2023-12-12 16:41:31

by Ninad Palsule

[permalink] [raw]
Subject: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

From: Johannes Holland <[email protected]>

Add a dt schema to support device tree bindings for the generic I2C
physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
Specification for TPM 2.0 v1.04 Revision 14.

This includes descriptions for the Nuvoton and Infineon devices.

OpenBMC-Staging-Count: 3
Signed-off-by: Johannes Holland <[email protected]>
Signed-off-by: Joel Stanley <[email protected]>
Signed-off-by: Ninad Palsule <[email protected]>
---
.../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml

diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
new file mode 100644
index 000000000000..de1e34065748
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: I2C PTP based TPM Devices
+
+maintainers:
+ - Johannes Holland <[email protected]>
+
+description:
+ Device Tree Bindings for I2C based Trusted Platform Module (TPM).
+
+properties:
+ $nodename:
+ pattern: "^tpm(@[0-9a-f]+)?$"
+
+ compatible:
+ oneOf:
+ - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
+ items:
+ - const: infineon,slb9673
+ - const: tcg,tpm-tis-i2c
+ - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
+ items:
+ - const: nuvoton,npct75x
+ - const: tcg,tpm-tis-i2c
+ - const: tcg,tpm-tis-i2c
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tpm@2e {
+ compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";
+ reg = <0x2e>;
+ };
+ };
+...
--
2.39.2


2023-12-12 17:14:56

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

Hey,

On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
> From: Johannes Holland <[email protected]>
>
> Add a dt schema to support device tree bindings

"Add bindings for..."

> for the generic I2C
> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
> Specification for TPM 2.0 v1.04 Revision 14.
>
> This includes descriptions for the Nuvoton and Infineon devices.
>

> OpenBMC-Staging-Count: 3

I have no idea what this is, but it needs to be removed from the patch.

> Signed-off-by: Johannes Holland <[email protected]>
> Signed-off-by: Joel Stanley <[email protected]>
> Signed-off-by: Ninad Palsule <[email protected]>
> ---
> .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>
> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> new file mode 100644
> index 000000000000..de1e34065748
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: I2C PTP based TPM Devices
> +
> +maintainers:
> + - Johannes Holland <[email protected]>
> +
> +description:
> + Device Tree Bindings for I2C based Trusted Platform Module (TPM).

s/Device Tree Bindings for //. Doesn't dt_binding_check now complain if
you have this in a title or description?

> +properties:
> + $nodename:
> + pattern: "^tpm(@[0-9a-f]+)?$"
> +
> + compatible:
> + oneOf:
> + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
> + items:
> + - const: infineon,slb9673
> + - const: tcg,tpm-tis-i2c
> + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
> + items:
> + - const: nuvoton,npct75x
> + - const: tcg,tpm-tis-i2c

> + - const: tcg,tpm-tis-i2c

IMO this should be removed and this fallback should only be used in
combination with device specific compatibles, like you have here for the
infineon and nuvoton devices.

Cheers,
Conor.

> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + tpm@2e {
> + compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";
> + reg = <0x2e>;
> + };
> + };
> +...
> --
> 2.39.2
>


Attachments:
(No filename) (2.79 kB)
signature.asc (235.00 B)
Download all attachments

2023-12-12 18:21:24

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

On Tue, Dec 12, 2023 at 05:14:26PM +0000, Conor Dooley wrote:
> Hey,
>
> On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
> > From: Johannes Holland <[email protected]>
> >
> > Add a dt schema to support device tree bindings
>
> "Add bindings for..."
>
> > for the generic I2C
> > physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
> > Specification for TPM 2.0 v1.04 Revision 14.
> >
> > This includes descriptions for the Nuvoton and Infineon devices.
> >
>
> > OpenBMC-Staging-Count: 3
>
> I have no idea what this is, but it needs to be removed from the patch.
>
> > Signed-off-by: Johannes Holland <[email protected]>
> > Signed-off-by: Joel Stanley <[email protected]>
> > Signed-off-by: Ninad Palsule <[email protected]>
> > ---
> > .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > new file mode 100644
> > index 000000000000..de1e34065748
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > @@ -0,0 +1,50 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: I2C PTP based TPM Devices
> > +
> > +maintainers:
> > + - Johannes Holland <[email protected]>
> > +
> > +description:
> > + Device Tree Bindings for I2C based Trusted Platform Module (TPM).
>
> s/Device Tree Bindings for //. Doesn't dt_binding_check now complain if
> you have this in a title or description?
>
> > +properties:
> > + $nodename:
> > + pattern: "^tpm(@[0-9a-f]+)?$"
> > +
> > + compatible:
> > + oneOf:
> > + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
> > + items:
> > + - const: infineon,slb9673
> > + - const: tcg,tpm-tis-i2c
> > + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
> > + items:
> > + - const: nuvoton,npct75x
> > + - const: tcg,tpm-tis-i2c
>
> > + - const: tcg,tpm-tis-i2c
>
> IMO this should be removed and this fallback should only be used in
> combination with device specific compatibles, like you have here for the
> infineon and nuvoton devices.

