diff mbox series

[1/7] dt-bindings: tpm: Introduce trivial-tpms.yaml

Message ID 20220930165116.13329-2-Alexander.Steffen@infineon.com (mailing list archive)
State New, archived
Headers show
Series Clean up TPM compatibles | expand

Commit Message

Alexander Steffen Sept. 30, 2022, 4:51 p.m. UTC 
Most TPM devices are very similar and only need a few common properties
to describe them. However, they may use more properties than other
trivial I2C or SPI devices, e.g. powered-while-suspended. Therefore,
move them to their own trivial-tpms.yaml.

Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
---
 .../bindings/security/tpm/trivial-tpms.yaml   | 54 +++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  | 16 ------
 2 files changed, 54 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml

Comments

Rob Herring Sept. 30, 2022, 10:15 p.m. UTC | #1 
On Fri, Sep 30, 2022 at 12:03 PM Alexander Steffen
<Alexander.Steffen@infineon.com> wrote:
>
> Most TPM devices are very similar and only need a few common properties
> to describe them. However, they may use more properties than other
> trivial I2C or SPI devices, e.g. powered-while-suspended. Therefore,
> move them to their own trivial-tpms.yaml.
>
> Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
> ---
>  .../bindings/security/tpm/trivial-tpms.yaml   | 54 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  | 16 ------
>  2 files changed, 54 insertions(+), 16 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
>
> diff --git a/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
> new file mode 100644
> index 000000000000..fadd4ca96554
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/security/tpm/trivial-tpms.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Trivial TPM devices that have simple device tree bindings
> +
> +maintainers:
> +  - linux-integrity@vger.kernel.org
> +
> +description: |
> +  This is a list of trivial TPM devices that share the same properties and
> +  therefore have simple device tree bindings.
> +
> +  If a device needs more specific bindings, such as properties to
> +  describe some aspect of it, there needs to be a specific binding
> +  document for it just like any other devices.
> +
> +properties:
> +  reg:
> +    maxItems: 1

blank line

> +  interrupts:
> +    maxItems: 1
> +
> +  spi-max-frequency: true

The SPI based devices need to reference spi-peripheral-props.yaml. So
I think these are going to need to be split up by bus some.

> +
> +  compatible:

compatible goes first by convention.

> +    contains:

'contains' can not be used here. That allows any other compatible
strings to be present. It's got to be exact lists of what are valid
combinations.

