Messages in this thread | | | From | Andrey Pronin <> | Date | Thu, 16 Jan 2020 09:58:52 -0800 | Subject | Re: [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML |
| |
On Sun, Jan 5, 2020 at 9:57 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Rob Herring (2019-12-20 15:10:40) > > On Tue, Dec 17, 2019 at 09:45:02AM -0800, Doug Anderson wrote: > > > On Mon, Dec 16, 2019 at 4:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > diff --git a/Documentation/devicetree/bindings/security/tpm/google,cr50.yaml b/Documentation/devicetree/bindings/security/tpm/google,cr50.yaml > > > > new file mode 100644 > > > > index 000000000000..8bfff0e757af > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/security/tpm/google,cr50.yaml > > > > @@ -0,0 +1,52 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/tpm/google,cr50.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: H1 Secure Microcontroller with Cr50 Firmware on SPI Bus > > > > + > > > > +description: > > > > + H1 Secure Microcontroller running Cr50 firmware provides several functions, > > > > + including TPM-like functionality. It communicates over SPI using the FIFO > > > > + protocol described in the PTP Spec, section 6. > > > > + > > > > +maintainers: > > > > + - Andrey Pronin <apronin@chromium.org> > > > > > > Does Andrey agree to be the maintainer here? > > I Cced Andrey in hopes of eliciting a response.
Yes, I finally can confirm I agree to be the maintainer.
> > > > > > > > > > I'd like to see if we can delete most of what you've written here. > > > Specifically in "spi/spi-controller.yaml" you can see a really nice > > > description of what SPI devices ought to look like. Can we just > > > reference that? To do that I _think_ we actually need to break that > > > description into a separate YAML file and then include it from there > > > and here. Maybe someone on the list can confirm or we can just post > > > some patches for that? > > I'm not sure what to do here. > > > > > > > > > > > +properties: > > > > + compatible: > > > > + const: google,cr50 > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > > > I'm curious if you need a minItems here. ...and if we don't somehow > > > include it, should we follow 'spi-controller.yaml' and treat this like > > > an int? > > > > Really, just 'true' is sufficient as you can't say which CS number it is > > here. > > Ok. > > > > > > > > > > > +required: > > > > + - compatible > > > > + - reg > > > > + - spi-max-frequency > > > > > > Technically spi-max-frequency might not be required (the SPI binding > > > doesn't list it as such), but I guess it was before... > > > > Generally, we expect a device knows its max and this should only be used > > it a board has a lower value. However, sometimes there's exceptions. > > > > Shouldn't really be debate here unless the old binding doc was wrong. > > The old binding doc had it as required and the spi framework seems to > bail out if this property isn't specified (see of_spi_parse_dt() for > more details). > > > > > > > > > > > > > + - interrupts > > > > + > > > > +additionalProperties: false > > > > + > > > > +examples: > > > > + - | > > > > + #include <dt-bindings/interrupt-controller/irq.h> > > > > + spi { > > > > + #address-cells = <0x1>; > > > > + #size-cells = <0x0>; > > > > + tpm@0 { > > > > + compatible = "google,cr50"; > > > > + reg = <0>; > > > > + spi-max-frequency = <800000>; > > > > + interrupts = <50 IRQ_TYPE_EDGE_RISING>; > > > > > > I would tend to prefer seeing the interrupt parent in the example > > > since it's pretty likely that the GPIO controller isn't the overall > > > parent and likely that our interrupt is a GPIO. I'm not sure the > > > convention, though. > > > > Example is fine, but shouldn't be in the schema. > > Ok. Will add an interrupt parent like > > interrupt-parent = <&gpio_controller>; >
-- Andrey
| |