[PATCH 0/5] Prepare sdhci-omap to support more SoCs

[PATCH 0/5] Prepare sdhci-omap to support more SoCs

[Tony Lindgren]

Hi,

Here are changes to prepare to add support for more SoCs to start
deprecating the old omap_hsmmc driver. I'll send another series
of changes to add support for more SoCs.

Regards,

Tony


Tony Lindgren (5):
  mmc: sdhci-omap: Fix NULL pointer exception if regulator is not
    configured
  mmc: sdhci-omap: Fix context restore
  mmc: sdhci-omap: Restore sysconfig after reset
  mmc: sdhci-omap: Parse legacy ti,non-removable property
  mmc: sdhci-omap: Check MMCHS_HL_HWINFO register for ADMA

 drivers/mmc/host/sdhci-omap.c | 61 ++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 5 deletions(-)

-- 
2.33.0

[PATCH 1/5] mmc: sdhci-omap: Fix NULL pointer exception if regulator is not configured

[Tony Lindgren]

If sdhci-omap is configured for an unused device instance and the device
is not set as disabled, we can get a NULL pointer dereference:

Unable to handle kernel NULL pointer dereference at virtual address
00000045
...
(regulator_set_voltage) from [<c07d7008>] (mmc_regulator_set_ocr+0x44/0xd0)
(mmc_regulator_set_ocr) from [<c07e2d80>] (sdhci_set_ios+0xa4/0x490)
(sdhci_set_ios) from [<c07ea690>] (sdhci_omap_set_ios+0x124/0x160)
(sdhci_omap_set_ios) from [<c07c8e94>] (mmc_power_up.part.0+0x3c/0x154)
(mmc_power_up.part.0) from [<c07c9d20>] (mmc_start_host+0x88/0x9c)
(mmc_start_host) from [<c07cad34>] (mmc_add_host+0x58/0x7c)
(mmc_add_host) from [<c07e2574>] (__sdhci_add_host+0xf0/0x22c)
(__sdhci_add_host) from [<c07eaf68>] (sdhci_omap_probe+0x318/0x72c)
(sdhci_omap_probe) from [<c06a39d8>] (platform_probe+0x58/0xb8)

AFAIK we are not seeing this with the devices configured in the mainline
kernel but this can cause issues for folks bringing up their boards.

Fixes: 7d326930d352 ("mmc: sdhci-omap: Add OMAP SDHCI driver")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mmc/host/sdhci-omap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -682,7 +682,8 @@ static void sdhci_omap_set_power(struct sdhci_host *host, unsigned char mode,
 {
 	struct mmc_host *mmc = host->mmc;
 
-	mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+	if (!IS_ERR(mmc->supply.vmmc))
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
 }
 
 static int sdhci_omap_enable_dma(struct sdhci_host *host)
-- 
2.33.0

[PATCH 2/5] mmc: sdhci-omap: Fix context restore

[Tony Lindgren]

We need to restore context in a specified order with HCTL set in two
phases. This is similar to what omap_hsmmc_context_restore() is doing.
Otherwise SDIO can stop working on resume.

And for PM runtime and SDIO cards, we need to also save SYSCTL, IE and
ISE.

This should not be a problem currently, and these patches can be applied
whenever suitable.

Fixes: ee0f309263a6 ("mmc: sdhci-omap: Add Support for Suspend/Resume")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mmc/host/sdhci-omap.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -62,6 +62,8 @@
 #define SDHCI_OMAP_IE		0x234
 #define INT_CC_EN		BIT(0)
 
+#define SDHCI_OMAP_ISE		0x238
+
 #define SDHCI_OMAP_AC12		0x23c
 #define AC12_V1V8_SIGEN		BIT(19)
 #define AC12_SCLK_SEL		BIT(23)
@@ -113,6 +115,8 @@ struct sdhci_omap_host {
 	u32			hctl;
 	u32			sysctl;
 	u32			capa;
+	u32			ie;
+	u32			ise;
 };
 
 static void sdhci_omap_start_clock(struct sdhci_omap_host *omap_host);
@@ -1245,14 +1249,23 @@ static void sdhci_omap_context_save(struct sdhci_omap_host *omap_host)
 {
 	omap_host->con = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON);
 	omap_host->hctl = sdhci_omap_readl(omap_host, SDHCI_OMAP_HCTL);
+	omap_host->sysctl = sdhci_omap_readl(omap_host, SDHCI_OMAP_SYSCTL);
 	omap_host->capa = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
+	omap_host->ie = sdhci_omap_readl(omap_host, SDHCI_OMAP_IE);
+	omap_host->ise = sdhci_omap_readl(omap_host, SDHCI_OMAP_ISE);
 }
 
