| Message ID | 20210625235532.19575-5-dipenp@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Intro to Hardware timestamping engine | expand |
On Fri, 25 Jun 2021 16:55:25 -0700, Dipen Patel wrote: > Introduces HTE devicetree binding details for the HTE subsystem. It > includes examples for the consumers, binding details for the providers > and specific binding details for the Tegra194 based HTE providers. > > Signed-off-by: Dipen Patel <dipenp@nvidia.com> > --- > .../devicetree/bindings/hte/hte-consumer.yaml | 47 +++++++++++ > .../devicetree/bindings/hte/hte.yaml | 34 ++++++++ > .../bindings/hte/nvidia,tegra194-hte.yaml | 83 +++++++++++++++++++ > 3 files changed, 164 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hte/hte-consumer.yaml > create mode 100644 Documentation/devicetree/bindings/hte/hte.yaml > create mode 100644 Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml:40:4: [warning] wrong indentation: expected 4 but found 3 (indentation) ./Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml:41:5: [warning] wrong indentation: expected 5 but found 4 (indentation) ./Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml:45:5: [warning] wrong indentation: expected 5 but found 4 (indentation) ./Documentation/devicetree/bindings/hte/hte.yaml:34:7: [error] no new line character at the end of file (new-line-at-end-of-file) dtschema/dtc warnings/errors: \ndoc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1497480 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On 6/27/21 3:56 AM, Linus Walleij wrote: > Hi Dipen, > > thanks a lot for this very interesting patch set! > > I'm gonna try to review properly, just pointing out some conceptual > things to begin with. Bindings is a good place to start. > > On Sat, Jun 26, 2021 at 1:48 AM Dipen Patel <dipenp@nvidia.com> wrote: > >> +description: | >> + HTE properties should be named "htes". The exact meaning of each htes >> + property must be documented in the device tree binding for each device. >> + An optional property "hte-names" may contain a list of strings to label >> + each of the HTE devices listed in the "htes" property. > I think this is a bit over-abbreviated. IIO has: > io-channels =... > io-channel-names =... > > Given DT:s infatuation with using english plural I would opt for: > hardware-timestamps = .. > hardware-timestamp-names = ... I can change it to suggested names in next RFC series. > > The "engine" part is a bit of an nVidia:ism I think and a too generic > term. Could as well be "processor" or "automata" but nVidia just > happened to name it an engine. (DMA engine would be a precedent > though, so no hard preference from my side.) > > When reading this it is pretty intuitively evident what is going on. > > Other than that it looks really good! > >> +++ b/Documentation/devicetree/bindings/hte/hte.yaml > I would name this hardware-timestamp-common.yamp or so. Sure, but do I have to change hte-consumer and other hte named yaml as well in this directory? If yes, I am referring HTE everywhere in the code (framework is named as hte itself), I hope that is fine and does not create any confusion. > >> +title: HTE providers > Spell this out: Hardware timestamp providers Can I do hardware timestamp engine provider instead? > >> +properties: >> + $nodename: >> + pattern: "^hte(@.*|-[0-9a-f])*$" > Likewise: > hardware-timestamp@ ... > > I think this is good because it is very unambiguous. > >> +examples: >> + - | >> + tegra_hte_aon: hte@c1e0000 { >> + compatible = "nvidia,tegra194-gte-aon"; >> + reg = <0xc1e0000 0x10000>; >> + interrupts = <0 13 0x4>; >> + int-threshold = <1>; >> + slices = <3>; >> + #hte-cells = <1>; >> + }; > The examples can be kept to the tegra194 bindings I think, this > generic binding doesn't need an example as such. Ok, will remove it. > >> +$id: http://devicetree.org/schemas/hte/nvidia,tegra194-hte.yaml# > This one should be named like this, that is great. > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Tegra194 on chip generic hardware timestamping engine (HTE) > This is clear and nice. > >> + int-threshold: >> + description: >> + HTE device generates its interrupt based on this u32 FIFO threshold >> + value. The recommended value is 1. >> + minimum: 1 >> + maximum: 256 > Does this mean a single timestamp in the FIFO will generate an IRQ? > Then spell that out so it is clear. In the description I said that. > >> + slices: >> + description: >> + HTE lines are arranged in 32 bit slice where each bit represents different >> + line/signal that it can enable/configure for the timestamp. It is u32 >> + property and depends on the HTE instance in the chip. >> + oneOf: >> + - items: >> + - const: 3 >> + - items: >> + - const: 11 > Can't you just use > enum: [3, 11] > ? Sure, will change it. > >> + '#hte-cells': >> + const: 1 > So IMO this would be something like > #hardware-timestamp-cells Sure. > > Other than this it overall looks very nice to me! > > Yours, > Linus Walleij
Thanks Rob, I will correct those warning. On 7/1/21 7:02 AM, Rob Herring wrote: > On Fri, 25 Jun 2021 16:55:25 -0700, Dipen Patel wrote: >> Introduces HTE devicetree binding details for the HTE subsystem. It >> includes examples for the consumers, binding details for the providers >> and specific binding details for the Tegra194 based HTE providers. >> >> Signed-off-by: Dipen Patel <dipenp@nvidia.com> >> --- >> .../devicetree/bindings/hte/hte-consumer.yaml | 47 +++++++++++ >> .../devicetree/bindings/hte/hte.yaml | 34 ++++++++ >> .../bindings/hte/nvidia,tegra194-hte.yaml | 83 +++++++++++++++++++ >> 3 files changed, 164 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hte/hte-consumer.yaml >> create mode 100644 Documentation/devicetree/bindings/hte/hte.yaml >> create mode 100644 Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml >> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml:40:4: [warning] wrong indentation: expected 4 but found 3 (indentation) > ./Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml:41:5: [warning] wrong indentation: expected 5 but found 4 (indentation) > ./Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml:45:5: [warning] wrong indentation: expected 5 but found 4 (indentation) > ./Documentation/devicetree/bindings/hte/hte.yaml:34:7: [error] no new line character at the end of file (new-line-at-end-of-file) > > dtschema/dtc warnings/errors: > \ndoc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/patch/1497480 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. >
On 7/1/21 8:54 AM, Rob Herring wrote: > On Fri, Jun 25, 2021 at 5:48 PM Dipen Patel <dipenp@nvidia.com> wrote: >> Introduces HTE devicetree binding details for the HTE subsystem. It >> includes examples for the consumers, binding details for the providers >> and specific binding details for the Tegra194 based HTE providers. >> >> Signed-off-by: Dipen Patel <dipenp@nvidia.com> >> --- >> .../devicetree/bindings/hte/hte-consumer.yaml | 47 +++++++++++ >> .../devicetree/bindings/hte/hte.yaml | 34 ++++++++ >> .../bindings/hte/nvidia,tegra194-hte.yaml | 83 +++++++++++++++++++ >> 3 files changed, 164 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hte/hte-consumer.yaml >> create mode 100644 Documentation/devicetree/bindings/hte/hte.yaml >> create mode 100644 Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml >> >> diff --git a/Documentation/devicetree/bindings/hte/hte-consumer.yaml b/Documentation/devicetree/bindings/hte/hte-consumer.yaml >> new file mode 100644 >> index 000000000000..79ae1f7d5185 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hte/hte-consumer.yaml >> @@ -0,0 +1,47 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hte/hte-consumer.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: HTE Consumer Device Tree Bindings >> + >> +maintainers: >> + - Dipen Patel <dipenp@nvidia.com> >> + >> +description: | >> + HTE properties should be named "htes". The exact meaning of each htes >> + property must be documented in the device tree binding for each device. >> + An optional property "hte-names" may contain a list of strings to label >> + each of the HTE devices listed in the "htes" property. >> + >> + The "hte-names" property if specified is used to map the name of the HTE >> + device requested by the devm_of_hte_request_ts() or of_hte_request_ts >> + call to an index into the list given by the "htes" property. >> + >> +properties: >> + htes: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: >> + The list of HTE provider phandle. The provider must document the number >> + of cell that must be passed in this property along with phandle. >> + >> + hte-names: >> + $ref: /schemas/types.yaml#/definitions/string-array >> + description: >> + An optional string property. >> + >> +required: >> + - "htes" >> + >> +dependencies: >> + hte-names: [ htes ] >> + >> +additionalProperties: true >> + >> +examples: >> + - | >> + hte_irq_consumer { >> + htes = <&tegra_hte_lic 0x19>; >> + hte-names = "hte-irq"; >> + }; >> diff --git a/Documentation/devicetree/bindings/hte/hte.yaml b/Documentation/devicetree/bindings/hte/hte.yaml >> new file mode 100644 >> index 000000000000..e285c38f1a05 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hte/hte.yaml >> @@ -0,0 +1,34 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hte/hte.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: HTE providers >> + >> +maintainers: >> + - Dipen Patel <dipenp@nvidia.com> >> + >> +properties: >> + $nodename: >> + pattern: "^hte(@.*|-[0-9a-f])*$" >> + >> + "#hte-cells": >> + description: >> + Number of cells in a HTE specifier. >> + >> +required: >> + - "#hte-cells" >> + >> +additionalProperties: true >> + >> +examples: >> + - | >> + tegra_hte_aon: hte@c1e0000 { >> + compatible = "nvidia,tegra194-gte-aon"; >> + reg = <0xc1e0000 0x10000>; >> + interrupts = <0 13 0x4>; >> + int-threshold = <1>; >> + slices = <3>; >> + #hte-cells = <1>; >> + }; >> \ No newline at end of file >> diff --git a/Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml b/Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml >> new file mode 100644 >> index 000000000000..bb76cc1971f0 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml >> @@ -0,0 +1,83 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hte/nvidia,tegra194-hte.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Tegra194 on chip generic hardware timestamping engine (HTE) > I had to read until here to know what HTE is. Do you think I should have added more description in commit message of hte.yaml for that matter? > > Is there another example of this type of h/w that this should be a > generic binding? For now, I have only this hardware. > > Rob
diff --git a/Documentation/devicetree/bindings/hte/hte-consumer.yaml b/Documentation/devicetree/bindings/hte/hte-consumer.yaml new file mode 100644 index 000000000000..79ae1f7d5185 --- /dev/null +++ b/Documentation/devicetree/bindings/hte/hte-consumer.yaml @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hte/hte-consumer.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HTE Consumer Device Tree Bindings + +maintainers: + - Dipen Patel <dipenp@nvidia.com> + +description: | + HTE properties should be named "htes". The exact meaning of each htes + property must be documented in the device tree binding for each device. + An optional property "hte-names" may contain a list of strings to label + each of the HTE devices listed in the "htes" property. + + The "hte-names" property if specified is used to map the name of the HTE + device requested by the devm_of_hte_request_ts() or of_hte_request_ts + call to an index into the list given by the "htes" property. + +properties: + htes: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: + The list of HTE provider phandle. The provider must document the number + of cell that must be passed in this property along with phandle. + + hte-names: + $ref: /schemas/types.yaml#/definitions/string-array + description: + An optional string property. + +required: + - "htes" + +dependencies: + hte-names: [ htes ] + +additionalProperties: true + +examples: + - | + hte_irq_consumer { + htes = <&tegra_hte_lic 0x19>; + hte-names = "hte-irq"; + }; diff --git a/Documentation/devicetree/bindings/hte/hte.yaml b/Documentation/devicetree/bindings/hte/hte.yaml new file mode 100644 index 000000000000..e285c38f1a05 --- /dev/null +++ b/Documentation/devicetree/bindings/hte/hte.yaml @@ -0,0 +1,34 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hte/hte.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HTE providers + +maintainers: + - Dipen Patel <dipenp@nvidia.com> + +properties: + $nodename: + pattern: "^hte(@.*|-[0-9a-f])*$" + + "#hte-cells": + description: + Number of cells in a HTE specifier. + +required: + - "#hte-cells" + +additionalProperties: true + +examples: + - | + tegra_hte_aon: hte@c1e0000 { + compatible = "nvidia,tegra194-gte-aon"; + reg = <0xc1e0000 0x10000>; + interrupts = <0 13 0x4>; + int-threshold = <1>; + slices = <3>; + #hte-cells = <1>; + }; \ No newline at end of file diff --git a/Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml b/Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml new file mode 100644 index 000000000000..bb76cc1971f0 --- /dev/null +++ b/Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hte/nvidia,tegra194-hte.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Tegra194 on chip generic hardware timestamping engine (HTE) + +maintainers: + - Dipen Patel <dipenp@nvidia.com> + +description: | + Tegra194 SoC has multiple generic hardware timestamping engines which can + monitor subset of GPIO and on chip IRQ lines for the state change, upon + detection it will record timestamp (taken from system counter) in its + internal hardware FIFO. It has bitmap array arranged in 32bit slices where + each bit represent signal/line to enable or disable for the hardware + timestamping. + +properties: + compatible: + enum: + - nvidia,tegra194-gte-aon + - nvidia,tegra194-gte-lic + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + int-threshold: + description: + HTE device generates its interrupt based on this u32 FIFO threshold + value. The recommended value is 1. + minimum: 1 + maximum: 256 + + slices: + description: + HTE lines are arranged in 32 bit slice where each bit represents different + line/signal that it can enable/configure for the timestamp. It is u32 + property and depends on the HTE instance in the chip. + oneOf: + - items: + - const: 3 + - items: + - const: 11 + + '#hte-cells': + const: 1 + +required: + - compatible + - reg + - interrupts + - slices + - "#hte-cells" + +additionalProperties: false + +examples: + - | + tegra_hte_aon: hte@c1e0000 { + compatible = "nvidia,tegra194-gte-aon"; + reg = <0xc1e0000 0x10000>; + interrupts = <0 13 0x4>; + int-threshold = <1>; + slices = <3>; + #hte-cells = <1>; + }; + + - | + tegra_hte_lic: hte@3aa0000 { + compatible = "nvidia,tegra194-gte-lic"; + reg = <0x3aa0000 0x10000>; + interrupts = <0 11 0x4>; + int-threshold = <1>; + slices = <11>; + #hte-cells = <1>; + }; + +...
Introduces HTE devicetree binding details for the HTE subsystem. It includes examples for the consumers, binding details for the providers and specific binding details for the Tegra194 based HTE providers. Signed-off-by: Dipen Patel <dipenp@nvidia.com> --- .../devicetree/bindings/hte/hte-consumer.yaml | 47 +++++++++++ .../devicetree/bindings/hte/hte.yaml | 34 ++++++++ .../bindings/hte/nvidia,tegra194-hte.yaml | 83 +++++++++++++++++++ 3 files changed, 164 insertions(+) create mode 100644 Documentation/devicetree/bindings/hte/hte-consumer.yaml create mode 100644 Documentation/devicetree/bindings/hte/hte.yaml create mode 100644 Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml