linux-kernel - Re: [PATCH v1 1/4] dt-bindings: gpio: rockchip,gpio-bank: add compatible string per SoC

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 for Android: free password hash cracker in your pocket

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

Message-ID: <a60e7f50-c39f-07e5-7238-e3a4031b45a4@linaro.org>
Date:   Wed, 18 Jan 2023 19:32:12 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Johan Jonker <jbx6244@...il.com>, Rob Herring <robh@...nel.org>
Cc:     linus.walleij@...aro.org, brgl@...ev.pl,
        krzysztof.kozlowski+dt@...aro.org, heiko@...ech.de,
        linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kever.yang@...k-chips.com, sjg@...omium.org,
        philipp.tomsich@...ll.eu
Subject: Re: [PATCH v1 1/4] dt-bindings: gpio: rockchip,gpio-bank: add
 compatible string per SoC

On 18/01/2023 18:12, Johan Jonker wrote:
> 
> 
> On 1/18/23 16:32, Rob Herring wrote:
>> On Wed, Jan 18, 2023 at 01:13:23PM +0100, Johan Jonker wrote:
>>> Currently all Rockchip gpio nodes have the same compatible.
>>> Replace all the compatibles in gpio nodes to be able to
>>> give them a consistent ID independent from probe order or alias.
>>
>> I fail to see how the compatible change affects probe order or aliases. 
>> It is also an ABI break if there is not the existing compatible as a 
>> fallback. State the problem you are trying to solve with this change, 
>> not just what your solution is.
> 
> Hi Rob,
> 
> Since the yaml conversion of rockchip,gpio-bank.yaml we have generic "gpio" names instead of "gpio-1".
> For both Linux and U-boot there's a need for a consisted ID order between the nodes.

Still do not see how compatible is related to this.

> 
> The kernel has no logic to decide between the first compatible and the fallback.
> A fallback doesn't have ability to add/select "data" with probe, but have to use
> of_device_is_compatible(np, "rockchip,rk3188-gpio-bank") for  "15"  SoCs instead.
> 
> I can produce a serie with fall back.
> Let us know how to move forward here.
> 
> Kind regards,
> 
> Johan Jonker
> 
> 
> ===
> Linux driver behavior:
> 
> 	id = of_alias_get_id(np, "gpio");
> 	if (id < 0)
> 		id = gpio++;
> 
> Problems:
> Alias not always available in existing DT files(not part of the binding)
> Probe order is not guarantied and possible number gap for rk3066a between gpio4 and gpio6.
> (id counter gives not consistent result)

Again, how compatible is related to this?

> 
> ===
> U-boot rk_gpio.c current behavior:
> 
> 	end = strrchr(dev->name, '@');
> 	priv->bank = trailing_strtoln(dev->name, end);
> 	priv->name[0] = 'A' + priv->bank;
> 	uc_priv->bank_name = priv->name;
> 
> Problems:
> Crash when node name has no "gpio-1" format
> 
> U-boot rk-gpio proposed: 
> 
> 	priv->name[0] = 'A' + dev_seq(dev);
> 	uc_priv->bank_name = priv->name;
> 
> Problems:
> Reduced FDT's and rk3066a gives number gaps.
> 
> ===
> 
> My proposal:
> 
> struct lookup_table rk_gpio_rk3188_data[] = {
> 	{0x2000a000, "A"},
> 	{0x2003c000, "B"},
> 	{0x2003e000, "C"},
> 	{0x20080000, "D"},
> };
> 
> 	{ .compatible = "rockchip,rk3188-gpio-bank", .data = &rk_gpio_rk3188_data },

Which you did not do... Your patch is doing something entirely else.

Best regards,
Krzysztof