+/* Order matters here, HCTL must be restored in two phases */
 static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
 {
-	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, omap_host->con);
 	sdhci_omap_writel(omap_host, SDHCI_OMAP_HCTL, omap_host->hctl);
 	sdhci_omap_writel(omap_host, SDHCI_OMAP_CAPA, omap_host->capa);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_HCTL, omap_host->hctl);
+
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_SYSCTL, omap_host->sysctl);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, omap_host->con);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_IE, omap_host->ie);
+	sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
 }
 
 static int __maybe_unused sdhci_omap_suspend(struct device *dev)
-- 
2.33.0

[PATCH 3/5] mmc: sdhci-omap: Restore sysconfig after reset

[Tony Lindgren]

The sysconfig register is managed in a generic way by PM runtime for us by
the interconnect target module layer code. SDHCI_RESET_ALL also resets the
target module configuration, so we need to restore sysconfig after reset.

Note that there is no need to save and restore sysconfig during PM runtime,
the PM runtime layer will do that for us.

Not sure if this issue is a problem with the current configurations, I
noticed the issue while adding support for older TI SoCs and testing with
wlcore SDIO wlan device.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mmc/host/sdhci-omap.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -21,6 +21,8 @@
 
 #include "sdhci-pltfm.h"
 
+#define SDHCI_OMAP_SYSCONFIG	0x110
+
 #define SDHCI_OMAP_CON		0x12c
 #define CON_DW8			BIT(5)
 #define CON_DMA_MASTER		BIT(20)
@@ -797,6 +799,11 @@ static void sdhci_omap_reset(struct sdhci_host *host, u8 mask)
 	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
 	unsigned long limit = MMC_TIMEOUT_US;
 	unsigned long i = 0;
+	u32 sysc;
+
+	/* Save target module sysconfig configured by SoC PM layer */
+	if (mask & SDHCI_RESET_ALL)
+		sysc = sdhci_omap_readl(omap_host, SDHCI_OMAP_SYSCONFIG);
 
 	/* Don't reset data lines during tuning operation */
 	if (omap_host->is_tuning)
@@ -816,10 +823,15 @@ static void sdhci_omap_reset(struct sdhci_host *host, u8 mask)
 			dev_err(mmc_dev(host->mmc),
 				"Timeout waiting on controller reset in %s\n",
 				__func__);
-		return;
+
+		goto restore_sysc;
 	}
 
 	sdhci_reset(host, mask);
+
+restore_sysc:
+	if (mask & SDHCI_RESET_ALL)
+		sdhci_omap_writel(omap_host, SDHCI_OMAP_SYSCONFIG, sysc);
 }
 
 #define CMD_ERR_MASK (SDHCI_INT_CRC | SDHCI_INT_END_BIT | SDHCI_INT_INDEX |\
-- 
2.33.0

[PATCH 4/5] mmc: sdhci-omap: Parse legacy ti,non-removable property

[Tony Lindgren]

We need to support the legacy ti,non-removable property too. Let's warn
about the legacy property and mark the device as non-removable.

Naturally all the mainline kernel devicetree files will get updated to use
the standard non-removable property with the sdhci-omap conversion. But we
also have folks updating their kernels with custom devicetree files that
we need to consider.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mmc/host/sdhci-omap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -1213,6 +1213,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	if (of_find_property(dev->of_node, "dmas", NULL))
 		sdhci_switch_external_dma(host, true);
 
+	if (device_property_read_bool(dev, "ti,non-removable")) {
+		dev_warn_once(dev, "using old ti,non-removable property\n");
+		mmc->caps |= MMC_CAP_NONREMOVABLE;
+	}
+
 	/* R1B responses is required to properly manage HW busy detection. */
 	mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
 
-- 
2.33.0

[PATCH 5/5] mmc: sdhci-omap: Check MMCHS_HL_HWINFO register for ADMA

[Tony Lindgren]

ADMA is only available on controller instances that are connected to the
L3 interconnect and are bus mastering capable.

As the MMCHS_HL_HWINFO is in the module registers before omap registers
and sdhci registers, and the omap registers and sdhci registers can be
at different offsets depending on the SoC, let's read MMCHS_HL_HWINFO
directly.

Let's also switch to using device_property_present() while at it.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mmc/host/sdhci-omap.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -692,6 +692,22 @@ static void sdhci_omap_set_power(struct sdhci_host *host, unsigned char mode,
 		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
 }
 
