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

[Dipen Patel <dipenp@nvidia.com>]

On 2/16/23 6:17 AM, Krzysztof Kozlowski wrote:
> On 14/02/2023 12:55, Dipen Patel wrote:
>> Added timestamp provider support for the Tegra234 in devicetree
>> bindings.
> 
> 1. Your commit does much more. You need to explain it why you drop some
> property.
ACK, will address it next patch
> 
> 2. Bindings go before its usage (in the patchset).
Ack...
> 
> 3. Please use scripts/get_maintainers.pl to get a list of necessary
> people and lists to CC.  It might happen, that command when run on an
> older kernel, gives you outdated entries.  Therefore please be sure you
> base your patches on recent Linux kernel.
It is based on recent linux at the time patch series was sent...
> 
> 
>>
>> Signed-off-by: Dipen Patel <dipenp@nvidia.com>
>> ---
>> v2:
>> - Removed nvidia,slices property
>> - Added nvidia,gpio-controller based on review comments from Thierry,
>>   this will help simplify the  hte provider driver.
>>
>>  .../timestamp/nvidia,tegra194-hte.yaml        | 30 ++++++++++++-------
>>  1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml b/Documentation/devicetree/bindings/timestamp/nvidia,tegra194-hte.yaml
>> index c31e207d1652..d0f4ed75baee 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@nvidia.com>
>> @@ -23,6 +23,8 @@ properties:
>>      enum:
>>        - nvidia,tegra194-gte-aon
>>        - nvidia,tegra194-gte-lic
>> +      - nvidia,tegra234-gte-aon
>> +      - nvidia,tegra234-gte-lic
>>  
>>    reg:
>>      maxItems: 1
>> @@ -38,14 +40,11 @@ properties:
>>      minimum: 1
>>      maximum: 256
>>  
>> -  nvidia,slices:
>> -    $ref: /schemas/types.yaml#/definitions/uint32
>> +  nvidia,gpio-controller:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>      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]
>> +      The phandle to AON gpio controller instance. This is required to handle
>> +      namespace conversion between GPIO and GTE.
>>  
>>    '#timestamp-cells':
>>      description:
>> @@ -55,11 +54,21 @@ properties:
>>        mentioned in the nvidia GPIO device tree binding document.
>>      const: 1
>>  
>> +if:
> 
> Keep it under allOf (so you no need to re-indent it on next if statement
> in the future) and put entire allOf after "required:".
> 
Ack...
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        enum:
>> +          - nvidia,tegra194-gte-aon
> 
> This is an ABI break. Does your driver handle it?
yes, handling patch is part of this patch series.
> 
>> +          - nvidia,tegra234-gte-aon
>> +then:
>> +  required:
>> +    - nvidia,gpio-controller
>> +
>>  required:
>>    - compatible
>>    - reg
>>    - interrupts
>> -  - nvidia,slices
>>    - "#timestamp-cells"
> 
> 
> Best regards,
> Krzysztof
>