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

Commit Message

Javier Martinez Canillas Dec. 18, 2017, 11:11 a.m. UTC

On 12/16/2017 09:59 PM, Javier Martinez Canillas wrote:
> On 12/16/2017 06:01 PM, Jarkko Sakkinen wrote:
>> On Fri, 2017-12-15 at 10:38 -0700, Jason Gunthorpe wrote:
>>> On Fri, Dec 15, 2017 at 04:56:30PM +0200, Jarkko Sakkinen wrote:
>>>> On Thu, Dec 14, 2017 at 12:10:52PM -0700, Jason Gunthorpe wrote:
>>>>> On Thu, Dec 14, 2017 at 08:08:58PM +0100, Javier Martinez Canillas wrote:
>>>>>
>>>>>> Although probably reverting the offending commits is the right thing to do
>>>>>> until a proper solution is proposed.
>>>>>
>>>>> Yes, if a fix is not forthcoming soon Jarkko should revert.
>>>>>
>>>>
>>>> I think I should drop the current patch going to 4.16 and revert the old
>>>> patch. Do we agree on this?
>>>
>>> I think the entire is_bsw thing needs to go, clearly manipulating
>>> CLKRUN directly was not fully thought out?
>>>
>>> Hopefully Intel will come up with a fix patch to preserve PS/2
>>> functionality and it won't come to a revert..
>>
>> Agreed.
>>
> 
> Agreed. Although it would be good to have a solution soon, because this has been
> broken since v4.13 and v4.15-rc4 is likely to come this weekend. So we may end
> with Braswell system being non-function for 3 kernel releases...
> 
> As mentioned I can write a patch to hide this behavior under a Kconfig option
> that's disabled by default (and maybe depend on BROKEN) or a module parameter
> until a proper solution is found.
> 

I meant something like the following [0]. That way, it'll be disabled by default
but still users that really need this workaround can do:

$ ./scripts/config --enable EXPERT
$ ./scripts/config --enable CONFIG_TCG_TIS_ENABLE_CLKRUN_QUIRK

What do you think?

[0]:
From 2ac0d743392ab96d5bf804830e0f0763a253c6e8 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <[e-mail]>
Date: Mon, 18 Dec 2017 11:10:01 +0100
Subject: [PATCH 1/1] tpm: Add Kconfig option to avoid disabling LPC CLKRUN
 unconditionally

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 transations.

Unfortunately this breaks other devices that are attached into the LPC
bus (e.g: PS/2 mouse and keyboards), so the CLKRUN signal shouldn't be
disabled unconditionally.

Until a proper solution is found, add a Kconfig option to explicitly
enable that behavior if needed instead of doing it unconditionally on
all Braswell machines.

Fixes: 5e572cab92f0 ("tpm: Enable CLKRUN protocol for Braswell systems")
Signed-off-by: Javier Martinez Canillas <[e-mail]>
---
 drivers/char/tpm/Kconfig        | 10 ++++++++++
 drivers/char/tpm/tpm_tis_core.c |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

Comments

Pavel Machek Jan. 2, 2018, 8:53 p.m. UTC | #1

> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index e7bd2e750f69..0858c08b3b89 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -688,7 +688,7 @@ static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value)
>  	struct tpm_tis_data *data = dev_get_drvdata(&chip->dev);
>  	u32 clkrun_val;
>  
> -	if (!IS_ENABLED(CONFIG_X86) || !is_bsw())
> +	if (!IS_ENABLED(TCG_TIS_ENABLE_CLKRUN_QUIRK) || !is_bsw())

Missing CONFIG_?
								pavel

Patch

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index a30352202f1f..220bd24e399f 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -43,6 +43,16 @@  config TCG_TIS
 	  within Linux. To compile this driver as a module, choose  M here;
 	  the module will be called tpm_tis.
 
+config TCG_TIS_ENABLE_CLKRUN_QUIRK
+	bool "Enable LPC CLKRUN workaround for Braswell systems TPM" if EXPERT
+	depends on TCG_TIS_CORE && X86
+	default n
+	---help---
+	  On Intel Braswell systems, the Low Pin Count bus CLKRUN signal has to
+	  be disabled during TPM devices transactions to operate correctly. This
+	  could break other devices attached to the LPC bus (i.e: PS/2 mouse and
+	  keyboards) so only enable if the TPM is the only device in the LPC bus.
+
 config TCG_TIS_SPI
 	tristate "TPM Interface Specification 1.3 Interface / TPM 2.0 FIFO Interface - (SPI)"
 	depends on SPI
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index e7bd2e750f69..0858c08b3b89 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -688,7 +688,7 @@  static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value)
 	struct tpm_tis_data *data = dev_get_drvdata(&chip->dev);
 	u32 clkrun_val;
 
-	if (!IS_ENABLED(CONFIG_X86) || !is_bsw())
+	if (!IS_ENABLED(TCG_TIS_ENABLE_CLKRUN_QUIRK) || !is_bsw())
 		return;
 
 	if (value) {