As mentioned in my response to the other patch, "only" isn't sufficient
since the tacoma devicetree file only references the generic entry.
It would also make support for chips from other vendors unnecessarily
complex.

Question should in my opinion be if the non-fallback entries are really
needed.

Thanks,
Guenter

2023-12-13 16:14:12

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
> From: Johannes Holland <[email protected]>
>
> Add a dt schema to support device tree bindings for the generic I2C
> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
> Specification for TPM 2.0 v1.04 Revision 14.
>
> This includes descriptions for the Nuvoton and Infineon devices.

This is incomplete and conflicts with this series[1]. Please help
review and make sure it works for the cases you care about.

Rob

[1] https://lore.kernel.org/all/[email protected]/

2023-12-13 18:21:12

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

On Tue Dec 12, 2023 at 6:39 PM EET, Ninad Palsule wrote:
> From: Johannes Holland <[email protected]>
>
> Add a dt schema to support device tree bindings for the generic I2C
> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
> Specification for TPM 2.0 v1.04 Revision 14.
>
> This includes descriptions for the Nuvoton and Infineon devices.
>
> OpenBMC-Staging-Count: 3

Please don't invent your own tags.

> Signed-off-by: Johannes Holland <[email protected]>
> Signed-off-by: Joel Stanley <[email protected]>
> Signed-off-by: Ninad Palsule <[email protected]>
> ---
> .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>
> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> new file mode 100644
> index 000000000000..de1e34065748
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: I2C PTP based TPM Devices
> +
> +maintainers:
> + - Johannes Holland <[email protected]>
> +
> +description:
> + Device Tree Bindings for I2C based Trusted Platform Module (TPM).
> +
> +properties:
> + $nodename:
> + pattern: "^tpm(@[0-9a-f]+)?$"
> +
> + compatible:
> + oneOf:
> + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
> + items:
> + - const: infineon,slb9673
> + - const: tcg,tpm-tis-i2c
> + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
> + items:
> + - const: nuvoton,npct75x
> + - const: tcg,tpm-tis-i2c
> + - const: tcg,tpm-tis-i2c
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + tpm@2e {
> + compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";
> + reg = <0x2e>;
> + };
> + };
> +...


BR, Jarkko

2023-12-13 18:39:54

by Ninad Palsule

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

Hello Jarkko,

On 12/13/23 12:20, Jarkko Sakkinen wrote:
> On Tue Dec 12, 2023 at 6:39 PM EET, Ninad Palsule wrote:
>> From: Johannes Holland <[email protected]>
>>
>> Add a dt schema to support device tree bindings for the generic I2C
>> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
>> Specification for TPM 2.0 v1.04 Revision 14.
>>
>> This includes descriptions for the Nuvoton and Infineon devices.
>>
>> OpenBMC-Staging-Count: 3
> Please don't invent your own tags.

Yes, Sorry. I have cherry-picked this commit from openbmc. Now I have
removed this line.

Thanks for the review.

Thanks & Regards,

Ninad

