Re: rtsx_pci not restoring ASPM state after suspend/resume

[James Ettle]

Hello,

On Tue, 2020-07-28 at 18:06 -0500, Bjorn Helgaas wrote:
> 
> I tried to deduce the problem from the code in aspm.c, but I didn't
> see the problem.  If you have the ability to build a kernel with a
> debug patch, can you boot with the patch below and collect the dmesg
> log?

I've built such a kernel and attached the dmesg output at

https://bugzilla.kernel.org/attachment.cgi?id=290713

For this, the machine was booted from off, no funny udev rules or sysfs
tinkering.

Thanks,
-James

Re: rtsx_pci not restoring ASPM state after suspend/resume

[James Ettle]

On Mon, 2020-07-27 at 16:47 -0500, Bjorn Helgaas wrote:
> 
> I don't see anything in rtsx that enables L0s.  Can you collect the
> dmesg log when booting with "pci=earlydump"?  That will show whether
> the BIOS left it this way.  The PCI core isn't supposed to do this,
> so
> if it did, we need to fix that.
> 

dmesg log attached to the bugzilla as:

https://bugzilla.kernel.org/attachment.cgi?id=290655

(to keep everything in one place).

Thanks,
-James

Re: rtsx_pci not restoring ASPM state after suspend/resume

[James Ettle]

On Mon, 2020-07-27 at 09:14 -0500, Bjorn Helgaas wrote:
> I don't know the connection between ASPM and package C-states, so I
> need to simplify this even more.  All I want to do right now is
> verify
> that if we don't have any outside influences on the ASPM
> configuration
> (eg, no manual changes and no udev rules), it stays the same across
> suspend/resume.

Basically this started from me observing deep package C-states weren't
being used, until I went and fiddled with the ASPM state of the
rtsx_pci card reader under sysfs -- so phenomenological poking on my
part.

> So let's read the ASPM state directly from the
> hardware like this:
> 
>   sudo lspci -vvs 00:1d.0 | egrep "^0|Lnk|L1|LTR|snoop"
>   sudo lspci -vvs 01:00   | egrep "^0|Lnk|L1|LTR|snoop"
> 
> Can you try that before and after suspend/resume?

I've attached these to the bugzilla entry at:

https://bugzilla.kernel.org/show_bug.cgi?id=208117

Spoiler: With no udev rules or suspend hooks, things are the same
before and after suspend/resume. One thing I do see (both before and
after) is that ASPM L0s and L1 is enabled for the card reader, but
disabled for the ethernet chip (does r8169 fiddle with ASPM too?).

[Oddly when I set ASPM (e.g. using udev) the lspci tools show ASPM
enabled after a suspend/resume, but still no deep package C-states
until I manually fiddle via sysfs on the card reader. Sorry if this
only muddies the water further!]

Thanks,
-James

Re: rtsx_pci not restoring ASPM state after suspend/resume

[James Ettle]

On Fri, 2020-07-24 at 18:13 -0500, Bjorn Helgaas wrote:
> 
> Maybe we should simplify this a little bit more.  James, if you don't
> touch ASPM config at all, either manually or via udev, does the ASPM
> configuration stay the same across suspend/resume?
> 

Yes, it stays the same. Explicitly: 

With the udev rule disabled, immediately following clean boot from
power-off (and no additional tinkering), ASPM is OFF to the best of my
knowledge:

 - link/l1_aspm in sysfs is 0 for PCI devices :01:00.[01];
 - the processor sleeps no deeper than package C3.

The situation above is the same following a suspend/resume cycle --
both in terms of sysfs, and observed package C-state occupancy.

[Tested on kernel 5.7.10, but the behaviour is the same as prior
kernels.]

Thanks,
James.

Re: linux-next: Signed-off-by missing for commit in the nfsd tree

[James Ettle]

Hello,

I'm OK with that. (This is the first time I've ventured into kernel space so I 
thought it better to at least sketch a solution and let the experts do it 
correctly ;) Glad it's passed the review!)

