Do not set interrupts which are not supported by the hardware. Instead use
the information from the capability query and activate only the reported
interrupts.
Signed-off-by: Lino Sanfilippo <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 68 ++++++++++++++++++++++-------------------
drivers/char/tpm/tpm_tis_core.h | 1 +
2 files changed, 38 insertions(+), 31 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index e8ab218..7b00161 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -980,13 +980,47 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
if (rc)
goto out_err;
+ /* Figure out the capabilities */
+ rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
+ if (rc < 0)
+ goto out_err;
+
+ dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
+ intfcaps);
+ if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
+ dev_dbg(dev, "\tBurst Count Static\n");
+ if (intfcaps & TPM_INTF_CMD_READY_INT) {
+ priv->supported_irqs |= TPM_INTF_CMD_READY_INT;
+ dev_dbg(dev, "\tCommand Ready Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
+ dev_dbg(dev, "\tInterrupt Edge Falling\n");
+ if (intfcaps & TPM_INTF_INT_EDGE_RISING)
+ dev_dbg(dev, "\tInterrupt Edge Rising\n");
+ if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
+ dev_dbg(dev, "\tInterrupt Level Low\n");
+ if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
+ dev_dbg(dev, "\tInterrupt Level High\n");
+ if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT) {
+ priv->supported_irqs |= TPM_INTF_LOCALITY_CHANGE_INT;
+ dev_dbg(dev, "\tLocality Change Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_STS_VALID_INT) {
+ priv->supported_irqs |= TPM_INTF_STS_VALID_INT;
+ dev_dbg(dev, "\tSts Valid Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_DATA_AVAIL_INT) {
+ priv->supported_irqs |= TPM_INTF_DATA_AVAIL_INT;
+ dev_dbg(dev, "\tData Avail Int Support\n");
+ }
+
/* Take control of the TPM's interrupt hardware and shut it off */
rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
if (rc < 0)
goto out_err;
- intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
- TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
+ intmask |= priv->supported_irqs;
+
intmask &= ~TPM_GLOBAL_INT_ENABLE;
tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
@@ -1015,32 +1049,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
goto out_err;
}
- /* Figure out the capabilities */
- rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
- if (rc < 0)
- goto out_err;
-
- dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
- intfcaps);
- if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
- dev_dbg(dev, "\tBurst Count Static\n");
- if (intfcaps & TPM_INTF_CMD_READY_INT)
- dev_dbg(dev, "\tCommand Ready Int Support\n");
- if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
- dev_dbg(dev, "\tInterrupt Edge Falling\n");
- if (intfcaps & TPM_INTF_INT_EDGE_RISING)
- dev_dbg(dev, "\tInterrupt Edge Rising\n");
- if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
- dev_dbg(dev, "\tInterrupt Level Low\n");
- if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
- dev_dbg(dev, "\tInterrupt Level High\n");
- if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
- dev_dbg(dev, "\tLocality Change Int Support\n");
- if (intfcaps & TPM_INTF_STS_VALID_INT)
- dev_dbg(dev, "\tSts Valid Int Support\n");
- if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
- dev_dbg(dev, "\tData Avail Int Support\n");
-
/* INTERRUPT Setup */
init_waitqueue_head(&priv->read_queue);
init_waitqueue_head(&priv->int_queue);
@@ -1110,9 +1118,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
if (rc < 0)
goto out;
- intmask |= TPM_INTF_CMD_READY_INT
- | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT
- | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE;
+ intmask |= chip->supported_irqs | TPM_GLOBAL_INT_ENABLE;
tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index dc5f92b..8ff62213 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -89,6 +89,7 @@ struct tpm_tis_data {
u16 manufacturer_id;
int locality;
int irq;
+ unsigned int supported_irqs;
unsigned int flags;
void __iomem *ilb_base_addr;
u16 clkrun_enabled;
--
2.7.4
Hi Lino,
I love your patch! Yet something to improve:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on kees/for-next/pstore linus/master v5.12 next-20210423]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lino-Sanfilippo/Fixes-for-TPM-interrupt-handling/20210426-075042
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git e2cb6b891ad2b8caa9131e3be70f45243df82a80
config: x86_64-randconfig-a001-20210426 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d941863de2becb3d8d2e00676fc7125974934c7f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/1b37b97e7f8601b56f1e8aa069aec29c7d80d175
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Lino-Sanfilippo/Fixes-for-TPM-interrupt-handling/20210426-075042
git checkout 1b37b97e7f8601b56f1e8aa069aec29c7d80d175
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/char/tpm/tpm_tis_core.c:1077:19: error: no member named 'supported_irqs' in 'struct tpm_chip'
intmask |= chip->supported_irqs | TPM_GLOBAL_INT_ENABLE;
~~~~ ^
1 error generated.
vim +1077 drivers/char/tpm/tpm_tis_core.c
1055
1056 #ifdef CONFIG_PM_SLEEP
1057 static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
1058 {
1059 struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
1060 u32 intmask;
1061 int rc;
1062
1063 if (chip->ops->clk_enable != NULL)
1064 chip->ops->clk_enable(chip, true);
1065
1066 /* reenable interrupts that device may have lost or
1067 * BIOS/firmware may have disabled
1068 */
1069 rc = tpm_tis_write8(priv, TPM_INT_VECTOR(priv->locality), priv->irq);
1070 if (rc < 0)
1071 goto out;
1072
1073 rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
1074 if (rc < 0)
1075 goto out;
1076
> 1077 intmask |= chip->supported_irqs | TPM_GLOBAL_INT_ENABLE;
1078
1079 tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
1080
1081 out:
1082 if (chip->ops->clk_enable != NULL)
1083 chip->ops->clk_enable(chip, false);
1084
1085 return;
1086 }
1087
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On 4/25/21 7:47 PM, Lino Sanfilippo wrote:
> Do not set interrupts which are not supported by the hardware. Instead use
> the information from the capability query and activate only the reported
> interrupts.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>
Reviewed-by: Stefan Berger <[email protected]>