+/*
+ * MMCHS_HL_HWINFO has the MADMA_EN bit set if the controller instance
+ * is connected to L3 interconnect and is bus master capable. Note that
+ * the MMCHS_HL_HWINFO register is in the module registers before the
+ * omap registers and sdhci registers. The offset can vary for omap
+ * registers depending on the SoC. Do not use sdhci_omap_readl() here.
+ */
+static bool sdhci_omap_has_adma(struct sdhci_omap_host *omap_host, int offset)
+{
+	/* MMCHS_HL_HWINFO register is only available on omap4 and later */
+	if (offset < 0x200)
+		return false;
+
+	return readl(omap_host->base + 4) & 1;
+}
+
 static int sdhci_omap_enable_dma(struct sdhci_host *host)
 {
 	u32 reg;
@@ -1209,8 +1225,12 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 	host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning;
 	host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq;
 
-	/* Switch to external DMA only if there is the "dmas" property */
-	if (of_find_property(dev->of_node, "dmas", NULL))
+	/*
+	 * Switch to external DMA only if there is the "dmas" property and
+	 * ADMA is not available on the controller instance.
+	 */
+	if (device_property_present(dev, "dmas") &&
+	    !sdhci_omap_has_adma(omap_host, offset))
 		sdhci_switch_external_dma(host, true);
 
 	if (device_property_read_bool(dev, "ti,non-removable")) {
-- 
2.33.0

Re: [PATCH 4/5] mmc: sdhci-omap: Parse legacy ti,non-removable property

[Ulf Hansson]

On Tue, 21 Sept 2021 at 13:00, Tony Lindgren <tony@atomide.com> wrote:
>
> We need to support the legacy ti,non-removable property too. Let's warn
> about the legacy property and mark the device as non-removable.
>
> Naturally all the mainline kernel devicetree files will get updated to use
> the standard non-removable property with the sdhci-omap conversion. But we
> also have folks updating their kernels with custom devicetree files that
> we need to consider.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/mmc/host/sdhci-omap.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -1213,6 +1213,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>         if (of_find_property(dev->of_node, "dmas", NULL))
>                 sdhci_switch_external_dma(host, true);
>
> +       if (device_property_read_bool(dev, "ti,non-removable")) {
> +               dev_warn_once(dev, "using old ti,non-removable property\n");

Perhaps we should document this property for sdhci-omap and thus also
set it as deprecated. What do you think?

> +               mmc->caps |= MMC_CAP_NONREMOVABLE;
> +       }
> +
>         /* R1B responses is required to properly manage HW busy detection. */
>         mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
>
> --
> 2.33.0

Kind regards
Uffe

Re: [PATCH 0/5] Prepare sdhci-omap to support more SoCs

[Ulf Hansson]

On Tue, 21 Sept 2021 at 13:00, Tony Lindgren <tony@atomide.com> wrote:
>
> Hi,
>
> Here are changes to prepare to add support for more SoCs to start
> deprecating the old omap_hsmmc driver. I'll send another series
> of changes to add support for more SoCs.
>
> Regards,
>
> Tony
>
>
> Tony Lindgren (5):
>   mmc: sdhci-omap: Fix NULL pointer exception if regulator is not
>     configured
>   mmc: sdhci-omap: Fix context restore
>   mmc: sdhci-omap: Restore sysconfig after reset
>   mmc: sdhci-omap: Parse legacy ti,non-removable property
>   mmc: sdhci-omap: Check MMCHS_HL_HWINFO register for ADMA
>
>  drivers/mmc/host/sdhci-omap.c | 61 ++++++++++++++++++++++++++++++++---
>  1 file changed, 56 insertions(+), 5 deletions(-)
>
> --
> 2.33.0

Applied for next, thanks!

Please tell me, if there are any of these changes that you think
deserves to be tagged for stable kernels.

Kind regards
Uffe

Re: [PATCH 4/5] mmc: sdhci-omap: Parse legacy ti,non-removable property

[Tony Lindgren]

* Ulf Hansson <ulf.hansson@linaro.org> [210923 18:43]:
> On Tue, 21 Sept 2021 at 13:00, Tony Lindgren <tony@atomide.com> wrote:
> > --- a/drivers/mmc/host/sdhci-omap.c
> > +++ b/drivers/mmc/host/sdhci-omap.c
> > @@ -1213,6 +1213,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> >         if (of_find_property(dev->of_node, "dmas", NULL))
> >                 sdhci_switch_external_dma(host, true);
> >
> > +       if (device_property_read_bool(dev, "ti,non-removable")) {
> > +               dev_warn_once(dev, "using old ti,non-removable property\n");
> 
> Perhaps we should document this property for sdhci-omap and thus also
> set it as deprecated. What do you think?

Yes good idea, I'll send a patch for that.

Regards,

Tony

Re: [PATCH 0/5] Prepare sdhci-omap to support more SoCs

[Tony Lindgren]

* Ulf Hansson <ulf.hansson@linaro.org> [210923 18:51]:
> Applied for next, thanks!

Thanks!

> Please tell me, if there are any of these changes that you think
> deserves to be tagged for stable kernels.

As the issues have not been seen so far with mainline kernels, I
think the fixes tags will be enough and no stable tags are needed.

Regaqrds,

Tony