Re: [PATCH V5 02/18] pinctrl: tegra: Add suspend and resume support

[Sowjanya Komatineni]

On 6/29/19 8:46 AM, Dmitry Osipenko wrote:
> 28.06.2019 5:12, Sowjanya Komatineni ÐÐÑÐÑ:
> > This patch adds support for Tegra pinctrl driver suspend and resume.
> >
> > During suspend, context of all pinctrl registers are stored and
> > on resume they are all restored to have all the pinmux and pad
> > configuration for normal operation.
> >
> > Acked-by: Thierry Reding <treding@xxxxxxxxxx>
> > Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
> > ---
> >   drivers/pinctrl/tegra/pinctrl-tegra.c    | 52 ++++++++++++++++++++++++++++++++
> >   drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
> >   drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
> >   3 files changed, 56 insertions(+)
> >
> > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > index 34596b246578..e7c0a1011cba 100644
> > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > @@ -621,6 +621,43 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> >   	}
> >   }
> >   
> > +static int tegra_pinctrl_suspend(struct device *dev)
> > +{
> > +	struct tegra_pmx *pmx = dev_get_drvdata(dev);
> > +	u32 *backup_regs = pmx->backup_regs;
> > +	u32 *regs;
> > +	unsigned int i, j;
> > +
> > +	for (i = 0; i < pmx->nbanks; i++) {
> > +		regs = pmx->regs[i];
> > +		for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
> > +			*backup_regs++ = readl(regs++);
> > +	}
> > +
> > +	return pinctrl_force_sleep(pmx->pctl);
> > +}
> > +
> > +static int tegra_pinctrl_resume(struct device *dev)
> > +{
> > +	struct tegra_pmx *pmx = dev_get_drvdata(dev);
> > +	u32 *backup_regs = pmx->backup_regs;
> > +	u32 *regs;
> > +	unsigned int i, j;
> > +
> > +	for (i = 0; i < pmx->nbanks; i++) {
> > +		regs = pmx->regs[i];
> > +		for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
> > +			writel(*backup_regs++, regs++);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +const struct dev_pm_ops tegra_pinctrl_pm = {
> > +	.suspend = &tegra_pinctrl_suspend,
> > +	.resume = &tegra_pinctrl_resume
> > +};
> Hm, so this are the generic platform-driver suspend-resume OPS here, which is very
> nice! But.. shouldn't pinctrl be resumed before the CLK driver (which is syscore_ops
> in this version of the series)? .. Given that "clock" function may need to be
> selected for some of the pins.

selection of clock functions on some Tegra pins through corresponding 
pinmux (like extperiph clks) can happen after clock driver resume as 
well where clock source is restored to state during suspend before 
selecting clock function on that pin.