Thanks,
James.

On 23 February 2018 01:05:45 GMT+00:00, "J. Bruce Fields" 
<bfie...@fieldses.org> wrote:
>On Thu, Feb 22, 2018 at 06:39:43AM +, James Ettle wrote:
>> I only really posted this as a demo of a fix. I was hoping someone
>who actually knows what they're doing in the kernel would pick it up
>and make it proper.
>
>Whoops, sorry, I didn't realize that.
>
>But I don't see a problem with the patch as it is.  Are you OK with me
>adding your signed-off-by?
>
>--b.
>
>> 
>> Thanks,
>> James.
>> 
>> 
>> On 22 February 2018 04:17:23 GMT+00:00, Stephen Rothwell
><s...@canb.auug.org.au> wrote:
>> >Hi,
>> >
>> >Commit
>> >
>> >  35da1ba8af70 ("sunrpc: Fix unaligned access on sparc64")
>> >
>> >is missing a Signed-off-by from its author.
>> >
>> >-- 
>> >Cheers,
>> >Stephen Rothwell

Re: linux-next: Signed-off-by missing for commit in the nfsd tree

[James Ettle]

Hello,

I'm OK with that. (This is the first time I've ventured into kernel space so I 
thought it better to at least sketch a solution and let the experts do it 
correctly ;) Glad it's passed the review!)

Thanks,
James.

On 23 February 2018 01:05:45 GMT+00:00, "J. Bruce Fields" 
 wrote:
>On Thu, Feb 22, 2018 at 06:39:43AM +, James Ettle wrote:
>> I only really posted this as a demo of a fix. I was hoping someone
>who actually knows what they're doing in the kernel would pick it up
>and make it proper.
>
>Whoops, sorry, I didn't realize that.
>
>But I don't see a problem with the patch as it is.  Are you OK with me
>adding your signed-off-by?
>
>--b.
>
>> 
>> Thanks,
>> James.
>> 
>> 
>> On 22 February 2018 04:17:23 GMT+00:00, Stephen Rothwell
> wrote:
>> >Hi,
>> >
>> >Commit
>> >
>> >  35da1ba8af70 ("sunrpc: Fix unaligned access on sparc64")
>> >
>> >is missing a Signed-off-by from its author.
>> >
>> >-- 
>> >Cheers,
>> >Stephen Rothwell

Re: [PATCH 0/4] tpm: fix PS/2 devices not working on Braswell systems due CLKRUN enabled

[James Ettle]

OK, I built a kernel based on:

- git clone of git.infradead.org/users/jjs/linux-tpmdd.git taken last night
- applying the patch in https://patchwork.kernel.org/patch/10119417/
- stock config-4.14.6-300.fc27.x86_64 + make oldconfig

The tpm modules are loaded and the keyboard is working.

Note: I won't have access to this machine over the next few weeks.

Thanks,
James.


On 20/12/17 11:35, Javier Martinez Canillas wrote:
> Hello,
> 
> Commit 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
> added logic in the TPM TIS driver to disable the Low Pin Count CLKRUN
> signal during TPM transactions.
> 
> Unfortunately this breaks other devices that are attached to the LPC bus
> like for example PS/2 mouse and keyboards.
> 
> The bug was reported to the Fedora kernel [0] and the kernel bugzilla [1].
> This issue and the propossed solution were discussed in this [2] thread,
> and the reporter (Jame Ettle) confirmed that his system works again after
> the fix in this series.
> 
> The patches are based on top or Jarkko Sakkinen's linux-tpmdd [3] tree.
> 
> James,
> 
> Even when there shouldn't be any functional changes, I included some other
> fixes / cleanups in this series so it would be great if you can test them
> again. I can't because I don't have access to a machine affected by this.
> 
> [0]: https://bugzilla.redhat.com/show_bug.cgi?id=1498987
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=197287
> [2]: https://patchwork.kernel.org/patch/10119417/
> [3]: git.infradead.org/users/jjs/linux-tpmdd.git
> 
> Best regards,
> Javier
> 
> 
> Javier Martinez Canillas (4):
>   tpm: fix access attempt to an already unmapped I/O memory region
>   tpm: delete the TPM_TIS_CLK_ENABLE flag
>   tpm: follow coding style for variable declaration in
> tpm_tis_core_init()
>   tpm: only attempt to disable the LPC CLKRUN if is already enabled
> 
>  drivers/char/tpm/tpm_tis.c  | 17 +
>  drivers/char/tpm/tpm_tis_core.c | 24 +---
>  drivers/char/tpm/tpm_tis_core.h |  1 -
>  3 files changed, 18 insertions(+), 24 deletions(-)
> 

