[PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

[PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ziyang Huang]

Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
Then we set rx_clk_phase to 11 or 15 which is out of range and make
hardware frozen. After we send command request, no irq will be
interrupted and the mmc driver will keep to wait for request finished,
even durning rebooting.

So let's set a common value - 1 just for initialization. Then let
meson_mx_sdhc_execute_tuning() to find the accurate value for data
transfer.

Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
---
 drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
index da85c2f2..a01090a2 100644
--- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
@@ -269,7 +269,6 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
 static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
-	u32 rx_clk_phase;
 	int ret;
 
 	meson_mx_sdhc_disable_clks(mmc);
@@ -290,31 +289,12 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
 		mmc->actual_clock = clk_get_rate(host->sd_clk);
 
 		/*
-		 * according to Amlogic the following latching points are
-		 * selected with empirical values, there is no (known) formula
-		 * to calculate these.
+		 * This value is just for initialization. For data transmission,
+		 * meson_mx_sdhc_execute_tuning() will find a accurate value
 		 */
-		if (mmc->actual_clock > 100000000) {
-			rx_clk_phase = 1;
-		} else if (mmc->actual_clock > 45000000) {
-			if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
-				rx_clk_phase = 15;
-			else
-				rx_clk_phase = 11;
-		} else if (mmc->actual_clock >= 25000000) {
-			rx_clk_phase = 15;
-		} else if (mmc->actual_clock > 5000000) {
-			rx_clk_phase = 23;
-		} else if (mmc->actual_clock > 1000000) {
-			rx_clk_phase = 55;
-		} else {
-			rx_clk_phase = 1061;
-		}
-
 		regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
 				   MESON_SDHC_CLK2_RX_CLK_PHASE,
-				   FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
-					      rx_clk_phase));
+				   FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE, 1));
 	} else {
 		mmc->actual_clock = 0;
 	}
-- 
2.34.1

Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Martin Blumenstingl]

Hello,

first of all: thank you for this patch!

On Mon, Jun 19, 2023 at 7:36 PM Ziyang Huang <hzyitc@outlook.com> wrote:
>
> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
> Then we set rx_clk_phase to 11 or 15 which is out of range and make
> hardware frozen. After we send command request, no irq will be
> interrupted and the mmc driver will keep to wait for request finished,
> even durning rebooting.
I think this is the exact same problem I reported some days ago: [0]
Ulf is questioning whether we properly support 52MHz clocks correctly,
so I think you're onto something!

So this is an excellent finding! I can confirm that using rx_clk_phase
of 1 makes my Odroid-C1 eMMC work again :-)

> So let's set a common value - 1 just for initialization. Then let
> meson_mx_sdhc_execute_tuning() to find the accurate value for data
> transfer.
As far as I know unconditionally using value 1 can negatively affect
other devices.
I'm assuming that you're testing on an Odroid-C1 or similar board with
HS200 eMMC:
On those SoC + eMMC combinations we do support. But on other boards
(for example Meson8b EC-100 / Endless Mini) there's no HS200 support
because the eMMC is connected with 3.3V IO lines. So tuning is not
executed there (if I recall correctly).

What do you think about adding a special case for the 51MHz "actual
clock rate" and adding a comment that it was found by manual testing?
For some reason (that I don't understand) Amlogic's vendor driver
maxes out at 47.22MHz (presumably because they limit themselves to
using FCLK_DIV3 as input only - but I don't get why...).


Best regards,
Martin


[0] https://lore.kernel.org/linux-amlogic/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@mail.gmail.com/
[1] https://lore.kernel.org/linux-amlogic/CAPDyKFpS-UwiaRPMqSpX0mNPrS5p=yJzu3g0=pGyCkWHSYyqWg@mail.gmail.com/

Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Martin Blumenstingl]

Hello,

On Mon, Jun 19, 2023 at 9:51 PM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hello,
>
> first of all: thank you for this patch!
>
> On Mon, Jun 19, 2023 at 7:36 PM Ziyang Huang <hzyitc@outlook.com> wrote:
> >
> > Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
> > HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
> > freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
> > Then we set rx_clk_phase to 11 or 15 which is out of range and make
> > hardware frozen. After we send command request, no irq will be
> > interrupted and the mmc driver will keep to wait for request finished,
> > even durning rebooting.
> I think this is the exact same problem I reported some days ago: [0]
> Ulf is questioning whether we properly support 52MHz clocks correctly,
> so I think you're onto something!
>
> So this is an excellent finding! I can confirm that using rx_clk_phase
> of 1 makes my Odroid-C1 eMMC work again :-)
>
> > So let's set a common value - 1 just for initialization. Then let
> > meson_mx_sdhc_execute_tuning() to find the accurate value for data
> > transfer.
> As far as I know unconditionally using value 1 can negatively affect
> other devices.
> I'm assuming that you're testing on an Odroid-C1 or similar board with
> HS200 eMMC:
> On those SoC + eMMC combinations we do support. But on other boards
> (for example Meson8b EC-100 / Endless Mini) there's no HS200 support
> because the eMMC is connected with 3.3V IO lines. So tuning is not
> executed there (if I recall correctly).
>
> What do you think about adding a special case for the 51MHz "actual
> clock rate" and adding a comment that it was found by manual testing?
> For some reason (that I don't understand) Amlogic's vendor driver
> maxes out at 47.22MHz (presumably because they limit themselves to
> using FCLK_DIV3 as input only - but I don't get why...).
Did you have the chance to look into my comment? I would like to hear
your opinion on this topic!


Best regards,
Martin

Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ziyang Huang]

在 2023/6/20 3:51, Martin Blumenstingl 写道:
> Hello,
> 
> first of all: thank you for this patch!
> 
> On Mon, Jun 19, 2023 at 7:36 PM Ziyang Huang <hzyitc@outlook.com> wrote:
>>
>> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
>> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
>> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
>> Then we set rx_clk_phase to 11 or 15 which is out of range and make
>> hardware frozen. After we send command request, no irq will be
>> interrupted and the mmc driver will keep to wait for request finished,
>> even durning rebooting.
> I think this is the exact same problem I reported some days ago: [0]
> Ulf is questioning whether we properly support 52MHz clocks correctly,
> so I think you're onto something!
> 
> So this is an excellent finding! I can confirm that using rx_clk_phase
> of 1 makes my Odroid-C1 eMMC work again :-)
> 
>> So let's set a common value - 1 just for initialization. Then let
>> meson_mx_sdhc_execute_tuning() to find the accurate value for data
>> transfer.
> As far as I know unconditionally using value 1 can negatively affect
> other devices.
> I'm assuming that you're testing on an Odroid-C1 or similar board with
> HS200 eMMC:
> On those SoC + eMMC combinations we do support. But on other boards
> (for example Meson8b EC-100 / Endless Mini) there's no HS200 support
> because the eMMC is connected with 3.3V IO lines. So tuning is not
> executed there (if I recall correctly).

