2021-12-01 00:00:42

by Rob Barnes

[permalink] [raw]
Subject: [PATCH] char: tpm: cr50: Set TPM_FIRMWARE_POWER_MANAGED based on device property

Set TPM_FIRMWARE_POWER_MANAGED flag based on 'firmware-power-managed'
ACPI DSD property. For the CR50 TPM, this flag defaults to true when
the property is unset.

When this flag is set to false, the CR50 TPM driver will always send
a shutdown command whenever the system suspends.

Signed-off-by: Rob Barnes <[email protected]>
---
drivers/char/tpm/tpm_tis_i2c_cr50.c | 14 +++++++++++++-
drivers/char/tpm/tpm_tis_spi_cr50.c | 14 +++++++++++++-
2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
index c89278103703..70143cc4f4e8 100644
--- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
+++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
@@ -628,6 +628,17 @@ static bool tpm_cr50_i2c_req_canceled(struct tpm_chip *chip, u8 status)
return status == TPM_STS_COMMAND_READY;
}

+static bool tpm_cr50_i2c_is_firmware_power_managed(struct device *dev)
+{
+ u8 val;
+ int ret;
+ /* This flag should default true when the device property is not present */
+ ret = device_property_read_u8(dev, "firmware-power-managed", &val);
+ if (ret)
+ return 1;
+ return val;
+}
+
static const struct tpm_class_ops cr50_i2c = {
.flags = TPM_OPS_AUTO_STARTUP,
.status = &tpm_cr50_i2c_tis_status,
@@ -686,7 +697,8 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)

/* cr50 is a TPM 2.0 chip */
chip->flags |= TPM_CHIP_FLAG_TPM2;
- chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
+ if (tpm_cr50_i2c_is_firmware_power_managed(dev))
+ chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;

/* Default timeouts */
chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
index dae98dbeeeac..e01f7cc258ca 100644
--- a/drivers/char/tpm/tpm_tis_spi_cr50.c
+++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
@@ -185,6 +185,17 @@ static int cr50_spi_flow_control(struct tpm_tis_spi_phy *phy,
return 0;
}

+static bool tpm_cr50_spi_is_firmware_power_managed(struct device *dev)
+{
+ u8 val;
+ int ret;
+ /* This flag should default true when the device property is not present */
+ ret = device_property_read_u8(dev, "firmware-power-managed", &val);
+ if (ret)
+ return 1;
+ return val;
+}
+
static int tpm_tis_spi_cr50_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
u8 *in, const u8 *out)
{
@@ -309,7 +320,8 @@ int cr50_spi_probe(struct spi_device *spi)
cr50_print_fw_version(&phy->priv);

chip = dev_get_drvdata(&spi->dev);
- chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
+ if (tpm_cr50_spi_is_firmware_power_managed(dev))
+ chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;

return 0;
}
--
2.34.0.rc2.393.gf8c9666880-goog



2021-12-01 06:36:17

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] char: tpm: cr50: Set TPM_FIRMWARE_POWER_MANAGED based on device property

