| Message ID | 1321074108-10025-1-git-send-email-inderpal.singh@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Sat, Nov 12, 2011 at 6:01 AM, Inderpal Singh <inderpal.singh@linaro.org> wrote: > GPIO driver strength settings are not preserved across suspend/resume > for s5pc100, s5pv210 and Exynos platforms which has been the cause of > mmc/sd card read/write failures after resume. Fix this by saving and > restoring the GPIO driver strength register settings across suspend/resume. > > Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> On a related theme: I am thinking about how to support preserving drive strength (etc) across suspend/resume and deepsleep in the pincontrol subsystem. Currently I am playing with the idea to let pin groups have states, as the different configurations seem to be 90% or so about very specific sleep modes, so say: pinconf_set_group_state("mmcgroup", PINCONF_STATE_ACTIVE); pinconf_set_group_state("mmcgroup", PINCONF_STATE_SUSPENDED); pinconf_set_group_state("mmcgroup", PINCONF_STATE_SLEEP); This would then instruct each pin controller driver to configure each pin apropriately for the given state, and that cross-references to a table keeping track of the preset per-pin for each state. My intuitive idea is that letting the core keep track of the state of every pin and letting pin groups harness the settings for a group of pins is the proper approach to the problem. Do you think something like this will work for the S5P:s? Yours, Linus Walleij
Hi Linus, On 12 November 2011 16:30, Linus Walleij <linus.walleij@linaro.org> wrote: > On Sat, Nov 12, 2011 at 6:01 AM, Inderpal Singh > <inderpal.singh@linaro.org> wrote: > >> GPIO driver strength settings are not preserved across suspend/resume >> for s5pc100, s5pv210 and Exynos platforms which has been the cause of >> mmc/sd card read/write failures after resume. Fix this by saving and >> restoring the GPIO driver strength register settings across suspend/resume. >> >> Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> > > On a related theme: I am thinking about how to support preserving > drive strength (etc) across suspend/resume and deepsleep in the > pincontrol subsystem. > > Currently I am playing with the idea to let pin groups have states, > as the different configurations seem to be 90% or so about very > specific sleep modes, so say: > > pinconf_set_group_state("mmcgroup", PINCONF_STATE_ACTIVE); > pinconf_set_group_state("mmcgroup", PINCONF_STATE_SUSPENDED); > pinconf_set_group_state("mmcgroup", PINCONF_STATE_SLEEP); The API name suggests that power state is set based on a pin group. For a driver, the following could be convenient as the driver transits through various power management states. struct pinmux pmx; pmx = pinmux_get(dev, "i2c0"); [...] pinmux_set_state(pmx, PINCONF_STATE_SLEEP); [...] pinmux_set_state(pmx, PINCONF_STATE_ACTIVE); > > This would then instruct each pin controller driver to configure > each pin apropriately for the given state, and that cross-references > to a table keeping track of the preset per-pin for each state. > > My intuitive idea is that letting the core keep track of the state of > every pin and letting pin groups harness the settings for a group > of pins is the proper approach to the problem. > > Do you think something like this will work for the S5P:s? Adding the responsibility of managing/configuring registers for different power states to a pin group does seem suitable for Exynos. Along with the pin list, a implementation of a pin group could include a callback to manage the registers for a particular pin state. Such a callback could be reused for all the pin groups as required. Thanks, Thomas. > > Yours, > Linus Walleij >
Thomas Abraham wrote at Monday, November 14, 2011 6:57 AM: > On 12 November 2011 16:30, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Sat, Nov 12, 2011 at 6:01 AM, Inderpal Singh > > <inderpal.singh@linaro.org> wrote: > > > >> GPIO driver strength settings are not preserved across suspend/resume > >> for s5pc100, s5pv210 and Exynos platforms which has been the cause of > >> mmc/sd card read/write failures after resume. Fix this by saving and > >> restoring the GPIO driver strength register settings across suspend/resume. > >> > >> Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> > > > > On a related theme: I am thinking about how to support preserving > > drive strength (etc) across suspend/resume and deepsleep in the > > pincontrol subsystem. > > > > Currently I am playing with the idea to let pin groups have states, > > as the different configurations seem to be 90% or so about very > > specific sleep modes, so say: > > > > pinconf_set_group_state("mmcgroup", PINCONF_STATE_ACTIVE); > > pinconf_set_group_state("mmcgroup", PINCONF_STATE_SUSPENDED); > > pinconf_set_group_state("mmcgroup", PINCONF_STATE_SLEEP); > > > The API name suggests that power state is set based on a pin group. > For a driver, the following could be convenient as the driver transits > through various power management states. > > struct pinmux pmx; > pmx = pinmux_get(dev, "i2c0"); > > [...] > > pinmux_set_state(pmx, PINCONF_STATE_SLEEP); > [...] > pinmux_set_state(pmx, PINCONF_STATE_ACTIVE); That seems a more consistent driver-oriented API, yes. I'd of course lean towards adding the pin config data into the mapping table, and using the existing feature struct pinmux_map name field to name those states, and allowing dynamic transitioning between states, rather than adding a complete new API and namespace for these states.
On 15 November 2011 00:30, Stephen Warren <swarren@nvidia.com> wrote: > Thomas Abraham wrote at Monday, November 14, 2011 6:57 AM: >> On 12 November 2011 16:30, Linus Walleij <linus.walleij@linaro.org> wrote: >> > On Sat, Nov 12, 2011 at 6:01 AM, Inderpal Singh >> > <inderpal.singh@linaro.org> wrote: >> > >> >> GPIO driver strength settings are not preserved across suspend/resume >> >> for s5pc100, s5pv210 and Exynos platforms which has been the cause of >> >> mmc/sd card read/write failures after resume. Fix this by saving and >> >> restoring the GPIO driver strength register settings across suspend/resume. >> >> >> >> Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> >> > >> > On a related theme: I am thinking about how to support preserving >> > drive strength (etc) across suspend/resume and deepsleep in the >> > pincontrol subsystem. >> > >> > Currently I am playing with the idea to let pin groups have states, >> > as the different configurations seem to be 90% or so about very >> > specific sleep modes, so say: >> > >> > pinconf_set_group_state("mmcgroup", PINCONF_STATE_ACTIVE); >> > pinconf_set_group_state("mmcgroup", PINCONF_STATE_SUSPENDED); >> > pinconf_set_group_state("mmcgroup", PINCONF_STATE_SLEEP); >> >> >> The API name suggests that power state is set based on a pin group. >> For a driver, the following could be convenient as the driver transits >> through various power management states. >> >> struct pinmux pmx; >> pmx = pinmux_get(dev, "i2c0"); >> >> [...] >> >> pinmux_set_state(pmx, PINCONF_STATE_SLEEP); >> [...] >> pinmux_set_state(pmx, PINCONF_STATE_ACTIVE); > > That seems a more consistent driver-oriented API, yes. > > I'd of course lean towards adding the pin config data into the mapping > table, and using the existing feature struct pinmux_map name field to > name those states, and allowing dynamic transitioning between states, > rather than adding a complete new API and namespace for these states. I am not sure how this works, but with dynamic transitioning, will it be possible to program the pinmux registers dynamically when a driver/controller decides to idle for sometime and come back up again (not a system wide suspend-resume cycle). There might be a need for the driver to inform the pinmux subsystem of this case and allow the pinmux subsystem to re-program pinmux registers as required. Thanks, Thomas. > > -- > nvpublic > >
Thomas Abraham wrote at Monday, November 14, 2011 12:37 PM: ... > I am not sure how this works, but with dynamic transitioning, will it > be possible to program the pinmux registers dynamically when a > driver/controller decides to idle for sometime and come back up again > (not a system wide suspend-resume cycle). There might be a need for > the driver to inform the pinmux subsystem of this case and allow the > pinmux subsystem to re-program pinmux registers as required. Well, such a feature doesn't exist yet. But, I envisaged extending the existing: pmx = pinmux_get() pinmux_enable(pmx) ... pinmux_disable(pmx) pinmux_put(pmx) to something like: pmx = pinmux_get(active) pinmux_enable(pmx) ... pinmux_switch(suspend) ... pinmux_switch(active) ... pinmux_disable(pmx) pinmux_put(pmx) (I think LinusW proposed an alternative a while back last time we talked about this) This has received only limited discussion though, and there are probably many gotchas to think through; I imagine any such feature is a way off.
diff --git a/arch/arm/plat-samsung/include/plat/gpio-core.h b/arch/arm/plat-samsung/include/plat/gpio-core.h index 1fe6917..8871b4c 100644 --- a/arch/arm/plat-samsung/include/plat/gpio-core.h +++ b/arch/arm/plat-samsung/include/plat/gpio-core.h @@ -69,7 +69,7 @@ struct samsung_gpio_chip { int group; spinlock_t lock; #ifdef CONFIG_PM - u32 pm_save[4]; + u32 pm_save[5]; #endif }; diff --git a/arch/arm/plat-samsung/pm-gpio.c b/arch/arm/plat-samsung/pm-gpio.c index 4be016e..5493f38 100644 --- a/arch/arm/plat-samsung/pm-gpio.c +++ b/arch/arm/plat-samsung/pm-gpio.c @@ -21,12 +21,14 @@ #include <plat/gpio-core.h> #include <plat/pm.h> +#include <plat/cpu.h> /* PM GPIO helpers */ #define OFFS_CON (0x00) #define OFFS_DAT (0x04) #define OFFS_UP (0x08) +#define OFFS_DRV_STRGTH (0x0C) static void samsung_gpio_pm_1bit_save(struct samsung_gpio_chip *chip) { @@ -199,6 +201,9 @@ static void samsung_gpio_pm_4bit_save(struct samsung_gpio_chip *chip) chip->pm_save[2] = __raw_readl(chip->base + OFFS_DAT); chip->pm_save[3] = __raw_readl(chip->base + OFFS_UP); + if (soc_is_s5pc100() || soc_is_s5pv210() || soc_is_exynos4210()) + chip->pm_save[4] = __raw_readl(chip->base + OFFS_DRV_STRGTH); + if (chip->chip.ngpio > 8) chip->pm_save[0] = __raw_readl(chip->base - 4); } @@ -285,6 +290,9 @@ static void samsung_gpio_pm_4bit_resume(struct samsung_gpio_chip *chip) __raw_writel(chip->pm_save[2], base + OFFS_DAT); __raw_writel(chip->pm_save[3], base + OFFS_UP); + if (soc_is_s5pc100() || soc_is_s5pv210() || soc_is_exynos4210()) + __raw_writel(chip->pm_save[4], base + OFFS_DRV_STRGTH); + if (chip->chip.ngpio > 8) { S3C_PMDBG("%s: CON4 %08x,%08x => %08x,%08x, DAT %08x => %08x\n", chip->chip.label, old_gpcon[0], old_gpcon[1], @@ -338,12 +346,13 @@ void samsung_pm_save_gpios(void) samsung_pm_save_gpio(ourchip); - S3C_PMDBG("%s: save %08x,%08x,%08x,%08x\n", + S3C_PMDBG("%s: save %08x,%08x,%08x,%08x,%08x\n", ourchip->chip.label, ourchip->pm_save[0], ourchip->pm_save[1], ourchip->pm_save[2], - ourchip->pm_save[3]); + ourchip->pm_save[3], + ourchip->pm_save[4]); gpio_nr += ourchip->chip.ngpio; gpio_nr += CONFIG_S3C_GPIO_SPACE;
GPIO driver strength settings are not preserved across suspend/resume for s5pc100, s5pv210 and Exynos platforms which has been the cause of mmc/sd card read/write failures after resume. Fix this by saving and restoring the GPIO driver strength register settings across suspend/resume. Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> --- 1. This change is applicable only for s5pc100, s5pv210 and Exynos platforms. For all other platforms, the driver strength registers are part of special port configuration register (SPCON) and these registers are saved and restored separately from the GPIO bank registers. For s5pc100, s5pv210 and Exynos platforms, the driver strength values are saved along with the GPIO bank registers. 2. An additional entry is added to the 'pm_save' array of 'struct samsung_gpio_chip' for saving driver strength values. 3. Tested with v210 and v310 smdk boards arch/arm/plat-samsung/include/plat/gpio-core.h | 2 +- arch/arm/plat-samsung/pm-gpio.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-)