Re: [PATCH 0/4] tpm: fix PS/2 devices not working on Braswell systems due CLKRUN enabled

[James Ettle]

OK, I built a kernel based on:

- git clone of git.infradead.org/users/jjs/linux-tpmdd.git taken last night
- applying the patch in https://patchwork.kernel.org/patch/10119417/
- stock config-4.14.6-300.fc27.x86_64 + make oldconfig

The tpm modules are loaded and the keyboard is working.

Note: I won't have access to this machine over the next few weeks.

Thanks,
James.


On 20/12/17 11:35, Javier Martinez Canillas wrote:
> Hello,
> 
> Commit 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
> added logic in the TPM TIS driver to disable the Low Pin Count CLKRUN
> signal during TPM transactions.
> 
> Unfortunately this breaks other devices that are attached to the LPC bus
> like for example PS/2 mouse and keyboards.
> 
> The bug was reported to the Fedora kernel [0] and the kernel bugzilla [1].
> This issue and the propossed solution were discussed in this [2] thread,
> and the reporter (Jame Ettle) confirmed that his system works again after
> the fix in this series.
> 
> The patches are based on top or Jarkko Sakkinen's linux-tpmdd [3] tree.
> 
> James,
> 
> Even when there shouldn't be any functional changes, I included some other
> fixes / cleanups in this series so it would be great if you can test them
> again. I can't because I don't have access to a machine affected by this.
> 
> [0]: https://bugzilla.redhat.com/show_bug.cgi?id=1498987
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=197287
> [2]: https://patchwork.kernel.org/patch/10119417/
> [3]: git.infradead.org/users/jjs/linux-tpmdd.git
> 
> Best regards,
> Javier
> 
> 
> Javier Martinez Canillas (4):
>   tpm: fix access attempt to an already unmapped I/O memory region
>   tpm: delete the TPM_TIS_CLK_ENABLE flag
>   tpm: follow coding style for variable declaration in
> tpm_tis_core_init()
>   tpm: only attempt to disable the LPC CLKRUN if is already enabled
> 
>  drivers/char/tpm/tpm_tis.c  | 17 +
>  drivers/char/tpm/tpm_tis_core.c | 24 +---
>  drivers/char/tpm/tpm_tis_core.h |  1 -
>  3 files changed, 18 insertions(+), 24 deletions(-)
> 

Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

I'm OK with that.

Do you need any more info on the specifics of the machine I'm using?

(Note: I'm not currently aware of any BIOS updates for it. If this is 
ultimately a BIOS problem I wouldn't know what sort of report to file, or how 
to file it anyway...)

Thanks,
James

On 19/12/17 13:00, Jarkko Sakkinen wrote:
> On Mon, Dec 18, 2017 at 06:26:35PM +0000, James Ettle wrote:
>> The keyboard and touchpad work OK with the patch quoted below and the 
>> earlier two applied, i.e. the three patches with signatures:
>>
>> 667dcc75be864ff4c17cf58891853b7393bba3e2
>> db3248e8a036c39141c8f7e9f1cf5c5ae6815f76
>> 370d45a34dc8914066a995a3a6d6df1953ea9f60
>>
>> I applied these to a vanilla kernel.org 4.14.7 source using Fedora 
>> 4.14.5-300.fc27 .config. Confirmed the tpm modules are loaded.
>>
>> Tests:
>> 1. Keyboard and touchpad work OK on boot - PASS
>> 2. Still work after suspend/resume - PASS
>>
>> Let me know if you want any further tests.
>>
>> Many thanks,
>> James.
> 
> Thank you for your awesome help! I guess Javier can add your
> Tested-by to the patch?
> 
> /Jarkko
> 

Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

