| Message ID | 1559084936-4610-3-git-send-email-skomatineni@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | LP0 entry and exit support for Tegra210 | expand |
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@nvidia.com> > --- > 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?
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@nvidia.com> >> --- >> 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.
29.05.2019 21:14, Sowjanya Komatineni пишет: > > 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@nvidia.com> >>> --- >>> 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. I'm looking at the tegra210_groups and it clearly has these both registers as a part of pinctrl setup because the rest of the bits configure drive of the pads. From pinctrl-tegra210.c: #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), ... 0xa9c - 0x8d4 = 0x1c8 0xab4 - 0x8d4 = 0x1e0 Hence the PARK bits are already getting unset by restoring the backup_regs because the CFGPADCTRL registers are a part of the "bank 0" registers. Am I still missing something? > 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. Okay, thank you for the clarification. > PAD is parked during LP0 entry to have it in DPD mode and it stays in > DPD till its cleared by SW on resume. Yes, this is documented in the public TRM. My main point is that it looks like the PARK bits are unneedlessly getting unset twice in your code (and it still looks like that to me).
On 5/29/19 12:32 PM, Dmitry Osipenko wrote: > 29.05.2019 21:14, Sowjanya Komatineni пишет: >> 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@nvidia.com> >>>> --- >>>> 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. > I'm looking at the tegra210_groups and it clearly has these both > registers as a part of pinctrl setup because the rest of the bits > configure drive of the pads. > > From pinctrl-tegra210.c: > > #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ > > DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), > DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), > > ... > > 0xa9c - 0x8d4 = 0x1c8 > 0xab4 - 0x8d4 = 0x1e0 > > Hence the PARK bits are already getting unset by restoring the > backup_regs because the CFGPADCTRL registers are a part of the "bank 0" > registers. > > Am I still missing something? DRV_PINGROUP parked_bit is -1 and will not be programmed so store and restore will not take care of it. Also EMMC PADCFG is the only padcfg register which has parked bit and for other IO pads its part of pinmux >> 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. > Okay, thank you for the clarification. > >> PAD is parked during LP0 entry to have it in DPD mode and it stays in >> DPD till its cleared by SW on resume. > Yes, this is documented in the public TRM. My main point is that it > looks like the PARK bits are unneedlessly getting unset twice in your > code (and it still looks like that to me).
29.05.2019 23:11, Sowjanya Komatineni пишет: > > On 5/29/19 12:32 PM, Dmitry Osipenko wrote: >> 29.05.2019 21:14, Sowjanya Komatineni пишет: >>> 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@nvidia.com> >>>>> --- >>>>> 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. >> I'm looking at the tegra210_groups and it clearly has these both >> registers as a part of pinctrl setup because the rest of the bits >> configure drive of the pads. >> >> From pinctrl-tegra210.c: >> >> #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ >> >> DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), >> DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), >> >> ... >> >> 0xa9c - 0x8d4 = 0x1c8 >> 0xab4 - 0x8d4 = 0x1e0 >> >> Hence the PARK bits are already getting unset by restoring the >> backup_regs because the CFGPADCTRL registers are a part of the "bank 0" >> registers. >> >> Am I still missing something? > > DRV_PINGROUP parked_bit is -1 and will not be programmed so store and > restore will not take care of it. > > Also EMMC PADCFG is the only padcfg register which has parked bit and > for other IO pads its part of pinmux You're storing raw values of all of the PINCTRL registers and then restoring the raw values (if I'm not misreading that part on the patch), it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK bits. In a result, the backup_regs array contains raw CFGPADCTRL value with the PARK bits being unset on store, that value is written out on the restore as-is and hence the PARK bits are getting unset as well. And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a driver's drawback that need to be addressed.
On 5/29/19 1:47 PM, Dmitry Osipenko wrote: > 29.05.2019 23:11, Sowjanya Komatineni пишет: >> On 5/29/19 12:32 PM, Dmitry Osipenko wrote: >>> 29.05.2019 21:14, Sowjanya Komatineni пишет: >>>> 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@nvidia.com> >>>>>> --- >>>>>> 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. >>> I'm looking at the tegra210_groups and it clearly has these both >>> registers as a part of pinctrl setup because the rest of the bits >>> configure drive of the pads. >>> >>> From pinctrl-tegra210.c: >>> >>> #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ >>> >>> DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), >>> DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), >>> >>> ... >>> >>> 0xa9c - 0x8d4 = 0x1c8 >>> 0xab4 - 0x8d4 = 0x1e0 >>> >>> Hence the PARK bits are already getting unset by restoring the >>> backup_regs because the CFGPADCTRL registers are a part of the "bank 0" >>> registers. >>> >>> Am I still missing something? >> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and >> restore will not take care of it. >> >> Also EMMC PADCFG is the only padcfg register which has parked bit and >> for other IO pads its part of pinmux > You're storing raw values of all of the PINCTRL registers and then > restoring the raw values (if I'm not misreading that part on the patch), > it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK bits. > > In a result, the backup_regs array contains raw CFGPADCTRL value with > the PARK bits being unset on store, that value is written out on the > restore as-is and hence the PARK bits are getting unset as well. > > And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a > driver's drawback that need to be addressed. Parked bits from padcfg are available only for couple of EMMC registers. default PARK bits are set so stored value contains park bit set. on resume, after restoring park bit is cleared. on subsequence DPD entry, stored value contains park bit 0 and HW clamps park bit to logic 1 during DPD entry and cleared again on resume.
On 5/29/19 1:56 PM, Sowjanya Komatineni wrote: > > On 5/29/19 1:47 PM, Dmitry Osipenko wrote: >> 29.05.2019 23:11, Sowjanya Komatineni пишет: >>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote: >>>> 29.05.2019 21:14, Sowjanya Komatineni пишет: >>>>> 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@nvidia.com> >>>>>>> --- >>>>>>> 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. >>>> I'm looking at the tegra210_groups and it clearly has these both >>>> registers as a part of pinctrl setup because the rest of the bits >>>> configure drive of the pads. >>>> >>>> From pinctrl-tegra210.c: >>>> >>>> #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ >>>> >>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), >>>> DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), >>>> >>>> ... >>>> >>>> 0xa9c - 0x8d4 = 0x1c8 >>>> 0xab4 - 0x8d4 = 0x1e0 >>>> >>>> Hence the PARK bits are already getting unset by restoring the >>>> backup_regs because the CFGPADCTRL registers are a part of the >>>> "bank 0" >>>> registers. >>>> >>>> Am I still missing something? >>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and >>> restore will not take care of it. >>> >>> Also EMMC PADCFG is the only padcfg register which has parked bit and >>> for other IO pads its part of pinmux >> You're storing raw values of all of the PINCTRL registers and then >> restoring the raw values (if I'm not misreading that part on the patch), >> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK >> bits. >> >> In a result, the backup_regs array contains raw CFGPADCTRL value with >> the PARK bits being unset on store, that value is written out on the >> restore as-is and hence the PARK bits are getting unset as well. >> >> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a >> driver's drawback that need to be addressed. > > Parked bits from padcfg are available only for couple of EMMC registers. > > default PARK bits are set so stored value contains park bit set. on > resume, after restoring park bit is cleared. > > on subsequence DPD entry, stored value contains park bit 0 and HW > clamps park bit to logic 1 during DPD entry and cleared again on resume. > > Other IOs park bit in pinmux gets cleared thru tegra_pinctrl_clear_parked_bits on probe and during suspend register values saved contains park bit = 0 which is same when restored on DPD resume. clearing park bit during resume for EMMC pads is same as clearing it during probe which is then saved during suspend and restored on resume similar to pinmux registers. So for more readability, probably can clear parked bit for EMMC during pinctrl_clear_parked_bits instead of on resume.
29.05.2019 23:56, Sowjanya Komatineni пишет: > > On 5/29/19 1:47 PM, Dmitry Osipenko wrote: >> 29.05.2019 23:11, Sowjanya Komatineni пишет: >>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote: >>>> 29.05.2019 21:14, Sowjanya Komatineni пишет: >>>>> 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@nvidia.com> >>>>>>> --- >>>>>>> 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. >>>> I'm looking at the tegra210_groups and it clearly has these both >>>> registers as a part of pinctrl setup because the rest of the bits >>>> configure drive of the pads. >>>> >>>> From pinctrl-tegra210.c: >>>> >>>> #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ >>>> >>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), >>>> DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), >>>> >>>> ... >>>> >>>> 0xa9c - 0x8d4 = 0x1c8 >>>> 0xab4 - 0x8d4 = 0x1e0 >>>> >>>> Hence the PARK bits are already getting unset by restoring the >>>> backup_regs because the CFGPADCTRL registers are a part of the "bank 0" >>>> registers. >>>> >>>> Am I still missing something? >>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and >>> restore will not take care of it. >>> >>> Also EMMC PADCFG is the only padcfg register which has parked bit and >>> for other IO pads its part of pinmux >> You're storing raw values of all of the PINCTRL registers and then >> restoring the raw values (if I'm not misreading that part on the patch), >> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK >> bits. >> >> In a result, the backup_regs array contains raw CFGPADCTRL value with >> the PARK bits being unset on store, that value is written out on the >> restore as-is and hence the PARK bits are getting unset as well. >> >> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a >> driver's drawback that need to be addressed. > > Parked bits from padcfg are available only for couple of EMMC registers. > > default PARK bits are set so stored value contains park bit set. on > resume, after restoring park bit is cleared. TRM says that the PARK bits are set on HW reset (and on DPD IIUC) and then SW should unset the bits. I assume that the PARK bits of the EMMC pads are unset by bootloader and that's why it doesn't cause problems for kernel, in oppose to PARK bits of the rest of pinmux that are unset by the driver in tegra_pinctrl_clear_parked_bits() on driver's probe. > on subsequence DPD entry, stored value contains park bit 0 and HW clamps > park bit to logic 1 during DPD entry and cleared again on resume. I assume that EMMC won't work properly if pads are "parked" and the PARK bits should be in unset state on kernel boot-up as I wrote above, hence stored value should always contain 0 for the EMMC PARK bits. No?
On 5/29/19 2:25 PM, Dmitry Osipenko wrote: > 29.05.2019 23:56, Sowjanya Komatineni пишет: >> On 5/29/19 1:47 PM, Dmitry Osipenko wrote: >>> 29.05.2019 23:11, Sowjanya Komatineni пишет: >>>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote: >>>>> 29.05.2019 21:14, Sowjanya Komatineni пишет: >>>>>> 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@nvidia.com> >>>>>>>> --- >>>>>>>> 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. >>>>> I'm looking at the tegra210_groups and it clearly has these both >>>>> registers as a part of pinctrl setup because the rest of the bits >>>>> configure drive of the pads. >>>>> >>>>> From pinctrl-tegra210.c: >>>>> >>>>> #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ >>>>> >>>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), >>>>> DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), >>>>> >>>>> ... >>>>> >>>>> 0xa9c - 0x8d4 = 0x1c8 >>>>> 0xab4 - 0x8d4 = 0x1e0 >>>>> >>>>> Hence the PARK bits are already getting unset by restoring the >>>>> backup_regs because the CFGPADCTRL registers are a part of the "bank 0" >>>>> registers. >>>>> >>>>> Am I still missing something? >>>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and >>>> restore will not take care of it. >>>> >>>> Also EMMC PADCFG is the only padcfg register which has parked bit and >>>> for other IO pads its part of pinmux >>> You're storing raw values of all of the PINCTRL registers and then >>> restoring the raw values (if I'm not misreading that part on the patch), >>> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK >>> bits. >>> >>> In a result, the backup_regs array contains raw CFGPADCTRL value with >>> the PARK bits being unset on store, that value is written out on the >>> restore as-is and hence the PARK bits are getting unset as well. >>> >>> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a >>> driver's drawback that need to be addressed. >> Parked bits from padcfg are available only for couple of EMMC registers. >> >> default PARK bits are set so stored value contains park bit set. on >> resume, after restoring park bit is cleared. > TRM says that the PARK bits are set on HW reset (and on DPD IIUC) and > then SW should unset the bits. I assume that the PARK bits of the EMMC > pads are unset by bootloader and that's why it doesn't cause problems > for kernel, in oppose to PARK bits of the rest of pinmux that are unset > by the driver in tegra_pinctrl_clear_parked_bits() on driver's probe. > >> on subsequence DPD entry, stored value contains park bit 0 and HW clamps >> park bit to logic 1 during DPD entry and cleared again on resume. > I assume that EMMC won't work properly if pads are "parked" and the PARK > bits should be in unset state on kernel boot-up as I wrote above, hence > stored value should always contain 0 for the EMMC PARK bits. No? On bootup, pads still works. PARK bit is to keep pads in DPD once they enter into DPD (LP0) and on resume they need to be cleared to be out of DPD
30.05.2019 0:27, Sowjanya Komatineni пишет: > > On 5/29/19 2:25 PM, Dmitry Osipenko wrote: >> 29.05.2019 23:56, Sowjanya Komatineni пишет: >>> On 5/29/19 1:47 PM, Dmitry Osipenko wrote: >>>> 29.05.2019 23:11, Sowjanya Komatineni пишет: >>>>> On 5/29/19 12:32 PM, Dmitry Osipenko wrote: >>>>>> 29.05.2019 21:14, Sowjanya Komatineni пишет: >>>>>>> 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@nvidia.com> >>>>>>>>> --- >>>>>>>>> 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. >>>>>> I'm looking at the tegra210_groups and it clearly has these both >>>>>> registers as a part of pinctrl setup because the rest of the bits >>>>>> configure drive of the pads. >>>>>> >>>>>> From pinctrl-tegra210.c: >>>>>> >>>>>> #define DRV_PINGROUP_REG_A 0x8d4 /* bank 0 */ >>>>>> >>>>>> DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2), >>>>>> DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2), >>>>>> >>>>>> ... >>>>>> >>>>>> 0xa9c - 0x8d4 = 0x1c8 >>>>>> 0xab4 - 0x8d4 = 0x1e0 >>>>>> >>>>>> Hence the PARK bits are already getting unset by restoring the >>>>>> backup_regs because the CFGPADCTRL registers are a part of the >>>>>> "bank 0" >>>>>> registers. >>>>>> >>>>>> Am I still missing something? >>>>> DRV_PINGROUP parked_bit is -1 and will not be programmed so store and >>>>> restore will not take care of it. >>>>> >>>>> Also EMMC PADCFG is the only padcfg register which has parked bit and >>>>> for other IO pads its part of pinmux >>>> You're storing raw values of all of the PINCTRL registers and then >>>> restoring the raw values (if I'm not misreading that part on the >>>> patch), >>>> it's absolutely meaningless that DRV_PINGROUP doesn't define the PARK >>>> bits. >>>> >>>> In a result, the backup_regs array contains raw CFGPADCTRL value with >>>> the PARK bits being unset on store, that value is written out on the >>>> restore as-is and hence the PARK bits are getting unset as well. >>>> >>>> And why DRV_PINGROUP misses PARK bits for the EMMC's? Looks like a >>>> driver's drawback that need to be addressed. >>> Parked bits from padcfg are available only for couple of EMMC registers. >>> >>> default PARK bits are set so stored value contains park bit set. on >>> resume, after restoring park bit is cleared. >> TRM says that the PARK bits are set on HW reset (and on DPD IIUC) and >> then SW should unset the bits. I assume that the PARK bits of the EMMC >> pads are unset by bootloader and that's why it doesn't cause problems >> for kernel, in oppose to PARK bits of the rest of pinmux that are unset >> by the driver in tegra_pinctrl_clear_parked_bits() on driver's probe. >> >>> on subsequence DPD entry, stored value contains park bit 0 and HW clamps >>> park bit to logic 1 during DPD entry and cleared again on resume. >> I assume that EMMC won't work properly if pads are "parked" and the PARK >> bits should be in unset state on kernel boot-up as I wrote above, hence >> stored value should always contain 0 for the EMMC PARK bits. No? > > On bootup, pads still works. PARK bit is to keep pads in DPD once they > enter into DPD (LP0) and on resume they need to be cleared to be out of DPD > > Ah okay, thank you for the clarification. Indeed, it will be better to unset the PARK bits in pinctrl_clear_parked_bits, as you suggested in the other email. Seems it should be simple to turn parked_bit into parked_bitmask.
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); + } +} + +static struct syscore_ops pinctrl_syscore_ops = { + .suspend = tegra_pinctrl_suspend, + .resume = tegra_pinctrl_resume, +}; + static bool gpio_node_has_range(const char *compatible) { struct device_node *np; @@ -648,11 +699,11 @@ static bool gpio_node_has_range(const char *compatible) int tegra_pinctrl_probe(struct platform_device *pdev, const struct tegra_pinctrl_soc_data *soc_data) { - struct tegra_pmx *pmx; struct resource *res; int i; const char **group_pins; int fn, gn, gfn; + unsigned long backup_regs_size = 0; pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL); if (!pmx) @@ -705,6 +756,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev, res = platform_get_resource(pdev, IORESOURCE_MEM, i); if (!res) break; + backup_regs_size += resource_size(res); } pmx->nbanks = i; @@ -713,11 +765,24 @@ int tegra_pinctrl_probe(struct platform_device *pdev, if (!pmx->regs) return -ENOMEM; + pmx->reg_bank_size = devm_kcalloc(&pdev->dev, pmx->nbanks, + sizeof(*pmx->reg_bank_size), + GFP_KERNEL); + if (!pmx->reg_bank_size) + return -ENOMEM; + + pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size, + GFP_KERNEL); + if (!pmx->backup_regs) + return -ENOMEM; + for (i = 0; i < pmx->nbanks; i++) { res = platform_get_resource(pdev, IORESOURCE_MEM, i); pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(pmx->regs[i])) return PTR_ERR(pmx->regs[i]); + + pmx->reg_bank_size[i] = resource_size(res); } pmx->pctl = devm_pinctrl_register(&pdev->dev, &tegra_pinctrl_desc, pmx); @@ -732,6 +797,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev, pinctrl_add_gpio_range(pmx->pctl, &tegra_pinctrl_gpio_range); platform_set_drvdata(pdev, pmx); + register_syscore_ops(&pinctrl_syscore_ops); dev_dbg(&pdev->dev, "Probed Tegra pinctrl driver\n"); diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h index 44c71941b5f8..b405df6fd390 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra.h +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h @@ -25,6 +25,8 @@ struct tegra_pmx { int nbanks; void __iomem **regs; + size_t *reg_bank_size; + u32 *backup_regs; }; enum tegra_pinconf_param { @@ -199,6 +201,7 @@ struct tegra_pinctrl_soc_data { bool hsm_in_mux; bool schmitt_in_mux; bool drvtype_in_mux; + bool has_park_padcfg; }; int tegra_pinctrl_probe(struct platform_device *pdev, diff --git a/drivers/pinctrl/tegra/pinctrl-tegra114.c b/drivers/pinctrl/tegra/pinctrl-tegra114.c index d43c209e9c30..4ac44f34dccf 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra114.c +++ b/drivers/pinctrl/tegra/pinctrl-tegra114.c @@ -1849,6 +1849,7 @@ static const struct tegra_pinctrl_soc_data tegra114_pinctrl = { .hsm_in_mux = false, .schmitt_in_mux = false, .drvtype_in_mux = false, + .has_park_padcfg = false, }; static int tegra114_pinctrl_probe(struct platform_device *pdev) diff --git a/drivers/pinctrl/tegra/pinctrl-tegra124.c b/drivers/pinctrl/tegra/pinctrl-tegra124.c index 5b07a5834d15..1dac7648b41f 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra124.c +++ b/drivers/pinctrl/tegra/pinctrl-tegra124.c @@ -2061,6 +2061,7 @@ static const struct tegra_pinctrl_soc_data tegra124_pinctrl = { .hsm_in_mux = false, .schmitt_in_mux = false, .drvtype_in_mux = false, + .has_park_padcfg = false, }; static int tegra124_pinctrl_probe(struct platform_device *pdev) diff --git a/drivers/pinctrl/tegra/pinctrl-tegra20.c b/drivers/pinctrl/tegra/pinctrl-tegra20.c index 1fc82a9576e0..9d2b25200f32 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra20.c +++ b/drivers/pinctrl/tegra/pinctrl-tegra20.c @@ -2231,6 +2231,7 @@ static const struct tegra_pinctrl_soc_data tegra20_pinctrl = { .hsm_in_mux = false, .schmitt_in_mux = false, .drvtype_in_mux = false, + .has_park_padcfg = false, }; static const char *cdev1_parents[] = { diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c index 3e77f5474dd8..dc06c36e698a 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra210.c +++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c @@ -1563,6 +1563,7 @@ static const struct tegra_pinctrl_soc_data tegra210_pinctrl = { .hsm_in_mux = true, .schmitt_in_mux = true, .drvtype_in_mux = true, + .has_park_padcfg = true, }; static int tegra210_pinctrl_probe(struct platform_device *pdev) diff --git a/drivers/pinctrl/tegra/pinctrl-tegra30.c b/drivers/pinctrl/tegra/pinctrl-tegra30.c index 10e617003e9c..42182d714950 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra30.c +++ b/drivers/pinctrl/tegra/pinctrl-tegra30.c @@ -2484,6 +2484,7 @@ static const struct tegra_pinctrl_soc_data tegra30_pinctrl = { .hsm_in_mux = false, .schmitt_in_mux = false, .drvtype_in_mux = false, + .has_park_padcfg = false, }; static int tegra30_pinctrl_probe(struct platform_device *pdev)
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@nvidia.com> --- 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(-)