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

[Krzysztof Kozlowski]

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