Message-ID: <18f9a6ca-a61b-4cbb-b729-1fdb6d48651a@nvidia.com>
Date:   Wed, 8 Mar 2023 12:09:14 -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
Content-Language: en-US
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>
 <7a8027c9-dc73-3684-c5f2-3071f315b3cd@nvidia.com>
 <a5e897e5-4cb9-d50f-47a8-ffb8bd8774cb@kernel.org>
From:   Dipen Patel <dipenp@nvidia.com>
In-Reply-To: <a5e897e5-4cb9-d50f-47a8-ffb8bd8774cb@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?Zm5DeDByaUxjcHExYTcwSU5tZUZWeHh1dDlvQ2VJRW8xOHdCN0dSdEp4bUZI?=
 =?utf-8?B?ODdZUEZPSkJoTSs4THVuMXluVU1Ca293aW5BTU4wSXVBRy9PdkdFM2ZrdXpK?=
 =?utf-8?B?NUdhY1p1NW5MT1BadXp4VUl4UE5STmtHbHBiYk05MmNZRk85VFc5b2VrVnVR?=
 =?utf-8?B?dXZZKzdNaEdLMGt1L3ZxdXJRa1FVNytFUWV6Z2xmTDlkQXUxVUJSZ3NEWC90?=
 =?utf-8?B?cjlGbENJdm0rakhyTVVxSVM5cWtsRExsZkduRFVtanRiUTZLbmNHSDBOTk5z?=
 =?utf-8?B?YXFXQTNGbWVzRUcvY3U5cit3L3g3WmtxNmVhQkZOYWtEcld1TFAyaGxYUDBo?=
 =?utf-8?B?NzhCWXF4Ym5UL3FWNHNaMDU1d1BQV2pxYnRmKzRhTFV0S1pFbzA4ZS9mdDlp?=
 =?utf-8?B?L3pDTFV6ejF2NWZVZHgrc1M1SUdNdnpER2Fna01GQjg0SkMyVmVkUjcwQ0o2?=
 =?utf-8?B?MU5neW4xOTBQajdGMnNqanFRcEVVTHZSbVBmazg1Tm1vdkMzYVJqd0NPMStn?=
 =?utf-8?B?S0JtZ2VzQ1FvZlZQQzhNdEpxd1ltOWFMRGFnMmNpV3JjT2QrV1Q4R3Z3Qm41?=
 =?utf-8?B?NVh2UVdDRzYxdXByYW5xVnBwQ1FHTmlwQW9aZXE4Unc2akd4QWFndnh6UTNE?=
 =?utf-8?B?RmY3azBtK0wwNGQ4ZmtXZzRabnhyYXRQSERkODgyUmtyRFpRVWhFclJoaGti?=
 =?utf-8?B?VUN1M2lybFpTVUhiZVZpbmdDejBSVjJJQzlnY05qNitGZWxyWXdTWFlreCtn?=
 =?utf-8?B?VmxBY0hSRHU5ak43aUlvb2QyL084RlVrUVdMNnNIMi80UnlYUzlQanRlQ215?=
 =?utf-8?B?cENzMllwZWNWUFZkTFJSeDB3bFhUcG1ZL05vRHJjMUtkSlpxRk1QSm14M3Er?=
 =?utf-8?B?QWozcTRvQ29oeUdZOThyWHhaeUtocmhOVjZqY3pGWlVZbnJIR0h0LzllMWh0?=
 =?utf-8?B?eWZUMzRRVGhOV1dmcXhlczBvQVNNZlBVUS93WjU3YU5xdEo1cGJUZzd5c3VE?=
 =?utf-8?B?UFUyOHphZ3pYOXpZajk0NE02bjI0L3NQaXlSa2FrNDBzN1VsVHUwckNTYlBv?=
 =?utf-8?B?NmlxQW9YbGdleDZ0S3VpbGNLdW13UC9QZXloUWY5cVdPT2F0SEVuL3dCU2F1?=
 =?utf-8?B?Zm5oV2V5dlNBWUx6N3JwUURyMEZrZ2VNOU03cVppUzNUVXp5cGpnWGhWY1h6?=
 =?utf-8?B?cncrQ2ZOeTZSTUJLWFUwdGdQd2d6Ry9kWmkvRDJVNTdISzZ3azBpb3BGNVEr?=
 =?utf-8?B?Z1JvVy9DRFRIZEJHTE1ZTTkrdHBaQmxkKzE1dEMyaEhHNzhxdGFsa3VjbTNo?=
 =?utf-8?B?NXprMkM5enN0MXB0cUFDSXJlZVcxUEo3cEhKZ0RtbjVxQllKeDhDclBRcDZK?=
 =?utf-8?B?Z05ERGJhSkxMVDRQY0pDKzJpUTJtMXlrN1RDaUN4Y09GTy9YQzhCaTg1U2xv?=
 =?utf-8?B?ZG1sajZNQnpJS0p5MmpBbWtrN1QvYUY3YXN6N0RrY1ZsSVk0RnFpN3lsSHRi?=
 =?utf-8?B?bVVlTi9MQzZIZnY3V2xrc0I2c0FKQTdZU3U3REpaZlZuZkQyMUN3SEMvUVBO?=
 =?utf-8?B?OXJwcWlUYUhuODQvNGtCa05GUEwwS3RteGgweW53WjlSZ2VIbUhNeGtseDBX?=
 =?utf-8?B?MEVvc2prUTg4RkF0Qkk4citLS3lqc3diVFpXbnl3ZVFMLzVnams2NDhQSkxK?=
 =?utf-8?B?ODFRRWtrbVhYZjlGU1UyR0J6NWxKUkhBRHhKeHhsa1pRV3hlVGp4Tlh6aG80?=
 =?utf-8?B?U2ZpUnhoZlJtTFVDbzYyYmthOVNOMGE1WjhNOUlTZGRjNTBjTVU5cWZtV3RJ?=
 =?utf-8?B?NzFhejF1SHlJeVhIZ3lrR3VNalFKYmwyNUhpTFV5N25kUGttWGJsdkhwUmE5?=
 =?utf-8?B?YUdTSXNtRkdTRkpjWkQwS1RzYnlTZktzeGRNcnVENEhoWmEwcGt4WmM1QlNW?=
 =?utf-8?B?VFZ0dGpiMFpLdkFMbGI0SkVyYlF4MkU1dXdFRG9YbVVsZ0xQd1M4Mm04WktM?=
 =?utf-8?B?WHI3cytRSGlCL1dUZ0x4V1h3SEpKdTlKUi9LWlBQQkhlbnZXOTNPbjBkN2dP?=
 =?utf-8?B?M1NkNnVBM0g4d0FWUTVmNEtKV0RsQXBEUG9yWVYwdFVMemduSlZoYUFOeklz?=
 =?utf-8?Q?231VmzOsdNXnOIRhcVpJxPGWB?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ea2b6031-ec97-4fb0-3505-08db2010ffbd
