On Tue, 25 Mar 2025 01:16:41 +0000 Andre Przywara <[email protected]> wrote:
Hi, the U-Boot CI just told me off: .... > On Sun, 9 Mar 2025 07:12:41 +0100 > Jernej Skrabec <[email protected]> wrote: > > Hi Jernej, > > many thanks for your investigation and this fix here! Not having > working eMMC access was a major annoyance for those TV boxes, and this > indeed seems to be fixed now, judging by my experiments. Also checked > boot partition access, works fine (though data partitions seem to take > precedence on most devices). > > > Cards should always be reset and threshold set. This fixes eMMC on H616. > > > > Signed-off-by: Jernej Skrabec <[email protected]> > > Acked-by: Andre Przywara <[email protected]> > > Cheers, > Andre > > > --- > > drivers/mmc/sunxi_mmc.c | 28 ++++++++++++++++++++++------ > > drivers/mmc/sunxi_mmc.h | 15 +++++++++++++-- > > 2 files changed, 35 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c > > index 0b56d1405bee..335def4b9738 100644 > > --- a/drivers/mmc/sunxi_mmc.c > > +++ b/drivers/mmc/sunxi_mmc.c > > @@ -442,6 +442,26 @@ out: > > return error; > > } > > > > +static void sunxi_mmc_reset(struct sunxi_mmc *regs) > > +{ > > + /* Reset controller */ > > + writel(SUNXI_MMC_GCTRL_RESET, ®s->gctrl); > > + udelay(1000); > > + > > + if (IS_ENABLED(CONFIG_SUN50I_GEN_H6) || > > IS_ENABLED(CONFIG_SUNXI_GEN_NCAT2)) { > > + /* Reset card */ > > + writel(SUNXI_MMC_HWRST_ASSERT, ®s->hwrst); > > + udelay(10); > > + writel(SUNXI_MMC_HWRST_DEASSERT, ®s->hwrst); > > + udelay(300); > > + > > + /* Setup FIFO R/W threshold. Needed on H616. */ > > + writel(SUNXI_MMC_THLDC_READ_THLD(512) | > > + SUNXI_MMC_THLDC_WRITE_EN | > > + SUNXI_MMC_THLDC_READ_EN, ®s->thldc); > > + } > > +} > > + > > /* non-DM code here is used by the (ARM) SPL only */ > > > > #if !CONFIG_IS_ENABLED(DM_MMC) > > @@ -489,9 +509,7 @@ static int sunxi_mmc_core_init(struct mmc *mmc) > > { > > struct sunxi_mmc_priv *priv = mmc->priv; > > > > - /* Reset controller */ > > - writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl); > > - udelay(1000); > > + sunxi_mmc_reset(priv->reg); > > > > return 0; > > } > > @@ -684,9 +702,7 @@ static int sunxi_mmc_probe(struct udevice *dev) > > > > upriv->mmc = &plat->mmc; > > > > - /* Reset controller */ > > - writel(SUNXI_MMC_GCTRL_RESET, &priv->reg->gctrl); > > - udelay(1000); > > + sunxi_mmc_reset(priv->reg); > > > > return 0; > > } > > diff --git a/drivers/mmc/sunxi_mmc.h b/drivers/mmc/sunxi_mmc.h > > index f4ae5a790c87..9d55904c213c 100644 > > --- a/drivers/mmc/sunxi_mmc.h > > +++ b/drivers/mmc/sunxi_mmc.h > > @@ -37,7 +37,9 @@ struct sunxi_mmc { > > u32 res0; /* 0x54 reserved */ > > u32 a12a; /* 0x58 Auto command 12 argument */ > > u32 ntsr; /* 0x5c New timing set register */ > > - u32 res1[8]; > > + u32 res1[6]; > > + u32 hwrst; /* 0x78 Hardware Reset */ > > + u32 res5; > > u32 dmac; /* 0x80 internal DMA control */ > > u32 dlba; /* 0x84 internal DMA descr list base address */ > > u32 idst; /* 0x88 internal DMA status */ > > @@ -46,7 +48,8 @@ struct sunxi_mmc { > > u32 cbda; /* 0x94 */ > > u32 res2[26]; > > #if defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_SUN50I_GEN_H6) || > > defined(CONFIG_SUNXI_GEN_NCAT2) > > - u32 res3[17]; > > + u32 thldc; /* 0x100 Threshold control */ That being in an #ifdef triggers a build failure for older SoCs (A10), as reported by the CI. You did the right thing by using IS_ENABLED() above, but this means the thldc member name is still visible to the compiler in the .c file. Another reason for not describing register frames in a struct ;-) There are like 18 registers used, so rewriting that is a bit of churn. Not sure we want to do that now, or just #define SUNXI_MMC_THLDC_OFFSET and use that in the .c file, as an interim measure? Because I would really like to take this patch rather sooner than later. Cheers, Andre > > + u32 res3[16]; > > u32 samp_dl; > > u32 res4[46]; > > #endif > > @@ -123,6 +126,9 @@ struct sunxi_mmc { > > > > #define SUNXI_MMC_NTSR_MODE_SEL_NEW (0x1 << 31) > > > > +#define SUNXI_MMC_HWRST_ASSERT (0x0 << 0) > > +#define SUNXI_MMC_HWRST_DEASSERT (0x1 << 0) > > + > > #define SUNXI_MMC_IDMAC_RESET (0x1 << 0) > > #define SUNXI_MMC_IDMAC_FIXBURST (0x1 << 1) > > #define SUNXI_MMC_IDMAC_ENABLE (0x1 << 7) > > @@ -133,6 +139,11 @@ struct sunxi_mmc { > > #define SUNXI_MMC_COMMON_CLK_GATE (1 << 16) > > #define SUNXI_MMC_COMMON_RESET (1 << 18) > > > > +#define SUNXI_MMC_THLDC_READ_EN (0x1 << 0) > > +#define SUNXI_MMC_THLDC_BSY_CLR_INT_EN (0x1 << 1) > > +#define SUNXI_MMC_THLDC_WRITE_EN (0x1 << 2) > > +#define SUNXI_MMC_THLDC_READ_THLD(x) (((x) & 0xfff) << 16) > > + > > #define SUNXI_MMC_CAL_DL_SW_EN (0x1 << 7) > > > > #endif /* _SUNXI_MMC_H */ > >