Sorry for the later reply. I'm so busy these day.

After checking the code, I found the following flow, so I think 
tuningshould work for all cards.

   mmc_start_request()
     -> __mmc_start_request()
       -> mmc_retune()
         -> mmc_execute_tuning()          -> host->ops->execute_tuning()

And yes, 1 may be not a good choice. I consider 3 values:
   - 1
   - div_val / 2
   - div_val - 1

Maybe, (div_val / 2) is a good choice. How do you think?

> What do you think about adding a special case for the 51MHz "actual
> clock rate" and adding a comment that it was found by manual testing?
> For some reason (that I don't understand) Amlogic's vendor driver
> maxes out at 47.22MHz (presumably because they limit themselves to
> using FCLK_DIV3 as input only - but I don't get why...).

With some modifications, I found that the mmc controller of S805 can 
work with all frequency, not only those mentioned in commit 
e4bf1b0970ef("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson 
SDHC host"). It seem like that every things work fine. I will use a 
oscilloscope to confirm the hardware clock is correct when I have time.

For this future commit, I don't want to add a special case.

> Best regards,
> Martin
> 
> 
> [0] https://lore.kernel.org/linux-amlogic/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@mail.gmail.com/
> [1] https://lore.kernel.org/linux-amlogic/CAPDyKFpS-UwiaRPMqSpX0mNPrS5p=yJzu3g0=pGyCkWHSYyqWg@mail.gmail.com/

Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ulf Hansson]

+ Thorsten

On Mon, 19 Jun 2023 at 19:36, Ziyang Huang <hzyitc@outlook.com> wrote:
>
> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
> Then we set rx_clk_phase to 11 or 15 which is out of range and make
> hardware frozen. After we send command request, no irq will be
> interrupted and the mmc driver will keep to wait for request finished,
> even durning rebooting.
>
> So let's set a common value - 1 just for initialization. Then let
> meson_mx_sdhc_execute_tuning() to find the accurate value for data
> transfer.
>
> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>

I don't quite understand if this patch is ok for everybody for me to apply?

It seems like it solves at least some part of the problems that
Martin/Thorsten were looking at too [1], right?

Kind regards
Uffe

[1]
https://lore.kernel.org/all/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@mail.gmail.com/#r

> ---
>  drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++-----------------------
>  1 file changed, 3 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> index da85c2f2..a01090a2 100644
> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> @@ -269,7 +269,6 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
>  static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
>         struct meson_mx_sdhc_host *host = mmc_priv(mmc);
> -       u32 rx_clk_phase;
>         int ret;
>
>         meson_mx_sdhc_disable_clks(mmc);
> @@ -290,31 +289,12 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>                 mmc->actual_clock = clk_get_rate(host->sd_clk);
>
>                 /*
> -                * according to Amlogic the following latching points are
> -                * selected with empirical values, there is no (known) formula
> -                * to calculate these.
> +                * This value is just for initialization. For data transmission,
> +                * meson_mx_sdhc_execute_tuning() will find a accurate value
>                  */
> -               if (mmc->actual_clock > 100000000) {
> -                       rx_clk_phase = 1;
> -               } else if (mmc->actual_clock > 45000000) {
> -                       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> -                               rx_clk_phase = 15;
> -                       else
> -                               rx_clk_phase = 11;
> -               } else if (mmc->actual_clock >= 25000000) {
> -                       rx_clk_phase = 15;
> -               } else if (mmc->actual_clock > 5000000) {
> -                       rx_clk_phase = 23;
> -               } else if (mmc->actual_clock > 1000000) {
> -                       rx_clk_phase = 55;
> -               } else {
> -                       rx_clk_phase = 1061;
> -               }
> -
>                 regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
>                                    MESON_SDHC_CLK2_RX_CLK_PHASE,
> -                                  FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
> -                                             rx_clk_phase));
> +                                  FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE, 1));
>         } else {
>                 mmc->actual_clock = 0;
>         }
> --
> 2.34.1
>

Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Thorsten Leemhuis]

