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

From: Hans de Goede
Date: Wed Dec 20 2017 - 06:43:48 EST


On 20-12-17 12:35, Javier Martinez Canillas wrote:

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.


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.


Best regards,

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: 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(-)

Note I'm just reading along here, but I'm wondering if both the TPM
and now also some PS/2 controllers need CLK_RUN to be disabled,
why don't we just disable it once permanently and be done with it?

It seems that on machines with a PS/2 controller connected to
the LPC bus the BIOS is already doing this, so I've a feeling that
it not being done on devices with a TPM is a bug in the firmware
there and we should just disable it everywhere (and probably
find a better place then the TPM driver to do the disabling).

Note this is just an observation, I could be completely wrong here,
but I've a feeling that just disabling CLKRUN all together is the
right thing to do and that seems like an easier fix to me.

Specifically what worries me is: what if another driver also takes
the temporarily disable CLK_RUN approach because of similar issues?
Now we've 2 drivers playing with the CLKRUN state and racing with
each others.