Re: [PATCH 4.13 015/109] pinctrl/amd: save pin registers over suspend/resume
On Tue, Sep 26, 2017 at 4:07 PM, Petr Mladek wrote: > IMHO, we need to select CONFIG_PINMUX for when CONFIG_PINCTRL_AMD > is selected. The following patch helped here: Took the patch out of this mail, applied it to fixes and tagged it for stable. Will send it to Torvals ASAP. Yours, Linus Walleij
Re: [PATCH 4.13 015/109] pinctrl/amd: save pin registers over suspend/resume
On Wed, Sep 27, 2017 at 1:29 PM, Petr Mladek wrote: > On Wed 2017-09-27 10:39:26, Greg Kroah-Hartman wrote: >> On Tue, Sep 26, 2017 at 04:07:45PM +0200, Petr Mladek wrote: >> > On Sun 2017-09-24 22:32:36, Greg Kroah-Hartman wrote: >> > > 4.13-stable review patch. If anyone has any objections, please let me >> > > know. >> > > >> > > -- >> > > >> > > From: Daniel Drake >> > > >> > > commit 79d2c8bede2c93f9432d7da0bc2f76a195c90fc0 upstream. >> > > >> > > The touchpad in the Asus laptop models X505BA/BP and X542BA/BP is >> > > unresponsive after suspend/resume. The following error appears during >> > > resume: >> > > >> > > i2c_hid i2c-ELAN1300:00: failed to reset device. >> > > >> > > The problem here is that i2c_hid does not notice the interrupt being >> > > generated at this point, because the GPIO is no longer configured >> > > for interrupts. >> > > >> > > Fix this by saving pinctrl-amd pin registers during suspend and >> > > restoring them at resume time. >> > > >> > > Based on code from pinctrl-intel. >> > > >> > > Signed-off-by: Daniel Drake >> > > Signed-off-by: Linus Walleij >> > > Signed-off-by: Greg Kroah-Hartman >> > > >> > > --- a/drivers/pinctrl/pinctrl-amd.c >> > > +++ b/drivers/pinctrl/pinctrl-amd.c >> > > @@ -725,6 +726,69 @@ static const struct pinconf_ops amd_pinc >> > > .pin_config_group_set = amd_pinconf_group_set, >> > > }; >> > > >> > > +#ifdef CONFIG_PM_SLEEP >> > > +static bool amd_gpio_should_save(struct amd_gpio *gpio_dev, unsigned >> > > int pin) >> > > +{ >> > > + const struct pin_desc *pd = pin_desc_get(gpio_dev->pctrl, pin); >> > > + >> > > + if (!pd) >> > > + return false; >> > > + >> > > + /* >> > > + * Only restore the pin if it is actually in use by the kernel (or >> > > + * by userspace). >> > > + */ >> > > + if (pd->mux_owner || pd->gpio_owner || >> > >> > This code causes a compilation error in the current Linus' tree >> > (4.14-rc2+). I guess that the same problem will be also in >> > older trees. >> > >> > IMHO, we need to select CONFIG_PINMUX for when CONFIG_PINCTRL_AMD >> > is selected. The following patch helped here: >> > >> > >From f83130d3b5c147b4d400922847ca2751d8020ab5 Mon Sep 17 00:00:00 2001 >> > From: Petr Mladek >> > Date: Tue, 26 Sep 2017 15:51:28 +0200 >> > Subject: [PATCH] pinctrl/amd: Fix build dependency on pinmux code >> > MIME-Version: 1.0 >> > Content-Type: text/plain; charset=UTF-8 >> > Content-Transfer-Encoding: 8bit >> > >> > The commit 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over >> > suspend/resume") caused the following compilation errors: >> > >> > drivers/pinctrl/pinctrl-amd.c: In function ‘amd_gpio_should_save’: >> > drivers/pinctrl/pinctrl-amd.c:741:8: error: ‘const struct pin_desc’ has no >> > member named ‘mux_owner’ >> > if (pd->mux_owner || pd->gpio_owner || >> > ^ >> > drivers/pinctrl/pinctrl-amd.c:741:25: error: ‘const struct pin_desc’ has >> > no member named ‘gpio_owner’ >> > if (pd->mux_owner || pd->gpio_owner || >> > >> > We need to enable CONFIG_PINMUX for this driver as well. >> > >> > Fixes: 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over >> > suspend/resume") >> > Signed-off-by: Petr Mladek >> > --- >> > drivers/pinctrl/Kconfig | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig >> > index 1778cf4f81c7..82cd8b08d71f 100644 >> > --- a/drivers/pinctrl/Kconfig >> > +++ b/drivers/pinctrl/Kconfig >> > @@ -100,6 +100,7 @@ config PINCTRL_AMD >> > tristate "AMD GPIO pin control" >> > depends on GPIOLIB >> > select GPIOLIB_IRQCHIP >> > + select PINMUX >> > select PINCONF >> > select GENERIC_PINCONF >> > help >> > -- >> > 1.8.5.6 >> >> What is the commit id for this patch in Linus's tree? I don't see it >> there... > > This extra patch has not been pushed to the Linus' tree yet. > It was only my proposal to fix the problem. I did not find > the original patch discussed on the kernel mailing list. > Therefore I sent this follow up fix as a reply to > the 4.13-stable review mail. > > I hope that Linus Walleij would notice, review, and push it > into Linus' tree. I missed it. OK looking for it so I can apply it. Yours, Linus Walleij
Re: [PATCH 4.13 015/109] pinctrl/amd: save pin registers over suspend/resume
On Wed 2017-09-27 10:39:26, Greg Kroah-Hartman wrote: > On Tue, Sep 26, 2017 at 04:07:45PM +0200, Petr Mladek wrote: > > On Sun 2017-09-24 22:32:36, Greg Kroah-Hartman wrote: > > > 4.13-stable review patch. If anyone has any objections, please let me > > > know. > > > > > > -- > > > > > > From: Daniel Drake > > > > > > commit 79d2c8bede2c93f9432d7da0bc2f76a195c90fc0 upstream. > > > > > > The touchpad in the Asus laptop models X505BA/BP and X542BA/BP is > > > unresponsive after suspend/resume. The following error appears during > > > resume: > > > > > > i2c_hid i2c-ELAN1300:00: failed to reset device. > > > > > > The problem here is that i2c_hid does not notice the interrupt being > > > generated at this point, because the GPIO is no longer configured > > > for interrupts. > > > > > > Fix this by saving pinctrl-amd pin registers during suspend and > > > restoring them at resume time. > > > > > > Based on code from pinctrl-intel. > > > > > > Signed-off-by: Daniel Drake > > > Signed-off-by: Linus Walleij > > > Signed-off-by: Greg Kroah-Hartman > > > > > > --- a/drivers/pinctrl/pinctrl-amd.c > > > +++ b/drivers/pinctrl/pinctrl-amd.c > > > @@ -725,6 +726,69 @@ static const struct pinconf_ops amd_pinc > > > .pin_config_group_set = amd_pinconf_group_set, > > > }; > > > > > > +#ifdef CONFIG_PM_SLEEP > > > +static bool amd_gpio_should_save(struct amd_gpio *gpio_dev, unsigned int > > > pin) > > > +{ > > > + const struct pin_desc *pd = pin_desc_get(gpio_dev->pctrl, pin); > > > + > > > + if (!pd) > > > + return false; > > > + > > > + /* > > > + * Only restore the pin if it is actually in use by the kernel (or > > > + * by userspace). > > > + */ > > > + if (pd->mux_owner || pd->gpio_owner || > > > > This code causes a compilation error in the current Linus' tree > > (4.14-rc2+). I guess that the same problem will be also in > > older trees. > > > > IMHO, we need to select CONFIG_PINMUX for when CONFIG_PINCTRL_AMD > > is selected. The following patch helped here: > > > > >From f83130d3b5c147b4d400922847ca2751d8020ab5 Mon Sep 17 00:00:00 2001 > > From: Petr Mladek > > Date: Tue, 26 Sep 2017 15:51:28 +0200 > > Subject: [PATCH] pinctrl/amd: Fix build dependency on pinmux code > > MIME-Version: 1.0 > > Content-Type: text/plain; charset=UTF-8 > > Content-Transfer-Encoding: 8bit > > > > The commit 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over > > suspend/resume") caused the following compilation errors: > > > > drivers/pinctrl/pinctrl-amd.c: In function ‘amd_gpio_should_save’: > > drivers/pinctrl/pinctrl-amd.c:741:8: error: ‘const struct pin_desc’ has no > > member named ‘mux_owner’ > > if (pd->mux_owner || pd->gpio_owner || > > ^ > > drivers/pinctrl/pinctrl-amd.c:741:25: error: ‘const struct pin_desc’ has no > > member named ‘gpio_owner’ > > if (pd->mux_owner || pd->gpio_owner || > > > > We need to enable CONFIG_PINMUX for this driver as well. > > > > Fixes: 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over > > suspend/resume") > > Signed-off-by: Petr Mladek > > --- > > drivers/pinctrl/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig > > index 1778cf4f81c7..82cd8b08d71f 100644 > > --- a/drivers/pinctrl/Kconfig > > +++ b/drivers/pinctrl/Kconfig > > @@ -100,6 +100,7 @@ config PINCTRL_AMD > > tristate "AMD GPIO pin control" > > depends on GPIOLIB > > select GPIOLIB_IRQCHIP > > + select PINMUX > > select PINCONF > > select GENERIC_PINCONF > > help > > -- > > 1.8.5.6 > > What is the commit id for this patch in Linus's tree? I don't see it > there... This extra patch has not been pushed to the Linus' tree yet. It was only my proposal to fix the problem. I did not find the original patch discussed on the kernel mailing list. Therefore I sent this follow up fix as a reply to the 4.13-stable review mail. I hope that Linus Walleij would notice, review, and push it into Linus' tree. Best Regards, Petr
Re: [PATCH 4.13 015/109] pinctrl/amd: save pin registers over suspend/resume
On Tue, Sep 26, 2017 at 04:07:45PM +0200, Petr Mladek wrote: > On Sun 2017-09-24 22:32:36, Greg Kroah-Hartman wrote: > > 4.13-stable review patch. If anyone has any objections, please let me know. > > > > -- > > > > From: Daniel Drake > > > > commit 79d2c8bede2c93f9432d7da0bc2f76a195c90fc0 upstream. > > > > The touchpad in the Asus laptop models X505BA/BP and X542BA/BP is > > unresponsive after suspend/resume. The following error appears during > > resume: > > > > i2c_hid i2c-ELAN1300:00: failed to reset device. > > > > The problem here is that i2c_hid does not notice the interrupt being > > generated at this point, because the GPIO is no longer configured > > for interrupts. > > > > Fix this by saving pinctrl-amd pin registers during suspend and > > restoring them at resume time. > > > > Based on code from pinctrl-intel. > > > > Signed-off-by: Daniel Drake > > Signed-off-by: Linus Walleij > > Signed-off-by: Greg Kroah-Hartman > > > > --- a/drivers/pinctrl/pinctrl-amd.c > > +++ b/drivers/pinctrl/pinctrl-amd.c > > @@ -725,6 +726,69 @@ static const struct pinconf_ops amd_pinc > > .pin_config_group_set = amd_pinconf_group_set, > > }; > > > > +#ifdef CONFIG_PM_SLEEP > > +static bool amd_gpio_should_save(struct amd_gpio *gpio_dev, unsigned int > > pin) > > +{ > > + const struct pin_desc *pd = pin_desc_get(gpio_dev->pctrl, pin); > > + > > + if (!pd) > > + return false; > > + > > + /* > > +* Only restore the pin if it is actually in use by the kernel (or > > +* by userspace). > > +*/ > > + if (pd->mux_owner || pd->gpio_owner || > > This code causes a compilation error in the current Linus' tree > (4.14-rc2+). I guess that the same problem will be also in > older trees. > > IMHO, we need to select CONFIG_PINMUX for when CONFIG_PINCTRL_AMD > is selected. The following patch helped here: > > >From f83130d3b5c147b4d400922847ca2751d8020ab5 Mon Sep 17 00:00:00 2001 > From: Petr Mladek > Date: Tue, 26 Sep 2017 15:51:28 +0200 > Subject: [PATCH] pinctrl/amd: Fix build dependency on pinmux code > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > The commit 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over > suspend/resume") caused the following compilation errors: > > drivers/pinctrl/pinctrl-amd.c: In function ‘amd_gpio_should_save’: > drivers/pinctrl/pinctrl-amd.c:741:8: error: ‘const struct pin_desc’ has no > member named ‘mux_owner’ > if (pd->mux_owner || pd->gpio_owner || > ^ > drivers/pinctrl/pinctrl-amd.c:741:25: error: ‘const struct pin_desc’ has no > member named ‘gpio_owner’ > if (pd->mux_owner || pd->gpio_owner || > > We need to enable CONFIG_PINMUX for this driver as well. > > Fixes: 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over > suspend/resume") > Signed-off-by: Petr Mladek > --- > drivers/pinctrl/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig > index 1778cf4f81c7..82cd8b08d71f 100644 > --- a/drivers/pinctrl/Kconfig > +++ b/drivers/pinctrl/Kconfig > @@ -100,6 +100,7 @@ config PINCTRL_AMD > tristate "AMD GPIO pin control" > depends on GPIOLIB > select GPIOLIB_IRQCHIP > + select PINMUX > select PINCONF > select GENERIC_PINCONF > help > -- > 1.8.5.6 What is the commit id for this patch in Linus's tree? I don't see it there... thanks, greg k-h
Re: [PATCH 4.13 015/109] pinctrl/amd: save pin registers over suspend/resume
On Sun 2017-09-24 22:32:36, Greg Kroah-Hartman wrote: > 4.13-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Daniel Drake > > commit 79d2c8bede2c93f9432d7da0bc2f76a195c90fc0 upstream. > > The touchpad in the Asus laptop models X505BA/BP and X542BA/BP is > unresponsive after suspend/resume. The following error appears during > resume: > > i2c_hid i2c-ELAN1300:00: failed to reset device. > > The problem here is that i2c_hid does not notice the interrupt being > generated at this point, because the GPIO is no longer configured > for interrupts. > > Fix this by saving pinctrl-amd pin registers during suspend and > restoring them at resume time. > > Based on code from pinctrl-intel. > > Signed-off-by: Daniel Drake > Signed-off-by: Linus Walleij > Signed-off-by: Greg Kroah-Hartman > > --- a/drivers/pinctrl/pinctrl-amd.c > +++ b/drivers/pinctrl/pinctrl-amd.c > @@ -725,6 +726,69 @@ static const struct pinconf_ops amd_pinc > .pin_config_group_set = amd_pinconf_group_set, > }; > > +#ifdef CONFIG_PM_SLEEP > +static bool amd_gpio_should_save(struct amd_gpio *gpio_dev, unsigned int pin) > +{ > + const struct pin_desc *pd = pin_desc_get(gpio_dev->pctrl, pin); > + > + if (!pd) > + return false; > + > + /* > + * Only restore the pin if it is actually in use by the kernel (or > + * by userspace). > + */ > + if (pd->mux_owner || pd->gpio_owner || This code causes a compilation error in the current Linus' tree (4.14-rc2+). I guess that the same problem will be also in older trees. IMHO, we need to select CONFIG_PINMUX for when CONFIG_PINCTRL_AMD is selected. The following patch helped here: >From f83130d3b5c147b4d400922847ca2751d8020ab5 Mon Sep 17 00:00:00 2001 From: Petr Mladek Date: Tue, 26 Sep 2017 15:51:28 +0200 Subject: [PATCH] pinctrl/amd: Fix build dependency on pinmux code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The commit 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over suspend/resume") caused the following compilation errors: drivers/pinctrl/pinctrl-amd.c: In function ‘amd_gpio_should_save’: drivers/pinctrl/pinctrl-amd.c:741:8: error: ‘const struct pin_desc’ has no member named ‘mux_owner’ if (pd->mux_owner || pd->gpio_owner || ^ drivers/pinctrl/pinctrl-amd.c:741:25: error: ‘const struct pin_desc’ has no member named ‘gpio_owner’ if (pd->mux_owner || pd->gpio_owner || We need to enable CONFIG_PINMUX for this driver as well. Fixes: 79d2c8bede2c93f943 ("pinctrl/amd: save pin registers over suspend/resume") Signed-off-by: Petr Mladek --- drivers/pinctrl/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 1778cf4f81c7..82cd8b08d71f 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -100,6 +100,7 @@ config PINCTRL_AMD tristate "AMD GPIO pin control" depends on GPIOLIB select GPIOLIB_IRQCHIP + select PINMUX select PINCONF select GENERIC_PINCONF help -- 1.8.5.6
[PATCH 4.13 015/109] pinctrl/amd: save pin registers over suspend/resume
4.13-stable review patch. If anyone has any objections, please let me know. -- From: Daniel Drake commit 79d2c8bede2c93f9432d7da0bc2f76a195c90fc0 upstream. The touchpad in the Asus laptop models X505BA/BP and X542BA/BP is unresponsive after suspend/resume. The following error appears during resume: i2c_hid i2c-ELAN1300:00: failed to reset device. The problem here is that i2c_hid does not notice the interrupt being generated at this point, because the GPIO is no longer configured for interrupts. Fix this by saving pinctrl-amd pin registers during suspend and restoring them at resume time. Based on code from pinctrl-intel. Signed-off-by: Daniel Drake Signed-off-by: Linus Walleij Signed-off-by: Greg Kroah-Hartman --- drivers/pinctrl/pinctrl-amd.c | 75 ++ drivers/pinctrl/pinctrl-amd.h |1 2 files changed, 76 insertions(+) --- a/drivers/pinctrl/pinctrl-amd.c +++ b/drivers/pinctrl/pinctrl-amd.c @@ -36,6 +36,7 @@ #include #include +#include "core.h" #include "pinctrl-utils.h" #include "pinctrl-amd.h" @@ -725,6 +726,69 @@ static const struct pinconf_ops amd_pinc .pin_config_group_set = amd_pinconf_group_set, }; +#ifdef CONFIG_PM_SLEEP +static bool amd_gpio_should_save(struct amd_gpio *gpio_dev, unsigned int pin) +{ + const struct pin_desc *pd = pin_desc_get(gpio_dev->pctrl, pin); + + if (!pd) + return false; + + /* +* Only restore the pin if it is actually in use by the kernel (or +* by userspace). +*/ + if (pd->mux_owner || pd->gpio_owner || + gpiochip_line_is_irq(&gpio_dev->gc, pin)) + return true; + + return false; +} + +int amd_gpio_suspend(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct amd_gpio *gpio_dev = platform_get_drvdata(pdev); + struct pinctrl_desc *desc = gpio_dev->pctrl->desc; + int i; + + for (i = 0; i < desc->npins; i++) { + int pin = desc->pins[i].number; + + if (!amd_gpio_should_save(gpio_dev, pin)) + continue; + + gpio_dev->saved_regs[i] = readl(gpio_dev->base + pin*4); + } + + return 0; +} + +int amd_gpio_resume(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct amd_gpio *gpio_dev = platform_get_drvdata(pdev); + struct pinctrl_desc *desc = gpio_dev->pctrl->desc; + int i; + + for (i = 0; i < desc->npins; i++) { + int pin = desc->pins[i].number; + + if (!amd_gpio_should_save(gpio_dev, pin)) + continue; + + writel(gpio_dev->saved_regs[i], gpio_dev->base + pin*4); + } + + return 0; +} + +static const struct dev_pm_ops amd_gpio_pm_ops = { + SET_LATE_SYSTEM_SLEEP_PM_OPS(amd_gpio_suspend, +amd_gpio_resume) +}; +#endif + static struct pinctrl_desc amd_pinctrl_desc = { .pins = kerncz_pins, .npins = ARRAY_SIZE(kerncz_pins), @@ -764,6 +828,14 @@ static int amd_gpio_probe(struct platfor return -EINVAL; } +#ifdef CONFIG_PM_SLEEP + gpio_dev->saved_regs = devm_kcalloc(&pdev->dev, amd_pinctrl_desc.npins, + sizeof(*gpio_dev->saved_regs), + GFP_KERNEL); + if (!gpio_dev->saved_regs) + return -ENOMEM; +#endif + gpio_dev->pdev = pdev; gpio_dev->gc.direction_input= amd_gpio_direction_input; gpio_dev->gc.direction_output = amd_gpio_direction_output; @@ -853,6 +925,9 @@ static struct platform_driver amd_gpio_d .driver = { .name = "amd_gpio", .acpi_match_table = ACPI_PTR(amd_gpio_acpi_match), +#ifdef CONFIG_PM_SLEEP + .pm = &amd_gpio_pm_ops, +#endif }, .probe = amd_gpio_probe, .remove = amd_gpio_remove, --- a/drivers/pinctrl/pinctrl-amd.h +++ b/drivers/pinctrl/pinctrl-amd.h @@ -97,6 +97,7 @@ struct amd_gpio { unsigned inthwbank_num; struct resource *res; struct platform_device *pdev; + u32 *saved_regs; }; /* KERNCZ configuration*/