X-MS-Exchange-CrossTenant-AuthSource: IA1PR12MB6604.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2023 20:09:18.7556
 (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: UTKLb41pNZ834RFgQjRmogosdz5ComvC9NDdEw06c7pJDaI3RYpqCZHBW08TV8pnWjsMPyzISQF3cBpLwWlk+Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB6884
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/8/23 11:05 AM, Krzysztof Kozlowski wrote:
> On 08/03/2023 19:45, Dipen Patel wrote:
>> 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...
> 
> That's good but then why you do not use scripts/get_maintainers.pl? The
> hint about recent kernel was just a hint... Just do not invent addresses
> by yourself and use the tool to get them right.
> 
I will take a note for the next patch series to add any missing people. The current
list of people/group is what historically helped review this new timestamp/hte subsystem.

> (...)
> 
>>>> +  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.
> 
> Can you point me to the code which does it? I see "return -ENODEV;", so
> I think you do not handle ABI break. I could miss something but since
> you disagree with me, please at least bring some arguments...
Refer to patch https://patchwork.kernel.org/project/timestamp/patch/20230214115553.10416-3-dipenp@nvidia.com/
which has compatible properties added and also code changes to reflect addition/deletion of some
properties.

I am not sure I have understood about ABI break comment. How else one should handle if
there is no related gpio controller property found? I am assuming you are referring to the
below code from the patch 2 (link above) when you said "return -ENODEV".


-		hte_dev->c = gpiochip_find("tegra194-gpio-aon",
-					   tegra_get_gpiochip_from_name);
+		gpio_ctrl = of_parse_phandle(dev->of_node,
+					     "nvidia,gpio-controller", 0);
+		if (!gpio_ctrl) {
+			dev_err(dev, "gpio controller node not found\n");
+			return -ENODEV;
+		}
+
+		hte_dev->c = gpiochip_find(gpio_ctrl,
+					   tegra_get_gpiochip_from_of_node)

> 
> 
> Best regards,
> Krzysztof
>