[v2] mmc: litex_mmc: Set mandatory idle clocks before CMD0

[PATCH v2] mmc: litex_mmc: Set mandatory idle clocks before CMD0

Posted by Inochi Amaoto 1 month, 3 weeks ago

The litex_mmc driver assumes the card is already probed in the BIOS
and skip the phy initialization. This will cause the command fail
like the following when the old card is unplugged and then insert
a new card:

[   62.923593] litex-mmc f0004000.mmc: Command (cmd 8) error, status -110
[   62.949717] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
[   62.976606] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
[   63.002516] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
[   63.028442] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110

Add required clock settings and initialization for the CMD 0, so it can
probe the new card.

Fixes: 92e099104729 ("mmc: Add driver for LiteX's LiteSDCard interface")
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
Change from v1:
- https://lore.kernel.org/linux-mmc/20260421025052.755471-1-inochiama@gmail.com/
1. use fsleep to replace udelay
---
 drivers/mmc/host/litex_mmc.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/host/litex_mmc.c b/drivers/mmc/host/litex_mmc.c
index d2f19c2dc673..ac0b47604c02 100644
--- a/drivers/mmc/host/litex_mmc.c
+++ b/drivers/mmc/host/litex_mmc.c
@@ -68,6 +68,9 @@
 #define SD_SLEEP_US       5
 #define SD_TIMEOUT_US 20000

+#define SD_INIT_DELAY_US  1000
+#define SD_INIT_CLK_HZ    400000
+
 #define SDIRQ_CARD_DETECT    1
 #define SDIRQ_SD_TO_MEM_DONE 2
 #define SDIRQ_MEM_TO_SD_DONE 4
@@ -99,6 +102,8 @@ struct litex_mmc_host {
 	bool app_cmd;
 };

