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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > ---
> > >   .../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@xxxxxxxxxxxxxxx
> > > +
> > > +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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux