Re: [PATCH 4/7] dt-bindings: timestamp: Add Tegra234 support

[Dipen Patel]

On 11/7/22 12:06 PM, Rob Herring wrote:
> On Thu, Nov 03, 2022 at 10:45:20AM -0700, Dipen Patel wrote:
>> Added timestamp provider support for the Tegra234 in devicetree
>> bindings.
>>
>> Signed-off-by: Dipen Patel <dipenp@xxxxxxxxxx>
>> ---
>>  .../timestamp/nvidia,tegra194-hte.yaml        | 44 +++++++++++++++++--
>>  1 file changed, 40 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml b/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml
>> index c31e207d1652..158dbe58c49f 100644
>> --- a/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml
>> +++ b/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml
>> @@ -4,7 +4,7 @@
>>  $id: http://devicetree.org/schemas/timestamp/nvidia,tegra194-hte.yaml#
>>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>>  
>> -title: Tegra194 on chip generic hardware timestamping engine (HTE)
>> +title: Tegra on chip generic hardware timestamping engine (HTE) provider
>>  
>>  maintainers:
>>    - Dipen Patel <dipenp@xxxxxxxxxx>
>> @@ -23,6 +23,8 @@ properties:
>>      enum:
>>        - nvidia,tegra194-gte-aon
>>        - nvidia,tegra194-gte-lic
>> +      - nvidia,tegra234-gte-aon
>> +      - nvidia,tegra234-gte-lic
> 
> How is the h/w in this chip different from the existing one? I'm 
> assuming it must be because you don't have a fallback compatible.
t234-gte-lic has different slices and signal lines it can support. t234-gte-aon
has again different slices and more gpio lines it can support and has
different GTE-GPIO mapping than t194-gte-aon.
> 
>>  
>>    reg:
>>      maxItems: 1
>> @@ -43,9 +45,8 @@ properties:
>>      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]
>> +      property and the value depends on the HTE instance in the chip.
> 
> If this statement was true, then this property makes sense...
> 
>> +    enum: [3, 11, 17]
>>  
>>    '#timestamp-cells':
>>      description:
>> @@ -55,6 +56,41 @@ properties:
>>        mentioned in the nvidia GPIO device tree binding document.
>>      const: 1
>>  
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - nvidia,tegra194-gte-aon
>> +              - nvidia,tegra234-gte-aon
>> +    then:
>> +      properties:
>> +        nvidia,slices:
>> +          const: 3
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - nvidia,tegra194-gte-lic
>> +    then:
>> +      properties:
>> +        nvidia,slices:
>> +          const: 11
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - nvidia,tegra234-gte-lic
>> +    then:
>> +      properties:
>> +        nvidia,slices:
>> +          const: 17
> 
> However, if there is only one possible value for each compatible, then 
> being per instance can't really be true. I guess 'aon' or 'lic' define 
> the instance? That's not normal practice. Are there other differences?
aon and lic are gte hardware instances but meant for different signals i.e.
lic is for interrupt lines while aon is for always on domain GPIO lines.
> 
> It seems like 'nvidia,slices' should be implied from the compatible 
> string.
thats true, assuming we have all those specified compatible strings from this patch.
> 
> Rob