On 14.09.23 16:45, Ulf Hansson wrote:
> + Thorsten

I recently gave up on this, as it seems nobody cared anymore, but let's
give this another try.

> On Mon, 19 Jun 2023 at 19:36, Ziyang Huang <hzyitc@outlook.com> wrote:
>>
>> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
>> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
>> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
>> Then we set rx_clk_phase to 11 or 15 which is out of range and make
>> hardware frozen. After we send command request, no irq will be
>> interrupted and the mmc driver will keep to wait for request finished,
>> even durning rebooting.
>>
>> So let's set a common value - 1 just for initialization. Then let
>> meson_mx_sdhc_execute_tuning() to find the accurate value for data
>> transfer.
>>
>> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
>> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> 
> I don't quite understand if this patch is ok for everybody for me to apply?
> 
> It seems like it solves at least some part of the problems that
> Martin/Thorsten were looking at too [1], right?

Martin, could you help clarifying the situation here? It seems Ziyang
Huang is busy.

I briefly skimmed this thread again and to me it sounded like there was
a plan for an improved patch that hasn't shown up yet.

Also CCing Brian Norris who according to the bisection from Martin in
that "[1]" caused the regression (or am I missing/confusing something
here?).

> [1]
> https://lore.kernel.org/all/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@mail.gmail.com/#r

Ciao, Thorsten

>> ---
>>  drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++-----------------------
>>  1 file changed, 3 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
>> index da85c2f2..a01090a2 100644
>> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
>> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
>> @@ -269,7 +269,6 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
>>  static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>>  {
>>         struct meson_mx_sdhc_host *host = mmc_priv(mmc);
>> -       u32 rx_clk_phase;
>>         int ret;
>>
>>         meson_mx_sdhc_disable_clks(mmc);
>> @@ -290,31 +289,12 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>>                 mmc->actual_clock = clk_get_rate(host->sd_clk);
>>
>>                 /*
>> -                * according to Amlogic the following latching points are
>> -                * selected with empirical values, there is no (known) formula
>> -                * to calculate these.
>> +                * This value is just for initialization. For data transmission,
>> +                * meson_mx_sdhc_execute_tuning() will find a accurate value
>>                  */
>> -               if (mmc->actual_clock > 100000000) {
>> -                       rx_clk_phase = 1;
>> -               } else if (mmc->actual_clock > 45000000) {
>> -                       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>> -                               rx_clk_phase = 15;
>> -                       else
>> -                               rx_clk_phase = 11;
>> -               } else if (mmc->actual_clock >= 25000000) {
>> -                       rx_clk_phase = 15;
>> -               } else if (mmc->actual_clock > 5000000) {
>> -                       rx_clk_phase = 23;
>> -               } else if (mmc->actual_clock > 1000000) {
>> -                       rx_clk_phase = 55;
>> -               } else {
>> -                       rx_clk_phase = 1061;
>> -               }
>> -
>>                 regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
>>                                    MESON_SDHC_CLK2_RX_CLK_PHASE,
>> -                                  FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
>> -                                             rx_clk_phase));
>> +                                  FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE, 1));
>>         } else {
>>                 mmc->actual_clock = 0;
>>         }
>> --
>> 2.34.1
>>
> 

[PATCH v2] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ziyang Huang]

Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
Then we set rx_clk_phase to 11 or 15 which is out of range and make
hardware frozen. After we send command request, no irq will be
interrupted and the mmc driver will keep to wait for request finished,
even durning rebooting.

So let's set it to Phase 90 which should work in most cases. Then let
meson_mx_sdhc_execute_tuning() to find the accurate value for data
transfer.

If this doesn't work, maybe need to define a factor in dts.

Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
---
Changes since v1:
  Use Phase 90 instand of value 1

 drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
index 97168cdfa8e9..29698fceb89c 100644
--- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
@@ -269,7 +269,7 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
 static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
-	u32 rx_clk_phase;
+	u32 val, rx_clk_phase;
 	int ret;
 
 	meson_mx_sdhc_disable_clks(mmc);
@@ -290,27 +290,11 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
 		mmc->actual_clock = clk_get_rate(host->sd_clk);
 
 		/*
-		 * according to Amlogic the following latching points are
-		 * selected with empirical values, there is no (known) formula
-		 * to calculate these.
+		 * Phase 90 should work in most cases. For data transmission,
+		 * meson_mx_sdhc_execute_tuning() will find a accurate value
 		 */
