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