> +      enum:
> +          # i2c trusted platform module (TPM)
> +        - atmel,at97sc3204t
> +          # Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
> +        - infineon,slb9635tt
> +          # Infineon SLB9645 I2C TPM (new protocol, max 400khz)
> +        - infineon,slb9645tt
> +          # Infineon SLB9673 I2C TPM 2.0
> +        - infineon,slb9673
> +          # i2c trusted platform module (TPM)
> +        - nuvoton,npct501
> +          # i2c trusted platform module (TPM2)
> +        - nuvoton,npct601
> +          # Socionext SynQuacer TPM MMIO module
> +        - socionext,synquacer-tpm-mmio
> +          # i2c trusted platform module (TPM)
> +        - winbond,wpct301
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +...
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 61746755c107..1cc7f82c0822 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -47,8 +47,6 @@ properties:
>            - ams,iaq-core
>              # i2c serial eeprom (24cxx)
>            - at,24c08
> -            # i2c trusted platform module (TPM)
> -          - atmel,at97sc3204t
>              # ATSHA204 - i2c h/w symmetric crypto module
>            - atmel,atsha204
>              # ATSHA204A - i2c h/w symmetric crypto module
> @@ -135,12 +133,6 @@ properties:
>            - infineon,ir38164
>              # Infineon IR38263 Voltage Regulator
>            - infineon,ir38263
> -            # Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
> -          - infineon,slb9635tt
> -            # Infineon SLB9645 I2C TPM (new protocol, max 400khz)
> -          - infineon,slb9645tt
> -            # Infineon SLB9673 I2C TPM 2.0
> -          - infineon,slb9673
>              # Infineon TLV493D-A1B6 I2C 3D Magnetic Sensor
>            - infineon,tlv493d-a1b6
>              # Infineon Multi-phase Digital VR Controller xdpe11280
> @@ -289,10 +281,6 @@ properties:
>            - national,lm85
>              # I2C ±0.33°C Accurate, 12-Bit + Sign Temperature Sensor and Thermal Window Comparator
>            - national,lm92
> -            # i2c trusted platform module (TPM)
> -          - nuvoton,npct501
> -            # i2c trusted platform module (TPM2)
> -          - nuvoton,npct601
>              # Nuvoton Temperature Sensor
>            - nuvoton,w83773g
>              # OKI ML86V7667 video decoder
> @@ -327,8 +315,6 @@ properties:
>            - silabs,si7020
>              # Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>            - skyworks,sky81452
> -            # Socionext SynQuacer TPM MMIO module
> -          - socionext,synquacer-tpm-mmio
>              # SparkFun Qwiic Joystick (COM-15168) with i2c interface
>            - sparkfun,qwiic-joystick
>              # i2c serial eeprom (24cxx)
> @@ -383,8 +369,6 @@ properties:
>            - winbond,w83793
>              # Vicor Corporation Digital Supervisor
>            - vicor,pli1209bc
> -            # i2c trusted platform module (TPM)
> -          - winbond,wpct301
>
>  required:
>    - compatible
> --
> 2.25.1
>
Alexander Steffen Oct. 4, 2022, 5:41 p.m. UTC | #2 
On 01.10.22 00:15, Rob Herring wrote:
> On Fri, Sep 30, 2022 at 12:03 PM Alexander Steffen
> <Alexander.Steffen@infineon.com> wrote:
>>
>> Most TPM devices are very similar and only need a few common properties
>> to describe them. However, they may use more properties than other
>> trivial I2C or SPI devices, e.g. powered-while-suspended. Therefore,
>> move them to their own trivial-tpms.yaml.
>>
>> Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
>> ---
>>   .../bindings/security/tpm/trivial-tpms.yaml   | 54 +++++++++++++++++++
>>   .../devicetree/bindings/trivial-devices.yaml  | 16 ------
>>   2 files changed, 54 insertions(+), 16 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
>> new file mode 100644
>> index 000000000000..fadd4ca96554
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
>> @@ -0,0 +1,54 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/security/tpm/trivial-tpms.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Trivial TPM devices that have simple device tree bindings
>> +
>> +maintainers:
>> +  - linux-integrity@vger.kernel.org
>> +
>> +description: |
>> +  This is a list of trivial TPM devices that share the same properties and
>> +  therefore have simple device tree bindings.
>> +
>> +  If a device needs more specific bindings, such as properties to
>> +  describe some aspect of it, there needs to be a specific binding
>> +  document for it just like any other devices.
>> +
>> +properties:
>> +  reg:
>> +    maxItems: 1
> 
> blank line
> 
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  spi-max-frequency: true
> 
> The SPI based devices need to reference spi-peripheral-props.yaml. So
> I think these are going to need to be split up by bus some.
> 
>> +
>> +  compatible:
> 
> compatible goes first by convention.

I had copied all three from trivial-devices.yaml ;-)

The style fixes are easy. But do you really think I should split 
trivial-tpms.yaml into i2c-tpms.yaml, spi-tpms.yaml, etc.? After all, 
trivial-devices.yaml also contains a mix of I2C and SPI devices.

Also, what about devices like "google,cr50", that support both I2C and 
SPI? Can they appear in two YAML files at the same time?

>> +    contains:
> 
> 'contains' can not be used here. That allows any other compatible
> strings to be present. It's got to be exact lists of what are valid
> combinations.

So what exactly are valid combinations then? If I look at what is in 
use, I find three possible combinations:

arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi: compatible = 
"tcg,tpm_tis-spi"
arch/arm/boot/dts/imx6dl-eckelmann-ci4x10.dts: compatible = 
"infineon,slb9670", "tcg,tpm_tis-spi";
arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts: compatible 
= "infineon,slb9670";

It is either a generic identifier or a specific device or both. Is it 
correct to allow all three variants? If so, how to specify that as YAML, 
ideally without duplicating any of the identifiers?

