Re: [PATCH v5 04/11] dt-bindings: Add HTE bindings

[Dipen Patel <dipenp@xxxxxxxxxx>]

Hi,

On 3/29/22 4:25 PM, Rob Herring wrote:
> On Mon, Mar 28, 2022 at 10:45:14PM -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@xxxxxxxxxx>
>> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>> ---
>> Changes in v2:
>> - Replace hte with hardware-timestamp for property names
>> - Renamed file
>> - Removed example from the common dt binding file.
>>
>> Changes in v3:
>> - Addressed grammatical errors.
>> - Removed double plural from the respective properties.
>> - Added dual license.
>> - Prefixed "nvidia" in nvidia specific properties.
>>
>> Changes in v4:
>> - Corrected make dt_binding_check error.
>>
>> Changes in v5:
>> - Addressed review comments.
>>
>>  .../hte/hardware-timestamps-common.yaml       | 29 +++++++
>>  .../devicetree/bindings/hte/hte-consumer.yaml | 43 ++++++++++
>>  .../bindings/hte/nvidia,tegra194-hte.yaml     | 82 +++++++++++++++++++
>>  3 files changed, 154 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/hte/hardware-timestamps-common.yaml
>>  create mode 100644 Documentation/devicetree/bindings/hte/hte-consumer.yaml
>>  create mode 100644 Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hte/hardware-timestamps-common.yaml b/Documentation/devicetree/bindings/hte/hardware-timestamps-common.yaml
>> new file mode 100644
>> index 000000000000..e8a69ceccd56
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hte/hardware-timestamps-common.yaml
>> @@ -0,0 +1,29 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fhte%2Fhardware-timestamps-common.yaml%23&amp;data=04%7C01%7Cdipenp%40nvidia.com%7C5793b3be05fd48a97ad108da11db79a7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637841931589163420%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=oyeG06oNMukYew%2Bkji%2FlXsDyGwIIrIvwxLHKxaiFBto%3D&amp;reserved=0
>> +$schema: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cdipenp%40nvidia.com%7C5793b3be05fd48a97ad108da11db79a7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637841931589163420%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=JOY3MmZlMo6Mopr5dwjUky%2BaQX4b0QSiDt8zo3mSu1k%3D&amp;reserved=0
>> +
>> +title: Hardware timestamp providers
>> +
>> +maintainers:
>> +  - Dipen Patel <dipenp@xxxxxxxxxx>
>> +
>> +description:
>> +  Some devices/SoCs have hardware time stamping engines which can use hardware
>> +  means to timestamp entity in realtime. The entity could be anything from
>> +  GPIOs, IRQs, Bus and so on. The hardware timestamp engine (HTE) present
>> +  itself as a provider with the bindings described in this document.
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^hardware-timestamp(@.*|-[0-9a-f])?$"
>> +
>> +  "#hardware-timestamp-cells":
>> +    description:
>> +      Number of cells in a HTE specifier.
>> +
>> +required:
>> +  - "#hardware-timestamp-cells"
>> +
>> +additionalProperties: true
>> diff --git a/Documentation/devicetree/bindings/hte/hte-consumer.yaml b/Documentation/devicetree/bindings/hte/hte-consumer.yaml
>> new file mode 100644
>> index 000000000000..be69f63aa8c3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hte/hte-consumer.yaml
>> @@ -0,0 +1,43 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fhte%2Fhte-consumer.yaml%23&amp;data=04%7C01%7Cdipenp%40nvidia.com%7C5793b3be05fd48a97ad108da11db79a7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637841931589319655%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=0f1FFB1IotZESaxDlXX5mo9YyMN25BlFAyq%2FOQJtVoE%3D&amp;reserved=0
>> +$schema: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cdipenp%40nvidia.com%7C5793b3be05fd48a97ad108da11db79a7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637841931589319655%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=B%2FvVGGwp9JghUpT33cGk0EZHDRlaOAzCbtv93Z%2Fa9YY%3D&amp;reserved=0
>> +
>> +title: HTE Consumer Device Tree Bindings
>> +
>> +maintainers:
>> +  - Dipen Patel <dipenp@xxxxxxxxxx>
>> +
>> +select: true
>> +
>> +description:
>> +  HTE properties should be named "hardware-timestamps". The exact meaning of
>> +  each hardware-timestamps property must be documented in the device tree
> The meaning of the cells needs to be documented. You are documenting the 
> meaning of 'hardware-timestamps' here.

