* [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML @ 2019-12-17 0:54 Stephen Boyd 2019-12-17 17:45 ` Doug Anderson 0 siblings, 1 reply; 6+ messages in thread From: Stephen Boyd @ 2019-12-17 0:54 UTC (permalink / raw) To: Rob Herring; +Cc: linux-kernel, devicetree, Andrey Pronin, Douglas Anderson This allows us to validate the dt binding to the implementation. Add the interrupt property too, because that's required but nobody noticed when the non-YAML binding was introduced. Cc: Andrey Pronin <apronin@chromium.org> Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- .../bindings/security/tpm/google,cr50.txt | 19 ------- .../bindings/security/tpm/google,cr50.yaml | 52 +++++++++++++++++++ 2 files changed, 52 insertions(+), 19 deletions(-) delete mode 100644 Documentation/devicetree/bindings/security/tpm/google,cr50.txt create mode 100644 Documentation/devicetree/bindings/security/tpm/google,cr50.yaml diff --git a/Documentation/devicetree/bindings/security/tpm/google,cr50.txt b/Documentation/devicetree/bindings/security/tpm/google,cr50.txt deleted file mode 100644 index cd69c2efdd37..000000000000 --- a/Documentation/devicetree/bindings/security/tpm/google,cr50.txt +++ /dev/null @@ -1,19 +0,0 @@ -* H1 Secure Microcontroller with Cr50 Firmware on SPI Bus. - -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. - -Required properties: -- compatible: Should be "google,cr50". -- spi-max-frequency: Maximum SPI frequency. - -Example: - -&spi0 { - tpm@0 { - compatible = "google,cr50"; - reg = <0>; - spi-max-frequency = <800000>; - }; -}; 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> + +properties: + compatible: + const: google,cr50 + + reg: + maxItems: 1 + + spi-max-frequency: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - spi-max-frequency + - 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>; + }; + }; + +... -- Sent by a computer, using git, on the internet ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML 2019-12-17 0:54 [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML Stephen Boyd @ 2019-12-17 17:45 ` Doug Anderson 2019-12-20 23:10 ` Rob Herring [not found] ` <5e174fec.1c69fb81.f3e14.8354@mx.google.com> 0 siblings, 2 replies; 6+ messages in thread From: Doug Anderson @ 2019-12-17 17:45 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, LKML, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Andrey Pronin Hi, On Mon, Dec 16, 2019 at 4:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > This allows us to validate the dt binding to the implementation. Add the > interrupt property too, because that's required but nobody noticed when > the non-YAML binding was introduced. > > Cc: Andrey Pronin <apronin@chromium.org> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > .../bindings/security/tpm/google,cr50.txt | 19 ------- > .../bindings/security/tpm/google,cr50.yaml | 52 +++++++++++++++++++ > 2 files changed, 52 insertions(+), 19 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/security/tpm/google,cr50.txt > create mode 100644 Documentation/devicetree/bindings/security/tpm/google,cr50.yaml > > diff --git a/Documentation/devicetree/bindings/security/tpm/google,cr50.txt b/Documentation/devicetree/bindings/security/tpm/google,cr50.txt > deleted file mode 100644 > index cd69c2efdd37..000000000000 > --- a/Documentation/devicetree/bindings/security/tpm/google,cr50.txt > +++ /dev/null > @@ -1,19 +0,0 @@ > -* H1 Secure Microcontroller with Cr50 Firmware on SPI Bus. > - > -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. > - > -Required properties: > -- compatible: Should be "google,cr50". > -- spi-max-frequency: Maximum SPI frequency. > - > -Example: > - > -&spi0 { > - tpm@0 { > - compatible = "google,cr50"; > - reg = <0>; > - spi-max-frequency = <800000>; > - }; > -}; > 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'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? > +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? > + spi-max-frequency: > + maxItems: 1 This is not an array type. Why do you need maxItems? Should treat like an int? Do we have any ranges of sane values we can put here? I'm sure that there is a maximum that Cr50 can talk at. > + interrupts: > + maxItems: 1 I'm curious if you need a minItems here. ...also: should we be trying to validate the flags at all? AKA that Cr50 expects a rising edge interrupt? > +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... > + - 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. > + }; > + }; > + > +... Is the "..." important here? I guess this is only if you're trying to jam two bindings into the same file, but I could be wrong. I guess a bunch of arm ones owned by Rob have it at the end (though the example doesn't?), so maybe it's right? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML 2019-12-17 17:45 ` Doug Anderson @ 2019-12-20 23:10 ` Rob Herring [not found] ` <5e12cc29.1c69fb81.fe838.d5f3@mx.google.com> [not found] ` <5e174fec.1c69fb81.f3e14.8354@mx.google.com> 1 sibling, 1 reply; 6+ messages in thread From: Rob Herring @ 2019-12-20 23:10 UTC (permalink / raw) To: Doug Anderson Cc: Stephen Boyd, LKML, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Andrey Pronin On Tue, Dec 17, 2019 at 09:45:02AM -0800, Doug Anderson wrote: > Hi, > > On Mon, Dec 16, 2019 at 4:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > This allows us to validate the dt binding to the implementation. Add the > > interrupt property too, because that's required but nobody noticed when > > the non-YAML binding was introduced. > > > > Cc: Andrey Pronin <apronin@chromium.org> > > Cc: Douglas Anderson <dianders@chromium.org> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > --- > > .../bindings/security/tpm/google,cr50.txt | 19 ------- > > .../bindings/security/tpm/google,cr50.yaml | 52 +++++++++++++++++++ > > 2 files changed, 52 insertions(+), 19 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/security/tpm/google,cr50.txt > > create mode 100644 Documentation/devicetree/bindings/security/tpm/google,cr50.yaml > > > > diff --git a/Documentation/devicetree/bindings/security/tpm/google,cr50.txt b/Documentation/devicetree/bindings/security/tpm/google,cr50.txt > > deleted file mode 100644 > > index cd69c2efdd37..000000000000 > > --- a/Documentation/devicetree/bindings/security/tpm/google,cr50.txt > > +++ /dev/null > > @@ -1,19 +0,0 @@ > > -* H1 Secure Microcontroller with Cr50 Firmware on SPI Bus. > > - > > -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. > > - > > -Required properties: > > -- compatible: Should be "google,cr50". > > -- spi-max-frequency: Maximum SPI frequency. > > - > > -Example: > > - > > -&spi0 { > > - tpm@0 { > > - compatible = "google,cr50"; > > - reg = <0>; > > - spi-max-frequency = <800000>; > > - }; > > -}; > > 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'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? > > > > +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. > > > > + spi-max-frequency: > > + maxItems: 1 > > This is not an array type. Why do you need maxItems? Should treat > like an int? Do we have any ranges of sane values we can put here? > I'm sure that there is a maximum that Cr50 can talk at. > > > > + interrupts: > > + maxItems: 1 > > I'm curious if you need a minItems here. No. It's implied to be the same. (The tooling adds it because that's not how json-schema works). > > ...also: should we be trying to validate the flags at all? AKA that > Cr50 expects a rising edge interrupt? You can't really because you don't know how many cells. > > > > +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. > > > > + - 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. > > + }; > > + }; > > + > > +... > > Is the "..." important here? I guess this is only if you're trying to > jam two bindings into the same file, but I could be wrong. I guess a > bunch of arm ones owned by Rob have it at the end (though the example > doesn't?), so maybe it's right? We won't ever have 2 because '$ref' lookups wouldn't work. It only matters that we are consistent because if/when we want to programatically edit files, the ruamel yaml parser writes out what it is told, not what it read in. That also means fixing these is pretty easy. My current position is to have them, but it's not anything I check ATM and I forget it too. Rob ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <5e12cc29.1c69fb81.fe838.d5f3@mx.google.com>]
* Re: [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML [not found] ` <5e12cc29.1c69fb81.fe838.d5f3@mx.google.com> @ 2020-01-08 0:40 ` Doug Anderson 2020-01-16 17:58 ` Andrey Pronin 1 sibling, 0 replies; 6+ messages in thread From: Doug Anderson @ 2020-01-08 0:40 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, LKML, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>, Andrey Pronin Hi, On Sun, Jan 5, 2020 at 9:57 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > 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. Ignore me. I guess it's overkill to try to do this and I suppose if someone ever wants to do it they can always do it later. -Doug ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML [not found] ` <5e12cc29.1c69fb81.fe838.d5f3@mx.google.com> 2020-01-08 0:40 ` Doug Anderson @ 2020-01-16 17:58 ` Andrey Pronin 1 sibling, 0 replies; 6+ messages in thread From: Andrey Pronin @ 2020-01-16 17:58 UTC (permalink / raw) To: Stephen Boyd; +Cc: Doug Anderson, Rob Herring, LKML 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <5e174fec.1c69fb81.f3e14.8354@mx.google.com>]
* Re: [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML [not found] ` <5e174fec.1c69fb81.f3e14.8354@mx.google.com> @ 2020-02-01 1:35 ` Stephen Boyd 0 siblings, 0 replies; 6+ messages in thread From: Stephen Boyd @ 2020-02-01 1:35 UTC (permalink / raw) To: Doug Anderson Cc: Rob Herring, LKML, OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Andrey Pronin Quoting Stephen Boyd (2020-01-09 08:08:11) > Quoting Doug Anderson (2019-12-17 09:45:02) > > On Mon, Dec 16, 2019 at 4:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > > + spi-max-frequency: > > > + maxItems: 1 > > > > This is not an array type. Why do you need maxItems? Should treat > > like an int? Do we have any ranges of sane values we can put here? > > I'm sure that there is a maximum that Cr50 can talk at. > > From what I see looking through downstream sources my best guess for a > max frequency is 1 MHz. > I'm leaning towards dropping this property. Is there any benefit? The driver should know the max anyway. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-01 1:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17 0:54 [PATCH] dt-bindings: tpm: Convert cr50 binding to YAML Stephen Boyd
2019-12-17 17:45 ` Doug Anderson
2019-12-20 23:10 ` Rob Herring
[not found] ` <5e12cc29.1c69fb81.fe838.d5f3@mx.google.com>
2020-01-08 0:40 ` Doug Anderson
2020-01-16 17:58 ` Andrey Pronin
[not found] ` <5e174fec.1c69fb81.f3e14.8354@mx.google.com>
2020-02-01 1:35 ` Stephen Boyd
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.