[PATCH] mmc: meson_gx_mmc: change a clock phase to stable value
Core clock phase value is changed from 180' to 270'. It's more stable than before. - Odroidn-N2/C4 : Working fine with 52MHz - VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com --- drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */ - meson_mmc_clk |= CLK_CO_PHASE_180; - - /* 180 phase tx clock */ + /* + * Clock Phase needs to set a proper value. + * It can be changed to other value. + * Because CORE : 270' Phase and TX : 0' Phase are stable, + * set to them by default. + */ + /* Core Clock Phase */ + meson_mmc_clk |= CLK_CO_PHASE_270; + + /* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
How did you test these ?
Neil
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
On 09/11/2020 15:10, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
So this is specific to SM1 SoCs then, because others families doesn't have such issues at 52MHz.
So the Linux must be fixes, including the bindings to introduce a new compatible, then ported to U-Boot.
So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this clarified.
Neil
Hi Neil,
On Mon, 9 Nov 2020 at 19:56, Neil Armstrong narmstrong@baylibre.com wrote:
On 09/11/2020 15:10, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
- Clock Phase needs to set a proper value.
- It can be changed to other value.
- Because CORE : 270' Phase and TX : 0' Phase are stable,
- set to them by default.
*/
/* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
So this is specific to SM1 SoCs then, because others families doesn't have such issues at 52MHz.
So the Linux must be fixes, including the bindings to introduce a new compatible, then ported to U-Boot.
So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this clarified.
Neil
Earlier I had a similar approach for this FIX but somehow it did not get merged.
Please add my Tested-by: Anand Moon linux.amoon@gmail.com
Best Regards -Anand
Dear Anand,
On 11/10/20 4:02 AM, Anand Moon wrote:
Hi Neil,
On Mon, 9 Nov 2020 at 19:56, Neil Armstrong narmstrong@baylibre.com wrote:
On 09/11/2020 15:10, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
- Clock Phase needs to set a proper value.
- It can be changed to other value.
- Because CORE : 270' Phase and TX : 0' Phase are stable,
- set to them by default.
*/
/* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
So this is specific to SM1 SoCs then, because others families doesn't have such issues at 52MHz.
So the Linux must be fixes, including the bindings to introduce a new compatible, then ported to U-Boot.
So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this clarified.
Neil
Earlier I had a similar approach for this FIX but somehow it did not get merged.
Please add my Tested-by: Anand Moon linux.amoon@gmail.com
Thanks for testing!
Best Regards, Jaehoon Chung
Best Regards -Anand
On 11/9/20 11:23 PM, Neil Armstrong wrote:
On 09/11/2020 15:10, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
So this is specific to SM1 SoCs then, because others families doesn't have such issues at 52MHz.
I don't have much knowledge of SM1 SoCs. But how did its phase value select them on Linux driver?
So the Linux must be fixes, including the bindings to introduce a new compatible, then ported to U-Boot.
So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this clarified.
If you want to limit to 26MHz, I don't have any objection about your opinion. But I wonder how to clarify all. And I also wonder that values what is used on Linux kernel are really right.
Best Regards, Jaehoon Chung
Neil
On 09/11/2020 23:19, Jaehoon Chung wrote:
On 11/9/20 11:23 PM, Neil Armstrong wrote:
On 09/11/2020 15:10, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
So this is specific to SM1 SoCs then, because others families doesn't have such issues at 52MHz.
I don't have much knowledge of SM1 SoCs. But how did its phase value select them on Linux driver?
So the Linux must be fixes, including the bindings to introduce a new compatible, then ported to U-Boot.
So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this clarified.
If you want to limit to 26MHz, I don't have any objection about your opinion. But I wonder how to clarify all. And I also wonder that values what is used on Linux kernel are really right.
OK, I'm not a MMC & SDCard expert on Amlogic, all the work was done in Linux.
I just want to keep a working and stable SDCard & eMMC support for the 7 SoCs Families.
Could you rewrite my "[PATCH 2/3] mmc: meson-gx: limit max frequency on SM1 SoCs" by instead changing the clock phase only for SM1 ? I'll then apply it with the 2 other patches and push then for the current development cycle.
The driver is functional for all the other SoCs, and I want to keep it stable unless extensively tested.
Neil
Best Regards, Jaehoon Chung
Neil
On 11/10/20 5:05 PM, Neil Armstrong wrote:
On 09/11/2020 23:19, Jaehoon Chung wrote:
On 11/9/20 11:23 PM, Neil Armstrong wrote:
On 09/11/2020 15:10, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
So this is specific to SM1 SoCs then, because others families doesn't have such issues at 52MHz.
I don't have much knowledge of SM1 SoCs. But how did its phase value select them on Linux driver?
So the Linux must be fixes, including the bindings to introduce a new compatible, then ported to U-Boot.
So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this clarified.
If you want to limit to 26MHz, I don't have any objection about your opinion. But I wonder how to clarify all. And I also wonder that values what is used on Linux kernel are really right.
OK, I'm not a MMC & SDCard expert on Amlogic, all the work was done in Linux.
I just want to keep a working and stable SDCard & eMMC support for the 7 SoCs Families.
Could you rewrite my "[PATCH 2/3] mmc: meson-gx: limit max frequency on SM1 SoCs" by instead changing the clock phase only for SM1 ? I'll then apply it with the 2 other patches and push then for the current development cycle.
Ok. I will rewrite with you patch. Is it ok that i send the patch on Tomorrow?
The driver is functional for all the other SoCs, and I want to keep it stable unless extensively tested.
Agreed. If i can test all Amlogic SoCs, I want to do that..But it's impossible to me. In future, i hope that it will be fixed with generic approach.
Best Regards, Jaehoon Chung
Neil
Best Regards, Jaehoon Chung
Neil
On 10/11/2020 09:36, Jaehoon Chung wrote:
On 11/10/20 5:05 PM, Neil Armstrong wrote:
On 09/11/2020 23:19, Jaehoon Chung wrote:
On 11/9/20 11:23 PM, Neil Armstrong wrote:
On 09/11/2020 15:10, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote: > Core clock phase value is changed from 180' to 270'. > It's more stable than before. > - Odroidn-N2/C4 : Working fine with 52MHz > - VIM3 : Working fine with 52MHz > > Before this patch, Odroid-C4 doesn't work fine with 52MHz. > > Signed-off-by: Jaehoon Chung jh80.chung@samsung.com > --- > drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c > index 719dd1e5e570..7c60e0566560 100644 > --- a/drivers/mmc/meson_gx_mmc.c > +++ b/drivers/mmc/meson_gx_mmc.c > @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) > } > clk_div = DIV_ROUND_UP(clk, mmc->clock); > > - /* 180 phase core clock */ > - meson_mmc_clk |= CLK_CO_PHASE_180; > - > - /* 180 phase tx clock */ > + /* > + * Clock Phase needs to set a proper value. > + * It can be changed to other value. > + * Because CORE : 270' Phase and TX : 0' Phase are stable, > + * set to them by default. > + */ > + /* Core Clock Phase */ > + meson_mmc_clk |= CLK_CO_PHASE_270; > + > + /* TX Clock Phase */ > meson_mmc_clk |= CLK_TX_PHASE_000; > > /* clock settings */ >
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
So this is specific to SM1 SoCs then, because others families doesn't have such issues at 52MHz.
I don't have much knowledge of SM1 SoCs. But how did its phase value select them on Linux driver?
So the Linux must be fixes, including the bindings to introduce a new compatible, then ported to U-Boot.
So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this clarified.
If you want to limit to 26MHz, I don't have any objection about your opinion. But I wonder how to clarify all. And I also wonder that values what is used on Linux kernel are really right.
OK, I'm not a MMC & SDCard expert on Amlogic, all the work was done in Linux.
I just want to keep a working and stable SDCard & eMMC support for the 7 SoCs Families.
Could you rewrite my "[PATCH 2/3] mmc: meson-gx: limit max frequency on SM1 SoCs" by instead changing the clock phase only for SM1 ? I'll then apply it with the 2 other patches and push then for the current development cycle.
Ok. I will rewrite with you patch. Is it ok that i send the patch on Tomorrow?
Agreed
The driver is functional for all the other SoCs, and I want to keep it stable unless extensively tested.
Agreed. If i can test all Amlogic SoCs, I want to do that..But it's impossible to me. In future, i hope that it will be fixed with generic approach.
Sure, thanks for your work & testing !
Neil
Best Regards, Jaehoon Chung
Neil
Best Regards, Jaehoon Chung
Neil
Hi,
On 11/9/20 11:10 PM, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Mon, 9 Nov 2020 14:37:09 +0100
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
Actually the Linux driver isn't really functional; the 52 MHz high-speed mode doesn't work. But since HS200 does work in Linux, probably nobody noticed this.
Well, i didn't check Linux driver. but i can also check on Linux side.
That said, I'm not confident a single clock phase setting will work across all Amlogic SoCs and across different boards. Maybe we need something in the device tree such that we can control the values on a per-board level.
Agreed. I can't mention that "it's working fine about all Amlogic SoCs". In exynos's case, there are sdr and ddr timing about mmc/sd IP. sdr/ddr timing are trying to get from dt's property, because it's possible that all Exynos SoCs have different values.
I think that Amlogic SoCs also needs to apply similar approach.
Best Regards, Jaehoon Chung
On 11/9/20 10:37 PM, Neil Armstrong wrote:
Hi,
On 09/11/2020 04:12, Jaehoon Chung wrote:
Core clock phase value is changed from 180' to 270'. It's more stable than before.
- Odroidn-N2/C4 : Working fine with 52MHz
- VIM3 : Working fine with 52MHz
Before this patch, Odroid-C4 doesn't work fine with 52MHz.
Signed-off-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/meson_gx_mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e570..7c60e0566560 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* 180 phase core clock */
- meson_mmc_clk |= CLK_CO_PHASE_180;
- /* 180 phase tx clock */
/*
* Clock Phase needs to set a proper value.* It can be changed to other value.* Because CORE : 270' Phase and TX : 0' Phase are stable,* set to them by default.*//* Core Clock Phase */
meson_mmc_clk |= CLK_CO_PHASE_270;
/* TX Clock Phase */ meson_mmc_clk |= CLK_TX_PHASE_000;
/* clock settings */
The previous values were aligned on the Linux driver, which is functional.
How did you test these ?
Actually, i have tested about all cases on targets what i have. (VIM3/Odroid-N2/Odroid-C4) I also have VIM3L, but i didn't test on VIM3L. (I can test with VIM3L) If check with oscilloscope, it will be a good way to find what's wrong.
When i have enabled MMC_DEBUG, it was always returned -5 (IO) error during switching mode. In meson_gx_mmc.c, meson_dm_mmc_send_cmd() is returned to -5. When i have checked status register, CRC error status bit (BIT[10]) is set. It means that clock timing is wrong. In my experiment, my debugging about CRC error is 1) GPIO setting 2) clock value 3) Driver strength 4) clock phase value
I assume that 1~3) are correct. So checked PHASE values.
I didn't check yet how to set value on Linux driver.
Best Regards, Jaehoon Chung
Neil
participants (4)
-
Anand Moon -
Jaehoon Chung -
Mark Kettenis -
Neil Armstrong