>> +      enum:
>> +          # i2c trusted platform module (TPM)
>> +        - atmel,at97sc3204t
>> +          # Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
>> +        - infineon,slb9635tt
>> +          # Infineon SLB9645 I2C TPM (new protocol, max 400khz)
>> +        - infineon,slb9645tt
>> +          # Infineon SLB9673 I2C TPM 2.0
>> +        - infineon,slb9673
>> +          # i2c trusted platform module (TPM)
>> +        - nuvoton,npct501
>> +          # i2c trusted platform module (TPM2)
>> +        - nuvoton,npct601
>> +          # Socionext SynQuacer TPM MMIO module
>> +        - socionext,synquacer-tpm-mmio
>> +          # i2c trusted platform module (TPM)
>> +        - winbond,wpct301
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +...
>> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
>> index 61746755c107..1cc7f82c0822 100644
>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>> @@ -47,8 +47,6 @@ properties:
>>             - ams,iaq-core
>>               # i2c serial eeprom (24cxx)
>>             - at,24c08
>> -            # i2c trusted platform module (TPM)
>> -          - atmel,at97sc3204t
>>               # ATSHA204 - i2c h/w symmetric crypto module
>>             - atmel,atsha204
>>               # ATSHA204A - i2c h/w symmetric crypto module
>> @@ -135,12 +133,6 @@ properties:
>>             - infineon,ir38164
>>               # Infineon IR38263 Voltage Regulator
>>             - infineon,ir38263
>> -            # Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
>> -          - infineon,slb9635tt
>> -            # Infineon SLB9645 I2C TPM (new protocol, max 400khz)
>> -          - infineon,slb9645tt
>> -            # Infineon SLB9673 I2C TPM 2.0
>> -          - infineon,slb9673
>>               # Infineon TLV493D-A1B6 I2C 3D Magnetic Sensor
>>             - infineon,tlv493d-a1b6
>>               # Infineon Multi-phase Digital VR Controller xdpe11280
>> @@ -289,10 +281,6 @@ properties:
>>             - national,lm85
>>               # I2C ±0.33°C Accurate, 12-Bit + Sign Temperature Sensor and Thermal Window Comparator
>>             - national,lm92
>> -            # i2c trusted platform module (TPM)
>> -          - nuvoton,npct501
>> -            # i2c trusted platform module (TPM2)
>> -          - nuvoton,npct601
>>               # Nuvoton Temperature Sensor
>>             - nuvoton,w83773g
>>               # OKI ML86V7667 video decoder
>> @@ -327,8 +315,6 @@ properties:
>>             - silabs,si7020
>>               # Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
>>             - skyworks,sky81452
>> -            # Socionext SynQuacer TPM MMIO module
>> -          - socionext,synquacer-tpm-mmio
>>               # SparkFun Qwiic Joystick (COM-15168) with i2c interface
>>             - sparkfun,qwiic-joystick
>>               # i2c serial eeprom (24cxx)
>> @@ -383,8 +369,6 @@ properties:
>>             - winbond,w83793
>>               # Vicor Corporation Digital Supervisor
>>             - vicor,pli1209bc
>> -            # i2c trusted platform module (TPM)
>> -          - winbond,wpct301
>>
>>   required:
>>     - compatible
>> --
>> 2.25.1
>>
Rob Herring (Arm) Nov. 1, 2022, 12:17 p.m. UTC | #3 
On Tue, Oct 04, 2022 at 07:41:36PM +0200, Alexander Steffen wrote:
> On 01.10.22 00:15, Rob Herring wrote:
> > On Fri, Sep 30, 2022 at 12:03 PM Alexander Steffen
> > <Alexander.Steffen@infineon.com> wrote:
> > > 
> > > Most TPM devices are very similar and only need a few common properties
> > > to describe them. However, they may use more properties than other
> > > trivial I2C or SPI devices, e.g. powered-while-suspended. Therefore,
> > > move them to their own trivial-tpms.yaml.
> > > 
> > > Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
> > > ---
> > >   .../bindings/security/tpm/trivial-tpms.yaml   | 54 +++++++++++++++++++
> > >   .../devicetree/bindings/trivial-devices.yaml  | 16 ------
> > >   2 files changed, 54 insertions(+), 16 deletions(-)
> > >   create mode 100644 Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
> > > new file mode 100644
> > > index 000000000000..fadd4ca96554
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
> > > @@ -0,0 +1,54 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/security/tpm/trivial-tpms.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Trivial TPM devices that have simple device tree bindings
> > > +
> > > +maintainers:
> > > +  - linux-integrity@vger.kernel.org
> > > +
> > > +description: |
> > > +  This is a list of trivial TPM devices that share the same properties and
> > > +  therefore have simple device tree bindings.
> > > +
> > > +  If a device needs more specific bindings, such as properties to
> > > +  describe some aspect of it, there needs to be a specific binding
> > > +  document for it just like any other devices.
> > > +
> > > +properties:
> > > +  reg:
> > > +    maxItems: 1
> > 
> > blank line
> > 
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  spi-max-frequency: true
> > 
> > The SPI based devices need to reference spi-peripheral-props.yaml. So
> > I think these are going to need to be split up by bus some.
> > 
> > > +
> > > +  compatible:
> > 
> > compatible goes first by convention.
> 
> I had copied all three from trivial-devices.yaml ;-)
> 
> The style fixes are easy. But do you really think I should split
> trivial-tpms.yaml into i2c-tpms.yaml, spi-tpms.yaml, etc.? After all,
> trivial-devices.yaml also contains a mix of I2C and SPI devices.

Well, you can leave them mixed. It just means that SPI bus properties 
would be allowed in I2C devices. That's okay, but if we can avoid it we 
should.

> Also, what about devices like "google,cr50", that support both I2C and SPI?
> Can they appear in two YAML files at the same time?

No, it can't. It can be a single schema for both, but perhaps a separate 
schema doc from the rest.

