Re: [PATCH RFC v2 2/2] mmc: sdhci-pxav3: add state_uhs pinctrl setting
From: Adrian Hunter <adrian.hunter@intel.com>
Date: 2025-08-04 13:39:39
Also in:
linux-mmc, lkml, phone-devel
On 01/08/2025 17:14, Duje Mihanović wrote:
quoted hunk ↗ jump to hunk
Different bus clocks require different pinctrl states to remain stable. Add support for selecting between a default and UHS state according to the bus clock. Signed-off-by: Duje Mihanović <duje@dujemihanovic.xyz> --- Changes in v2: - Don't attempt to lookup pinstates if getting pinctrl fails - Only select pinstates if both of them are valid - dev_warn() -> dev_dbg() --- drivers/mmc/host/sdhci-pxav3.c | 31 ++++++++++++++++++++++++++++++- include/linux/platform_data/pxa_sdhci.h | 7 +++++++ 2 files changed, 37 insertions(+), 1 deletion(-)diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c index 3fb56face3d81259b693c8569682d05c95be2880..fc6018de92fb19f028b776df0f87937846621e95 100644 --- a/drivers/mmc/host/sdhci-pxav3.c +++ b/drivers/mmc/host/sdhci-pxav3.c@@ -20,9 +20,11 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/pinctrl/consumer.h> #include <linux/pm.h> #include <linux/pm_runtime.h> #include <linux/mbus.h> +#include <linux/units.h> #include "sdhci.h" #include "sdhci-pltfm.h"@@ -313,8 +315,23 @@ static void pxav3_set_power(struct sdhci_host *host, unsigned char mode, mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); } +static void pxav3_set_clock(struct sdhci_host *host, unsigned int clock) +{ + struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc)); + struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data; + + if (!(IS_ERR(pdata->pinctrl) || IS_ERR(pdata->pins_default) || !IS_ERR(pdata->pins_uhs))) { + if (clock < 100 * HZ_PER_MHZ) + pinctrl_select_state(pdata->pinctrl, pdata->pins_default); + else + pinctrl_select_state(pdata->pinctrl, pdata->pins_uhs); + } + + sdhci_set_clock(host, clock); +}
Really pinctrl et al should be in struct sdhci_pxa not pdata. Also
it is neater to set pinctrl_state pointers to NULL when there is no
valid value, so this could become:
static void pxav3_set_clock(struct sdhci_host *host, unsigned int clock)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_pxa *pxa = sdhci_pltfm_priv(pltfm_host);
struct pinctrl_state *pins = clock < 100 * HZ_PER_MHZ ? pxa->pins_default : pxa->pins_uhs;
if (pins)
pinctrl_select_state(pxa->pinctrl, pins);
sdhci_set_clock(host, clock);
}
quoted hunk ↗ jump to hunk
+ static const struct sdhci_ops pxav3_sdhci_ops = { - .set_clock = sdhci_set_clock, + .set_clock = pxav3_set_clock, .set_power = pxav3_set_power, .platform_send_init_74_clocks = pxav3_gen_init_74_clocks, .get_max_clock = sdhci_pltfm_clk_get_max_clock,@@ -441,6 +458,18 @@ static int sdhci_pxav3_probe(struct platform_device *pdev) host->mmc->pm_caps |= pdata->pm_caps; } + pdata->pinctrl = devm_pinctrl_get(dev); + if (!IS_ERR(pdata->pinctrl)) { + pdata->pins_default = pinctrl_lookup_state(pdata->pinctrl, "default"); + if (IS_ERR(pdata->pins_default)) + dev_dbg(dev, "could not get default state: %ld\n", + PTR_ERR(pdata->pins_default)); + pdata->pins_uhs = pinctrl_lookup_state(pdata->pinctrl, "state_uhs"); + if (IS_ERR(pdata->pins_uhs)) + dev_dbg(dev, "could not get uhs state: %ld\n", PTR_ERR(pdata->pins_uhs)); + } else + dev_dbg(dev, "could not get pinctrl handle: %ld\n", PTR_ERR(pdata->pinctrl)); +
To make the code neater, perhaps add a helper like:
static struct pinctrl_state *pxav3_pinctrl_state(struct device *dev, struct pinctrl *pinctrl,
const char *name)
{
struct pinctrl_state *pins = pinctrl_lookup_state(pinctrl, name);
if (IS_ERR(pins)) {
dev_dbg(dev, "could not get pinctrl state '%s', error %ld\n", name, PTR_ERR(pins));
return NULL;
}
return pins;
}
Then it could be like:
pxa->pinctrl = devm_pinctrl_get(dev);
if (IS_ERR(pxa->pinctrl)) {
dev_dbg(dev, "could not get pinctrl handle: %ld\n", PTR_ERR(pxa->pinctrl));
} else {
pxa->pins_default = pxav3_pinctrl_state(dev, pxa->pinctrl, "default");
if (pxa->pins_default)
pxa->pins_uhs = pxav3_pinctrl_state(dev, pxa->pinctrl, "state_uhs");
}
quoted hunk ↗ jump to hunk
pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); pm_runtime_set_autosuspend_delay(&pdev->dev, PXAV3_RPM_DELAY_MS);diff --git a/include/linux/platform_data/pxa_sdhci.h b/include/linux/platform_data/pxa_sdhci.h index 899457cee425d33f82606f0b8c280003bc73d48d..540aa36db11243719707bdf22db23a8e2035674d 100644 --- a/include/linux/platform_data/pxa_sdhci.h +++ b/include/linux/platform_data/pxa_sdhci.h@@ -35,6 +35,9 @@ * @quirks: quirks of platfrom * @quirks2: quirks2 of platfrom * @pm_caps: pm_caps of platfrom + * @pinctrl: pinctrl handle + * @pins_default: default pinctrl state + * @pins_uhs: pinctrl state for fast (>100 MHz) bus clocks */ struct sdhci_pxa_platdata { unsigned int flags;@@ -47,5 +50,9 @@ struct sdhci_pxa_platdata { unsigned int quirks; unsigned int quirks2; unsigned int pm_caps; + + struct pinctrl *pinctrl; + struct pinctrl_state *pins_default; + struct pinctrl_state *pins_uhs;
Move to struct sdhci_pxa
}; #endif /* _PXA_SDHCI_H_ */