Re: [PATCH] mmc: dw_mmc-rockchip: add pinctrl support

[Shawn Lin]

On 2016/1/19 18:07, Heiko Stuebner wrote:
> Hi Shawn,
>
> Am Dienstag, 19. Januar 2016, 17:52:45 schrieb Shawn Lin:
> > We find rk3368 fail to enumerate sd card since
> > the default state is gpio function. Meahwhile, lots of
> > rockchip platform dts file assign pinctrl to dw_mmc but actually
> > dw_mmc never use it. Fortunately, those platforms' default io
> > state is sdmmc function, but no always right for all.
> >
> > Cc: Heiko Stuebner <heiko@xxxxxxxxx>
> > Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
>
> I do believe any issue you see, must come from somewhere else, because the
> driver-core does in drivers/base/dd.c really_probe():
> 	pinctrl_bind_pins()

yes, pinctrl_bind_pins does turn pinctrl state. But I try debug a little
more and seems somewhere else switch the state before dw_mmc probe. Now 
I *guess* that may relate to our ATF or MCU firmware since rk3368's 
sdmmc io can also be used as mcu_jtag for system debug. I need to double 
check that point, thanks.

Hi Caesar,
does your patch able to consider adding SDMMC/CPU_JTAG/MCU_JTAG 
managment for rk3368 as well?

> which in turn already does the needed pinctrl state handling.
>
> So that default-state should be set up even before the dwmmc probe function
> is called.
>
> The sharing between sdmmc and uart2 pins is the same on the rk3036 and so
> far Caesar seems to have been able to switch the pin-state to sdmmc without
> touching the dw_mmc driver itself.
>
>
> Heiko
>
>
> > ---
> >
> >   drivers/mmc/host/dw_mmc-rockchip.c | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
> > b/drivers/mmc/host/dw_mmc-rockchip.c index d9c92f3..308b424 100644
> > --- a/drivers/mmc/host/dw_mmc-rockchip.c
> > +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> > @@ -9,6 +9,7 @@
> >
> >   #include <linux/module.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/pinctrl/consumer.h>
> >   #include <linux/clk.h>
> >   #include <linux/mmc/host.h>
> >   #include <linux/mmc/dw_mmc.h>
> > @@ -21,6 +22,8 @@
> >   #define RK3288_CLKGEN_DIV       2
> >
> >   struct dw_mci_rockchip_priv_data {
> > +	struct pinctrl		*pinctrl;
> > +	struct pinctrl_state	*pins_default;
> >   	struct clk		*drv_clk;
> >   	struct clk		*sample_clk;
> >   	int			default_sample_phase;
> > @@ -78,6 +81,10 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host,
> > struct mmc_ios *ios) /* Make sure we use phases which we can enumerate
> > with */
> >   	if (!IS_ERR(priv->sample_clk))
> >   		clk_set_phase(priv->sample_clk, priv->default_sample_phase);
> > +
> > +	/* Make sure we use correct pinctrl which we can enumerate with */
> > +	if (!IS_ERR(priv->pinctrl) && !IS_ERR(priv->pins_default))
> > +		pinctrl_select_state(priv->pinctrl, priv->pins_default);
> >   }
> >
> >   #define NUM_PHASES			360
> > @@ -223,6 +230,14 @@ static int dw_mci_rk3288_parse_dt(struct dw_mci
> > *host) if (IS_ERR(priv->sample_clk))
> >   		dev_dbg(host->dev, "ciu_sample not available\n");
> >
> > +	priv->pinctrl = devm_pinctrl_get(host->dev);
> > +	if (!IS_ERR(priv->pinctrl)) {
> > +		priv->pins_default = pinctrl_lookup_state(priv->pinctrl,
> > +							  "default");
> > +		if (IS_ERR(priv->pins_default))
> > +			dev_dbg(host->dev, "pinctrl not available\n");
> > +	}
> > +
> >   	host->priv = priv;
> >
> >   	return 0;


--
Best Regards
Shawn Lin