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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b4195142-6cfe-df3c-6edf-0c40b64ad02a@nvidia.com>
Date:   Mon, 13 Mar 2023 10:05:24 -0700
From:   Dipen Patel <dipenp@...dia.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        thierry.reding@...il.com, jonathanh@...dia.com,
        linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-gpio@...r.kernel.org, linus.walleij@...aro.org,
        devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
        robh+dt@...nel.org, timestamp@...ts.linux.dev,
        krzysztof.kozlowski+dt@...aro.org, brgl@...ev.pl, corbet@....net,
        gregkh@...uxfoundation.org
Subject: Re: [PATCH V3 2/6] dt-bindings: timestamp: Add Tegra234 support

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.

> 
> And since DTS always go to separate branch, your patch #4 breaks
> existing DTS (return -ENODEV;) - it is not bisectable.
> 
>>
>> Signed-off-by: Dipen Patel <dipenp@...dia.com>
>> ---
> 
> 
> Best regards,
> Krzysztof
>