Hi Rob,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.16-rc3 next-20211201]
[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/Rob-Barnes/char-tpm-cr50-Set-TPM_FIRMWARE_POWER_MANAGED-based-on-device-property/20211201-080132
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 5d331b5922551637c586cdf5fdc1778910fc937f
config: hexagon-randconfig-r041-20211128 (https://download.01.org/0day-ci/archive/20211201/[email protected]/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
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
# https://github.com/0day-ci/linux/commit/4c5a69ab6ee4ba384abbbf714753053b5cd0de2c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rob-Barnes/char-tpm-cr50-Set-TPM_FIRMWARE_POWER_MANAGED-based-on-device-property/20211201-080132
git checkout 4c5a69ab6ee4ba384abbbf714753053b5cd0de2c
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/char/tpm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from drivers/char/tpm/tpm_tis_spi_cr50.c:18:
In file included from drivers/char/tpm/tpm_tis_core.h:22:
In file included from drivers/char/tpm/tpm.h:28:
include/linux/tpm_eventlog.h:167:6: warning: variable 'mapping_size' set but not used [-Wunused-but-set-variable]
int mapping_size;
^
>> drivers/char/tpm/tpm_tis_spi_cr50.c:319:45: error: use of undeclared identifier 'dev'
if (tpm_cr50_spi_is_firmware_power_managed(dev))
^
1 warning and 1 error generated.


vim +/dev +319 drivers/char/tpm/tpm_tis_spi_cr50.c

263
264 int cr50_spi_probe(struct spi_device *spi)
265 {
266 struct tpm_tis_spi_phy *phy;
267 struct cr50_spi_phy *cr50_phy;
268 int ret;
269 struct tpm_chip *chip;
270
271 cr50_phy = devm_kzalloc(&spi->dev, sizeof(*cr50_phy), GFP_KERNEL);
272 if (!cr50_phy)
273 return -ENOMEM;
274
275 phy = &cr50_phy->spi_phy;
276 phy->flow_control = cr50_spi_flow_control;
277 phy->wake_after = jiffies;
278 init_completion(&phy->ready);
279
280 cr50_phy->access_delay = CR50_NOIRQ_ACCESS_DELAY;
281 cr50_phy->last_access = jiffies;
282 mutex_init(&cr50_phy->time_track_mutex);
283
284 if (spi->irq > 0) {
285 ret = devm_request_irq(&spi->dev, spi->irq,
286 cr50_spi_irq_handler,
287 IRQF_TRIGGER_RISING | IRQF_ONESHOT,
288 "cr50_spi", cr50_phy);
289 if (ret < 0) {
290 if (ret == -EPROBE_DEFER)
291 return ret;
292 dev_warn(&spi->dev, "Requesting IRQ %d failed: %d\n",
293 spi->irq, ret);
294 /*
295 * This is not fatal, the driver will fall back to
296 * delays automatically, since ready will never
297 * be completed without a registered irq handler.
298 * So, just fall through.
299 */
300 } else {
301 /*
302 * IRQ requested, let's verify that it is actually
303 * triggered, before relying on it.
304 */
305 cr50_phy->irq_needs_confirmation = true;
306 }
307 } else {
308 dev_warn(&spi->dev,
309 "No IRQ - will use delays between transactions.\n");
310 }
311
312 ret = tpm_tis_spi_init(spi, phy, -1, &tpm_spi_cr50_phy_ops);
313 if (ret)
314 return ret;
315
316 cr50_print_fw_version(&phy->priv);
317
318 chip = dev_get_drvdata(&spi->dev);
> 319 if (tpm_cr50_spi_is_firmware_power_managed(dev))
320 chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
321
322 return 0;
323 }
324

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2021-12-02 20:07:07

by Rob Barnes

[permalink] [raw]
Subject: [PATCH v2] char: tpm: cr50: Set TPM_FIRMWARE_POWER_MANAGED based on device property

Set TPM_FIRMWARE_POWER_MANAGED flag based on 'firmware-power-managed'
ACPI DSD property. For the CR50 TPM, this flag defaults to true when
the property is unset.

When this flag is set to false, the CR50 TPM driver will always send
a shutdown command whenever the system suspends.

Signed-off-by: Rob Barnes <[email protected]>
---
drivers/char/tpm/tpm_tis_i2c_cr50.c | 14 +++++++++++++-
drivers/char/tpm/tpm_tis_spi_cr50.c | 14 +++++++++++++-
2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
index c89278103703..70143cc4f4e8 100644
--- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
+++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
@@ -628,6 +628,17 @@ static bool tpm_cr50_i2c_req_canceled(struct tpm_chip *chip, u8 status)
return status == TPM_STS_COMMAND_READY;
}

+static bool tpm_cr50_i2c_is_firmware_power_managed(struct device *dev)
+{
+ u8 val;
+ int ret;
+ /* This flag should default true when the device property is not present */
+ ret = device_property_read_u8(dev, "firmware-power-managed", &val);
+ if (ret)
+ return 1;
+ return val;
+}
+
static const struct tpm_class_ops cr50_i2c = {
.flags = TPM_OPS_AUTO_STARTUP,
.status = &tpm_cr50_i2c_tis_status,
@@ -686,7 +697,8 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)

/* cr50 is a TPM 2.0 chip */
chip->flags |= TPM_CHIP_FLAG_TPM2;
- chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
+ if (tpm_cr50_i2c_is_firmware_power_managed(dev))
+ chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;

/* Default timeouts */
chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
index dae98dbeeeac..6c40ff99d3ea 100644
--- a/drivers/char/tpm/tpm_tis_spi_cr50.c
+++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
@@ -185,6 +185,17 @@ static int cr50_spi_flow_control(struct tpm_tis_spi_phy *phy,
return 0;
}

+static bool tpm_cr50_spi_is_firmware_power_managed(struct device *dev)
+{
+ u8 val;
+ int ret;
+ /* This flag should default true when the device property is not present */
+ ret = device_property_read_u8(dev, "firmware-power-managed", &val);
+ if (ret)
+ return 1;
+ return val;
+}
+
static int tpm_tis_spi_cr50_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
u8 *in, const u8 *out)
{
@@ -309,7 +320,8 @@ int cr50_spi_probe(struct spi_device *spi)
cr50_print_fw_version(&phy->priv);

chip = dev_get_drvdata(&spi->dev);
- chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
+ if (tpm_cr50_spi_is_firmware_power_managed(&spi->dev))
+ chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;

return 0;
}
--
2.34.0.384.gca35af8252-goog


