pinctrl: intel: Actually disable Tx and Rx buffers on GPIO request
Subsystems (3)
PATCH 1/1 pinctrl: intel: Actually disable Tx and Rx buffers on GPIO request
Mistakenly the buffers (input and output) become together enabled for a little period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit af7e3eeb84e2 ("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmware of some touch pads in an awkward state that needs a full power off to recover. Fix this by, as stated in a culprit commit, disabling the buffers.
--- drivers/pinctrl/intel/pinctrl-intel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pinctrl/intel/pinctrl-intel.c +2 -2
@@ -452,8 +452,8 @@ static void intel_gpio_set_gpio_mode(void __iomem *padcfg0) value |=PADCFG0_PMODE_GPIO ;/* Disable input and output buffers */ - value &= ~PADCFG0_GPIORXDIS ; - value &= ~PADCFG0_GPIOTXDIS ; + value |=PADCFG0_GPIORXDIS ; + value |=PADCFG0_GPIOTXDIS ;/* Disable SCI/SMI/NMI generation */ value &= ~(PADCFG0_GPIROUTIOXAPIC |PADCFG0_GPIROUTSCI );
Signature · 1 line
2.29.2
Quoted 8 lines · Andy Shevchenko
Mistakenly the buffers (input and output) become together enabled for a little period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit
af7e3eeb84e2("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmware of some touch pads in an awkward state that needs a full power off to recover. Fix this by, as stated in a culprit commit, disabling the buffers.
Linus, since we are at rc7 I'm not going to send a PR for this single fix (I truly do not expect anything to come). Please, proceed as usual (either to v5.10 or to v5.11 cycle, there is not much difference b/c culptin is sitting in v5.9).
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 11 lines · Andy Shevchenko
Quoted 8 lines · Andy Shevchenko
Mistakenly the buffers (input and output) become together enabled for a little period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit
af7e3eeb84e2("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmware of some touch pads in an awkward state that needs a full power off to recover. Fix this by, as stated in a culprit commit, disabling the buffers.
Quoted 4 lines
Linus, since we are at rc7 I'm not going to send a PR for this single fix (I truly do not expect anything to come). Please, proceed as usual (either to v5.10 or to v5.11 cycle, there is not much difference b/c culptin is sitting in v5.9).
OK no problem, I applied the patch directly for fixes so we get it in ASAP.
Yours,
Linus Walleij
Quoted 13 lines · Linus Walleij
Quoted 11 lines · Andy Shevchenko
Quoted 8 lines · Andy Shevchenko
Mistakenly the buffers (input and output) become together enabled for a little period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit
af7e3eeb84e2("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmware of some touch pads in an awkward state that needs a full power off to recover. Fix this by, as stated in a culprit commit, disabling the buffers.
Quoted 6 lines
Quoted 4 lines
Linus, since we are at rc7 I'm not going to send a PR for this single fix (I truly do not expect anything to come). Please, proceed as usual (either to v5.10 or to v5.11 cycle, there is not much difference b/c culptin is sitting in v5.9).
OK no problem, I applied the patch directly for fixes so we get it in ASAP.
Linus, thanks, however something happened to it. i.e. BugLink tag disappeared. It's not good, because due to that link and Kai's report I quickly understood what was the issue.
Besides that commit message has some grammar / style issues. Can you please fix above and use below (fixed) text as commit message?
---8<---8<---
Mistakenly the buffers (input and output) become together enabled for a short period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit af7e3eeb84e2 ("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmwares of some touch pads to an awkward state that needs a full power off to recover. Fix this by, as stated in the culprit commit, disabling the buffers.
P.S. I'm looking at 5f4592bc20c8 in your pin control tree, fixes branch.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Linus Walleij
Quoted 2 lines · Andy Shevchenko
On Mon, Dec 7, 2020 at 8:02 PM Andy Shevchenko
<[email protected]> wrote:
...
Quoted 24 lines
Quoted 6 lines
Quoted 4 lines
Linus, since we are at rc7 I'm not going to send a PR for this single fix (I truly do not expect anything to come). Please, proceed as usual (either to v5.10 or to v5.11 cycle, there is not much difference b/c culptin is sitting in v5.9).
OK no problem, I applied the patch directly for fixes so we get it in ASAP.
Linus, thanks, however something happened to it. i.e. BugLink tag disappeared. It's not good, because due to that link and Kai's report I quickly understood what was the issue.
Besides that commit message has some grammar / style issues. Can you please fix above and use below (fixed) text as commit message?
---8<---8<---
Mistakenly the buffers (input and output) become together enabled for a short period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit
af7e3eeb84e2("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmwares of some touch pads to an awkward state that needs a full power off to recover. Fix this by, as stated in the culprit commit, disabling the buffers.
Quoted 1 line
P.S. I'm looking at5f4592bc20c8in your pin control tree, fixes branch.
I can send a v2 for your convenience.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 1 line · Andy Shevchenko
On Tue, Dec 08, 2020 at 11:48:53AM +0200, Andy Shevchenko wrote:
...
Quoted 1 line
I can send a v2 for your convenience.
v2 has been sent.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 3 lines · Andy Shevchenko
Linus, thanks, however something happened to it. i.e. BugLink tag disappeared. It's not good, because due to that link and Kai's report I quickly understood what was the issue.
Oh I think that is an issue with the b4 tool that I use to extract patches. It was reported some time back and might not be fixed in the version I'm running.
Yours,
Linus Walleij
Quoted 8 lines · Andy Shevchenko
Mistakenly the buffers (input and output) become together enabled for a little period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit
af7e3eeb84e2("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmware of some touch pads in an awkward state that needs a full power off to recover. Fix this by, as stated in a culprit commit, disabling the buffers.
Quoted 4 lines
Fixes af7e3eeb84e2 ("pinctrl: intel: Disable input and output buffer when switching to GPIO")Reported-by Pierre-Louis Bossart <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>
Pierre-Louis Bossart
Tested-byQuoted 9 lines · Mika Westerberg
Quoted 8 lines · Andy Shevchenko
Mistakenly the buffers (input and output) become together enabled for a little period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit
af7e3eeb84e2("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmware of some touch pads in an awkward state that needs a full power off to recover. Fix this by, as stated in a culprit commit, disabling the buffers.
Quoted 6 lines
Quoted 4 lines
Fixes af7e3eeb84e2 ("pinctrl: intel: Disable input and output buffer when switching to GPIO")Reported-by Pierre-Louis Bossart <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>Acked-by Mika Westerberg <[email protected]>
On the initial device where I reported the problem, and with the battery disconnected to make sure there's no EC issue, this patch restores the touchpad functionality.
Quoted 12 lines · Andy Shevchenko
Mistakenly the buffers (input and output) become together enabled for a little period of time during GPIO request. This is problematic, because instead of initial motive to disable them in the commit
af7e3eeb84e2("pinctrl: intel: Disable input and output buffer when switching to GPIO"), the driven value on the pin, which might be used as an IRQ line, brings firmware of some touch pads in an awkward state that needs a full power off to recover. Fix this by, as stated in a culprit commit, disabling the buffers.
Quoted 4 lines
Fixes af7e3eeb84e2 ("pinctrl: intel: Disable input and output buffer when switching to GPIO")Reported-by Pierre-Louis Bossart <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>
Quoted 23 lines
---
drivers/pinctrl/intel/pinctrl-intel.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)drivers/pinctrl/intel/pinctrl-intel.c +2
-2@@ -452,8 +452,8 @@ static void intel_gpio_set_gpio_mode(void __iomem *padcfg0) value |=PADCFG0_PMODE_GPIO ;/* Disable input and output buffers */ - value &= ~PADCFG0_GPIORXDIS ; - value &= ~PADCFG0_GPIOTXDIS ; + value |=PADCFG0_GPIORXDIS ; + value |=PADCFG0_GPIOTXDIS ;/* Disable SCI/SMI/NMI generation */ value &= ~(PADCFG0_GPIROUTIOXAPIC |PADCFG0_GPIROUTSCI );Signature · 1 line
2.29.2