Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <52cfa5fc-9fc5-dee4-6d53-70d241510269@infineon.com>
Date:   Mon, 11 Apr 2022 13:30:53 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH 4/4] tpm: Add YAML schema for the TPM TIS I2C options
Content-Language: en-US
To:     Rob Herring <robh@kernel.org>
CC:     <peterhuewe@gmx.de>, <jarkko@kernel.org>, <jgg@ziepe.ca>,
        <linux-integrity@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <amirmizi6@gmail.com>
References: <20220404081835.495-1-johannes.holland@infineon.com>
 <20220404081835.495-4-johannes.holland@infineon.com>
 <YksaVw74Eotowyse@robh.at.kernel.org>
From:   Johannes Holland <johannes.holland@infineon.com>
In-Reply-To: <YksaVw74Eotowyse@robh.at.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 04.04.2022 18:18, Rob Herring wrote:
> On Mon, Apr 04, 2022 at 10:18:35AM +0200, Johannes Holland wrote:
>> Add a YAML 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.
>
> Bindings are for devices. A protocol layer does not make a device.

Agreed. I will change this in my next patch.

>
>>
>> Signed-off-by: Johannes Holland <johannes.holland@infineon.com>
>> ---
>>  .../bindings/security/tpm/tpm-tis-i2c.yaml    | 48 +++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>
> We already have a binding for I2C TPM. That one should be converted.

Will do. There are two required properties which are in fact not needed
by any I2C driver. If that is ok with you, I would like to turn them
optional.

- linux,sml-base
- linux,sml-size

>>>
>> 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..7948867ff3f7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-tis-i2c.yaml
>> @@ -0,0 +1,48 @@
>> +# 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 Device Tree Bindings
>> +
>> +maintainers:
>> +  - Johannes Holland <johannes.holland@infineon.com>
>> +
>> +description:
>> +  Device Tree Bindings for I2C based Trusted Platform Module (TPM).
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          # Infineon's Trusted Platform Module (TPM) (SLB9673)
>> +          - infineon,slb9673
>> +          # Nuvoton's Trusted Platform Module (TPM) (NPCT75x)
>> +          - nuvoton,npct75x
>
> I see this is already used, but in general wildcards should not be used
> in device compatibles.

Ok, I took this over from a previous patch. Since I am not familiar with
Nuvoton products, so I am going to remove this for now.

>
>> +      - const: tcg,tpm-tis-i2c
>
> Pretty sure I killed this off when originally reviewing the TPM I2C
> binding.
>

Sorry, I did not see any discussion related to this.

IMHO, the TPM is a open standard device. That should allow for plug
and play, regardless of the manufacturer. For SPI, we also have
tcg,tpm_tis-spi. However, if you want it removed, I can do that.

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