Re: [PATCH V2 02/12] pinctrl: tegra: add suspend and resume support

[Sowjanya Komatineni <skomatineni@xxxxxxxxxx>]

On 5/29/19 8:29 AM, Dmitry Osipenko wrote:
> 29.05.2019 2:08, Sowjanya Komatineni пишет:
> > This patch adds suspend and resume support for Tegra pinctrl driver
> > and registers them to syscore so the pinmux settings are restored
> > before the devices resume.
> >
> > Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
> > ---
> >   drivers/pinctrl/tegra/pinctrl-tegra.c    | 68 +++++++++++++++++++++++++++++++-
> >   drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++
> >   drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
> >   drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
> >   drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
> >   drivers/pinctrl/tegra/pinctrl-tegra210.c |  1 +
> >   drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
> >   7 files changed, 75 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > index a5008c066bac..bdc47e62c457 100644
> > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > @@ -28,11 +28,18 @@
> >   #include <linux/pinctrl/pinmux.h>
> >   #include <linux/pinctrl/pinconf.h>
> >   #include <linux/slab.h>
> > +#include <linux/syscore_ops.h>
> >   
> >   #include "../core.h"
> >   #include "../pinctrl-utils.h"
> >   #include "pinctrl-tegra.h"
> >   
> > +#define EMMC2_PAD_CFGPADCTRL_0			0x1c8
> > +#define EMMC4_PAD_CFGPADCTRL_0			0x1e0
> > +#define EMMC_DPD_PARKING			(0x1fff << 14)
> > +
> > +static struct tegra_pmx *pmx;
> > +
> >   static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> >   {
> >   	return readl(pmx->regs[bank] + reg);
> > @@ -629,6 +636,50 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> >   	}
> >   }
> >   
> > +static int __maybe_unused tegra_pinctrl_suspend(void)
> > +{
> > +	u32 *backup_regs = pmx->backup_regs;
> > +	u32 *regs;
> > +	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 void __maybe_unused tegra_pinctrl_resume(void)
> > +{
> > +	u32 *backup_regs = pmx->backup_regs;
> > +	u32 *regs;
> > +	u32 val;
> > +	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++);
> > +	}
> > +
> > +	if (pmx->soc->has_park_padcfg) {
> > +		val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> > +		val &= ~EMMC_DPD_PARKING;
> > +		pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> > +
> > +		val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> > +		val &= ~EMMC_DPD_PARKING;
> > +		pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> > +	}
> > +}
> But the CFGPADCTRL registers are already programmed by restoring the
> backup_regs and hence the relevant EMMC's are already unparked. Hence
> why do you need to force-unpark both of the EMMC's? What if EMMC is
> unpopulated on a board, why do you need to unpark it then?

PARK bit for EMMC2/EMMC4 (EMMC2_PAD_CFGPADCTRL and EMMC4_PAD_CFGPADCTRL) 
are not part of pinmux.

They are part of CFGPADCTRL register so pinctrl driver pingroup doesn't 
include these registers.

backup_regs doesn't take care of this and need to handled separately for 
Tegra210.


During resume we have to clear PARK bit for the pads on Tegra210 and 
this is not related to presence/absence of eMMC on the board.

PAD is parked during LP0 entry to have it in DPD mode and it stays in 
DPD till its cleared by SW on resume.