>
>> Signed-off-by: Johannes Holland <[email protected]>
>> Signed-off-by: Joel Stanley <[email protected]>
>> Signed-off-by: Ninad Palsule <[email protected]>
>> ---
>> .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
>> 1 file changed, 50 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>> new file mode 100644
>> index 000000000000..de1e34065748
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: I2C PTP based TPM Devices
>> +
>> +maintainers:
>> + - Johannes Holland <[email protected]>
>> +
>> +description:
>> + Device Tree Bindings for I2C based Trusted Platform Module (TPM).
>> +
>> +properties:
>> + $nodename:
>> + pattern: "^tpm(@[0-9a-f]+)?$"
>> +
>> + compatible:
>> + oneOf:
>> + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
>> + items:
>> + - const: infineon,slb9673
>> + - const: tcg,tpm-tis-i2c
>> + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
>> + items:
>> + - const: nuvoton,npct75x
>> + - const: tcg,tpm-tis-i2c
>> + - const: tcg,tpm-tis-i2c
>> + reg:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + tpm@2e {
>> + compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";
>> + reg = <0x2e>;
>> + };
>> + };
>> +...
>
> BR, Jarkko

2023-12-14 15:36:14

by Ninad Palsule

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

Hello Conor,

On 12/12/23 11:14, Conor Dooley wrote:
> Hey,
>
> On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
>> From: Johannes Holland <[email protected]>
>>
>> Add a dt schema to support device tree bindings
> "Add bindings for..."
Fixed.
>
>> for the generic I2C
>> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
>> Specification for TPM 2.0 v1.04 Revision 14.
>>
>> This includes descriptions for the Nuvoton and Infineon devices.
>>
>> OpenBMC-Staging-Count: 3
> I have no idea what this is, but it needs to be removed from the patch.
Removed.
>
>> Signed-off-by: Johannes Holland <[email protected]>
>> Signed-off-by: Joel Stanley <[email protected]>
>> Signed-off-by: Ninad Palsule <[email protected]>
>> ---
>> .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
>> 1 file changed, 50 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>> new file mode 100644
>> index 000000000000..de1e34065748
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: I2C PTP based TPM Devices
>> +
>> +maintainers:
>> + - Johannes Holland <[email protected]>
>> +
>> +description:
>> + Device Tree Bindings for I2C based Trusted Platform Module (TPM).
> s/Device Tree Bindings for //. Doesn't dt_binding_check now complain if
> you have this in a title or description?
Fixed.
>
>> +properties:
>> + $nodename:
>> + pattern: "^tpm(@[0-9a-f]+)?$"
>> +
>> + compatible:
>> + oneOf:
>> + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
>> + items:
>> + - const: infineon,slb9673
>> + - const: tcg,tpm-tis-i2c
>> + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
>> + items:
>> + - const: nuvoton,npct75x
>> + - const: tcg,tpm-tis-i2c
>> + - const: tcg,tpm-tis-i2c
> IMO this should be removed and this fallback should only be used in
> combination with device specific compatibles, like you have here for the
> infineon and nuvoton devices.

As Guenter mentioned I need to keep it as tacoma board is just using
this string.

Thanks for the review.

Regards,

Ninad

>
> Cheers,
> Conor.
>
>> + reg:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + tpm@2e {
>> + compatible = "nuvoton,npct75x", "tcg,tpm-tis-i2c";
>> + reg = <0x2e>;
>> + };
>> + };
>> +...
>> --
>> 2.39.2
>>

2023-12-14 15:38:51

by Ninad Palsule

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

Hello Guenter,