I'm OK with that.

Do you need any more info on the specifics of the machine I'm using?

(Note: I'm not currently aware of any BIOS updates for it. If this is 
ultimately a BIOS problem I wouldn't know what sort of report to file, or how 
to file it anyway...)

Thanks,
James

On 19/12/17 13:00, Jarkko Sakkinen wrote:
> On Mon, Dec 18, 2017 at 06:26:35PM +0000, James Ettle wrote:
>> The keyboard and touchpad work OK with the patch quoted below and the 
>> earlier two applied, i.e. the three patches with signatures:
>>
>> 667dcc75be864ff4c17cf58891853b7393bba3e2
>> db3248e8a036c39141c8f7e9f1cf5c5ae6815f76
>> 370d45a34dc8914066a995a3a6d6df1953ea9f60
>>
>> I applied these to a vanilla kernel.org 4.14.7 source using Fedora 
>> 4.14.5-300.fc27 .config. Confirmed the tpm modules are loaded.
>>
>> Tests:
>> 1. Keyboard and touchpad work OK on boot - PASS
>> 2. Still work after suspend/resume - PASS
>>
>> Let me know if you want any further tests.
>>
>> Many thanks,
>> James.
> 
> Thank you for your awesome help! I guess Javier can add your
> Tested-by to the patch?
> 
> /Jarkko
> 

Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

The keyboard and touchpad work OK with the patch quoted below and the earlier 
two applied, i.e. the three patches with signatures:

667dcc75be864ff4c17cf58891853b7393bba3e2
db3248e8a036c39141c8f7e9f1cf5c5ae6815f76
370d45a34dc8914066a995a3a6d6df1953ea9f60

I applied these to a vanilla kernel.org 4.14.7 source using Fedora 
4.14.5-300.fc27 .config. Confirmed the tpm modules are loaded.

Tests:
1. Keyboard and touchpad work OK on boot - PASS
2. Still work after suspend/resume - PASS

Let me know if you want any further tests.

Many thanks,
James.