2021-12-04 18:05:19

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v2] char: tpm: cr50: Set TPM_FIRMWARE_POWER_MANAGED based on device property

On Thu, Dec 02, 2021 at 08:03:40PM +0000, Rob Barnes wrote:
> Set TPM_FIRMWARE_POWER_MANAGED flag based on 'firmware-power-managed'
> ACPI DSD property. For the CR50 TPM, this flag defaults to true when
> the property is unset.
>
> When this flag is set to false, the CR50 TPM driver will always send
> a shutdown command whenever the system suspends.
>
> Signed-off-by: Rob Barnes <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_i2c_cr50.c | 14 +++++++++++++-
> drivers/char/tpm/tpm_tis_spi_cr50.c | 14 +++++++++++++-
> 2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> index c89278103703..70143cc4f4e8 100644
> --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> @@ -628,6 +628,17 @@ static bool tpm_cr50_i2c_req_canceled(struct tpm_chip *chip, u8 status)
> return status == TPM_STS_COMMAND_READY;
> }
>
> +static bool tpm_cr50_i2c_is_firmware_power_managed(struct device *dev)
> +{
> + u8 val;
> + int ret;

empty line here.

> + /* This flag should default true when the device property is not present */
> + ret = device_property_read_u8(dev, "firmware-power-managed", &val);
> + if (ret)
> + return 1;

"return true;" and empty line here.

> + return val;
> +}
> +
> static const struct tpm_class_ops cr50_i2c = {
> .flags = TPM_OPS_AUTO_STARTUP,
> .status = &tpm_cr50_i2c_tis_status,
> @@ -686,7 +697,8 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
>
> /* cr50 is a TPM 2.0 chip */
> chip->flags |= TPM_CHIP_FLAG_TPM2;
> - chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
> + if (tpm_cr50_i2c_is_firmware_power_managed(dev))
> + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
>
> /* Default timeouts */
> chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
> index dae98dbeeeac..6c40ff99d3ea 100644
> --- a/drivers/char/tpm/tpm_tis_spi_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
> @@ -185,6 +185,17 @@ static int cr50_spi_flow_control(struct tpm_tis_spi_phy *phy,
> return 0;
> }
>
> +static bool tpm_cr50_spi_is_firmware_power_managed(struct device *dev)
> +{
> + u8 val;
> + int ret;

Ditto.

> + /* This flag should default true when the device property is not present */
> + ret = device_property_read_u8(dev, "firmware-power-managed", &val);
> + if (ret)
> + return 1;

Ditto.

> + return val;
> +}
> +
> static int tpm_tis_spi_cr50_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> u8 *in, const u8 *out)
> {
> @@ -309,7 +320,8 @@ int cr50_spi_probe(struct spi_device *spi)
> cr50_print_fw_version(&phy->priv);
>
> chip = dev_get_drvdata(&spi->dev);
> - chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
> + if (tpm_cr50_spi_is_firmware_power_managed(&spi->dev))
> + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
>
> return 0;
> }
> --
> 2.34.0.384.gca35af8252-goog
>

/Jarkko

2021-12-06 12:03:45

by Rob Barnes

[permalink] [raw]
Subject: [PATCH v3] char: tpm: cr50: Set TPM_FIRMWARE_POWER_MANAGED based on device property

Set TPM_FIRMWARE_POWER_MANAGED flag based on 'firmware-power-managed'
ACPI DSD property. For the CR50 TPM, this flag defaults to true when
the property is unset.