> 
> > > +    contains:
> > 
> > 'contains' can not be used here. That allows any other compatible
> > strings to be present. It's got to be exact lists of what are valid
> > combinations.
> 
> So what exactly are valid combinations then? If I look at what is in use, I
> find three possible combinations:
> 
> arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi: compatible =
> "tcg,tpm_tis-spi"
> arch/arm/boot/dts/imx6dl-eckelmann-ci4x10.dts: compatible =
> "infineon,slb9670", "tcg,tpm_tis-spi";
> arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts: compatible =
> "infineon,slb9670";
> 
> It is either a generic identifier or a specific device or both. Is it
> correct to allow all three variants? If so, how to specify that as YAML,
> ideally without duplicating any of the identifiers?

At a minimum you need an 'items' entry for each length of compatible 
entries and generally an entry for each fallback. So you will have to 
have some duplication.

In cases like imx8mq-kontron-pitx-imx8m.dts, the dts should be fixed 
adding "tcg,tpm_tis-spi". If all the users were just "infineon,slb9670", 
then we'd leave it. So define the schema with what matches existing 
users, but fix users when inconsistent. 

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
new file mode 100644
index 000000000000..fadd4ca96554
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/trivial-tpms.yaml
@@ -0,0 +1,54 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/security/tpm/trivial-tpms.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Trivial TPM devices that have simple device tree bindings
+
+maintainers:
+  - linux-integrity@vger.kernel.org
+
+description: |
+  This is a list of trivial TPM devices that share the same properties and
+  therefore have simple device tree bindings.
+
+  If a device needs more specific bindings, such as properties to
+  describe some aspect of it, there needs to be a specific binding
+  document for it just like any other devices.
+
+properties:
+  reg:
+    maxItems: 1
+  interrupts:
+    maxItems: 1
+
+  spi-max-frequency: true
+
+  compatible:
+    contains:
+      enum:
+          # i2c trusted platform module (TPM)
+        - atmel,at97sc3204t
+          # Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
+        - infineon,slb9635tt
+          # Infineon SLB9645 I2C TPM (new protocol, max 400khz)
+        - infineon,slb9645tt
+          # Infineon SLB9673 I2C TPM 2.0
+        - infineon,slb9673
+          # i2c trusted platform module (TPM)
+        - nuvoton,npct501
+          # i2c trusted platform module (TPM2)
+        - nuvoton,npct601
+          # Socionext SynQuacer TPM MMIO module
+        - socionext,synquacer-tpm-mmio
+          # i2c trusted platform module (TPM)
+        - winbond,wpct301
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+...
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 61746755c107..1cc7f82c0822 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -47,8 +47,6 @@  properties:
           - ams,iaq-core
             # i2c serial eeprom (24cxx)
           - at,24c08
-            # i2c trusted platform module (TPM)
-          - atmel,at97sc3204t
             # ATSHA204 - i2c h/w symmetric crypto module
           - atmel,atsha204
             # ATSHA204A - i2c h/w symmetric crypto module
@@ -135,12 +133,6 @@  properties:
           - infineon,ir38164
             # Infineon IR38263 Voltage Regulator
           - infineon,ir38263
-            # Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz)
-          - infineon,slb9635tt
-            # Infineon SLB9645 I2C TPM (new protocol, max 400khz)
-          - infineon,slb9645tt
-            # Infineon SLB9673 I2C TPM 2.0
-          - infineon,slb9673
             # Infineon TLV493D-A1B6 I2C 3D Magnetic Sensor
           - infineon,tlv493d-a1b6
             # Infineon Multi-phase Digital VR Controller xdpe11280
@@ -289,10 +281,6 @@  properties:
           - national,lm85
             # I2C ±0.33°C Accurate, 12-Bit + Sign Temperature Sensor and Thermal Window Comparator
           - national,lm92
-            # i2c trusted platform module (TPM)
-          - nuvoton,npct501
-            # i2c trusted platform module (TPM2)
-          - nuvoton,npct601
             # Nuvoton Temperature Sensor
           - nuvoton,w83773g
             # OKI ML86V7667 video decoder
@@ -327,8 +315,6 @@  properties:
           - silabs,si7020
             # Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
           - skyworks,sky81452
-            # Socionext SynQuacer TPM MMIO module
-          - socionext,synquacer-tpm-mmio
             # SparkFun Qwiic Joystick (COM-15168) with i2c interface
           - sparkfun,qwiic-joystick
             # i2c serial eeprom (24cxx)
@@ -383,8 +369,6 @@  properties:
           - winbond,w83793
             # Vicor Corporation Digital Supervisor
           - vicor,pli1209bc
-            # i2c trusted platform module (TPM)
-          - winbond,wpct301
 
 required:
   - compatible