| Message ID | 20241030-imx-emmc-reset-v2-0-b3a823393974@solid-run.com |
|---|---|
| Headers | show |
| Series | mmc: host: sdhci-esdhc-imx: implement emmc hardware reset | expand |
Am 30.10.24 um 14:37 schrieb Josua Mayer: > NXP ESDHC supports setting data timeout using uSDHCx_SYS_CTRL register > DTOCV bits (bits 16-19). > Currently the driver accesses those bits by 32-bit write using > SDHCI_TIMEOUT_CONTROL (0x2E) defined in drivers/mmc/host/sdhci.h. > This is offset by two bytes relative to uSDHCx_SYS_CTRL (0x2C). > The driver also defines ESDHC_SYS_CTRL_DTOCV_MASK as first 4 bits, which > is correct relative to SDHCI_TIMEOUT_CONTROL but not relative to > uSDHCx_SYS_CTRL. The definition carrying control register in its name is > therefore inconsistent. > > Update the bitmask definition for bits 16-19 to be correct relative to > control register base. > Update the esdhc_set_timeout function to set timeout value at control > register base, not timeout offset. > > This solves a purely cosmetic problem. > > Signed-off-by: Josua Mayer <josua@solid-run.com> > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index a830d9a9490408d3148b927bf1acc719a13e8975..101feabb24fb41bd10a2e796f4f3f8d4357dc245 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -30,10 +30,10 @@ > #include "sdhci-esdhc.h" > #include "cqhci.h" > > -#define ESDHC_SYS_CTRL_DTOCV_MASK 0x0f > #define ESDHC_CTRL_D3CD 0x08 > #define ESDHC_BURST_LEN_EN_INCR (1 << 27) > #define ESDHC_SYS_CTRL 0x2c > +#define ESDHC_SYS_CTRL_DTOCV_MASK GENMASK(19, 16) > #define ESDHC_SYS_CTRL_IPP_RST_N BIT(23) > /* VENDOR SPEC register */ > #define ESDHC_VENDOR_SPEC 0xc0 > @@ -1387,7 +1387,7 @@ static void esdhc_set_timeout(struct sdhci_host *host, struct mmc_command *cmd) > > /* use maximum timeout counter */ > esdhc_clrset_le(host, ESDHC_SYS_CTRL_DTOCV_MASK, > - esdhc_is_usdhc(imx_data) ? 0xF : 0xE, > + esdhc_is_usdhc(imx_data) ? 0x0F0000 : 0x0E0000, > SDHCI_TIMEOUT_CONTROL); ^^ There is a mistake here, intended: ESDHC_SYS_CTRL (I changed that last second while writing commit description :( ) > } > >
> -----Original Message----- > From: Josua Mayer <josua@solid-run.com> > Sent: 2024年10月30日 21:38 > To: Adrian Hunter <adrian.hunter@intel.com>; Bough Chen > <haibo.chen@nxp.com>; Ulf Hansson <ulf.hansson@linaro.org>; Shawn Guo > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > <festevam@gmail.com> > Cc: Mikhail Anikin <mikhail.anikin@solid-run.com>; Jon Nettleton > <jon@solid-run.com>; yazan.shhady <yazan.shhady@solid-run.com>; Rabeeh > Khoury <rabeeh@solid-run.com>; imx@lists.linux.dev; > linux-mmc@vger.kernel.org; dl-S32 <S32@nxp.com>; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Josua > Mayer <josua@solid-run.com> > Subject: [PATCH v2 1/2] mmc: host: sdhci-esdhc-imx: implement emmc > hardware reset > > NXP ESDHC supports control of native emmc reset signal when pinmux is set > accordingly, using uSDHCx_SYS_CTRL register IPP_RST_N bit. > Documentation is available in NXP i.MX6Q Reference Manual. > > Implement the hw_reset function in sdhci_ops asserting reset for at least 1us > and waiting at least 200us after deassertion. > Lower bounds are based on: > JEDEC Standard No. 84-B51, 6.15.10 H/W Reset Operation, page 159. > Upper bounds are chosen allowing flexibility to the scheduler. > > Tested on SolidRun i.MX8DXL SoM with a scope, and confirmed that eMMC is > still accessible after boot: > - eMMC extcsd has RST_N_FUNCTION=0x01 > - sdhc node has cap-mmc-hw-reset > - pinmux set for EMMC0_RESET_B > - Linux v5.15 > > Signed-off-by: Josua Mayer <josua@solid-run.com> > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > b/drivers/mmc/host/sdhci-esdhc-imx.c > index > 8f0bc6dca2b0402fd2a0695903cf261a5b4e19dc..a830d9a9490408d3148b927bf > 1acc719a13e8975 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -33,6 +33,8 @@ > #define ESDHC_SYS_CTRL_DTOCV_MASK 0x0f > #define ESDHC_CTRL_D3CD 0x08 > #define ESDHC_BURST_LEN_EN_INCR (1 << 27) > +#define ESDHC_SYS_CTRL 0x2c No need this definition, already done in drivers/mmc/host/sdhci-esdhc.h #define ESDHC_SYSTEM_CONTROL 0x2c Regards Haibo Chen > +#define ESDHC_SYS_CTRL_IPP_RST_N BIT(23) > /* VENDOR SPEC register */ > #define ESDHC_VENDOR_SPEC 0xc0 > #define ESDHC_VENDOR_SPEC_SDIO_QUIRK (1 << 1) > @@ -1402,6 +1404,17 @@ static u32 esdhc_cqhci_irq(struct sdhci_host *host, > u32 intmask) > return 0; > } > > +static void esdhc_hw_reset(struct sdhci_host *host) { > + esdhc_clrset_le(host, ESDHC_SYS_CTRL_IPP_RST_N, 0, ESDHC_SYS_CTRL); > + /* eMMC spec requires minimum 1us, here delay between 1-10us */ > + usleep_range(1, 10); > + esdhc_clrset_le(host, ESDHC_SYS_CTRL_IPP_RST_N, > + ESDHC_SYS_CTRL_IPP_RST_N, ESDHC_SYS_CTRL); > + /* eMMC spec requires minimum 200us, here delay between 200-300us */ > + usleep_range(200, 300); > +} > + > static struct sdhci_ops sdhci_esdhc_ops = { > .read_l = esdhc_readl_le, > .read_w = esdhc_readw_le, > @@ -1420,6 +1433,7 @@ static struct sdhci_ops sdhci_esdhc_ops = { > .reset = esdhc_reset, > .irq = esdhc_cqhci_irq, > .dump_vendor_regs = esdhc_dump_debug_regs, > + .hw_reset = esdhc_hw_reset, > }; > > static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = { > > -- > 2.43.0
> -----Original Message----- > From: Josua Mayer <josua@solid-run.com> > Sent: 2024年10月30日 21:44 > To: Adrian Hunter <adrian.hunter@intel.com>; Bough Chen > <haibo.chen@nxp.com>; Ulf Hansson <ulf.hansson@linaro.org>; Shawn Guo > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > <festevam@gmail.com> > Cc: Mikhail Anikin <mikhail.anikin@solid-run.com>; Jon Nettleton > <jon@solid-run.com>; yazan.shhady <yazan.shhady@solid-run.com>; Rabeeh > Khoury <rabeeh@solid-run.com>; imx@lists.linux.dev; > linux-mmc@vger.kernel.org; dl-S32 <S32@nxp.com>; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 2/2] mmc: host: sdhci-esdhc-imx: update esdhc sysctl > dtocv bitmask > > Am 30.10.24 um 14:37 schrieb Josua Mayer: > > NXP ESDHC supports setting data timeout using uSDHCx_SYS_CTRL register > > DTOCV bits (bits 16-19). > > Currently the driver accesses those bits by 32-bit write using > > SDHCI_TIMEOUT_CONTROL (0x2E) defined in drivers/mmc/host/sdhci.h. > > This is offset by two bytes relative to uSDHCx_SYS_CTRL (0x2C). > > The driver also defines ESDHC_SYS_CTRL_DTOCV_MASK as first 4 bits, > > which is correct relative to SDHCI_TIMEOUT_CONTROL but not relative to > > uSDHCx_SYS_CTRL. The definition carrying control register in its name > > is therefore inconsistent. > > > > Update the bitmask definition for bits 16-19 to be correct relative to > > control register base. > > Update the esdhc_set_timeout function to set timeout value at control > > register base, not timeout offset. > > > > This solves a purely cosmetic problem. > > > > Signed-off-by: Josua Mayer <josua@solid-run.com> > > --- > > drivers/mmc/host/sdhci-esdhc-imx.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > > b/drivers/mmc/host/sdhci-esdhc-imx.c > > index > > > a830d9a9490408d3148b927bf1acc719a13e8975..101feabb24fb41bd10a2e796f > 4f3 > > f8d4357dc245 100644 > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > > @@ -30,10 +30,10 @@ > > #include "sdhci-esdhc.h" > > #include "cqhci.h" > > > > -#define ESDHC_SYS_CTRL_DTOCV_MASK 0x0f > > #define ESDHC_CTRL_D3CD 0x08 > > #define ESDHC_BURST_LEN_EN_INCR (1 << 27) > > #define ESDHC_SYS_CTRL 0x2c > > +#define ESDHC_SYS_CTRL_DTOCV_MASK GENMASK(19, 16) > > #define ESDHC_SYS_CTRL_IPP_RST_N BIT(23) > > /* VENDOR SPEC register */ > > #define ESDHC_VENDOR_SPEC 0xc0 > > @@ -1387,7 +1387,7 @@ static void esdhc_set_timeout(struct sdhci_host > > *host, struct mmc_command *cmd) > > > > /* use maximum timeout counter */ > > esdhc_clrset_le(host, ESDHC_SYS_CTRL_DTOCV_MASK, > > - esdhc_is_usdhc(imx_data) ? 0xF : 0xE, > > + esdhc_is_usdhc(imx_data) ? 0x0F0000 : 0x0E0000, > > SDHCI_TIMEOUT_CONTROL); > ^^ There is a mistake here, intended: ESDHC_SYS_CTRL (I changed that last > second while writing commit description :( ) Yes, please fix this. Regards Haibo Chen > > } > > > >
Signed-off-by: Josua Mayer <josua@solid-run.com> --- Changes in v2: - replaced udelay with usleep_range (Reported-by: Adrian Hunter <adrian.hunter@intel.com>) - added comments for delay values (Reported-by: Peng Fan <peng.fan@nxp.com>) - delay values based on JEDEC Standard No. 84-B51, 6.15.10 H/W Reset Operation, on page 159 (Thanks to Bough Chen <haibo.chen@nxp.com>) - added a second patch demonstrating a cosmetic issue revealed by first patch - it bothered me during development but is not important - Link to v1: https://lore.kernel.org/r/20241027-imx-emmc-reset-v1-1-d5d0c672864a@solid-run.com --- Josua Mayer (2): mmc: host: sdhci-esdhc-imx: implement emmc hardware reset mmc: host: sdhci-esdhc-imx: update esdhc sysctl dtocv bitmask drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) --- base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc change-id: 20241027-imx-emmc-reset-7127d311174c Best regards,