-		if (mmc->actual_clock > 100000000) {
-			rx_clk_phase = 1;
-		} else if (mmc->actual_clock > 45000000) {
-			if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
-				rx_clk_phase = 15;
-			else
-				rx_clk_phase = 11;
-		} else if (mmc->actual_clock >= 25000000) {
-			rx_clk_phase = 15;
-		} else if (mmc->actual_clock > 5000000) {
-			rx_clk_phase = 23;
-		} else if (mmc->actual_clock > 1000000) {
-			rx_clk_phase = 55;
-		} else {
-			rx_clk_phase = 1061;
-		}
-
+		regmap_read(host->regmap, MESON_SDHC_CLKC, &val);
+		rx_clk_phase = FIELD_GET(MESON_SDHC_CLKC_CLK_DIV, val) / 4;
 		regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
 				   MESON_SDHC_CLK2_RX_CLK_PHASE,
 				   FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
-- 
2.34.1

Re: [PATCH v2] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ulf Hansson]

+ Thorsten

On Tue, 10 Oct 2023 at 18:44, Ziyang Huang <hzyitc@outlook.com> wrote:
>
> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
> Then we set rx_clk_phase to 11 or 15 which is out of range and make
> hardware frozen. After we send command request, no irq will be
> interrupted and the mmc driver will keep to wait for request finished,
> even durning rebooting.
>
> So let's set it to Phase 90 which should work in most cases. Then let
> meson_mx_sdhc_execute_tuning() to find the accurate value for data
> transfer.
>
> If this doesn't work, maybe need to define a factor in dts.
>
> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>

I have re-added Thorsten to see if he has some time to test this on his end.

Kind regards
Uffe

> ---
> Changes since v1:
>   Use Phase 90 instand of value 1
>
>  drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++++---------------------
>  1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> index 97168cdfa8e9..29698fceb89c 100644
> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> @@ -269,7 +269,7 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
>  static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
>         struct meson_mx_sdhc_host *host = mmc_priv(mmc);
> -       u32 rx_clk_phase;
> +       u32 val, rx_clk_phase;
>         int ret;
>
>         meson_mx_sdhc_disable_clks(mmc);
> @@ -290,27 +290,11 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>                 mmc->actual_clock = clk_get_rate(host->sd_clk);
>
>                 /*
> -                * according to Amlogic the following latching points are
> -                * selected with empirical values, there is no (known) formula
> -                * to calculate these.
> +                * Phase 90 should work in most cases. For data transmission,
> +                * meson_mx_sdhc_execute_tuning() will find a accurate value
>                  */
> -               if (mmc->actual_clock > 100000000) {
> -                       rx_clk_phase = 1;
> -               } else if (mmc->actual_clock > 45000000) {
> -                       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> -                               rx_clk_phase = 15;
> -                       else
> -                               rx_clk_phase = 11;
> -               } else if (mmc->actual_clock >= 25000000) {
> -                       rx_clk_phase = 15;
> -               } else if (mmc->actual_clock > 5000000) {
> -                       rx_clk_phase = 23;
> -               } else if (mmc->actual_clock > 1000000) {
> -                       rx_clk_phase = 55;
> -               } else {
> -                       rx_clk_phase = 1061;
> -               }
> -
> +               regmap_read(host->regmap, MESON_SDHC_CLKC, &val);
> +               rx_clk_phase = FIELD_GET(MESON_SDHC_CLKC_CLK_DIV, val) / 4;
>                 regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
>                                    MESON_SDHC_CLK2_RX_CLK_PHASE,
>                                    FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
> --
> 2.34.1
>

Re: [PATCH v2] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Linux regression tracking (Thorsten Leemhuis)]

On 23.10.23 13:14, Ulf Hansson wrote:
> On Tue, 10 Oct 2023 at 18:44, Ziyang Huang <hzyitc@outlook.com> wrote:
>>
>> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
>> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
>> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
>> Then we set rx_clk_phase to 11 or 15 which is out of range and make
>> hardware frozen. After we send command request, no irq will be
>> interrupted and the mmc driver will keep to wait for request finished,
>> even durning rebooting.
>>
>> So let's set it to Phase 90 which should work in most cases. Then let
>> meson_mx_sdhc_execute_tuning() to find the accurate value for data
>> transfer.
>>
>> If this doesn't work, maybe need to define a factor in dts.
>>
>> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
>> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> 
> I have re-added Thorsten to see if he has some time to test this on his end.

FWIW, I was only involved wrt to regression tracking. It iirc was was
Martin that had reported the problem here:
https://lore.kernel.org/all/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@mail.gmail.com/

Side note: a proper Reported-by: tag together with a Link or Closes tag
would be good to have in the patch descriptions, as explained in the Docs.

Ciao, Thorsten