On 12/12/23 12:20, Guenter Roeck wrote:
> On Tue, Dec 12, 2023 at 05:14:26PM +0000, Conor Dooley wrote:
>> Hey,
>>
>> On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
>>> From: Johannes Holland <[email protected]>
>>>
>>> Add a dt schema to support device tree bindings
>> "Add bindings for..."
>>
>>> for the generic I2C
>>> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
>>> Specification for TPM 2.0 v1.04 Revision 14.
>>>
>>> This includes descriptions for the Nuvoton and Infineon devices.
>>>
>>> OpenBMC-Staging-Count: 3
>> I have no idea what this is, but it needs to be removed from the patch.
>>
>>> Signed-off-by: Johannes Holland <[email protected]>
>>> Signed-off-by: Joel Stanley <[email protected]>
>>> Signed-off-by: Ninad Palsule <[email protected]>
>>> ---
>>> .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
>>> 1 file changed, 50 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>>> new file mode 100644
>>> index 000000000000..de1e34065748
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>>> @@ -0,0 +1,50 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: I2C PTP based TPM Devices
>>> +
>>> +maintainers:
>>> + - Johannes Holland <[email protected]>
>>> +
>>> +description:
>>> + Device Tree Bindings for I2C based Trusted Platform Module (TPM).
>> s/Device Tree Bindings for //. Doesn't dt_binding_check now complain if
>> you have this in a title or description?
>>
>>> +properties:
>>> + $nodename:
>>> + pattern: "^tpm(@[0-9a-f]+)?$"
>>> +
>>> + compatible:
>>> + oneOf:
>>> + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
>>> + items:
>>> + - const: infineon,slb9673
>>> + - const: tcg,tpm-tis-i2c
>>> + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
>>> + items:
>>> + - const: nuvoton,npct75x
>>> + - const: tcg,tpm-tis-i2c
>>> + - const: tcg,tpm-tis-i2c
>> IMO this should be removed and this fallback should only be used in
>> combination with device specific compatibles, like you have here for the
>> infineon and nuvoton devices.
> As mentioned in my response to the other patch, "only" isn't sufficient
> since the tacoma devicetree file only references the generic entry.
> It would also make support for chips from other vendors unnecessarily
> complex.
>
> Question should in my opinion be if the non-fallback entries are really
> needed.

Thanks for the response. I think generic option is in-case we have a
chip whose specific driver is not available.

Regards,

Ninad

>
> Thanks,
> Guenter

2023-12-14 16:36:03

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

On Thu, Dec 14, 2023 at 09:34:39AM -0600, Ninad Palsule wrote:
> Hello Conor,
>
> On 12/12/23 11:14, Conor Dooley wrote:
> > Hey,
> >
> > On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
> > > From: Johannes Holland <[email protected]>
> > >
> > > Add a dt schema to support device tree bindings
> > "Add bindings for..."
> Fixed.
> >
> > > for the generic I2C
> > > physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
> > > Specification for TPM 2.0 v1.04 Revision 14.
> > >
> > > This includes descriptions for the Nuvoton and Infineon devices.
> > >
> > > OpenBMC-Staging-Count: 3
> > I have no idea what this is, but it needs to be removed from the patch.
> Removed.
> >
> > > Signed-off-by: Johannes Holland <[email protected]>
> > > Signed-off-by: Joel Stanley <[email protected]>
> > > Signed-off-by: Ninad Palsule <[email protected]>
> > > ---
> > > .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
> > > 1 file changed, 50 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > > new file mode 100644
> > > index 000000000000..de1e34065748
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > > @@ -0,0 +1,50 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: I2C PTP based TPM Devices
> > > +
> > > +maintainers:
> > > + - Johannes Holland <[email protected]>
> > > +
> > > +description:
> > > + Device Tree Bindings for I2C based Trusted Platform Module (TPM).
> > s/Device Tree Bindings for //. Doesn't dt_binding_check now complain if
> > you have this in a title or description?
> Fixed.
> >
> > > +properties:
> > > + $nodename:
> > > + pattern: "^tpm(@[0-9a-f]+)?$"
> > > +
> > > + compatible:
> > > + oneOf:
> > > + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
> > > + items:
> > > + - const: infineon,slb9673
> > > + - const: tcg,tpm-tis-i2c
> > > + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
> > > + items:
> > > + - const: nuvoton,npct75x
> > > + - const: tcg,tpm-tis-i2c

Also, another thought - the bus is not usually encoded in the compatible
string, so it would be good to remove that.

> > > + - const: tcg,tpm-tis-i2c
> > IMO this should be removed and this fallback should only be used in
> > combination with device specific compatibles, like you have here for the
> > infineon and nuvoton devices.
>
> As Guenter mentioned I need to keep it as tacoma board is just using this
> string.

No, that does not mean that you have to keep this in the binding. I know
Rob had some comments that might invalidate this binding entirely, but
if that does not happen then I think think that the tacoma devicetree
needs to have a device-specific compatible added for the tpm that it has.
You could of course retain the generic fallback compatible however.


Attachments:
(No filename) (3.36 kB)
signature.asc (235.00 B)
Download all attachments

2023-12-14 20:13:33

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

