Re: [PATCH v3 2/2] mmc: sdhci-pxav3: add state_uhs pinctrl setting
From: Adrian Hunter <adrian.hunter@intel.com>
Date: 2025-08-18 07:26:07
Also in:
linux-mmc, lkml, phone-devel
On 06/08/2025 12:43, Duje Mihanović wrote:
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>
One minor cosmetic issue below. Address that and add: Acked-by: Adrian Hunter <adrian.hunter@intel.com>
quoted hunk ↗ jump to hunk
--- Changes in v3: - Move pinctrl stuff out of platdata - Add helper for pinstate lookup - Thanks to Adrian for the suggestions 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 | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c index 3fb56face3d81259b693c8569682d05c95be2880..ceb06f1c18146eac296c6d20483a9f006567a978 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"@@ -51,6 +53,9 @@ struct sdhci_pxa { struct clk *clk_io; u8 power_mode; void __iomem *sdio3_conf_reg; + struct pinctrl *pinctrl; + struct pinctrl_state *pins_default; + struct pinctrl_state *pins_uhs; }; /*@@ -313,8 +318,20 @@ 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 sdhci_pltfm_host *phost = sdhci_priv(host); + struct sdhci_pxa *pxa = sdhci_pltfm_priv(phost); + 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); +} + 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,@@ -366,6 +383,19 @@ static inline struct sdhci_pxa_platdata *pxav3_get_mmc_pdata(struct device *dev) } #endif +static struct pinctrl_state *pxav3_lookup_pinstate(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 pinstate '%s': %ld\n", name, PTR_ERR(pins)); + return NULL; + } + + return pins; +} + static int sdhci_pxav3_probe(struct platform_device *pdev) { struct sdhci_pltfm_host *pltfm_host;@@ -441,6 +471,14 @@ static int sdhci_pxav3_probe(struct platform_device *pdev) host->mmc->pm_caps |= pdata->pm_caps; } + pxa->pinctrl = devm_pinctrl_get(dev); + if (!IS_ERR(pxa->pinctrl)) { + pxa->pins_default = pxav3_lookup_pinstate(dev, pxa->pinctrl, "default"); + if (pxa->pins_default) + pxa->pins_uhs = pxav3_lookup_pinstate(dev, pxa->pinctrl, "state_uhs"); + } else + dev_dbg(dev, "could not get pinctrl handle: %ld\n", PTR_ERR(pxa->pinctrl));
Kernel style is to have braces here. ./scripts/checkpatch.pl --strict
can tell about that kind of thing:
CHECK: braces {} should be used on all arms of this statement
#92: FILE: drivers/mmc/host/sdhci-pxav3.c:474:
+ if (!IS_ERR(pxa->pinctrl)) {
[...]
+ } else
[...]
CHECK: Unbalanced braces around else statement
#96: FILE: drivers/mmc/host/sdhci-pxav3.c:478:
+ } else
+ pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); pm_runtime_set_autosuspend_delay(&pdev->dev, PXAV3_RPM_DELAY_MS);