Message-ID: <7a8027c9-dc73-3684-c5f2-3071f315b3cd@nvidia.com>
Date:   Wed, 8 Mar 2023 10:45:24 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
Subject: Re: [PATCH V2 4/6] dt-bindings: timestamp: Add Tegra234 support
To:     Krzysztof Kozlowski <krzk@kernel.org>, thierry.reding@gmail.com,
        jonathanh@nvidia.com, linux-kernel@vger.kernel.org,
        linux-tegra@vger.kernel.org, linux-gpio@vger.kernel.org,
        linus.walleij@linaro.org, devicetree@vger.kernel.org,
        linux-doc@vger.kernel.org, robh+dt@kernel.org,
        timestamp@lists.linux.dev
References: <20230214115553.10416-1-dipenp@nvidia.com>
 <20230214115553.10416-5-dipenp@nvidia.com>
 <3c0ad963-ce69-bd5b-20cd-888e5fbdecaf@kernel.org>
Content-Language: en-US
From:   Dipen Patel <dipenp@nvidia.com>
In-Reply-To: <3c0ad963-ce69-bd5b-20cd-888e5fbdecaf@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aGhVajQzM1ZHanJpQkJrK20xVDlnbURzeW4xSk5ZYy8ycFhKM056ZjdjMWVH?=
 =?utf-8?B?NzMzeE8rVmFUR08wbWdWWHZpOTlDY2M5aVlSakJaSk91bFJIN1RLc1ZuR0NY?=
 =?utf-8?B?cVFOYVZHZVl6akFBa2R5ZTdza3cvODdLKzJoVlFiYWl4aVpLYWRPNGs0ZHI1?=
 =?utf-8?B?N2NmMFVFSnI2ZW1WUHRBMXFoaTg4M3UyR0MzRHBoYnFJdUVPM2hBR2xWak1E?=
 =?utf-8?B?eVo5UVNjSnljK2JDTndNYXlyNUhMZnlMMkJxTlJ1WVF5anoxa0pzdG5xQ1Fj?=
 =?utf-8?B?a25KTitDNmRQYW91TzZKK0dKTEhWcHdLVk9MbEVrVXJFaGdOTVhhY2hHblBn?=
 =?utf-8?B?WUZKMzU5SVhYLzFOcG5KRmlwcHNIZWw0QjEzeXI0UWRmeFkvYjZ2NGYzUU1L?=
 =?utf-8?B?cXRGdk5tWTB5QWpUVklQMWU1ZTYyZ2U4Y2NWK0hoNVVYbWxCM2szZ1JZek45?=
 =?utf-8?B?R0xMU1pEaEFjSnNhMEIvbDI0RkNQK1QyZlg5ZHJGYnFGWGxGM1QyUUtiaU1Z?=
 =?utf-8?B?Mldvd2tLbWlKQjZuVDU1QndpV2VaK2dRK09JSUNhcUI4TDRCMlVDRThRcy9p?=
 =?utf-8?B?b29oeWI3S1pRYjM3d3NoYnBhS0svUUpWelR4d0V1aTBuSzlrRjhYSnZHdVRy?=
 =?utf-8?B?VlY5cmJlZGk5SFBVREhKRXFzaEVjelZ4dUFSSzhiU244eGdqREgxK2hvS1l1?=
 =?utf-8?B?U216VTl0L0VCWlkyQ1pwblhKL0lBdkk3MlBuZHdXTk5NamxaN2Z0bjhXdUgz?=
 =?utf-8?B?U0hSVGRKUmo2UDRHYUZnNkcyelowSFBMdlhDUkRDOXQ1OXN6SGI4Mjk3cnVv?=
 =?utf-8?B?bWZTZ0VpcU1SdDdxZzJ3aWlwaU9qV0prYWxvWFhTaklTRGxXTFpBVnlLK1oy?=
 =?utf-8?B?R2h5YkxZbFg5N1ErU3VzN1Y0dDk2ODhxd3dzd1pNdlB5YmhSRDAybXFUMGxK?=
 =?utf-8?B?aU1YVGpKUlpNWEVkY3R5TXlrSktIaGpoa0N3S1ZjOWsyWTZaSTBVZlhzcmRt?=
 =?utf-8?B?SldNYkxEK0o5ZzI0K3V0YUtZeTU4RitwRFdzTExhQkltdy90ekd6SHVqYnBj?=
 =?utf-8?B?ZTE0dzNXd2RGeHVwRGtYeTZoRzU5eHRGSUhnc0xvYVdBU2FrU28wWVlkSFJX?=
 =?utf-8?B?TkZRa3YxbGRsZEd6NTlyL3NPZlVKREFqTHhkT3I5bmJhazNkQk54ejNrRGF0?=
 =?utf-8?B?M2c4RlNjV1hyWi80MUlydmYzQjE4WFFVQ2ZIK2lIUGZ5eW1NTnJIdlZ2YWwy?=
 =?utf-8?B?WUJMcVVSSU5RdzFYblQyR0xOUDJabGJQaG5xYXlqMzhpOUY2ekF6NzcrdWJL?=
 =?utf-8?B?a1VKblplaUszY0ZlMXJJT21YV0JSUFFCcWxIMngwQ1JVUFIrMWk2WlVHeWM5?=
 =?utf-8?B?bmNvWDdibC8vbWpocXhJQ0Q5UDlvd0R1QzI0MWxyU0pqTldPTW9lVno5NVpa?=
 =?utf-8?B?aXNoSkxqV25OSkczVFJqUnZ1d3JvZ3dia0wvelIzRzBOOWNGU2dqb2xBVkdw?=
 =?utf-8?B?b3hLdlc3ckhYYXgrc011R2J4M0VrcEFubVhxTXB2YTlLa3RaaDdyR3hQMU1h?=
 =?utf-8?B?YXVFT0lDeU54L1M4UkI4cGlaWExVbjM4bHEvYW5xT0dMRWNrNERwZ01EY2xu?=
 =?utf-8?B?em1xNCtnMEk0Ukh4cldBL1ZwMm8vWlYrVDRLVWtKb1R2NSs4ZUpFOUZNMkpZ?=
 =?utf-8?B?RmVCMTRzMDRTbExFQlBuS3Qyd2szZVR6ei9QQmMybzk0WlBRQ1RkVlEyK05I?=
 =?utf-8?B?bFBnaVhJOVZYcTQvLzU2bGxrdCtjNXo4TUs1djFaOTNRWFh0d0M5MW9tQzFT?=
 =?utf-8?B?Rkx4SU1iYjhLajJoblBFeDRJaGNrelNnK0FPOHF6NklXR2NhRWJJQjhvZXFl?=
 =?utf-8?B?N3lVaSsya1NYMWpSdGZnMnF1TFppVlowNWl6czU4OHA0UDFPU2c1RjZRTlc5?=
 =?utf-8?B?dVQ5bTlKSFFBRlVUTFRDQXRzdW51VVpTTDdpRXVRQnFuTVR5WGRKSEFGWU9s?=
 =?utf-8?B?RTJ1a3lSK2QvWkZORHd4SWtQb1BNQkZkOWlDcnlpM1NsclMxRFVVTEFXN21T?=
 =?utf-8?B?U1JIZytnanRJd0VWdGQrZzhRekJ4RUxmVmx6ZE1ZT3Jtb3Z0L1ZHZk0zc0dr?=
 =?utf-8?Q?Ka4nDZc7RI7JnVTGtpFzMYCaC?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1ad0ad9b-5775-4d68-84f1-08db20054953
X-MS-Exchange-CrossTenant-AuthSource: IA1PR12MB6604.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2023 18:45:28.3942
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ytB2Au/kmlslpGW1/sPBKRQxVf2y2PwOmmtwuOKnXwcdsWGvfcBx88uERixd8+E0m8ryvheYukLr/C9wHU3Icw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB6563
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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
>