Re: [PATCH V3 2/6] dt-bindings: timestamp: Add Tegra234 support

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 13/03/2023 22:49, Dipen Patel wrote:
> On 3/13/23 10:55 AM, Krzysztof Kozlowski wrote:
>> On 13/03/2023 18:05, Dipen Patel wrote:
>>> On 3/12/23 8:47 AM, Krzysztof Kozlowski wrote:
>>>> On 10/03/2023 20:06, Dipen Patel wrote:
>>>>> Added timestamp provider support for the Tegra234 in devicetree
>>>>> bindings. In addition, it addresses review comments from the
>>>>> previous review round as follows:
>>>>> - Removes nvidia,slices property. This was not necessary as it
>>>>> is a constant value and can be hardcoded inside the driver code.
>>>>> - Adds nvidia,gpio-controller property. This simplifies how GTE driver
>>>>> retrieves GPIO controller instance, see below explanation.
>>>>>
>>>>> Without this property code would look like:
>>>>> if (of_device_is_compatible(dev->of_node, "nvidia,tegra194-gte-aon"))
>>>>> 	hte_dev->c = gpiochip_find("tegra194-gpio-aon",
>>>>> 				   tegra_get_gpiochip_from_name);
>>>>> else if (of_device_is_compatible(dev->of_node, "nvidia,tegra234-gte-aon"))
>>>>> 	hte_dev->c = gpiochip_find("tegra234-gpio-aon",
>>>>> 				   tegra_get_gpiochip_from_name);
>>>>> else
>>>>> 	return -ENODEV;
>>>>>
>>>>> This means for every future addition of the compatible string, if else
>>>>> condition statements have to be expanded.
>>>>>
>>>>> With the property:
>>>>> gpio_ctrl = of_parse_phandle(dev->of_node, "nvidia,gpio-controller", 0);
>>>>> ....
>>>>> hte_dev->c = gpiochip_find(gpio_ctrl, tegra_get_gpiochip_from_of_node);
>>>>>
>>>>> We haven't technically started making use of these bindings, so
>>>>> backwards-compatibility shouldn't be an issue yet.
>>>>
>>>> Unfortunately, I don't understand this statement. The
>>>> nvidia,tegra194-gte-aon with removed property is in a released kernel
>>>> v6.2. What does it mean "technically"? It's a released kernel thus it is
>>>> a released ABI.
>>>
>>> There is no active user of that driver, so even if it breaks 6.2, it is fine
>>> as there is no one to complain about it.
>>
>> How do you know? It's a released kernel, thus how can you ask millions
>> of people if they use it or not?
> 
> Please help me understand, if I am targeting these set of changes for the kernel
> 6.4, wouldn't all the patches land on v6.4 at the same time no matter the tree it

No, that's not how we do things. Changes *must be bisectable* and *DTS
always* goes to separate branch, so how do you ensure this in your
current flow? I don't see it. The patch #4 should break the bisectability.

> will go from? Also, if user is at v6.2, how this will break as at that version, it
> will have the old bindings and old driver, right?

Bindings define ABI. You defined them like this in v6.2 thus someone is
using them:
1. In other systems, bootloaders, firmwares, SW.
2. via DTS written for v6.2 ABI. Newer kernel should not break existing
DTS and we do not talk about in-kernel DTS, just like we do not talk
about in-kernel user-space applications when using same argument for
their compatibility.


Best regards,
Krzysztof