Message-ID: <c45df83a-a6ac-16bd-1872-64fd0794b421@linaro.org>
Date:   Tue, 14 Mar 2023 09:43:14 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Subject: Re: [PATCH V3 2/6] dt-bindings: timestamp: Add Tegra234 support
To:     Dipen Patel <dipenp@nvidia.com>, 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, krzysztof.kozlowski+dt@linaro.org,
        brgl@bgdev.pl, corbet@lwn.net, gregkh@linuxfoundation.org
References: <20230310190634.5053-1-dipenp@nvidia.com>
 <20230310190634.5053-3-dipenp@nvidia.com>
 <f6d9c84a-1c75-d9b4-59ed-39d6c5b310a9@linaro.org>
 <b4195142-6cfe-df3c-6edf-0c40b64ad02a@nvidia.com>
 <6c5045d9-4f4a-5018-3f3f-7746b08ab2b5@linaro.org>
 <fad52df6-38e8-ba8a-117a-8514e09af0ee@nvidia.com>
Content-Language: en-US
In-Reply-To: <fad52df6-38e8-ba8a-117a-8514e09af0ee@nvidia.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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