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ć <[email protected]>
One minor cosmetic issue below. Address that and add: Acked-by: Adrian Hunter <[email protected]> > --- > 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); >