On 18/12/17 12:29, Javier Martinez Canillas wrote:
> On 12/18/2017 01:22 PM, Javier Martinez Canillas wrote:
> 
> [snip]
> 
>>
>> James,
>>
>> Can you please test the following (untested) patch on top of the other two
>> mentioned patches to see if it makes a difference for you?
>>
> 
> I should had tried to at least compile the patch :)
> 
> Updated patch below:
> 
> From 370d45a34dc8914066a995a3a6d6df1953ea9f60 Mon Sep 17 00:00:00 2001
> From: Javier Martinez Canillas 
> Date: Mon, 18 Dec 2017 12:56:28 +0100
> Subject: [PATCH v2] tpm: only attempt to disable the LPC CLKRUN if is already
>  enabled
> 
> Commit 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
> added logic in the TPM TIS driver to disable the Low Pin Count CLKRUN
> signal during TPM transactions.
> 
> Unfortunately this breaks other devices that are attached to the LPC bus
> like for example PS/2 mouse and keyboards.
> 
> One flaw with the logic is that it assumes that the CLKRUN is always
> enabled, and so it unconditionally enables it after a TPM transaction.
> 
> But it could be that the CLKRUN signal was already disabled in the LPC
> bus and so after the driver probes, the signal will remain enabled which
> may break other devices transactions since the clocks will be restarted
> by the CLKRUN# signal.
> 
> Fixes: 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
> Signed-off-by: Javier Martinez Canillas 
> ---
>  drivers/char/tpm/tpm_tis_core.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index e7bd2e750f69..5f2b1fc2194f 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -688,7 +688,8 @@ static void tpm_tis_clkrun_enable(struct tpm_chip *chip, 
> bool value)
>   struct tpm_tis_data *data = dev_get_drvdata(>dev);
>   u32 clkrun_val;
>  
> - if (!IS_ENABLED(CONFIG_X86) || !is_bsw())
> + if (!IS_ENABLED(CONFIG_X86) || !is_bsw() ||
> + !data->ilb_base_addr)
>   return;
>  
>   if (value) {
> @@ -746,7 +747,7 @@ int tpm_tis_core_init(struct device *dev, struct 
> tpm_tis_data *priv, int irq,
> const struct tpm_tis_phy_ops *phy_ops,
> acpi_handle acpi_dev_handle)
>  {
> - u32 vendor, intfcaps, intmask;
> + u32 vendor, intfcaps, intmask, clkrun_val;
>   u8 rid;
>   int rc, probe;
>   struct tpm_chip *chip;
> @@ -772,6 +773,14 @@ int tpm_tis_core_init(struct device *dev, struct 
> tpm_tis_data *priv, int irq,
>   ILB_REMAP_SIZE);
>   if (!priv->ilb_base_addr)
>   return -ENOMEM;
> +
> + clkrun_val = ioread32(priv->ilb_base_addr + LPC_CNTRL_OFFSET);
> + /* Check if CLKRUN# is already not enabled in the LPC bus */
> + if (!(clkrun_val & LPC_CLKRUN_EN)) {
> + priv->flags |= TPM_TIS_CLK_ENABLE;
> + iounmap(priv->ilb_base_addr);
> + priv->ilb_base_addr = NULL;
> + }
>   }
>  
>   if (chip->ops->clk_enable != NULL)
> @@ -868,7 +877,7 @@ int tpm_tis_core_init(struct device *dev, struct 
> tpm_tis_data *priv, int irq,
>   }
>  
>   rc = tpm_chip_register(chip);
> - if (rc && is_bsw())
> + if (rc && is_bsw() && priv->ilb_base_addr)
>   iounmap(priv->ilb_base_addr);
>  
>   if (chip->ops->clk_enable != NULL)
> 

Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

The keyboard and touchpad work OK with the patch quoted below and the earlier 
two applied, i.e. the three patches with signatures:

667dcc75be864ff4c17cf58891853b7393bba3e2
db3248e8a036c39141c8f7e9f1cf5c5ae6815f76
370d45a34dc8914066a995a3a6d6df1953ea9f60

I applied these to a vanilla kernel.org 4.14.7 source using Fedora 
4.14.5-300.fc27 .config. Confirmed the tpm modules are loaded.

Tests:
1. Keyboard and touchpad work OK on boot - PASS
2. Still work after suspend/resume - PASS

Let me know if you want any further tests.

Many thanks,
James.


On 18/12/17 12:29, Javier Martinez Canillas wrote:
> On 12/18/2017 01:22 PM, Javier Martinez Canillas wrote:
> 
> [snip]
> 
>>
>> James,
>>
>> Can you please test the following (untested) patch on top of the other two
>> mentioned patches to see if it makes a difference for you?
>>
> 
> I should had tried to at least compile the patch :)
> 
> Updated patch below:
> 
> From 370d45a34dc8914066a995a3a6d6df1953ea9f60 Mon Sep 17 00:00:00 2001
> From: Javier Martinez Canillas 
> Date: Mon, 18 Dec 2017 12:56:28 +0100
> Subject: [PATCH v2] tpm: only attempt to disable the LPC CLKRUN if is already
>  enabled
> 
> Commit 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
> added logic in the TPM TIS driver to disable the Low Pin Count CLKRUN
> signal during TPM transactions.
> 
> Unfortunately this breaks other devices that are attached to the LPC bus
> like for example PS/2 mouse and keyboards.
> 
> One flaw with the logic is that it assumes that the CLKRUN is always
> enabled, and so it unconditionally enables it after a TPM transaction.
> 
> But it could be that the CLKRUN signal was already disabled in the LPC
> bus and so after the driver probes, the signal will remain enabled which
> may break other devices transactions since the clocks will be restarted
> by the CLKRUN# signal.
> 
> Fixes: 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
> Signed-off-by: Javier Martinez Canillas 
> ---
>  drivers/char/tpm/tpm_tis_core.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index e7bd2e750f69..5f2b1fc2194f 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -688,7 +688,8 @@ static void tpm_tis_clkrun_enable(struct tpm_chip *chip, 
> bool value)
>   struct tpm_tis_data *data = dev_get_drvdata(>dev);
>   u32 clkrun_val;
>  
> - if (!IS_ENABLED(CONFIG_X86) || !is_bsw())
> + if (!IS_ENABLED(CONFIG_X86) || !is_bsw() ||
> + !data->ilb_base_addr)
>   return;
>  
>   if (value) {
> @@ -746,7 +747,7 @@ int tpm_tis_core_init(struct device *dev, struct 
> tpm_tis_data *priv, int irq,
> const struct tpm_tis_phy_ops *phy_ops,
> acpi_handle acpi_dev_handle)
>  {
> - u32 vendor, intfcaps, intmask;
> + u32 vendor, intfcaps, intmask, clkrun_val;
>   u8 rid;
>   int rc, probe;
>   struct tpm_chip *chip;
> @@ -772,6 +773,14 @@ int tpm_tis_core_init(struct device *dev, struct 
> tpm_tis_data *priv, int irq,
>   ILB_REMAP_SIZE);
>   if (!priv->ilb_base_addr)
>   return -ENOMEM;
> +
> + clkrun_val = ioread32(priv->ilb_base_addr + LPC_CNTRL_OFFSET);
> + /* Check if CLKRUN# is already not enabled in the LPC bus */
> + if (!(clkrun_val & LPC_CLKRUN_EN)) {
> + priv->flags |= TPM_TIS_CLK_ENABLE;
> + iounmap(priv->ilb_base_addr);
> + priv->ilb_base_addr = NULL;
> + }
>   }
>  
>   if (chip->ops->clk_enable != NULL)
> @@ -868,7 +877,7 @@ int tpm_tis_core_init(struct device *dev, struct 
> tpm_tis_data *priv, int irq,
>   }
>  
>   rc = tpm_chip_register(chip);
> - if (rc && is_bsw())
> + if (rc && is_bsw() && priv->ilb_base_addr)
>   iounmap(priv->ilb_base_addr);
>  
>   if (chip->ops->clk_enable != NULL)
> 

Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

OK, I built a kernel 4.14.5 from vanilla kernel.org sources using the Fedora 
.config (couldn't get the Fedora package to build).

I see no difference with both [0, 1] patches applied and the tpm modules loaded 
-- no keyboard or touchpad.

[Note: I'm not *consciously* using the TPM for anything, but /dev/tpm0 is 
present and have never initialised it. I don't know if anything in Fedora uses 
it by default if present.]

Regards,
James.


On 12/12/17 11:38, Javier Martinez Canillas wrote:
> Hello James,
> 
> On 12/11/2017 08:37 PM, James Ettle wrote:
>> Hello,
>>
>> [First: Apologies if cross-posting from Kernel.org BZ is bad form; my distro 
>> BZ advised I post this to your mailing list as well.]
>>
>> Situation: enabling TPM on a Clevo W510LU with an Intel N3160 CPU breaks 
>> PS/2 keyboard and mouse. They just don't respond until after a 
>> suspend/resume cycle, and after that they later stop after a while.
>>
>> I have confirmed this by blacklisting tpm modules. I noticed this first with 
>> kernel 4.13, and have bisected it down to:
>>
>> 5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad is the first bad commit
>> commit 5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad
>> Author: Azhar Shaikh <azhar.sha...@intel.com>
>> Date:   Sun Jun 18 19:17:59 2017 -0700
>>
>> tpm: Enable CLKRUN protocol for Braswell systems
>> 
>> To overcome a hardware limitation on Intel Braswell systems,
>> disable CLKRUN protocol during TPM transactions and re-enable
>> once the transaction is completed.
>> 
>> Signed-off-by: Azhar Shaikh <azhar.sha...@intel.com>
>> Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>
>> Tested-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>
>> Signed-off-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>
>> Signed-off-by: James Morris <james.l.mor...@oracle.com>
>>
>> :04 04 5437c91886cb62c497255f2c60dbedd7268ab50d 
>> 1863a1738ded35a817aad52f9f2b451bd43623d7 M   drivers
>>
>> Currently in Kernel.org bugzilla 197287.
>>
>> Please let me know if you need any further info.
>>
> 
> I don't have access to a Braswell machine to reproduce this. I tried to do it
> on different Intel systems by modifying is_bsw() to always return true, but
> that didn't work either. They work correctly even when CLKRUN_EN is toggled.
> 
> I'm not familiar with LPC so please let me know if my assumptions are wrong,
> but I find suspicious that a driver for a single device attached to the bus
> can control the CLKRUN# signal which AFAIU may be needed for other devices.
> 
> So that would explain why the mentioned commit causes issues for PS/2 mouse
> and keyboards, since these are attached to the LPC bus and may rely on the
> CLKRUN# signal for proper operation.
> 
> I see that the following [0,1] patches for the tpm_tis driver landed a few
> days ago, they change how the CLKRUN protocol is enabled/disabled. Instead
> of doing it per each TPM transaction, it does it once for the duration of
> a TPM command.
> 
> Not sure if that will make things better or worse for you, but it would be
> good to try in case it makes a difference.
> 
> [0]: 
> http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/667dcc75be864ff4c17cf58891853b7393bba3e2
> [1]: 
> http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/db3248e8a036c39141c8f7e9f1cf5c5ae6815f76
> 
>> Many thanks,
>> James.
>>
> 
> Best regards,
> 

Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

OK, I built a kernel 4.14.5 from vanilla kernel.org sources using the Fedora 
.config (couldn't get the Fedora package to build).

I see no difference with both [0, 1] patches applied and the tpm modules loaded 
-- no keyboard or touchpad.

[Note: I'm not *consciously* using the TPM for anything, but /dev/tpm0 is 
present and have never initialised it. I don't know if anything in Fedora uses 
it by default if present.]

Regards,
James.


On 12/12/17 11:38, Javier Martinez Canillas wrote:
> Hello James,
> 
> On 12/11/2017 08:37 PM, James Ettle wrote:
>> Hello,
>>
>> [First: Apologies if cross-posting from Kernel.org BZ is bad form; my distro 
>> BZ advised I post this to your mailing list as well.]
>>
>> Situation: enabling TPM on a Clevo W510LU with an Intel N3160 CPU breaks 
>> PS/2 keyboard and mouse. They just don't respond until after a 
>> suspend/resume cycle, and after that they later stop after a while.
>>
>> I have confirmed this by blacklisting tpm modules. I noticed this first with 
>> kernel 4.13, and have bisected it down to:
>>
>> 5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad is the first bad commit
>> commit 5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad
>> Author: Azhar Shaikh 
>> Date:   Sun Jun 18 19:17:59 2017 -0700
>>
>> tpm: Enable CLKRUN protocol for Braswell systems
>> 
>> To overcome a hardware limitation on Intel Braswell systems,
>> disable CLKRUN protocol during TPM transactions and re-enable
>> once the transaction is completed.
>> 
>> Signed-off-by: Azhar Shaikh 
>> Reviewed-by: Jarkko Sakkinen 
>> Tested-by: Jarkko Sakkinen 
>> Signed-off-by: Jarkko Sakkinen 
>> Signed-off-by: James Morris 
>>
>> :04 04 5437c91886cb62c497255f2c60dbedd7268ab50d 
>> 1863a1738ded35a817aad52f9f2b451bd43623d7 M   drivers
>>
>> Currently in Kernel.org bugzilla 197287.
>>
>> Please let me know if you need any further info.
>>
> 
> I don't have access to a Braswell machine to reproduce this. I tried to do it
> on different Intel systems by modifying is_bsw() to always return true, but
> that didn't work either. They work correctly even when CLKRUN_EN is toggled.
> 
> I'm not familiar with LPC so please let me know if my assumptions are wrong,
> but I find suspicious that a driver for a single device attached to the bus
> can control the CLKRUN# signal which AFAIU may be needed for other devices.
> 
> So that would explain why the mentioned commit causes issues for PS/2 mouse
> and keyboards, since these are attached to the LPC bus and may rely on the
> CLKRUN# signal for proper operation.
> 
> I see that the following [0,1] patches for the tpm_tis driver landed a few
> days ago, they change how the CLKRUN protocol is enabled/disabled. Instead
> of doing it per each TPM transaction, it does it once for the duration of
> a TPM command.
> 
> Not sure if that will make things better or worse for you, but it would be
> good to try in case it makes a difference.
> 
> [0]: 
> http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/667dcc75be864ff4c17cf58891853b7393bba3e2
> [1]: 
> http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/db3248e8a036c39141c8f7e9f1cf5c5ae6815f76
> 
>> Many thanks,
>> James.
>>
> 
> Best regards,
> 

[BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

Hello,

[First: Apologies if cross-posting from Kernel.org BZ is bad form; my distro BZ 
advised I post this to your mailing list as well.]

Situation: enabling TPM on a Clevo W510LU with an Intel N3160 CPU breaks PS/2 
keyboard and mouse. They just don't respond until after a suspend/resume cycle, 
and after that they later stop after a while.

I have confirmed this by blacklisting tpm modules. I noticed this first with 
kernel 4.13, and have bisected it down to:

5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad is the first bad commit
commit 5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad
Author: Azhar Shaikh 
Date:   Sun Jun 18 19:17:59 2017 -0700

tpm: Enable CLKRUN protocol for Braswell systems

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh 
Reviewed-by: Jarkko Sakkinen 
Tested-by: Jarkko Sakkinen 
Signed-off-by: Jarkko Sakkinen 
Signed-off-by: James Morris 

:04 04 5437c91886cb62c497255f2c60dbedd7268ab50d 
1863a1738ded35a817aad52f9f2b451bd43623d7 M  drivers

Currently in Kernel.org bugzilla 197287.

Please let me know if you need any further info.

Many thanks,
James.

[BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system

[James Ettle]

Hello,

[First: Apologies if cross-posting from Kernel.org BZ is bad form; my distro BZ 
advised I post this to your mailing list as well.]

Situation: enabling TPM on a Clevo W510LU with an Intel N3160 CPU breaks PS/2 
keyboard and mouse. They just don't respond until after a suspend/resume cycle, 
and after that they later stop after a while.

I have confirmed this by blacklisting tpm modules. I noticed this first with 
kernel 4.13, and have bisected it down to:

5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad is the first bad commit
commit 5e572cab92f0bb56ca1e6e5ee4d807663a7ccbad
Author: Azhar Shaikh 
Date:   Sun Jun 18 19:17:59 2017 -0700

tpm: Enable CLKRUN protocol for Braswell systems

To overcome a hardware limitation on Intel Braswell systems,
disable CLKRUN protocol during TPM transactions and re-enable
once the transaction is completed.

Signed-off-by: Azhar Shaikh 
Reviewed-by: Jarkko Sakkinen 
Tested-by: Jarkko Sakkinen 
Signed-off-by: Jarkko Sakkinen 
Signed-off-by: James Morris 

:04 04 5437c91886cb62c497255f2c60dbedd7268ab50d 
1863a1738ded35a817aad52f9f2b451bd43623d7 M  drivers

Currently in Kernel.org bugzilla 197287.

Please let me know if you need any further info.

Many thanks,
James.