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

[Inochi Amaoto]

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@xxxxxxxxx>
> > ---
> > Change from v1:
> > - https://lore.kernel.org/linux-mmc/20260421025052.755471-1-inochiama@xxxxxxxxx/
> > 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