>> ---
>> Changes since v1:
>>   Use Phase 90 instand of value 1
>>
>>  drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++++---------------------
>>  1 file changed, 5 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
>> index 97168cdfa8e9..29698fceb89c 100644
>> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
>> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
>> @@ -269,7 +269,7 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
>>  static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>>  {
>>         struct meson_mx_sdhc_host *host = mmc_priv(mmc);
>> -       u32 rx_clk_phase;
>> +       u32 val, rx_clk_phase;
>>         int ret;
>>
>>         meson_mx_sdhc_disable_clks(mmc);
>> @@ -290,27 +290,11 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>>                 mmc->actual_clock = clk_get_rate(host->sd_clk);
>>
>>                 /*
>> -                * according to Amlogic the following latching points are
>> -                * selected with empirical values, there is no (known) formula
>> -                * to calculate these.
>> +                * Phase 90 should work in most cases. For data transmission,
>> +                * meson_mx_sdhc_execute_tuning() will find a accurate value
>>                  */
>> -               if (mmc->actual_clock > 100000000) {
>> -                       rx_clk_phase = 1;
>> -               } else if (mmc->actual_clock > 45000000) {
>> -                       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
>> -                               rx_clk_phase = 15;
>> -                       else
>> -                               rx_clk_phase = 11;
>> -               } else if (mmc->actual_clock >= 25000000) {
>> -                       rx_clk_phase = 15;
>> -               } else if (mmc->actual_clock > 5000000) {
>> -                       rx_clk_phase = 23;
>> -               } else if (mmc->actual_clock > 1000000) {
>> -                       rx_clk_phase = 55;
>> -               } else {
>> -                       rx_clk_phase = 1061;
>> -               }
>> -
>> +               regmap_read(host->regmap, MESON_SDHC_CLKC, &val);
>> +               rx_clk_phase = FIELD_GET(MESON_SDHC_CLKC_CLK_DIV, val) / 4;
>>                 regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
>>                                    MESON_SDHC_CLK2_RX_CLK_PHASE,
>>                                    FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
>> --
>> 2.34.1
>>
> 

Re: [PATCH v2] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Anand Moon]

Hi Ziyang,

On Tue, 10 Oct 2023 at 22:15, Ziyang Huang <hzyitc@outlook.com> wrote:
>
> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
> Then we set rx_clk_phase to 11 or 15 which is out of range and make
> hardware frozen. After we send command request, no irq will be
> interrupted and the mmc driver will keep to wait for request finished,
> even durning rebooting.
>
> So let's set it to Phase 90 which should work in most cases. Then let
> meson_mx_sdhc_execute_tuning() to find the accurate value for data
> transfer.
>
> If this doesn't work, maybe need to define a factor in dts.
>
> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> ---
> Changes since v1:
>   Use Phase 90 instand of value 1
>

I have tested this patch on my Odroid C1+ board.
Please add my

Tested-by: Anand Moon <linux.amoon@gmail.com>

[alarm@alarm ~]$ sudo cat /sys/kernel/debug/mmc1/ios
clock:          100000000 Hz
actual clock:   94444445 Hz
vdd:            21 (3.3 ~ 3.4 V)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     2 (on)
bus width:      3 (8 bits)
timing spec:    9 (mmc HS200)
signal voltage: 1 (1.80 V)
driver type:    0 (driver type B)
[alarm@alarm ~]$ sync && dd if=/dev/zero of=~/testfile bs=100M count=1
oflag=dsync && sync
1+0 records in
1+0 records out
104857600 bytes (105 MB, 100 MiB) copied, 5.70235 s, 18.4 MB/s
[alarm@alarm ~]$ sync && dd if=~/testfile of=/dev/null bs=100M count=1
iflag=dsync && sync
1+0 records in
1+0 records out
104857600 bytes (105 MB, 100 MiB) copied, 0.20267 s, 517 MB/s

Thanks
-Anand

>  drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++++---------------------
>  1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> index 97168cdfa8e9..29698fceb89c 100644
> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> @@ -269,7 +269,7 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
>  static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
>         struct meson_mx_sdhc_host *host = mmc_priv(mmc);
> -       u32 rx_clk_phase;
> +       u32 val, rx_clk_phase;
>         int ret;
>
>         meson_mx_sdhc_disable_clks(mmc);
> @@ -290,27 +290,11 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>                 mmc->actual_clock = clk_get_rate(host->sd_clk);
>
>                 /*
> -                * according to Amlogic the following latching points are
> -                * selected with empirical values, there is no (known) formula
> -                * to calculate these.
> +                * Phase 90 should work in most cases. For data transmission,
> +                * meson_mx_sdhc_execute_tuning() will find a accurate value
>                  */
> -               if (mmc->actual_clock > 100000000) {
> -                       rx_clk_phase = 1;
> -               } else if (mmc->actual_clock > 45000000) {
> -                       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> -                               rx_clk_phase = 15;
> -                       else
> -                               rx_clk_phase = 11;
> -               } else if (mmc->actual_clock >= 25000000) {
> -                       rx_clk_phase = 15;
> -               } else if (mmc->actual_clock > 5000000) {
> -                       rx_clk_phase = 23;
> -               } else if (mmc->actual_clock > 1000000) {
> -                       rx_clk_phase = 55;
> -               } else {
> -                       rx_clk_phase = 1061;
> -               }
> -
> +               regmap_read(host->regmap, MESON_SDHC_CLKC, &val);
> +               rx_clk_phase = FIELD_GET(MESON_SDHC_CLKC_CLK_DIV, val) / 4;
>                 regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
>                                    MESON_SDHC_CLK2_RX_CLK_PHASE,
>                                    FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
> --
> 2.34.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ziyang Huang]