This is for the consumer side, meaning of the cells will be documented in the provider

binding document.

>
>> +  binding for each device. An optional property "hardware-timestamp-names" may
>> +  contain a list of strings to label each of the HTE devices listed in the
>> +  "hardware-timestamps" property.
>> +
>> +properties:
>> +  hardware-timestamps:
> I'm wondering if we should just drop 'hardware'. What other kind of 
> timestamps are we going to have in DT? software-timestamps? No.

I believe this makes it explicit and leaves no room for second guess. If

only timestamps, ambiguity then will be which timestamp it is i.e. through hardware

engine, pps, ptp and so on...

>
>
>> +    $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.
>> +
>> +  hardware-timestamp-names:
>> +    $ref: /schemas/types.yaml#/definitions/string-array
>> +    description:
>> +      An optional string property.
>> +
>> +dependencies:
>> +  hardware-timestamp-names: [ hardware-timestamps ]
>> +
>> +additionalProperties: true
>> +
>> +examples:
>> +  - |
>> +    hte_irq_consumer {
>> +              hardware-timestamps = <&tegra_hte_lic 0x19>;
>> +              hardware-timestamp-names = "hte-irq";
>> +    };
>> 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..ce6f1313f52d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hte/nvidia,tegra194-hte.yaml
>> @@ -0,0 +1,82 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fhte%2Fnvidia%2Ctegra194-hte.yaml%23&amp;data=04%7C01%7Cdipenp%40nvidia.com%7C5793b3be05fd48a97ad108da11db79a7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637841931589319655%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=op7oS69fzQyvxJjQkPq8ScLuEEd32eCRjL%2BuCOPIKko%3D&amp;reserved=0
>> +$schema: https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cdipenp%40nvidia.com%7C5793b3be05fd48a97ad108da11db79a7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637841931589319655%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=B%2FvVGGwp9JghUpT33cGk0EZHDRlaOAzCbtv93Z%2Fa9YY%3D&amp;reserved=0
>> +
>> +title: Tegra194 on chip generic hardware timestamping engine (HTE)
>> +
>> +maintainers:
>> +  - Dipen Patel <dipenp@xxxxxxxxxx>
>> +
>> +description:
>> +  Tegra194 SoC has multiple generic hardware timestamping engines (GTE) 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 a 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
>> +
>> +  nvidia,int-threshold:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      HTE device generates its interrupt based on this u32 FIFO threshold
>> +      value. The recommended value is 1.
>> +    minimum: 1
>> +    maximum: 256
>> +
>> +  nvidia,slices:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    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. The value 3 is for
>> +      GPIO GTE and 11 for IRQ GTE.
>> +    enum: [3, 11]
>> +
>> +  '#hardware-timestamp-cells':
>> +    const: 1
> Need to describe what the cell represents.
Agree...
>
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - nvidia,slices
>> +  - "#hardware-timestamp-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    tegra_hte_aon: hardware-timestamp@c1e0000 {
>> +              compatible = "nvidia,tegra194-gte-aon";
>> +              reg = <0xc1e0000 0x10000>;
>> +              interrupts = <0 13 0x4>;
>> +              nvidia,int-threshold = <1>;
>> +              nvidia,slices = <3>;
>> +              #hardware-timestamp-cells = <1>;
>> +    };
>> +
>> +  - |
>> +    tegra_hte_lic: hardware-timestamp@3aa0000 {
>> +              compatible = "nvidia,tegra194-gte-lic";
>> +              reg = <0x3aa0000 0x10000>;
>> +              interrupts = <0 11 0x4>;
>> +              nvidia,int-threshold = <1>;
>> +              nvidia,slices = <11>;
>> +              #hardware-timestamp-cells = <1>;
>> +    };
>> +
>> +...
>> -- 
>> 2.17.1
>>
>>