When this flag is set to false, the CR50 TPM driver will always send
a shutdown command whenever the system suspends.

Signed-off-by: Rob Barnes <[email protected]>
---
drivers/char/tpm/tpm_tis_i2c_cr50.c | 16 +++++++++++++++-
drivers/char/tpm/tpm_tis_spi_cr50.c | 16 +++++++++++++++-
2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
index c89278103703..f6c0affbb456 100644
--- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
+++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
@@ -628,6 +628,19 @@ static bool tpm_cr50_i2c_req_canceled(struct tpm_chip *chip, u8 status)
return status == TPM_STS_COMMAND_READY;
}

+static bool tpm_cr50_i2c_is_firmware_power_managed(struct device *dev)
+{
+ u8 val;
+ int ret;
+
+ /* This flag should default true when the device property is not present */
+ ret = device_property_read_u8(dev, "firmware-power-managed", &val);
+ if (ret)
+ return true;
+
+ return val;
+}
+
static const struct tpm_class_ops cr50_i2c = {
.flags = TPM_OPS_AUTO_STARTUP,
.status = &tpm_cr50_i2c_tis_status,
@@ -686,7 +699,8 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)

/* cr50 is a TPM 2.0 chip */
chip->flags |= TPM_CHIP_FLAG_TPM2;
- chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
+ if (tpm_cr50_i2c_is_firmware_power_managed(dev))
+ chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;

/* Default timeouts */
chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
index dae98dbeeeac..7bf123d3c537 100644
--- a/drivers/char/tpm/tpm_tis_spi_cr50.c
+++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
@@ -185,6 +185,19 @@ static int cr50_spi_flow_control(struct tpm_tis_spi_phy *phy,
return 0;
}

+static bool tpm_cr50_spi_is_firmware_power_managed(struct device *dev)
+{
+ u8 val;
+ int ret;
+
+ /* This flag should default true when the device property is not present */
+ ret = device_property_read_u8(dev, "firmware-power-managed", &val);
+ if (ret)
+ return true;
+
+ return val;
+}
+
static int tpm_tis_spi_cr50_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
u8 *in, const u8 *out)
{
@@ -309,7 +322,8 @@ int cr50_spi_probe(struct spi_device *spi)
cr50_print_fw_version(&phy->priv);

chip = dev_get_drvdata(&spi->dev);
- chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
+ if (tpm_cr50_spi_is_firmware_power_managed(&spi->dev))
+ chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;

return 0;
}
--
2.34.1.400.ga245620fadb-goog


2021-12-06 23:53:20

by Rob Barnes

[permalink] [raw]
Subject: Re: [PATCH] char: tpm: cr50: Set TPM_FIRMWARE_POWER_MANAGED based on device property

Including [email protected]

>
> Set TPM_FIRMWARE_POWER_MANAGED flag based on 'firmware-power-managed'
> ACPI DSD property. For the CR50 TPM, this flag defaults to true when
> the property is unset.
>
> When this flag is set to false, the CR50 TPM driver will always send
> a shutdown command whenever the system suspends.
>
> Signed-off-by: Rob Barnes <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_i2c_cr50.c | 14 +++++++++++++-
> drivers/char/tpm/tpm_tis_spi_cr50.c | 14 +++++++++++++-
> 2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> index c89278103703..70143cc4f4e8 100644
> --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> @@ -628,6 +628,17 @@ static bool tpm_cr50_i2c_req_canceled(struct tpm_chip *chip, u8 status)
> return status == TPM_STS_COMMAND_READY;
> }
>
> +static bool tpm_cr50_i2c_is_firmware_power_managed(struct device *dev)
> +{
> + u8 val;
> + int ret;
> + /* This flag should default true when the device property is not present */
> + ret = device_property_read_u8(dev, "firmware-power-managed", &val);
> + if (ret)
> + return 1;
> + return val;
> +}
> +
> static const struct tpm_class_ops cr50_i2c = {
> .flags = TPM_OPS_AUTO_STARTUP,
> .status = &tpm_cr50_i2c_tis_status,
> @@ -686,7 +697,8 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
>
> /* cr50 is a TPM 2.0 chip */
> chip->flags |= TPM_CHIP_FLAG_TPM2;
> - chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
> + if (tpm_cr50_i2c_is_firmware_power_managed(dev))
> + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
>
> /* Default timeouts */
> chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
> index dae98dbeeeac..e01f7cc258ca 100644
> --- a/drivers/char/tpm/tpm_tis_spi_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
> @@ -185,6 +185,17 @@ static int cr50_spi_flow_control(struct tpm_tis_spi_phy *phy,
> return 0;
> }
>
> +static bool tpm_cr50_spi_is_firmware_power_managed(struct device *dev)
> +{
> + u8 val;
> + int ret;
> + /* This flag should default true when the device property is not present */
> + ret = device_property_read_u8(dev, "firmware-power-managed", &val);
> + if (ret)
> + return 1;
> + return val;
> +}
> +
> static int tpm_tis_spi_cr50_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> u8 *in, const u8 *out)
> {
> @@ -309,7 +320,8 @@ int cr50_spi_probe(struct spi_device *spi)
> cr50_print_fw_version(&phy->priv);
>
> chip = dev_get_drvdata(&spi->dev);
> - chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
> + if (tpm_cr50_spi_is_firmware_power_managed(dev))
> + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
>
> return 0;
> }
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>