+static void litex_mmc_setclk(struct litex_mmc_host *host, unsigned int freq);
+
 static int litex_mmc_sdcard_wait_done(void __iomem *reg, struct device *dev)
 {
 	u8 evt;
@@ -128,6 +133,12 @@ static int litex_mmc_send_cmd(struct litex_mmc_host *host,
 	int ret;
 	u8 evt;

+	if (cmd == MMC_GO_IDLE_STATE) {
+		litex_mmc_setclk(host, SD_INIT_CLK_HZ);
+		litex_write8(host->sdphy + LITEX_PHY_INITIALIZE, 1);
+		fsleep(SD_INIT_DELAY_US);
+	}
+
 	litex_write32(host->sdcore + LITEX_CORE_CMDARG, arg);
 	litex_write32(host->sdcore + LITEX_CORE_CMDCMD,
 		      cmd << 8 | transfer << 5 | response_len);
--
2.54.0

Re: [PATCH v2] mmc: litex_mmc: Set mandatory idle clocks before CMD0

Posted by Gabriel L. Somlo 1 month, 3 weeks ago

Hi,

Thanks for the patch!

On Fri, Apr 24, 2026 at 09:36:14AM +0800, Inochi Amaoto wrote:
> The litex_mmc driver assumes the card is already probed in the BIOS
> and skip the phy initialization. This will cause the command fail
> like the following when the old card is unplugged and then insert
> a new card:
> 
> [   62.923593] litex-mmc f0004000.mmc: Command (cmd 8) error, status -110
> [   62.949717] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> [   62.976606] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> [   63.002516] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> [   63.028442] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> 
> Add required clock settings and initialization for the CMD 0, so it can
> probe the new card.
> 
> Fixes: 92e099104729 ("mmc: Add driver for LiteX's LiteSDCard interface")
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---
> Change from v1:
> - https://lore.kernel.org/linux-mmc/20260421025052.755471-1-inochiama@gmail.com/
> 1. use fsleep to replace udelay
> ---
>  drivers/mmc/host/litex_mmc.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mmc/host/litex_mmc.c b/drivers/mmc/host/litex_mmc.c
> index d2f19c2dc673..ac0b47604c02 100644
> --- a/drivers/mmc/host/litex_mmc.c
> +++ b/drivers/mmc/host/litex_mmc.c
> @@ -68,6 +68,9 @@
>  #define SD_SLEEP_US       5
>  #define SD_TIMEOUT_US 20000
> 
> +#define SD_INIT_DELAY_US  1000
> +#define SD_INIT_CLK_HZ    400000
> +
>  #define SDIRQ_CARD_DETECT    1
>  #define SDIRQ_SD_TO_MEM_DONE 2
>  #define SDIRQ_MEM_TO_SD_DONE 4
> @@ -99,6 +102,8 @@ struct litex_mmc_host {
>  	bool app_cmd;
>  };
> 
> +static void litex_mmc_setclk(struct litex_mmc_host *host, unsigned int freq);
> +

Instead of the forward reference, can you simply move the entire
`litex_mmc_setclk` function definition to this location in the file?
(i.e., add a separate, "cosmetic" patch simply cut'n'pasting the
function body from one place to another, with a commit blurb stating
that it's in preparation for the idle-clock fix, and that it contains
only cosmetic, no functional changes)?

The idle-clock fix patch can then contain nothing but the
"#define SD_INIT_*" lines and the hunk below:

>  static int litex_mmc_sdcard_wait_done(void __iomem *reg, struct device *dev)
>  {
>  	u8 evt;
> @@ -128,6 +133,12 @@ static int litex_mmc_send_cmd(struct litex_mmc_host *host,
>  	int ret;
>  	u8 evt;
> 
> +	if (cmd == MMC_GO_IDLE_STATE) {
> +		litex_mmc_setclk(host, SD_INIT_CLK_HZ);
> +		litex_write8(host->sdphy + LITEX_PHY_INITIALIZE, 1);
> +		fsleep(SD_INIT_DELAY_US);
> +	}
> +
>  	litex_write32(host->sdcore + LITEX_CORE_CMDARG, arg);
>  	litex_write32(host->sdcore + LITEX_CORE_CMDCMD,
>  		      cmd << 8 | transfer << 5 | response_len);

Thanks much,
--Gabriel

Re: [PATCH v2] mmc: litex_mmc: Set mandatory idle clocks before CMD0

Posted by Inochi Amaoto 1 month, 3 weeks ago

On Fri, Apr 24, 2026 at 11:12:21AM -0400, Gabriel L. Somlo wrote:
> Hi,
> 
> Thanks for the patch!
> 
> On Fri, Apr 24, 2026 at 09:36:14AM +0800, Inochi Amaoto wrote:
> > The litex_mmc driver assumes the card is already probed in the BIOS
> > and skip the phy initialization. This will cause the command fail
> > like the following when the old card is unplugged and then insert
> > a new card:
> > 
> > [   62.923593] litex-mmc f0004000.mmc: Command (cmd 8) error, status -110
> > [   62.949717] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> > [   62.976606] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> > [   63.002516] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> > [   63.028442] litex-mmc f0004000.mmc: Command (cmd 55) error, status -110
> > 
> > Add required clock settings and initialization for the CMD 0, so it can
> > probe the new card.
> > 
> > Fixes: 92e099104729 ("mmc: Add driver for LiteX's LiteSDCard interface")
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> > Change from v1:
> > - https://lore.kernel.org/linux-mmc/20260421025052.755471-1-inochiama@gmail.com/
> > 1. use fsleep to replace udelay
> > ---
> >  drivers/mmc/host/litex_mmc.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/mmc/host/litex_mmc.c b/drivers/mmc/host/litex_mmc.c
> > index d2f19c2dc673..ac0b47604c02 100644
> > --- a/drivers/mmc/host/litex_mmc.c
> > +++ b/drivers/mmc/host/litex_mmc.c
> > @@ -68,6 +68,9 @@
> >  #define SD_SLEEP_US       5
> >  #define SD_TIMEOUT_US 20000
> > 
> > +#define SD_INIT_DELAY_US  1000
> > +#define SD_INIT_CLK_HZ    400000
> > +
> >  #define SDIRQ_CARD_DETECT    1
> >  #define SDIRQ_SD_TO_MEM_DONE 2
> >  #define SDIRQ_MEM_TO_SD_DONE 4
> > @@ -99,6 +102,8 @@ struct litex_mmc_host {
> >  	bool app_cmd;
> >  };
> > 
> > +static void litex_mmc_setclk(struct litex_mmc_host *host, unsigned int freq);
> > +
> 
> Instead of the forward reference, can you simply move the entire
> `litex_mmc_setclk` function definition to this location in the file?
> (i.e., add a separate, "cosmetic" patch simply cut'n'pasting the
> function body from one place to another, with a commit blurb stating
> that it's in preparation for the idle-clock fix, and that it contains
> only cosmetic, no functional changes)?
> 
> The idle-clock fix patch can then contain nothing but the
> "#define SD_INIT_*" lines and the hunk below:
> 

Thanks, it is fine for me. I will send a new version for this.

Regards,
Inochi


> >  static int litex_mmc_sdcard_wait_done(void __iomem *reg, struct device *dev)
> >  {
> >  	u8 evt;
> > @@ -128,6 +133,12 @@ static int litex_mmc_send_cmd(struct litex_mmc_host *host,
> >  	int ret;
> >  	u8 evt;
> > 
> > +	if (cmd == MMC_GO_IDLE_STATE) {
> > +		litex_mmc_setclk(host, SD_INIT_CLK_HZ);
> > +		litex_write8(host->sdphy + LITEX_PHY_INITIALIZE, 1);
> > +		fsleep(SD_INIT_DELAY_US);
> > +	}
> > +
> >  	litex_write32(host->sdcore + LITEX_CORE_CMDARG, arg);
> >  	litex_write32(host->sdcore + LITEX_CORE_CMDCMD,
> >  		      cmd << 8 | transfer << 5 | response_len);
> 
> Thanks much,
> --Gabriel