On Thu, Dec 14, 2023 at 10:35 AM Conor Dooley <[email protected]> wrote:
>
> On Thu, Dec 14, 2023 at 09:34:39AM -0600, Ninad Palsule wrote:
> > Hello Conor,
> >
> > On 12/12/23 11:14, Conor Dooley wrote:
> > > Hey,
> > >
> > > On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
> > > > From: Johannes Holland <[email protected]>
> > > >
> > > > Add a dt schema to support device tree bindings
> > > "Add bindings for..."
> > Fixed.
> > >
> > > > for the generic I2C
> > > > physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
> > > > Specification for TPM 2.0 v1.04 Revision 14.
> > > >
> > > > This includes descriptions for the Nuvoton and Infineon devices.
> > > >
> > > > OpenBMC-Staging-Count: 3
> > > I have no idea what this is, but it needs to be removed from the patch.
> > Removed.
> > >
> > > > Signed-off-by: Johannes Holland <[email protected]>
> > > > Signed-off-by: Joel Stanley <[email protected]>
> > > > Signed-off-by: Ninad Palsule <[email protected]>
> > > > ---
> > > > .../bindings/security/tpm/tpm-tis-i2c.yaml | 50 +++++++++++++++++++
> > > > 1 file changed, 50 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > > > new file mode 100644
> > > > index 000000000000..de1e34065748
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
> > > > @@ -0,0 +1,50 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/security/tpm/tpm-tis-i2c.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: I2C PTP based TPM Devices
> > > > +
> > > > +maintainers:
> > > > + - Johannes Holland <[email protected]>
> > > > +
> > > > +description:
> > > > + Device Tree Bindings for I2C based Trusted Platform Module (TPM).
> > > s/Device Tree Bindings for //. Doesn't dt_binding_check now complain if
> > > you have this in a title or description?
> > Fixed.
> > >
> > > > +properties:
> > > > + $nodename:
> > > > + pattern: "^tpm(@[0-9a-f]+)?$"
> > > > +
> > > > + compatible:
> > > > + oneOf:
> > > > + - description: Infineon's Trusted Platform Module (TPM) (SLB9673).
> > > > + items:
> > > > + - const: infineon,slb9673
> > > > + - const: tcg,tpm-tis-i2c
> > > > + - description: Nuvoton's Trusted Platform Module (TPM) (NPCT75x).
> > > > + items:
> > > > + - const: nuvoton,npct75x
> > > > + - const: tcg,tpm-tis-i2c
>
> Also, another thought - the bus is not usually encoded in the compatible
> string, so it would be good to remove that.

True, but we already have 3 different bus variants in this case. So
that ship has sailed.

Rob

2023-12-14 22:26:51

by Ninad Palsule

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

Hello Rob,

On 12/13/23 10:13, Rob Herring wrote:
> On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
>> From: Johannes Holland <[email protected]>
>>
>> Add a dt schema to support device tree bindings for the generic I2C
>> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
>> Specification for TPM 2.0 v1.04 Revision 14.
>>
>> This includes descriptions for the Nuvoton and Infineon devices.
> This is incomplete and conflicts with this series[1]. Please help
> review and make sure it works for the cases you care about.
>
> Rob
>
> [1] https://lore.kernel.org/all/[email protected]/

I will take a look at the patchset. How do you want to handle mine? Do
you want me to send patch as per the new directory structure?

Thanks for the review.

Regards,

Ninad


2024-01-08 19:46:17

by Ninad Palsule

[permalink] [raw]
Subject: Re: [PATCH v1 2/8] dt-bindings: tpm: Add schema for TIS I2C devices

Hi Rob,

On 12/13/23 10:13, Rob Herring wrote:
> On Tue, Dec 12, 2023 at 10:39:58AM -0600, Ninad Palsule wrote:
>> From: Johannes Holland <[email protected]>
>>
>> Add a dt schema to support device tree bindings for the generic I2C
>> physical layer. Refer to the TCG PC Client Platform TPM Profile (PTP)
>> Specification for TPM 2.0 v1.04 Revision 14.
>>
>> This includes descriptions for the Nuvoton and Infineon devices.
> This is incomplete and conflicts with this series[1]. Please help
> review and make sure it works for the cases you care about.
>
> Rob
>
> [1] https://lore.kernel.org/all/[email protected]/

Looks like my patch "dt-bindings: tpm: Add schema for TIS I2C devices"
is not required. I am removing it.

Thanks for the review.

Regards,

Ninad