在 2023/10/29 21:08, Anand Moon 写道:
> Hi Ziyang,
> 
> On Tue, 10 Oct 2023 at 22:15, Ziyang Huang <hzyitc@outlook.com> wrote:
>>
>> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
>> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
>> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
>> Then we set rx_clk_phase to 11 or 15 which is out of range and make
>> hardware frozen. After we send command request, no irq will be
>> interrupted and the mmc driver will keep to wait for request finished,
>> even durning rebooting.
>>
>> So let's set it to Phase 90 which should work in most cases. Then let
>> meson_mx_sdhc_execute_tuning() to find the accurate value for data
>> transfer.
>>
>> If this doesn't work, maybe need to define a factor in dts.
>>
>> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
>> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
>> ---
>> Changes since v1:
>>    Use Phase 90 instand of value 1
>>
> 
> I have tested this patch on my Odroid C1+ board.
> Please add my
> 
> Tested-by: Anand Moon <linux.amoon@gmail.com>
> 
> [alarm@alarm ~]$ sudo cat /sys/kernel/debug/mmc1/ios
> clock:          100000000 Hz
> actual clock:   94444445 Hz
> vdd:            21 (3.3 ~ 3.4 V)
> bus mode:       2 (push-pull)
> chip select:    0 (don't care)
> power mode:     2 (on)
> bus width:      3 (8 bits)
> timing spec:    9 (mmc HS200)
> signal voltage: 1 (1.80 V)
> driver type:    0 (driver type B)
> [alarm@alarm ~]$ sync && dd if=/dev/zero of=~/testfile bs=100M count=1
> oflag=dsync && sync
> 1+0 records in
> 1+0 records out
> 104857600 bytes (105 MB, 100 MiB) copied, 5.70235 s, 18.4 MB/s
> [alarm@alarm ~]$ sync && dd if=~/testfile of=/dev/null bs=100M count=1
> iflag=dsync && sync
> 1+0 records in
> 1+0 records out
> 104857600 bytes (105 MB, 100 MiB) copied, 0.20267 s, 517 MB/s
> 
> Thanks
> -Anand
> 

Oh, thank you.

Re: [PATCH v2] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ulf Hansson]

+ Anand

On Tue, 10 Oct 2023 at 18:44, Ziyang Huang <hzyitc@outlook.com> wrote:
>
> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
> Then we set rx_clk_phase to 11 or 15 which is out of range and make
> hardware frozen. After we send command request, no irq will be
> interrupted and the mmc driver will keep to wait for request finished,
> even durning rebooting.
>
> So let's set it to Phase 90 which should work in most cases. Then let
> meson_mx_sdhc_execute_tuning() to find the accurate value for data
> transfer.
>
> If this doesn't work, maybe need to define a factor in dts.
>
> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>

Let's give this a try! Although, rather than queuing it as a fix for
v6.7, I am picking it for v6.8 and adding a stable tag, this should
allow it to become a bit more tested first.

Kind regards
Uffe