2021-12-11 05:06:17

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v3] char: tpm: cr50: Set TPM_FIRMWARE_POWER_MANAGED based on device property

On Mon, 2021-12-06 at 12:03 +0000, Rob Barnes wrote:
> Set TPM_FIRMWARE_POWER_MANAGED flag based on 'firmware-power-managed'
> ACPI DSD property. For the CR50 TPM, this flag defaults to true when
> the property is unset.
>
> When this flag is set to false, the CR50 TPM driver will always send
> a shutdown command whenever the system suspends.
>
> Signed-off-by: Rob Barnes <[email protected]>
> ---
> drivers/char/tpm/tpm_tis_i2c_cr50.c | 16 +++++++++++++++-
> drivers/char/tpm/tpm_tis_spi_cr50.c | 16 +++++++++++++++-
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> index c89278103703..f6c0affbb456 100644
> --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> @@ -628,6 +628,19 @@ static bool tpm_cr50_i2c_req_canceled(struct tpm_chip *chip, u8 status)
> return status == TPM_STS_COMMAND_READY;
> }
>
> +static bool tpm_cr50_i2c_is_firmware_power_managed(struct device *dev)
> +{
> + u8 val;
> + int ret;
> +
> + /* This flag should default true when the device property is not present */
> + ret = device_property_read_u8(dev, "firmware-power-managed", &val);
> + if (ret)
> + return true;
> +
> + return val;
> +}
> +
> static const struct tpm_class_ops cr50_i2c = {
> .flags = TPM_OPS_AUTO_STARTUP,
> .status = &tpm_cr50_i2c_tis_status,
> @@ -686,7 +699,8 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
>
> /* cr50 is a TPM 2.0 chip */
> chip->flags |= TPM_CHIP_FLAG_TPM2;
> - chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
> + if (tpm_cr50_i2c_is_firmware_power_managed(dev))
> + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
>
> /* Default timeouts */
> chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
> index dae98dbeeeac..7bf123d3c537 100644
> --- a/drivers/char/tpm/tpm_tis_spi_cr50.c
> +++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
> @@ -185,6 +185,19 @@ static int cr50_spi_flow_control(struct tpm_tis_spi_phy *phy,
> return 0;
> }
>
> +static bool tpm_cr50_spi_is_firmware_power_managed(struct device *dev)
> +{
> + u8 val;
> + int ret;
> +
> + /* This flag should default true when the device property is not present */
> + ret = device_property_read_u8(dev, "firmware-power-managed", &val);
> + if (ret)
> + return true;
> +
> + return val;
> +}
> +
> static int tpm_tis_spi_cr50_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> u8 *in, const u8 *out)
> {
> @@ -309,7 +322,8 @@ int cr50_spi_probe(struct spi_device *spi)
> cr50_print_fw_version(&phy->priv);
>
> chip = dev_get_drvdata(&spi->dev);
> - chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
> + if (tpm_cr50_spi_is_firmware_power_managed(&spi->dev))
> + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
>
> return 0;
> }

Thank you.

Reviewed-by: Jarkko Sakkinen <[email protected]>

I applied this to my tree, and it should be visible in linux-next soon.

/Jarkko