| Message ID | 20230517122931.22385-1-peter.ujfalusi@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-,for,6.4] tpm: tpm_tis: Disable interrupts for AEON UPX-i11 | expand |
On 17/05/2023 15:29, Peter Ujfalusi wrote: > The interrupts initially works on the device but they will stop arriving > after about 200 interrupts. > > On system reboot/shutdown this will cause a long wait (120000 jiffies). > > The interrupts on this device got enabled by commit > e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > Prior to this point the interrupts were not enabled on this machine. > > Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> > --- > Hi, > > This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to > reboot on this machine, linux-next have > e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices > > I'm not sure if I shouold send this on top of next or mainline is fine, please > let me know the preferred way to get this to 6.4. In 6.3 the kernel prints this on boot: # dmesg -w | grep tpm tpm_tis MSFT0101:00: 2.0 TPM (device-id 0x1B, rev-id 22) tpm tpm0: [Firmware Bug]: TPM interrupt not working, polling instead It is interesting that with 6.4 most of the times the interrupts got enabled (without this patch) resulting stall during reboot/shutdown but there are few boots when the driver falls back to polling and thus the TPM driver works. The command which 'locks' the system is TPM2_CC_SHUTDOWN, it is given TPM_UNDEFINED as duration index by tpm2_ordinal_duration_index(). > > Regards, > Peter > > drivers/char/tpm/tpm_tis.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 7af389806643..aad682c2ab21 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { > DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), > }, > }, > + { > + .callback = tpm_tis_disable_irq, > + .ident = "UPX-TGL", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "AAEON"), > + }, > + }, > {} > }; >
On Wed May 17, 2023 at 3:29 PM EEST, Peter Ujfalusi wrote: > The interrupts initially works on the device but they will stop arriving > after about 200 interrupts. > > On system reboot/shutdown this will cause a long wait (120000 jiffies). > > The interrupts on this device got enabled by commit > e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > Prior to this point the interrupts were not enabled on this machine. > > Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") Complements -> Fixes > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> > --- > Hi, > > This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to > reboot on this machine, linux-next have > e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices > > I'm not sure if I shouold send this on top of next or mainline is fine, please > let me know the preferred way to get this to 6.4. > > Regards, > Peter > > drivers/char/tpm/tpm_tis.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 7af389806643..aad682c2ab21 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { > DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), > }, > }, > + { > + .callback = tpm_tis_disable_irq, > + .ident = "UPX-TGL", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "AAEON"), > + }, > + }, > {} > }; > > -- > 2.40.1 Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
On Thu May 18, 2023 at 9:37 PM EEST, Jarkko Sakkinen wrote: > On Wed May 17, 2023 at 3:29 PM EEST, Peter Ujfalusi wrote: > > The interrupts initially works on the device but they will stop arriving > > after about 200 interrupts. > > > > On system reboot/shutdown this will cause a long wait (120000 jiffies). > > > > The interrupts on this device got enabled by commit > > e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > > > Prior to this point the interrupts were not enabled on this machine. > > > > Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > Complements -> Fixes > > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> > > --- > > Hi, > > > > This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to > > reboot on this machine, linux-next have > > e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices > > > > I'm not sure if I shouold send this on top of next or mainline is fine, please > > let me know the preferred way to get this to 6.4. > > > > Regards, > > Peter > > > > drivers/char/tpm/tpm_tis.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > > index 7af389806643..aad682c2ab21 100644 > > --- a/drivers/char/tpm/tpm_tis.c > > +++ b/drivers/char/tpm/tpm_tis.c > > @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { > > DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), > > }, > > }, > > + { > > + .callback = tpm_tis_disable_irq, > > + .ident = "UPX-TGL", > > + .matches = { > > + DMI_MATCH(DMI_SYS_VENDOR, "AAEON"), > > + }, > > + }, > > {} > > }; > > > > -- > > 2.40.1 > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> I adjusted the commit message a bit: https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=d46f47575cf97d397fbe8a6150353f41d2917936 BR, Jarkko
On Wed, May 17, 2023 at 03:29:31PM +0300, Peter Ujfalusi wrote: > The interrupts initially works on the device but they will stop arriving > after about 200 interrupts. > > On system reboot/shutdown this will cause a long wait (120000 jiffies). > > The interrupts on this device got enabled by commit > e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > Prior to this point the interrupts were not enabled on this machine. > > Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> > --- > Hi, > > This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to > reboot on this machine, linux-next have > e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices > > I'm not sure if I shouold send this on top of next or mainline is fine, please > let me know the preferred way to get this to 6.4. > > Regards, > Peter > > drivers/char/tpm/tpm_tis.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 7af389806643..aad682c2ab21 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { > DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), > }, > }, > + { > + .callback = tpm_tis_disable_irq, > + .ident = "UPX-TGL", Is this the product version returned by dmidecode? If yes, then the entry could be made more specific by adding a DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable for this device instead of any system that matches the vendor AAEON. > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "AAEON"), > + }, > + }, > {} > }; > > -- > 2.40.1 >
On Thu May 18, 2023 at 9:50 PM EEST, Jerry Snitselaar wrote: > On Wed, May 17, 2023 at 03:29:31PM +0300, Peter Ujfalusi wrote: > > The interrupts initially works on the device but they will stop arriving > > after about 200 interrupts. > > > > On system reboot/shutdown this will cause a long wait (120000 jiffies). > > > > The interrupts on this device got enabled by commit > > e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > > > Prior to this point the interrupts were not enabled on this machine. > > > > Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> > > --- > > Hi, > > > > This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to > > reboot on this machine, linux-next have > > e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices > > > > I'm not sure if I shouold send this on top of next or mainline is fine, please > > let me know the preferred way to get this to 6.4. > > > > Regards, > > Peter > > > > drivers/char/tpm/tpm_tis.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > > index 7af389806643..aad682c2ab21 100644 > > --- a/drivers/char/tpm/tpm_tis.c > > +++ b/drivers/char/tpm/tpm_tis.c > > @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { > > DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), > > }, > > }, > > + { > > + .callback = tpm_tis_disable_irq, > > + .ident = "UPX-TGL", > > Is this the product version returned by dmidecode? If yes, > then the entry could be made more specific by adding a > DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable > for this device instead of any system that matches the vendor > AAEON. I can squash this to the commit I pushed (it is not yet mirrored to linux-next), if I get the dmidecode info. BR, Jarkko
On 18/05/2023 21:53, Jarkko Sakkinen wrote: > On Thu May 18, 2023 at 9:50 PM EEST, Jerry Snitselaar wrote: >> On Wed, May 17, 2023 at 03:29:31PM +0300, Peter Ujfalusi wrote: >>> The interrupts initially works on the device but they will stop arriving >>> after about 200 interrupts. >>> >>> On system reboot/shutdown this will cause a long wait (120000 jiffies). >>> >>> The interrupts on this device got enabled by commit >>> e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") >>> >>> Prior to this point the interrupts were not enabled on this machine. >>> >>> Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") >>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> >>> --- >>> Hi, >>> >>> This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to >>> reboot on this machine, linux-next have >>> e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices >>> >>> I'm not sure if I shouold send this on top of next or mainline is fine, please >>> let me know the preferred way to get this to 6.4. >>> >>> Regards, >>> Peter >>> >>> drivers/char/tpm/tpm_tis.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c >>> index 7af389806643..aad682c2ab21 100644 >>> --- a/drivers/char/tpm/tpm_tis.c >>> +++ b/drivers/char/tpm/tpm_tis.c >>> @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { >>> DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), >>> }, >>> }, >>> + { >>> + .callback = tpm_tis_disable_irq, >>> + .ident = "UPX-TGL", >> >> Is this the product version returned by dmidecode? If yes, >> then the entry could be made more specific by adding a >> DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable >> for this device instead of any system that matches the vendor >> AAEON. The version is used to differentiate the revisions of the UPX-i11 boards, and this issue present in all revisions. > I can squash this to the commit I pushed (it is not yet mirrored > to linux-next), if I get the dmidecode info. System Information Manufacturer: AAEON Product Name: UPX-TGL01 Version: V1.0 Serial Number: Default string UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 Wake-up Type: Power Switch SKU Number: Default string Family: Default string I have used this description as it it is used for SOF, probably DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01") should be added? Oh, yes, the product name match should be there, we have TigerLake specific matching, so SOF is looking for AAEON device with TGL. Sorry for missing this. > > BR, Jarkko
On 18/05/2023 23:24, Péter Ujfalusi wrote: > > The version is used to differentiate the revisions of the UPX-i11 > boards, and this issue present in all revisions. > >> I can squash this to the commit I pushed (it is not yet mirrored >> to linux-next), if I get the dmidecode info. > > System Information > Manufacturer: AAEON > Product Name: UPX-TGL01 > Version: V1.0 > Serial Number: Default string > UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 > Wake-up Type: Power Switch > SKU Number: Default string > Family: Default string > > I have used this description as it it is used for SOF, probably > DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01") > should be added? Jarkko: I have tested that adding the DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01") works. I would also do a small update to commit message: "120000 jiffies" to "120000 msec". On my setup 120000 msec ends up to be 120000 jiffies. How do you prefer to handle this? I can send a v2 on top of linux-next / mainline I can send a fixup patch which can be squashed to the patch you have in your master branch atm Or you add this line by yourself? Either way is fine for me, whichever works best for you.
On Thu, 2023-05-18 at 23:24 +0300, Péter Ujfalusi wrote: > > On 18/05/2023 21:53, Jarkko Sakkinen wrote: > > On Thu May 18, 2023 at 9:50 PM EEST, Jerry Snitselaar wrote: > > > On Wed, May 17, 2023 at 03:29:31PM +0300, Peter Ujfalusi wrote: > > > > The interrupts initially works on the device but they will stop arriving > > > > after about 200 interrupts. > > > > > > > > On system reboot/shutdown this will cause a long wait (120000 jiffies). > > > > > > > > The interrupts on this device got enabled by commit > > > > e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > > > > > > > Prior to this point the interrupts were not enabled on this machine. > > > > > > > > Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") > > > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> > > > > --- > > > > Hi, > > > > > > > > This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to > > > > reboot on this machine, linux-next have > > > > e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices > > > > > > > > I'm not sure if I shouold send this on top of next or mainline is fine, please > > > > let me know the preferred way to get this to 6.4. > > > > > > > > Regards, > > > > Peter > > > > > > > > drivers/char/tpm/tpm_tis.c | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > > > > index 7af389806643..aad682c2ab21 100644 > > > > --- a/drivers/char/tpm/tpm_tis.c > > > > +++ b/drivers/char/tpm/tpm_tis.c > > > > @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { > > > > DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), > > > > }, > > > > }, > > > > + { > > > > + .callback = tpm_tis_disable_irq, > > > > + .ident = "UPX-TGL", > > > > > > Is this the product version returned by dmidecode? If yes, > > > then the entry could be made more specific by adding a > > > DMI_MATCH(DMI_PRODUCT_VERSION, "UPX-TGL"), and only disable > > > for this device instead of any system that matches the vendor > > > AAEON. > > The version is used to differentiate the revisions of the UPX-i11 > boards, and this issue present in all revisions. > > > I can squash this to the commit I pushed (it is not yet mirrored > > to linux-next), if I get the dmidecode info. > > System Information > Manufacturer: AAEON > Product Name: UPX-TGL01 > Version: V1.0 > Serial Number: Default string > UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 > Wake-up Type: Power Switch > SKU Number: Default string > Family: Default string > > I have used this description as it it is used for SOF, probably > DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01") > should be added? > > Oh, yes, the product name match should be there, we have TigerLake > specific matching, so SOF is looking for AAEON device with TGL. > > Sorry for missing this. I sent a PR with the original patch because I know that you've tested it and it has worked for you. BR, Jarkko
On Mon May 22, 2023 at 10:40 AM EEST, Péter Ujfalusi wrote: > > On 18/05/2023 23:24, Péter Ujfalusi wrote: > > > > The version is used to differentiate the revisions of the UPX-i11 > > boards, and this issue present in all revisions. > > > >> I can squash this to the commit I pushed (it is not yet mirrored > >> to linux-next), if I get the dmidecode info. > > > > System Information > > Manufacturer: AAEON > > Product Name: UPX-TGL01 > > Version: V1.0 > > Serial Number: Default string > > UUID: a300091d-fb1c-ce1c-1d30-0007328efc11 > > Wake-up Type: Power Switch > > SKU Number: Default string > > Family: Default string > > > > I have used this description as it it is used for SOF, probably > > DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01") > > should be added? > > Jarkko: I have tested that adding the > DMI_MATCH(DMI_PRODUCT_NAME, "UPX-TGL01") > works. > > I would also do a small update to commit message: "120000 jiffies" to > "120000 msec". > On my setup 120000 msec ends up to be 120000 jiffies. > > How do you prefer to handle this? > I can send a v2 on top of linux-next / mainline > I can send a fixup patch which can be squashed to the patch you have in > your master branch atm > Or you add this line by yourself? > > Either way is fine for me, whichever works best for you. If you want to send a follow-up please do, and I can pick it. BR, Jarkko
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index 7af389806643..aad682c2ab21 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -122,6 +122,13 @@ static const struct dmi_system_id tpm_tis_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), }, }, + { + .callback = tpm_tis_disable_irq, + .ident = "UPX-TGL", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "AAEON"), + }, + }, {} };
The interrupts initially works on the device but they will stop arriving after about 200 interrupts. On system reboot/shutdown this will cause a long wait (120000 jiffies). The interrupts on this device got enabled by commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") Prior to this point the interrupts were not enabled on this machine. Complements: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> --- Hi, This patch applies on top of mainline since 6.4-rc1 takes about 2 minutes to reboot on this machine, linux-next have e7d3e5c4b1dd tpm/tpm_tis: Disable interrupts for more Lenovo devices I'm not sure if I shouold send this on top of next or mainline is fine, please let me know the preferred way to get this to 6.4. Regards, Peter drivers/char/tpm/tpm_tis.c | 7 +++++++ 1 file changed, 7 insertions(+)