* [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc
@ 2021-10-12 10:37 Tony Lindgren
2021-10-12 10:37 ` [PATCH 1/6] dt-bindings: sdhci-omap: Update binding for legacy SoCs Tony Lindgren
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Tony Lindgren @ 2021-10-12 10:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring,
devicetree
Hi,
Here are v3 changes to add support to sdhci-omap for earlier SoCs so we
can start deprecating the old omap_hsmmc driver.
For most part these changes can be tested just by changing the old hsmmc
devicetree compatible value for the SoC as described in the first binding
patch. Then after some testing, I'll post patches to enable sdhci-omap
for all the omap variants instead of omap_hsmmc.
These patches are against current Linux next.
Regards,
Tony
Changes since v2:
- Fix up runtime PM issues and enable autosuspend based on comments
from Ulf
Changes since v1:
- Added Rob's ack for the binding changes
- Fix wakeirq assignment as noted by Grygorii
Tony Lindgren (6):
dt-bindings: sdhci-omap: Update binding for legacy SoCs
mmc: sdhci-omap: Handle voltages to add support omap4
mmc: sdhci-omap: Add omap_offset to support omap3 and earlier
mmc: sdhci-omap: Implement PM runtime functions
sdhci: omap: Enable aggressive PM
mmc: sdhci-omap: Configure optional wakeirq
.../devicetree/bindings/mmc/sdhci-omap.txt | 6 +-
drivers/mmc/host/sdhci-omap.c | 290 ++++++++++++++----
2 files changed, 236 insertions(+), 60 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/6] dt-bindings: sdhci-omap: Update binding for legacy SoCs
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
@ 2021-10-12 10:37 ` Tony Lindgren
2021-10-12 10:37 ` [PATCH 2/6] mmc: sdhci-omap: Handle voltages to add support omap4 Tony Lindgren
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Tony Lindgren @ 2021-10-12 10:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, devicetree,
Rob Herring
Let's add compatible values for the legacy SoCs so we can continue
deprecating omap_hsmmc in favor of sdhci-omap driver.
For omap5, we want to have a separate compatible from omap4 for the
additional features available on omap5. AFAIK ti81 can just use the
omap4 compatible.
Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Documentation/devicetree/bindings/mmc/sdhci-omap.txt | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
--- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
@@ -5,7 +5,11 @@ Refer to mmc.txt for standard MMC bindings.
For UHS devices which require tuning, the device tree should have a "cpu_thermal" node which maps to the appropriate thermal zone. This is used to get the temperature of the zone during tuning.
Required properties:
-- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
+- compatible: Should be "ti,omap2430-sdhci" for omap2430 controllers
+ Should be "ti,omap3-sdhci" for omap3 controllers
+ Should be "ti,omap4-sdhci" for omap4 and ti81 controllers
+ Should be "ti,omap5-sdhci" for omap5 controllers
+ Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
Should be "ti,k2g-sdhci" for K2G
Should be "ti,am335-sdhci" for am335x controllers
Should be "ti,am437-sdhci" for am437x controllers
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/6] mmc: sdhci-omap: Handle voltages to add support omap4
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
2021-10-12 10:37 ` [PATCH 1/6] dt-bindings: sdhci-omap: Update binding for legacy SoCs Tony Lindgren
@ 2021-10-12 10:37 ` Tony Lindgren
2021-10-12 14:16 ` kernel test robot
2021-10-12 10:37 ` [PATCH 3/6] mmc: sdhci-omap: Add omap_offset to support omap3 and earlier Tony Lindgren
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Tony Lindgren @ 2021-10-12 10:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring,
devicetree
In order to start deprecating the custom omap_hsmmc.c in favor of the
generic sdhci-omap driver, we need to add support for voltages for earlier
SoCs.
The PBIAS regulator on omap4 and earlier only supports nominal values of
1.8V and 3.0V, while omap5 and later support nominal values of 1.8V and
3.3V IO voltage.
This gets omap4/5 working with sdhci-omap driver.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/mmc/host/sdhci-omap.c | 124 ++++++++++++++++++++++++++--------
1 file changed, 96 insertions(+), 28 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
@@ -178,7 +178,7 @@ static int sdhci_omap_set_pbias(struct sdhci_omap_host *omap_host,
}
static int sdhci_omap_enable_iov(struct sdhci_omap_host *omap_host,
- unsigned int iov)
+ unsigned int iov_pbias)
{
int ret;
struct sdhci_host *host = omap_host->host;
@@ -189,14 +189,15 @@ static int sdhci_omap_enable_iov(struct sdhci_omap_host *omap_host,
return ret;
if (!IS_ERR(mmc->supply.vqmmc)) {
- ret = regulator_set_voltage(mmc->supply.vqmmc, iov, iov);
- if (ret) {
+ /* Pick the right voltage to allow 3.0V for 3.3V nominal PBIAS */
+ ret = mmc_regulator_set_vqmmc(mmc, &mmc->ios);
+ if (ret < 0) {
dev_err(mmc_dev(mmc), "vqmmc set voltage failed\n");
return ret;
}
}
- ret = sdhci_omap_set_pbias(omap_host, true, iov);
+ ret = sdhci_omap_set_pbias(omap_host, true, iov_pbias);
if (ret)
return ret;
@@ -206,16 +207,28 @@ static int sdhci_omap_enable_iov(struct sdhci_omap_host *omap_host,
static void sdhci_omap_conf_bus_power(struct sdhci_omap_host *omap_host,
unsigned char signal_voltage)
{
- u32 reg;
+ u32 reg, capa;
ktime_t timeout;
reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_HCTL);
reg &= ~HCTL_SDVS_MASK;
- if (signal_voltage == MMC_SIGNAL_VOLTAGE_330)
- reg |= HCTL_SDVS_33;
- else
+ switch (signal_voltage) {
+ case MMC_SIGNAL_VOLTAGE_330:
+ capa = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
+ if (capa & CAPA_VS33)
+ reg |= HCTL_SDVS_33;
+ else if (capa & CAPA_VS30)
+ reg |= HCTL_SDVS_30;
+ else
+ dev_warn(omap_host->dev, "misconfigured CAPA: %08x\n",
+ capa);
+ break;
+ case MMC_SIGNAL_VOLTAGE_180:
+ default:
reg |= HCTL_SDVS_18;
+ break;
+ }
sdhci_omap_writel(omap_host, SDHCI_OMAP_HCTL, reg);
@@ -533,28 +546,32 @@ static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc,
if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
- if (!(reg & CAPA_VS33))
+ if (!(reg & (CAPA_VS30 | CAPA_VS33)))
return -EOPNOTSUPP;
+ if (reg & CAPA_VS30)
+ iov = IOV_3V0;
+ else
+ iov = IOV_3V3;
+
sdhci_omap_conf_bus_power(omap_host, ios->signal_voltage);
reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
reg &= ~AC12_V1V8_SIGEN;
sdhci_omap_writel(omap_host, SDHCI_OMAP_AC12, reg);
- iov = IOV_3V3;
} else if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
if (!(reg & CAPA_VS18))
return -EOPNOTSUPP;
+ iov = IOV_1V8;
+
sdhci_omap_conf_bus_power(omap_host, ios->signal_voltage);
reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_AC12);
reg |= AC12_V1V8_SIGEN;
sdhci_omap_writel(omap_host, SDHCI_OMAP_AC12, reg);
-
- iov = IOV_1V8;
} else {
return -EOPNOTSUPP;
}
@@ -910,34 +927,73 @@ static struct sdhci_ops sdhci_omap_ops = {
.set_timeout = sdhci_omap_set_timeout,
};
-static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host)
+static unsigned int sdhci_omap_regulator_get_caps(struct device *dev,
+ const char *name)
{
- u32 reg;
- int ret = 0;
+ struct regulator *reg;
+ unsigned int caps = 0;
+
+ reg = regulator_get(dev, name);
+ if (IS_ERR(reg))
+ return ~0UL;
+
+ if (regulator_is_supported_voltage(reg, 1700000, 1950000))
+ caps |= SDHCI_CAN_VDD_180;
+ if (regulator_is_supported_voltage(reg, 2700000, 3150000))
+ caps |= SDHCI_CAN_VDD_300;
+ if (regulator_is_supported_voltage(reg, 3150000, 3600000))
+ caps |= SDHCI_CAN_VDD_330;
+
+ regulator_put(reg);
+
+ return caps;
+}
+
+static int sdhci_omap_set_capabilities(struct sdhci_host *host)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
struct device *dev = omap_host->dev;
- struct regulator *vqmmc;
+ const u32 mask = SDHCI_CAN_VDD_180 | SDHCI_CAN_VDD_300 | SDHCI_CAN_VDD_330;
+ unsigned int pbias, vqmmc, caps = 0;
+ u32 reg;
- vqmmc = regulator_get(dev, "vqmmc");
- if (IS_ERR(vqmmc)) {
- ret = PTR_ERR(vqmmc);
- goto reg_put;
- }
+ pbias = sdhci_omap_regulator_get_caps(dev, "pbias");
+ vqmmc = sdhci_omap_regulator_get_caps(dev, "vqmmc");
+ caps = pbias & vqmmc;
+
+ if (pbias != ~0UL && vqmmc == ~0UL)
+ dev_warn(dev, "vqmmc regulator missing for pbias\n");
+ else if (caps == ~0UL)
+ return 0;
+
+ /*
+ * Quirk handling to allow 3.0V vqmmc with a valid 3.3V PBIAS. This is
+ * needed for 3.0V ldo9_reg on omap5 at least.
+ */
+ if (pbias != ~0UL && (pbias & SDHCI_CAN_VDD_330) &&
+ (vqmmc & SDHCI_CAN_VDD_300))
+ caps |= SDHCI_CAN_VDD_330;
/* voltage capabilities might be set by boot loader, clear it */
reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
reg &= ~(CAPA_VS18 | CAPA_VS30 | CAPA_VS33);
- if (regulator_is_supported_voltage(vqmmc, IOV_3V3, IOV_3V3))
- reg |= CAPA_VS33;
- if (regulator_is_supported_voltage(vqmmc, IOV_1V8, IOV_1V8))
+ if (caps & SDHCI_CAN_VDD_180)
reg |= CAPA_VS18;
+ if (caps & SDHCI_CAN_VDD_300)
+ reg |= CAPA_VS30;
+
+ if (caps & SDHCI_CAN_VDD_330)
+ reg |= CAPA_VS33;
+
sdhci_omap_writel(omap_host, SDHCI_OMAP_CAPA, reg);
-reg_put:
- regulator_put(vqmmc);
+ host->caps &= ~mask;
+ host->caps |= caps;
- return ret;
+ return 0;
}
static const struct sdhci_pltfm_data sdhci_omap_pdata = {
@@ -953,6 +1009,16 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = {
.ops = &sdhci_omap_ops,
};
+static const struct sdhci_omap_data omap4_data = {
+ .offset = 0x200,
+ .flags = SDHCI_OMAP_SPECIAL_RESET,
+};
+
+static const struct sdhci_omap_data omap5_data = {
+ .offset = 0x200,
+ .flags = SDHCI_OMAP_SPECIAL_RESET,
+};
+
static const struct sdhci_omap_data k2g_data = {
.offset = 0x200,
};
@@ -973,6 +1039,8 @@ static const struct sdhci_omap_data dra7_data = {
};
static const struct of_device_id omap_sdhci_match[] = {
+ { .compatible = "ti,omap4-sdhci", .data = &omap4_data },
+ { .compatible = "ti,omap5-sdhci", .data = &omap5_data },
{ .compatible = "ti,dra7-sdhci", .data = &dra7_data },
{ .compatible = "ti,k2g-sdhci", .data = &k2g_data },
{ .compatible = "ti,am335-sdhci", .data = &am335_data },
@@ -1212,7 +1280,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
goto err_rpm_disable;
}
- ret = sdhci_omap_set_capabilities(omap_host);
+ ret = sdhci_omap_set_capabilities(host);
if (ret) {
dev_err(dev, "failed to set system capabilities\n");
goto err_put_sync;
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/6] mmc: sdhci-omap: Add omap_offset to support omap3 and earlier
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
2021-10-12 10:37 ` [PATCH 1/6] dt-bindings: sdhci-omap: Update binding for legacy SoCs Tony Lindgren
2021-10-12 10:37 ` [PATCH 2/6] mmc: sdhci-omap: Handle voltages to add support omap4 Tony Lindgren
@ 2021-10-12 10:37 ` Tony Lindgren
2021-10-12 10:37 ` [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Tony Lindgren
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Tony Lindgren @ 2021-10-12 10:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring,
devicetree
The omap specific registers are at offset 0x100 from base for omap4 and
later, and for omap3 and earlier they are at offset 0. Let's handle also
the earlier SoCs by adding omap_offset.
Note that eventually we should just move to using standard sdhci register
access for the sdhci range with new offsets starting at 0x100.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/mmc/host/sdhci-omap.c | 61 ++++++++++++++++++++++++++---------
1 file changed, 45 insertions(+), 16 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
@@ -21,9 +21,14 @@
#include "sdhci-pltfm.h"
-#define SDHCI_OMAP_SYSCONFIG 0x110
+/*
+ * Note that the register offsets used here are from omap_regs
+ * base which is 0x100 for omap4 and later, and 0 for omap3 and
+ * earlier.
+ */
+#define SDHCI_OMAP_SYSCONFIG 0x10
-#define SDHCI_OMAP_CON 0x12c
+#define SDHCI_OMAP_CON 0x2c
#define CON_DW8 BIT(5)
#define CON_DMA_MASTER BIT(20)
#define CON_DDR BIT(19)
@@ -33,20 +38,20 @@
#define CON_INIT BIT(1)
#define CON_OD BIT(0)
-#define SDHCI_OMAP_DLL 0x0134
+#define SDHCI_OMAP_DLL 0x34
#define DLL_SWT BIT(20)
#define DLL_FORCE_SR_C_SHIFT 13
#define DLL_FORCE_SR_C_MASK (0x7f << DLL_FORCE_SR_C_SHIFT)
#define DLL_FORCE_VALUE BIT(12)
#define DLL_CALIB BIT(1)
-#define SDHCI_OMAP_CMD 0x20c
+#define SDHCI_OMAP_CMD 0x10c
-#define SDHCI_OMAP_PSTATE 0x0224
+#define SDHCI_OMAP_PSTATE 0x124
#define PSTATE_DLEV_DAT0 BIT(20)
#define PSTATE_DATI BIT(1)
-#define SDHCI_OMAP_HCTL 0x228
+#define SDHCI_OMAP_HCTL 0x128
#define HCTL_SDBP BIT(8)
#define HCTL_SDVS_SHIFT 9
#define HCTL_SDVS_MASK (0x7 << HCTL_SDVS_SHIFT)
@@ -54,28 +59,28 @@
#define HCTL_SDVS_30 (0x6 << HCTL_SDVS_SHIFT)
#define HCTL_SDVS_18 (0x5 << HCTL_SDVS_SHIFT)
-#define SDHCI_OMAP_SYSCTL 0x22c
+#define SDHCI_OMAP_SYSCTL 0x12c
#define SYSCTL_CEN BIT(2)
#define SYSCTL_CLKD_SHIFT 6
#define SYSCTL_CLKD_MASK 0x3ff
-#define SDHCI_OMAP_STAT 0x230
+#define SDHCI_OMAP_STAT 0x130
-#define SDHCI_OMAP_IE 0x234
+#define SDHCI_OMAP_IE 0x134
#define INT_CC_EN BIT(0)
-#define SDHCI_OMAP_ISE 0x238
+#define SDHCI_OMAP_ISE 0x138
-#define SDHCI_OMAP_AC12 0x23c
+#define SDHCI_OMAP_AC12 0x13c
#define AC12_V1V8_SIGEN BIT(19)
#define AC12_SCLK_SEL BIT(23)
-#define SDHCI_OMAP_CAPA 0x240
+#define SDHCI_OMAP_CAPA 0x140
#define CAPA_VS33 BIT(24)
#define CAPA_VS30 BIT(25)
#define CAPA_VS18 BIT(26)
-#define SDHCI_OMAP_CAPA2 0x0244
+#define SDHCI_OMAP_CAPA2 0x144
#define CAPA2_TSDR50 BIT(13)
#define SDHCI_OMAP_TIMEOUT 1 /* 1 msec */
@@ -93,7 +98,8 @@
#define SDHCI_OMAP_SPECIAL_RESET BIT(1)
struct sdhci_omap_data {
- u32 offset;
+ int omap_offset; /* Offset for omap regs from base */
+ u32 offset; /* Offset for SDHCI regs from base */
u8 flags;
};
@@ -112,6 +118,10 @@ struct sdhci_omap_host {
struct pinctrl *pinctrl;
struct pinctrl_state **pinctrl_state;
bool is_tuning;
+
+ /* Offset for omap specific registers from base */
+ int omap_offset;
+
/* Omap specific context save */
u32 con;
u32 hctl;
@@ -127,13 +137,13 @@ static void sdhci_omap_stop_clock(struct sdhci_omap_host *omap_host);
static inline u32 sdhci_omap_readl(struct sdhci_omap_host *host,
unsigned int offset)
{
- return readl(host->base + offset);
+ return readl(host->base + host->omap_offset + offset);
}
static inline void sdhci_omap_writel(struct sdhci_omap_host *host,
unsigned int offset, u32 data)
{
- writel(data, host->base + offset);
+ writel(data, host->base + host->omap_offset + offset);
}
static int sdhci_omap_set_pbias(struct sdhci_omap_host *omap_host,
@@ -1009,36 +1019,54 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = {
.ops = &sdhci_omap_ops,
};
+static const struct sdhci_omap_data omap2430_data = {
+ .omap_offset = 0,
+ .offset = 0x100,
+};
+
+static const struct sdhci_omap_data omap3_data = {
+ .omap_offset = 0,
+ .offset = 0x100,
+};
+
static const struct sdhci_omap_data omap4_data = {
+ .omap_offset = 0x100,
.offset = 0x200,
.flags = SDHCI_OMAP_SPECIAL_RESET,
};
static const struct sdhci_omap_data omap5_data = {
+ .omap_offset = 0x100,
.offset = 0x200,
.flags = SDHCI_OMAP_SPECIAL_RESET,
};
static const struct sdhci_omap_data k2g_data = {
+ .omap_offset = 0x100,
.offset = 0x200,
};
static const struct sdhci_omap_data am335_data = {
+ .omap_offset = 0x100,
.offset = 0x200,
.flags = SDHCI_OMAP_SPECIAL_RESET,
};
static const struct sdhci_omap_data am437_data = {
+ .omap_offset = 0x100,
.offset = 0x200,
.flags = SDHCI_OMAP_SPECIAL_RESET,
};
static const struct sdhci_omap_data dra7_data = {
+ .omap_offset = 0x100,
.offset = 0x200,
.flags = SDHCI_OMAP_REQUIRE_IODELAY,
};
static const struct of_device_id omap_sdhci_match[] = {
+ { .compatible = "ti,omap2430-sdhci", .data = &omap2430_data },
+ { .compatible = "ti,omap3-sdhci", .data = &omap3_data },
{ .compatible = "ti,omap4-sdhci", .data = &omap4_data },
{ .compatible = "ti,omap5-sdhci", .data = &omap5_data },
{ .compatible = "ti,dra7-sdhci", .data = &dra7_data },
@@ -1223,6 +1251,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
omap_host->power_mode = MMC_POWER_UNDEFINED;
omap_host->timing = MMC_TIMING_LEGACY;
omap_host->flags = data->flags;
+ omap_host->omap_offset = data->omap_offset;
host->ioaddr += offset;
host->mapbase = regs->start + offset;
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
` (2 preceding siblings ...)
2021-10-12 10:37 ` [PATCH 3/6] mmc: sdhci-omap: Add omap_offset to support omap3 and earlier Tony Lindgren
@ 2021-10-12 10:37 ` Tony Lindgren
2021-10-12 15:11 ` kernel test robot
2021-10-12 15:16 ` Ulf Hansson
2021-10-12 10:37 ` [PATCH 5/6] sdhci: omap: Enable aggressive PM Tony Lindgren
2021-10-12 10:37 ` [PATCH 6/6] mmc: sdhci-omap: Configure optional wakeirq Tony Lindgren
5 siblings, 2 replies; 13+ messages in thread
From: Tony Lindgren @ 2021-10-12 10:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring,
devicetree
Implement PM runtime functions and enable autosuspend.
Note that we save context in probe to avoid restoring invalid context
on the first resume. For system suspend, we have the new PM runtime
functions do most of the work.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/mmc/host/sdhci-omap.c | 78 ++++++++++++++++++++++++++++-------
1 file changed, 63 insertions(+), 15 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
@@ -1207,6 +1207,8 @@ static const struct soc_device_attribute sdhci_omap_soc_devices[] = {
}
};
+static void sdhci_omap_context_save(struct sdhci_omap_host *omap_host);
+
static int sdhci_omap_probe(struct platform_device *pdev)
{
int ret;
@@ -1252,6 +1254,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
omap_host->timing = MMC_TIMING_LEGACY;
omap_host->flags = data->flags;
omap_host->omap_offset = data->omap_offset;
+ omap_host->con = -EINVAL;
host->ioaddr += offset;
host->mapbase = regs->start + offset;
@@ -1302,6 +1305,8 @@ static int sdhci_omap_probe(struct platform_device *pdev)
* SYSCONFIG register of omap devices. The callback will be invoked
* as part of pm_runtime_get_sync.
*/
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 50);
pm_runtime_enable(dev);
ret = pm_runtime_resume_and_get(dev);
if (ret) {
@@ -1312,7 +1317,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
ret = sdhci_omap_set_capabilities(host);
if (ret) {
dev_err(dev, "failed to set system capabilities\n");
- goto err_put_sync;
+ goto err_rpm_put;
}
host->mmc_host_ops.start_signal_voltage_switch =
@@ -1340,7 +1345,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
ret = sdhci_setup_host(host);
if (ret)
- goto err_put_sync;
+ goto err_rpm_put;
ret = sdhci_omap_config_iodelay_pinctrl_state(omap_host);
if (ret)
@@ -1350,15 +1355,21 @@ static int sdhci_omap_probe(struct platform_device *pdev)
if (ret)
goto err_cleanup_host;
+ sdhci_omap_context_save(omap_host);
+
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
return 0;
err_cleanup_host:
sdhci_cleanup_host(host);
-err_put_sync:
- pm_runtime_put_sync(dev);
-
+err_rpm_put:
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
err_rpm_disable:
+ pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);
err_pltfm_free:
@@ -1371,8 +1382,12 @@ static int sdhci_omap_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct sdhci_host *host = platform_get_drvdata(pdev);
+ pm_runtime_get_sync(dev);
sdhci_remove_host(host, true);
+ pm_runtime_dont_use_autosuspend(dev);
pm_runtime_put_sync(dev);
+ /* Ensure device gets idled despite userspace sysfs config */
+ pm_runtime_force_suspend(dev);
pm_runtime_disable(dev);
sdhci_pltfm_free(pdev);
@@ -1402,42 +1417,75 @@ static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
}
-static int __maybe_unused sdhci_omap_suspend(struct device *dev)
+static int __maybe_unused sdhci_omap_runtime_suspend(struct device *dev)
{
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
- sdhci_suspend_host(host);
+ sdhci_runtime_suspend_host(host);
sdhci_omap_context_save(omap_host);
pinctrl_pm_select_idle_state(dev);
- pm_runtime_force_suspend(dev);
-
return 0;
}
-static int __maybe_unused sdhci_omap_resume(struct device *dev)
+static int __maybe_unused sdhci_omap_runtime_resume(struct device *dev)
{
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
- pm_runtime_force_resume(dev);
-
pinctrl_pm_select_default_state(dev);
- sdhci_omap_context_restore(omap_host);
+ if (omap_host->con != -EINVAL)
+ sdhci_omap_context_restore(omap_host);
+
+ sdhci_runtime_resume_host(host, 0);
+
+ return 0;
+}
+
+static int __maybe_unused sdhci_omap_suspend(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+ int err;
+
+ /* Enable for configuring wakeups, paired in resume */
+ err = pm_runtime_resume_and_get(dev);
+ if (err < 0)
+ return err;
+
+ sdhci_suspend_host(host);
+
+ return pm_runtime_force_suspend(dev);
+}
+
+static int __maybe_unused sdhci_omap_resume(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+ int err;
+
+ err = pm_runtime_force_resume(dev);
+ if (err < 0)
+ dev_warn(dev, "force resume failed: %i\n", err);
sdhci_resume_host(host);
+ /* Balance pm_runtime_resume_and_get() done in suspend */
+ pm_runtime_put(dev);
+
return 0;
}
#endif
-static SIMPLE_DEV_PM_OPS(sdhci_omap_dev_pm_ops, sdhci_omap_suspend,
- sdhci_omap_resume);
+
+static const struct dev_pm_ops sdhci_omap_dev_pm_ops = {
+ SET_RUNTIME_PM_OPS(sdhci_omap_runtime_suspend,
+ sdhci_omap_runtime_resume, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(sdhci_omap_suspend, sdhci_omap_resume)
+};
static struct platform_driver sdhci_omap_driver = {
.probe = sdhci_omap_probe,
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/6] sdhci: omap: Enable aggressive PM
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
` (3 preceding siblings ...)
2021-10-12 10:37 ` [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Tony Lindgren
@ 2021-10-12 10:37 ` Tony Lindgren
2021-10-12 14:14 ` Ulf Hansson
2021-10-12 10:37 ` [PATCH 6/6] mmc: sdhci-omap: Configure optional wakeirq Tony Lindgren
5 siblings, 1 reply; 13+ messages in thread
From: Tony Lindgren @ 2021-10-12 10:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring,
devicetree
Enable runtime PM for eMMC/SD card devices. Without this, SDIO WLAN
devices will not idle.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/mmc/host/sdhci-omap.c | 3 +++
1 file changed, 3 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
@@ -1343,6 +1343,9 @@ static int sdhci_omap_probe(struct platform_device *pdev)
/* R1B responses is required to properly manage HW busy detection. */
mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
+ /* Enable runtime PM for eMMC/SD card devices */
+ mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
+
ret = sdhci_setup_host(host);
if (ret)
goto err_rpm_put;
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/6] mmc: sdhci-omap: Configure optional wakeirq
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
` (4 preceding siblings ...)
2021-10-12 10:37 ` [PATCH 5/6] sdhci: omap: Enable aggressive PM Tony Lindgren
@ 2021-10-12 10:37 ` Tony Lindgren
2021-10-12 15:02 ` Ulf Hansson
5 siblings, 1 reply; 13+ messages in thread
From: Tony Lindgren @ 2021-10-12 10:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring,
devicetree
Configure optional wakeirq. This may be optionally configured for SDIO
dat1 pin for wake-up events for SoCs that support deeper idle states.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/mmc/host/sdhci-omap.c | 24 ++++++++++++++++++++++++
1 file changed, 24 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
@@ -12,8 +12,10 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/pm_wakeirq.h>
#include <linux/regulator/consumer.h>
#include <linux/pinctrl/consumer.h>
#include <linux/sys_soc.h>
@@ -117,6 +119,7 @@ struct sdhci_omap_host {
struct pinctrl *pinctrl;
struct pinctrl_state **pinctrl_state;
+ int wakeirq;
bool is_tuning;
/* Offset for omap specific registers from base */
@@ -1360,6 +1363,25 @@ static int sdhci_omap_probe(struct platform_device *pdev)
sdhci_omap_context_save(omap_host);
+ /*
+ * SDIO devices can use the dat1 pin as a wake-up interrupt. Some
+ * devices like wl1xxx, use an out-of-band GPIO interrupt instead.
+ */
+ omap_host->wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+ if (omap_host->wakeirq == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_cleanup_host;
+ }
+ if (omap_host->wakeirq > 0) {
+ device_init_wakeup(dev, true);
+ ret = dev_pm_set_dedicated_wake_irq(dev, omap_host->wakeirq);
+ if (ret) {
+ device_init_wakeup(dev, false);
+ goto err_cleanup_host;
+ }
+ host->mmc->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
+ }
+
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
@@ -1387,6 +1409,8 @@ static int sdhci_omap_remove(struct platform_device *pdev)
pm_runtime_get_sync(dev);
sdhci_remove_host(host, true);
+ device_init_wakeup(dev, false);
+ dev_pm_clear_wake_irq(dev);
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_put_sync(dev);
/* Ensure device gets idled despite userspace sysfs config */
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/6] sdhci: omap: Enable aggressive PM
2021-10-12 10:37 ` [PATCH 5/6] sdhci: omap: Enable aggressive PM Tony Lindgren
@ 2021-10-12 14:14 ` Ulf Hansson
0 siblings, 0 replies; 13+ messages in thread
From: Ulf Hansson @ 2021-10-12 14:14 UTC (permalink / raw)
To: Tony Lindgren
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring, DTML
On Tue, 12 Oct 2021 at 12:38, Tony Lindgren <tony@atomide.com> wrote:
>
> Enable runtime PM for eMMC/SD card devices. Without this, SDIO WLAN
> devices will not idle.
Just wanted to add some clarification. If the platform is able to idle
SDIO card/func devices, you can set MMC_CAP_POWER_OFF_CARD.
MMC_CAP_AGGRESSIVE_PM is for eMMC and SD. Perhaps we should consider
merging the capabilities flags, as they are in principle indicating
the same thing. Anyway, that's a different story.
Kind regards
Uffe
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/mmc/host/sdhci-omap.c | 3 +++
> 1 file changed, 3 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
> @@ -1343,6 +1343,9 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> /* R1B responses is required to properly manage HW busy detection. */
> mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
>
> + /* Enable runtime PM for eMMC/SD card devices */
> + mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
> +
> ret = sdhci_setup_host(host);
> if (ret)
> goto err_rpm_put;
> --
> 2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/6] mmc: sdhci-omap: Handle voltages to add support omap4
2021-10-12 10:37 ` [PATCH 2/6] mmc: sdhci-omap: Handle voltages to add support omap4 Tony Lindgren
@ 2021-10-12 14:16 ` kernel test robot
0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-10-12 14:16 UTC (permalink / raw)
To: Tony Lindgren, Ulf Hansson
Cc: llvm, kbuild-all, Adrian Hunter, Chunyan Zhang, Faiz Abbas,
Kishon Vijay Abraham I, Santosh Shilimkar, linux-mmc, linux-omap,
Rob Herring, devicetree
[-- Attachment #1: Type: text/plain, Size: 5591 bytes --]
Hi Tony,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20211011]
[also build test WARNING on v5.15-rc5]
[cannot apply to robh/for-next linus/master ulf-hansson-mmc-mirror/next v5.15-rc5 v5.15-rc4 v5.15-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Tony-Lindgren/More-SoCs-for-sdhci-omap-to-deprecate-omap_hsmmc/20211012-183855
base: d3134eb5de8546a214c028fb7195e764b89da7d4
config: riscv-randconfig-r042-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c3dcf39554dbea780d6cb7e12239451ba47a2668)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/86b011af48d7f1cd6e2810dddcdf5d1b5ee819af
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tony-Lindgren/More-SoCs-for-sdhci-omap-to-deprecate-omap_hsmmc/20211012-183855
git checkout 86b011af48d7f1cd6e2810dddcdf5d1b5ee819af
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/mmc/host/sdhci-omap.c:938:10: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
return ~0UL;
~~~~~~ ^~~~
>> drivers/mmc/host/sdhci-omap.c:965:29: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
if (pbias != ~0UL && vqmmc == ~0UL)
~~~~~ ^ ~~~~
>> drivers/mmc/host/sdhci-omap.c:965:12: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Wtautological-constant-out-of-range-compare]
if (pbias != ~0UL && vqmmc == ~0UL)
~~~~~ ^ ~~~~
drivers/mmc/host/sdhci-omap.c:967:16: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
else if (caps == ~0UL)
~~~~ ^ ~~~~
drivers/mmc/host/sdhci-omap.c:974:12: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Wtautological-constant-out-of-range-compare]
if (pbias != ~0UL && (pbias & SDHCI_CAN_VDD_330) &&
~~~~~ ^ ~~~~
5 warnings generated.
vim +938 drivers/mmc/host/sdhci-omap.c
929
930 static unsigned int sdhci_omap_regulator_get_caps(struct device *dev,
931 const char *name)
932 {
933 struct regulator *reg;
934 unsigned int caps = 0;
935
936 reg = regulator_get(dev, name);
937 if (IS_ERR(reg))
> 938 return ~0UL;
939
940 if (regulator_is_supported_voltage(reg, 1700000, 1950000))
941 caps |= SDHCI_CAN_VDD_180;
942 if (regulator_is_supported_voltage(reg, 2700000, 3150000))
943 caps |= SDHCI_CAN_VDD_300;
944 if (regulator_is_supported_voltage(reg, 3150000, 3600000))
945 caps |= SDHCI_CAN_VDD_330;
946
947 regulator_put(reg);
948
949 return caps;
950 }
951
952 static int sdhci_omap_set_capabilities(struct sdhci_host *host)
953 {
954 struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
955 struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
956 struct device *dev = omap_host->dev;
957 const u32 mask = SDHCI_CAN_VDD_180 | SDHCI_CAN_VDD_300 | SDHCI_CAN_VDD_330;
958 unsigned int pbias, vqmmc, caps = 0;
959 u32 reg;
960
961 pbias = sdhci_omap_regulator_get_caps(dev, "pbias");
962 vqmmc = sdhci_omap_regulator_get_caps(dev, "vqmmc");
963 caps = pbias & vqmmc;
964
> 965 if (pbias != ~0UL && vqmmc == ~0UL)
966 dev_warn(dev, "vqmmc regulator missing for pbias\n");
967 else if (caps == ~0UL)
968 return 0;
969
970 /*
971 * Quirk handling to allow 3.0V vqmmc with a valid 3.3V PBIAS. This is
972 * needed for 3.0V ldo9_reg on omap5 at least.
973 */
974 if (pbias != ~0UL && (pbias & SDHCI_CAN_VDD_330) &&
975 (vqmmc & SDHCI_CAN_VDD_300))
976 caps |= SDHCI_CAN_VDD_330;
977
978 /* voltage capabilities might be set by boot loader, clear it */
979 reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CAPA);
980 reg &= ~(CAPA_VS18 | CAPA_VS30 | CAPA_VS33);
981
982 if (caps & SDHCI_CAN_VDD_180)
983 reg |= CAPA_VS18;
984
985 if (caps & SDHCI_CAN_VDD_300)
986 reg |= CAPA_VS30;
987
988 if (caps & SDHCI_CAN_VDD_330)
989 reg |= CAPA_VS33;
990
991 sdhci_omap_writel(omap_host, SDHCI_OMAP_CAPA, reg);
992
993 host->caps &= ~mask;
994 host->caps |= caps;
995
996 return 0;
997 }
998
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27114 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/6] mmc: sdhci-omap: Configure optional wakeirq
2021-10-12 10:37 ` [PATCH 6/6] mmc: sdhci-omap: Configure optional wakeirq Tony Lindgren
@ 2021-10-12 15:02 ` Ulf Hansson
0 siblings, 0 replies; 13+ messages in thread
From: Ulf Hansson @ 2021-10-12 15:02 UTC (permalink / raw)
To: Tony Lindgren
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring, DTML
On Tue, 12 Oct 2021 at 12:38, Tony Lindgren <tony@atomide.com> wrote:
>
> Configure optional wakeirq. This may be optionally configured for SDIO
> dat1 pin for wake-up events for SoCs that support deeper idle states.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/mmc/host/sdhci-omap.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 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
> @@ -12,8 +12,10 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/of_irq.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> +#include <linux/pm_wakeirq.h>
> #include <linux/regulator/consumer.h>
> #include <linux/pinctrl/consumer.h>
> #include <linux/sys_soc.h>
> @@ -117,6 +119,7 @@ struct sdhci_omap_host {
>
> struct pinctrl *pinctrl;
> struct pinctrl_state **pinctrl_state;
> + int wakeirq;
> bool is_tuning;
>
> /* Offset for omap specific registers from base */
> @@ -1360,6 +1363,25 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>
> sdhci_omap_context_save(omap_host);
>
> + /*
> + * SDIO devices can use the dat1 pin as a wake-up interrupt. Some
> + * devices like wl1xxx, use an out-of-band GPIO interrupt instead.
> + */
Ah, right I recall this now. Very clever.
> + omap_host->wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> + if (omap_host->wakeirq == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto err_cleanup_host;
> + }
> + if (omap_host->wakeirq > 0) {
> + device_init_wakeup(dev, true);
> + ret = dev_pm_set_dedicated_wake_irq(dev, omap_host->wakeirq);
> + if (ret) {
> + device_init_wakeup(dev, false);
> + goto err_cleanup_host;
> + }
> + host->mmc->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
To prevent the mmc core from powering off the SDIO card in system
suspend (which certainly must be prevented if wakeups should be
delivered), you need to set MMC_PM_KEEP_POWER, too.
FYI: We also have common mmc DT properties for these caps, which are
being parsed in mmc_of_parse().
These are the DT properties:
"keep-power-in-suspend" - > MMC_PM_KEEP_POWER
"wakeup-source" || "enable-sdio-wakeup" (/* legacy */) -> MMC_PM_WAKE_SDIO_IRQ;
> + }
> +
> pm_runtime_mark_last_busy(dev);
> pm_runtime_put_autosuspend(dev);
>
> @@ -1387,6 +1409,8 @@ static int sdhci_omap_remove(struct platform_device *pdev)
>
> pm_runtime_get_sync(dev);
> sdhci_remove_host(host, true);
> + device_init_wakeup(dev, false);
> + dev_pm_clear_wake_irq(dev);
> pm_runtime_dont_use_autosuspend(dev);
> pm_runtime_put_sync(dev);
> /* Ensure device gets idled despite userspace sysfs config */
> --
> 2.33.0
I now think I better understand what is needed during system
suspend/resume to support system wakeups. I will comment on patch 4,
let's see what that brings us to.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions
2021-10-12 10:37 ` [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Tony Lindgren
@ 2021-10-12 15:11 ` kernel test robot
2021-10-12 15:16 ` Ulf Hansson
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-10-12 15:11 UTC (permalink / raw)
To: Tony Lindgren, Ulf Hansson
Cc: llvm, kbuild-all, Adrian Hunter, Chunyan Zhang, Faiz Abbas,
Kishon Vijay Abraham I, Santosh Shilimkar, linux-mmc, linux-omap,
Rob Herring, devicetree
[-- Attachment #1: Type: text/plain, Size: 5857 bytes --]
Hi Tony,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20211011]
[cannot apply to robh/for-next linus/master ulf-hansson-mmc-mirror/next v5.15-rc5 v5.15-rc4 v5.15-rc3 v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Tony-Lindgren/More-SoCs-for-sdhci-omap-to-deprecate-omap_hsmmc/20211012-183855
base: d3134eb5de8546a214c028fb7195e764b89da7d4
config: riscv-randconfig-r042-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c3dcf39554dbea780d6cb7e12239451ba47a2668)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/66e8ed22a31746643373772ef2fca668ca7d1a8f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tony-Lindgren/More-SoCs-for-sdhci-omap-to-deprecate-omap_hsmmc/20211012-183855
git checkout 66e8ed22a31746643373772ef2fca668ca7d1a8f
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/mmc/host/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/mmc/host/sdhci-omap.c:948:10: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
return ~0UL;
~~~~~~ ^~~~
drivers/mmc/host/sdhci-omap.c:975:29: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
if (pbias != ~0UL && vqmmc == ~0UL)
~~~~~ ^ ~~~~
drivers/mmc/host/sdhci-omap.c:975:12: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Wtautological-constant-out-of-range-compare]
if (pbias != ~0UL && vqmmc == ~0UL)
~~~~~ ^ ~~~~
drivers/mmc/host/sdhci-omap.c:977:16: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
else if (caps == ~0UL)
~~~~ ^ ~~~~
drivers/mmc/host/sdhci-omap.c:984:12: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Wtautological-constant-out-of-range-compare]
if (pbias != ~0UL && (pbias & SDHCI_CAN_VDD_330) &&
~~~~~ ^ ~~~~
>> drivers/mmc/host/sdhci-omap.c:1485:21: error: use of undeclared identifier 'sdhci_omap_runtime_suspend'; did you mean '__pm_runtime_suspend'?
SET_RUNTIME_PM_OPS(sdhci_omap_runtime_suspend,
^~~~~~~~~~~~~~~~~~~~~~~~~~
__pm_runtime_suspend
include/linux/pm.h:341:21: note: expanded from macro 'SET_RUNTIME_PM_OPS'
.runtime_suspend = suspend_fn, \
^
include/linux/pm_runtime.h:39:12: note: '__pm_runtime_suspend' declared here
extern int __pm_runtime_suspend(struct device *dev, int rpmflags);
^
>> drivers/mmc/host/sdhci-omap.c:1486:7: error: use of undeclared identifier 'sdhci_omap_runtime_resume'; did you mean '__pm_runtime_resume'?
sdhci_omap_runtime_resume, NULL)
^~~~~~~~~~~~~~~~~~~~~~~~~
__pm_runtime_resume
include/linux/pm.h:342:20: note: expanded from macro 'SET_RUNTIME_PM_OPS'
.runtime_resume = resume_fn, \
^
include/linux/pm_runtime.h:40:12: note: '__pm_runtime_resume' declared here
extern int __pm_runtime_resume(struct device *dev, int rpmflags);
^
>> drivers/mmc/host/sdhci-omap.c:1485:21: error: incompatible function pointer types initializing 'int (*)(struct device *)' with an expression of type 'int (struct device *, int)' [-Werror,-Wincompatible-function-pointer-types]
SET_RUNTIME_PM_OPS(sdhci_omap_runtime_suspend,
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/pm.h:341:21: note: expanded from macro 'SET_RUNTIME_PM_OPS'
.runtime_suspend = suspend_fn, \
^~~~~~~~~~
drivers/mmc/host/sdhci-omap.c:1486:7: error: incompatible function pointer types initializing 'int (*)(struct device *)' with an expression of type 'int (struct device *, int)' [-Werror,-Wincompatible-function-pointer-types]
sdhci_omap_runtime_resume, NULL)
^~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/pm.h:342:20: note: expanded from macro 'SET_RUNTIME_PM_OPS'
.runtime_resume = resume_fn, \
^~~~~~~~~
5 warnings and 4 errors generated.
vim +1485 drivers/mmc/host/sdhci-omap.c
1483
1484 static const struct dev_pm_ops sdhci_omap_dev_pm_ops = {
> 1485 SET_RUNTIME_PM_OPS(sdhci_omap_runtime_suspend,
> 1486 sdhci_omap_runtime_resume, NULL)
1487 SET_SYSTEM_SLEEP_PM_OPS(sdhci_omap_suspend, sdhci_omap_resume)
1488 };
1489
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27114 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions
2021-10-12 10:37 ` [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Tony Lindgren
2021-10-12 15:11 ` kernel test robot
@ 2021-10-12 15:16 ` Ulf Hansson
2021-10-15 9:34 ` Tony Lindgren
1 sibling, 1 reply; 13+ messages in thread
From: Ulf Hansson @ 2021-10-12 15:16 UTC (permalink / raw)
To: Tony Lindgren
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring, DTML
On Tue, 12 Oct 2021 at 12:38, Tony Lindgren <tony@atomide.com> wrote:
>
> Implement PM runtime functions and enable autosuspend.
>
> Note that we save context in probe to avoid restoring invalid context
> on the first resume. For system suspend, we have the new PM runtime
> functions do most of the work.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/mmc/host/sdhci-omap.c | 78 ++++++++++++++++++++++++++++-------
> 1 file changed, 63 insertions(+), 15 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
> @@ -1207,6 +1207,8 @@ static const struct soc_device_attribute sdhci_omap_soc_devices[] = {
> }
> };
>
> +static void sdhci_omap_context_save(struct sdhci_omap_host *omap_host);
> +
> static int sdhci_omap_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -1252,6 +1254,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> omap_host->timing = MMC_TIMING_LEGACY;
> omap_host->flags = data->flags;
> omap_host->omap_offset = data->omap_offset;
> + omap_host->con = -EINVAL;
> host->ioaddr += offset;
> host->mapbase = regs->start + offset;
>
> @@ -1302,6 +1305,8 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> * SYSCONFIG register of omap devices. The callback will be invoked
> * as part of pm_runtime_get_sync.
> */
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_autosuspend_delay(dev, 50);
> pm_runtime_enable(dev);
> ret = pm_runtime_resume_and_get(dev);
> if (ret) {
> @@ -1312,7 +1317,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> ret = sdhci_omap_set_capabilities(host);
> if (ret) {
> dev_err(dev, "failed to set system capabilities\n");
> - goto err_put_sync;
> + goto err_rpm_put;
> }
>
> host->mmc_host_ops.start_signal_voltage_switch =
> @@ -1340,7 +1345,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>
> ret = sdhci_setup_host(host);
> if (ret)
> - goto err_put_sync;
> + goto err_rpm_put;
>
> ret = sdhci_omap_config_iodelay_pinctrl_state(omap_host);
> if (ret)
> @@ -1350,15 +1355,21 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> if (ret)
> goto err_cleanup_host;
>
> + sdhci_omap_context_save(omap_host);
Calling sdhci_omap_context_save() here looks unnecessary. The device
is already runtime resumed at this point.
In other words, sdhci_omap_context_save() will be called from the
->runtime_suspend() callback, next time the device becomes runtime
suspended. That should be sufficient, right?
> +
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> return 0;
>
> err_cleanup_host:
> sdhci_cleanup_host(host);
>
> -err_put_sync:
> - pm_runtime_put_sync(dev);
> -
> +err_rpm_put:
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> err_rpm_disable:
> + pm_runtime_dont_use_autosuspend(dev);
> pm_runtime_disable(dev);
>
> err_pltfm_free:
> @@ -1371,8 +1382,12 @@ static int sdhci_omap_remove(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct sdhci_host *host = platform_get_drvdata(pdev);
>
> + pm_runtime_get_sync(dev);
> sdhci_remove_host(host, true);
> + pm_runtime_dont_use_autosuspend(dev);
> pm_runtime_put_sync(dev);
> + /* Ensure device gets idled despite userspace sysfs config */
> + pm_runtime_force_suspend(dev);
> pm_runtime_disable(dev);
The call to pm_runtime_disable() can be removed, as that is taken care
of in pm_runtime_force_suspend().
> sdhci_pltfm_free(pdev);
>
> @@ -1402,42 +1417,75 @@ static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
> sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
> }
>
> -static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> +static int __maybe_unused sdhci_omap_runtime_suspend(struct device *dev)
> {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
>
> - sdhci_suspend_host(host);
> + sdhci_runtime_suspend_host(host);
>
> sdhci_omap_context_save(omap_host);
>
> pinctrl_pm_select_idle_state(dev);
>
> - pm_runtime_force_suspend(dev);
> -
> return 0;
> }
>
> -static int __maybe_unused sdhci_omap_resume(struct device *dev)
> +static int __maybe_unused sdhci_omap_runtime_resume(struct device *dev)
> {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
>
> - pm_runtime_force_resume(dev);
> -
> pinctrl_pm_select_default_state(dev);
>
> - sdhci_omap_context_restore(omap_host);
> + if (omap_host->con != -EINVAL)
> + sdhci_omap_context_restore(omap_host);
> +
> + sdhci_runtime_resume_host(host, 0);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> +{
> + struct sdhci_host *host = dev_get_drvdata(dev);
> + int err;
> +
> + /* Enable for configuring wakeups, paired in resume */
> + err = pm_runtime_resume_and_get(dev);
> + if (err < 0)
> + return err;
> +
> + sdhci_suspend_host(host);
As far as I can tell, sdhci_suspend_host() doesn't really make sense
for the omap variant. What you need, is to put the device into the
same low power state as "runtime suspend", that should be sufficient.
The system wakeup will be armed (and later then disarmed) by the PM
core, when it calls device_wakeup_arm_wake_irqs() from the
dpm_suspend_noirq() phase.
In other words, pointing the system suspend/resume callbacks to
pm_runtime_force_suspend|resume() should work fine, I think.
> +
> + return pm_runtime_force_suspend(dev);
> +}
> +
> +static int __maybe_unused sdhci_omap_resume(struct device *dev)
> +{
> + struct sdhci_host *host = dev_get_drvdata(dev);
> + int err;
> +
> + err = pm_runtime_force_resume(dev);
> + if (err < 0)
> + dev_warn(dev, "force resume failed: %i\n", err);
>
> sdhci_resume_host(host);
>
> + /* Balance pm_runtime_resume_and_get() done in suspend */
> + pm_runtime_put(dev);
> +
> return 0;
> }
> #endif
> -static SIMPLE_DEV_PM_OPS(sdhci_omap_dev_pm_ops, sdhci_omap_suspend,
> - sdhci_omap_resume);
> +
> +static const struct dev_pm_ops sdhci_omap_dev_pm_ops = {
> + SET_RUNTIME_PM_OPS(sdhci_omap_runtime_suspend,
> + sdhci_omap_runtime_resume, NULL)
> + SET_SYSTEM_SLEEP_PM_OPS(sdhci_omap_suspend, sdhci_omap_resume)
> +};
>
> static struct platform_driver sdhci_omap_driver = {
> .probe = sdhci_omap_probe,
> --
> 2.33.0
Kind regards
Uffe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions
2021-10-12 15:16 ` Ulf Hansson
@ 2021-10-15 9:34 ` Tony Lindgren
0 siblings, 0 replies; 13+ messages in thread
From: Tony Lindgren @ 2021-10-15 9:34 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, Chunyan Zhang, Faiz Abbas, Kishon Vijay Abraham I,
Santosh Shilimkar, linux-mmc, linux-omap, Rob Herring, DTML
* Ulf Hansson <ulf.hansson@linaro.org> [211012 15:17]:
> On Tue, 12 Oct 2021 at 12:38, Tony Lindgren <tony@atomide.com> wrote:
> > @@ -1350,15 +1355,21 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> > if (ret)
> > goto err_cleanup_host;
> >
> > + sdhci_omap_context_save(omap_host);
>
> Calling sdhci_omap_context_save() here looks unnecessary. The device
> is already runtime resumed at this point.
>
> In other words, sdhci_omap_context_save() will be called from the
> ->runtime_suspend() callback, next time the device becomes runtime
> suspended. That should be sufficient, right?
Yup this can be now dropped with omap_host->con initialized to
-EINVAL earlier.
> > @@ -1371,8 +1382,12 @@ static int sdhci_omap_remove(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct sdhci_host *host = platform_get_drvdata(pdev);
> >
> > + pm_runtime_get_sync(dev);
> > sdhci_remove_host(host, true);
> > + pm_runtime_dont_use_autosuspend(dev);
> > pm_runtime_put_sync(dev);
> > + /* Ensure device gets idled despite userspace sysfs config */
> > + pm_runtime_force_suspend(dev);
> > pm_runtime_disable(dev);
>
> The call to pm_runtime_disable() can be removed, as that is taken care
> of in pm_runtime_force_suspend().
OK
> > +static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> > +{
> > + struct sdhci_host *host = dev_get_drvdata(dev);
> > + int err;
> > +
> > + /* Enable for configuring wakeups, paired in resume */
> > + err = pm_runtime_resume_and_get(dev);
> > + if (err < 0)
> > + return err;
> > +
> > + sdhci_suspend_host(host);
>
> As far as I can tell, sdhci_suspend_host() doesn't really make sense
> for the omap variant. What you need, is to put the device into the
> same low power state as "runtime suspend", that should be sufficient.
>
> The system wakeup will be armed (and later then disarmed) by the PM
> core, when it calls device_wakeup_arm_wake_irqs() from the
> dpm_suspend_noirq() phase.
>
> In other words, pointing the system suspend/resume callbacks to
> pm_runtime_force_suspend|resume() should work fine, I think.
OK sounds good to me.
Regards,
Tony
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-10-15 9:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
2021-10-12 10:37 ` [PATCH 1/6] dt-bindings: sdhci-omap: Update binding for legacy SoCs Tony Lindgren
2021-10-12 10:37 ` [PATCH 2/6] mmc: sdhci-omap: Handle voltages to add support omap4 Tony Lindgren
2021-10-12 14:16 ` kernel test robot
2021-10-12 10:37 ` [PATCH 3/6] mmc: sdhci-omap: Add omap_offset to support omap3 and earlier Tony Lindgren
2021-10-12 10:37 ` [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Tony Lindgren
2021-10-12 15:11 ` kernel test robot
2021-10-12 15:16 ` Ulf Hansson
2021-10-15 9:34 ` Tony Lindgren
2021-10-12 10:37 ` [PATCH 5/6] sdhci: omap: Enable aggressive PM Tony Lindgren
2021-10-12 14:14 ` Ulf Hansson
2021-10-12 10:37 ` [PATCH 6/6] mmc: sdhci-omap: Configure optional wakeirq Tony Lindgren
2021-10-12 15:02 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).