> ---
> Changes since v1:
>   Use Phase 90 instand of value 1
>
>  drivers/mmc/host/meson-mx-sdhc-mmc.c | 26 +++++---------------------
>  1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> index 97168cdfa8e9..29698fceb89c 100644
> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> @@ -269,7 +269,7 @@ static int meson_mx_sdhc_enable_clks(struct mmc_host *mmc)
>  static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
>         struct meson_mx_sdhc_host *host = mmc_priv(mmc);
> -       u32 rx_clk_phase;
> +       u32 val, rx_clk_phase;
>         int ret;
>
>         meson_mx_sdhc_disable_clks(mmc);
> @@ -290,27 +290,11 @@ static int meson_mx_sdhc_set_clk(struct mmc_host *mmc, struct mmc_ios *ios)
>                 mmc->actual_clock = clk_get_rate(host->sd_clk);
>
>                 /*
> -                * according to Amlogic the following latching points are
> -                * selected with empirical values, there is no (known) formula
> -                * to calculate these.
> +                * Phase 90 should work in most cases. For data transmission,
> +                * meson_mx_sdhc_execute_tuning() will find a accurate value
>                  */
> -               if (mmc->actual_clock > 100000000) {
> -                       rx_clk_phase = 1;
> -               } else if (mmc->actual_clock > 45000000) {
> -                       if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> -                               rx_clk_phase = 15;
> -                       else
> -                               rx_clk_phase = 11;
> -               } else if (mmc->actual_clock >= 25000000) {
> -                       rx_clk_phase = 15;
> -               } else if (mmc->actual_clock > 5000000) {
> -                       rx_clk_phase = 23;
> -               } else if (mmc->actual_clock > 1000000) {
> -                       rx_clk_phase = 55;
> -               } else {
> -                       rx_clk_phase = 1061;
> -               }
> -
> +               regmap_read(host->regmap, MESON_SDHC_CLKC, &val);
> +               rx_clk_phase = FIELD_GET(MESON_SDHC_CLKC_CLK_DIV, val) / 4;
>                 regmap_update_bits(host->regmap, MESON_SDHC_CLK2,
>                                    MESON_SDHC_CLK2_RX_CLK_PHASE,
>                                    FIELD_PREP(MESON_SDHC_CLK2_RX_CLK_PHASE,
> --
> 2.34.1
>

Re: [PATCH v2] mmc: meson-mx-sdhc: Fix initialization frozen issue

[Ziyang Huang]

在 2023/11/3 19:15, Ulf Hansson 写道:
> + Anand
> 
> On Tue, 10 Oct 2023 at 18:44, Ziyang Huang <hzyitc@outlook.com> wrote:
>>
>> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
>> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
>> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
>> Then we set rx_clk_phase to 11 or 15 which is out of range and make
>> hardware frozen. After we send command request, no irq will be
>> interrupted and the mmc driver will keep to wait for request finished,
>> even durning rebooting.
>>
>> So let's set it to Phase 90 which should work in most cases. Then let
>> meson_mx_sdhc_execute_tuning() to find the accurate value for data
>> transfer.
>>
>> If this doesn't work, maybe need to define a factor in dts.
>>
>> Fixes: e4bf1b0970ef ("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host")
>> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> 
> Let's give this a try! Although, rather than queuing it as a fix for
> v6.7, I am picking it for v6.8 and adding a stable tag, this should
> allow it to become a bit more tested first.
> 
> Kind regards
> Uffe
> 
> 

Yes, I think any fixes better than now since it's broken and can't work.

[PATCH v2] mtd: spinand: fmsh: add support for FM25G{01,02}B

[Ziyang Huang]

FM25G01B: https://www.fmsh.com/nvm/FM25G01B_ds_eng.pdf
FM25G02B: https://www.fmsh.com/nvm/FM25G02B_ds_eng.pdf

Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
---
Changes since v1:
  Fix copy-paste issue. (Correct FM25G01B size.)

 drivers/mtd/nand/spi/fmsh.c | 101 ++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/drivers/mtd/nand/spi/fmsh.c b/drivers/mtd/nand/spi/fmsh.c
index f417955f7d1c..a9b219ae6c29 100644
--- a/drivers/mtd/nand/spi/fmsh.c
+++ b/drivers/mtd/nand/spi/fmsh.c
@@ -9,6 +9,16 @@
 #include <linux/kernel.h>
 #include <linux/mtd/spinand.h>
 
+#define FM25G01B_STATUS_ECC_MASK		(7 << 4)
+	#define FM25G01B_STATUS_ECC_NO_BITFLIPS		(0 << 4)
+	#define FM25G01B_STATUS_ECC_1_3_BITFLIPS	(1 << 4)
+	#define FM25G01B_STATUS_ECC_4_BITFLIPS		(2 << 4)
+	#define FM25G01B_STATUS_ECC_5_BITFLIPS		(3 << 4)
+	#define FM25G01B_STATUS_ECC_6_BITFLIPS		(4 << 4)
+	#define FM25G01B_STATUS_ECC_7_BITFLIPS		(5 << 4)
+	#define FM25G01B_STATUS_ECC_8_BITFLIPS		(6 << 4)
+	#define FM25G01B_STATUS_ECC_UNCOR_ERROR		(7 << 4)
+
 #define FM25S01BI3_STATUS_ECC_MASK		(7 << 4)
 	#define FM25S01BI3_STATUS_ECC_NO_BITFLIPS	(0 << 4)
 	#define FM25S01BI3_STATUS_ECC_1_3_BITFLIPS	(1 << 4)
@@ -34,6 +44,72 @@ static SPINAND_OP_VARIANTS(update_cache_variants,
 		SPINAND_PROG_LOAD_1S_1S_4S_OP(false, 0, NULL, 0),
 		SPINAND_PROG_LOAD_1S_1S_1S_OP(false, 0, NULL, 0));
 
+
+static int fm25g01b_ooblayout_ecc(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	region->offset = 64;
+	region->length = 64;
+
+	return 0;
+}
+
+static int fm25g01b_ooblayout_free(struct mtd_info *mtd, int section,
+				   struct mtd_oob_region *region)
+{
+	if (section > 3)
+		return -ERANGE;
+
+	if (section == 0) {
+		/* reserve 2 bytes for the BBM */
+		region->offset = 2;
+		region->length = 14;
+	} else {
+		region->offset = section * 16;
+		region->length = 16;
+	}
+
+	return 0;
+}
+
+static int fm25g01b_ecc_get_status(struct spinand_device *spinand,
+				   u8 status)
+{
+	switch (status & FM25S01BI3_STATUS_ECC_MASK) {
+	case FM25G01B_STATUS_ECC_NO_BITFLIPS:
+		return 0;
+
+	case FM25G01B_STATUS_ECC_1_3_BITFLIPS:
+		return 3;
+
+	case FM25G01B_STATUS_ECC_4_BITFLIPS:
+		return 4;
+
+	case FM25G01B_STATUS_ECC_5_BITFLIPS:
+		return 5;
+
+	case FM25G01B_STATUS_ECC_6_BITFLIPS:
+		return 6;
+
+	case FM25G01B_STATUS_ECC_7_BITFLIPS:
+		return 7;
+
+	case FM25G01B_STATUS_ECC_8_BITFLIPS:
+		return 8;
+
+	case FM25G01B_STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
 static int fm25s01a_ooblayout_ecc(struct mtd_info *mtd, int section,
 				  struct mtd_oob_region *region)
 {
@@ -102,6 +178,11 @@ static int fm25s01bi3_ooblayout_free(struct mtd_info *mtd, int section,
 	return 0;
 }
 
+static const struct mtd_ooblayout_ops fm25g01b_ooblayout = {
+	.ecc = fm25g01b_ooblayout_ecc,
+	.free = fm25g01b_ooblayout_free,
+};
+
 static const struct mtd_ooblayout_ops fm25s01a_ooblayout = {
 	.ecc = fm25s01a_ooblayout_ecc,
 	.free = fm25s01a_ooblayout_free,
@@ -113,6 +194,26 @@ static const struct mtd_ooblayout_ops fm25s01bi3_ooblayout = {
 };
 
 static const struct spinand_info fmsh_spinand_table[] = {
+	SPINAND_INFO("FM25G01B",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xd1),
+		     NAND_MEMORG(1, 2048, 128, 64, 1024, 21, 1, 1, 1),
+		     NAND_ECCREQ(8, 528),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&fm25g01b_ooblayout,
+				     fm25g01b_ecc_get_status)),
+	SPINAND_INFO("FM25G02B",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xd2),
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 41, 1, 1, 1),
+		     NAND_ECCREQ(8, 528),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&fm25g01b_ooblayout,
+				     fm25g01b_ecc_get_status)),
 	SPINAND_INFO("FM25S01A",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xE4),
 		     NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
-- 
2.40.1

Re: [PATCH v2] mtd: spinand: fmsh: add support for FM25G{01,02}B

[Miquel Raynal]

Hello Ziyang,

On 15/04/2026 at 23:31:41 +08, Ziyang Huang <hzyitc@outlook.com> wrote:

> FM25G01B: https://www.fmsh.com/nvm/FM25G01B_ds_eng.pdf
> FM25G02B: https://www.fmsh.com/nvm/FM25G02B_ds_eng.pdf

Before giving the links, the commit could be slightly more verbose, such
as "Add support for...".

> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> ---
> Changes since v1:
>   Fix copy-paste issue. (Correct FM25G01B size.)
>
>  drivers/mtd/nand/spi/fmsh.c | 101 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>
> diff --git a/drivers/mtd/nand/spi/fmsh.c b/drivers/mtd/nand/spi/fmsh.c
> index f417955f7d1c..a9b219ae6c29 100644
> --- a/drivers/mtd/nand/spi/fmsh.c
> +++ b/drivers/mtd/nand/spi/fmsh.c
> @@ -9,6 +9,16 @@
>  #include <linux/kernel.h>
>  #include <linux/mtd/spinand.h>
>  
> +#define FM25G01B_STATUS_ECC_MASK		(7 << 4)
> +	#define FM25G01B_STATUS_ECC_NO_BITFLIPS		(0 << 4)
> +	#define FM25G01B_STATUS_ECC_1_3_BITFLIPS	(1 << 4)
> +	#define FM25G01B_STATUS_ECC_4_BITFLIPS		(2 << 4)
> +	#define FM25G01B_STATUS_ECC_5_BITFLIPS		(3 << 4)
> +	#define FM25G01B_STATUS_ECC_6_BITFLIPS		(4 << 4)
> +	#define FM25G01B_STATUS_ECC_7_BITFLIPS		(5 << 4)
> +	#define FM25G01B_STATUS_ECC_8_BITFLIPS		(6 << 4)
> +	#define FM25G01B_STATUS_ECC_UNCOR_ERROR		(7 << 4)
> +
>  #define FM25S01BI3_STATUS_ECC_MASK		(7 << 4)
>  	#define FM25S01BI3_STATUS_ECC_NO_BITFLIPS	(0 << 4)
>  	#define FM25S01BI3_STATUS_ECC_1_3_BITFLIPS	(1 << 4)
> @@ -34,6 +44,72 @@ static SPINAND_OP_VARIANTS(update_cache_variants,
>  		SPINAND_PROG_LOAD_1S_1S_4S_OP(false, 0, NULL, 0),
>  		SPINAND_PROG_LOAD_1S_1S_1S_OP(false, 0, NULL, 0));
>  
> +
> +static int fm25g01b_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				  struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	region->offset = 64;
> +	region->length = 64;
> +
> +	return 0;
> +}
> +
> +static int fm25g01b_ooblayout_free(struct mtd_info *mtd, int section,
> +				   struct mtd_oob_region *region)
> +{
> +	if (section > 3)
> +		return -ERANGE;
> +
> +	if (section == 0) {
> +		/* reserve 2 bytes for the BBM */
> +		region->offset = 2;
> +		region->length = 14;
> +	} else {
> +		region->offset = section * 16;
> +		region->length = 16;
> +	}

Isn't that just one big 62 bytes section starting at 2?

> +
> +	return 0;
> +}

Rest lgtm.

Thanks,
Miquèl