linux-rockchip.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4
       [not found] ` <31883268-dedf-6e3c-8647-0b7955c1ff16@theobroma-systems.com>
@ 2023-01-17 14:44   ` Johan Jonker
  2023-01-17 15:20     ` John Keeping
  2023-01-17 16:18     ` Quentin Schulz
  0 siblings, 2 replies; 6+ messages in thread
From: Johan Jonker @ 2023-01-17 14:44 UTC (permalink / raw)
  To: Quentin Schulz, kever.yang
  Cc: sjg, philipp.tomsich, u-boot, Heiko Stuebner,
	open list:ARM/Rockchip SoC support, Linus Walleij,
	Bartosz Golaszewski



On 1/17/23 10:46, Quentin Schulz wrote:
> Hi Johan,
> 
> On 1/16/23 20:45, Johan Jonker wrote:
>> Sync rk3066/rk3188 DT files from Linux.
>> This is the state as of linux-next v6.2-rc4.
>> New nfc node for MK808 rk3066a.
>> CRU nodes now have a clock property.
>> To prefend dtoc errors a fixed clock must also be
>> included for tpl/spl in the rk3xxx-u-boot.dtsi file.
>>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>> ---

[..]

>> @@ -223,7 +224,7 @@
>>           #size-cells = <1>;
>>           ranges;
>>
>> -        gpio0: gpio0@2000a000 {
>> +        gpio0: gpio@2000a000 {
>>               compatible = "rockchip,rk3188-gpio-bank0";
>>               reg = <0x2000a000 0x100>;
>>               interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
>> @@ -236,7 +237,7 @@
>>               #interrupt-cells = <2>;
>>           };
>>
>> -        gpio1: gpio1@2003c000 {

>> +        gpio1: gpio@2003c000 {

Hi,

LOL: I made that binding change on request from Linux DT maintainers.
Node names should generic.

===

My full u-boot is able to boot a Linux kernel for rk3066a.
Only when I give the command below it crashes:

gpio status -a

Could you confirm what other parts are effected?

If it's boots then it's good enough for me and move forward, so please merge.(Kever)

Driver fixes for u-boot depending on Linux DT changes is already very time consuming enough!

===

Using DT path or node name is wrong.

Comment by robh+dt:
/sys/bus/platform/devices/ paths are not an ABI. I'll consider
nodenames an ABI if a change is noticed, but not for sysfs path.

https://lore.kernel.org/linux-rockchip/CAL_JsqJGx_MwX9ynMiCUm_xP3t6xWbrtiUi-F+2ceXqZ2Su5tA@mail.gmail.com/

Make use of the properties and a compatible in a node instead.

===
A little test.
From: rockchip_gpio_probe():

int main() {
	//char dev_name[] = "gpio1@2003c000";
	char dev_name[] = "gpio@2003c000";
	char priv_name[2];
	int priv_bank;
	char *end;

	end = strrchr(dev_name, '@');
	priv_bank = trailing_strtoln(dev_name, end);

	priv_name[0] = 'A' + priv_bank;
	priv_name[1] = 0;

	printf("priv_bank: %d\n", priv_bank);
	printf("priv_name: %s\n", priv_name);
	return 0;
}

// priv_bank: 1
// priv_name: B

A change of node name gives:

// priv_bank: -1
// priv_name: @

===

Linux driver gpio-rockchip.c has an alias or else an increment id.

id = of_alias_get_id(np, "gpio");
	if (id < 0)
		id = gpio++;

Problem:
Probe order is not guarantied.
Aliases should go to board files and not in dtsi files anymore. 

===

The current compatible string is generic, but these strings should normally be SoC orientated.

Proposal:

replace:
compatible = "rockchip,gpio-bank";

by:
compatible = "rockchip,rk3188-gpio-bank";

With that known we can lookup the reg address in a lookup table and it's name for u-boot.

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 },

===

If needed I can help with changes to rockchip,gpio-bank.yaml

Let me know if it's useful or that you have an other solution as long as we get forward here.

Johan

> 

> I believe this is the same issue as on RK3399 and PX30: this breaks our GPIO controller driver.

> 
> c.f. https://lore.kernel.org/u-boot/0ab9a600-b517-0ce5-d189-99fc8eddfa60@theobroma-systems.com/ and its answers.
> 
> Cheers,
> Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4
  2023-01-17 14:44   ` [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4 Johan Jonker
@ 2023-01-17 15:20     ` John Keeping
  2023-01-17 15:58       ` Quentin Schulz
  2023-01-17 16:18     ` Quentin Schulz
  1 sibling, 1 reply; 6+ messages in thread
From: John Keeping @ 2023-01-17 15:20 UTC (permalink / raw)
  To: Johan Jonker
  Cc: Quentin Schulz, kever.yang, sjg, philipp.tomsich, u-boot,
	Heiko Stuebner, open list:ARM/Rockchip SoC support,
	Linus Walleij, Bartosz Golaszewski

Hi Johan,

On Tue, Jan 17, 2023 at 03:44:07PM +0100, Johan Jonker wrote:
> On 1/17/23 10:46, Quentin Schulz wrote:
> > On 1/16/23 20:45, Johan Jonker wrote:
> >> Sync rk3066/rk3188 DT files from Linux.
> >> This is the state as of linux-next v6.2-rc4.
> >> New nfc node for MK808 rk3066a.
> >> CRU nodes now have a clock property.
> >> To prefend dtoc errors a fixed clock must also be
> >> included for tpl/spl in the rk3xxx-u-boot.dtsi file.
> >>
> >> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> >> ---
> 
> [..]
> 
> >> @@ -223,7 +224,7 @@
> >>           #size-cells = <1>;
> >>           ranges;
> >>
> >> -        gpio0: gpio0@2000a000 {
> >> +        gpio0: gpio@2000a000 {
> >>               compatible = "rockchip,rk3188-gpio-bank0";
> >>               reg = <0x2000a000 0x100>;
> >>               interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
> >> @@ -236,7 +237,7 @@
> >>               #interrupt-cells = <2>;
> >>           };
> >>
> >> -        gpio1: gpio1@2003c000 {
> 
> >> +        gpio1: gpio@2003c000 {
> 
> Hi,
> 
> LOL: I made that binding change on request from Linux DT maintainers.
> Node names should generic.
> 
> ===
> 
> My full u-boot is able to boot a Linux kernel for rk3066a.
> Only when I give the command below it crashes:
> 
> gpio status -a
> 
> Could you confirm what other parts are effected?
> 
> If it's boots then it's good enough for me and move forward, so please merge.(Kever)
> 
> Driver fixes for u-boot depending on Linux DT changes is already very time consuming enough!

This reminded me of a patch that I never followed up with:

	https://lore.kernel.org/u-boot/20220726162509.1304234-1-john@metanate.com/

Can you test if that fixes `gpio status -a` for you?


John

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4
  2023-01-17 15:20     ` John Keeping
@ 2023-01-17 15:58       ` Quentin Schulz
  2023-01-17 17:27         ` John Keeping
  0 siblings, 1 reply; 6+ messages in thread
From: Quentin Schulz @ 2023-01-17 15:58 UTC (permalink / raw)
  To: John Keeping, Johan Jonker
  Cc: kever.yang, sjg, philipp.tomsich, u-boot, Heiko Stuebner,
	open list:ARM/Rockchip SoC support, Linus Walleij,
	Bartosz Golaszewski

Hi John,

On 1/17/23 16:20, John Keeping wrote:
> Hi Johan,
> 
> On Tue, Jan 17, 2023 at 03:44:07PM +0100, Johan Jonker wrote:
>> On 1/17/23 10:46, Quentin Schulz wrote:
>>> On 1/16/23 20:45, Johan Jonker wrote:
>>>> Sync rk3066/rk3188 DT files from Linux.
>>>> This is the state as of linux-next v6.2-rc4.
>>>> New nfc node for MK808 rk3066a.
>>>> CRU nodes now have a clock property.
>>>> To prefend dtoc errors a fixed clock must also be
>>>> included for tpl/spl in the rk3xxx-u-boot.dtsi file.
>>>>
>>>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>>> ---
>>
>> [..]
>>
>>>> @@ -223,7 +224,7 @@
>>>>            #size-cells = <1>;
>>>>            ranges;
>>>>
>>>> -        gpio0: gpio0@2000a000 {
>>>> +        gpio0: gpio@2000a000 {
>>>>                compatible = "rockchip,rk3188-gpio-bank0";
>>>>                reg = <0x2000a000 0x100>;
>>>>                interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
>>>> @@ -236,7 +237,7 @@
>>>>                #interrupt-cells = <2>;
>>>>            };
>>>>
>>>> -        gpio1: gpio1@2003c000 {
>>
>>>> +        gpio1: gpio@2003c000 {
>>
>> Hi,
>>
>> LOL: I made that binding change on request from Linux DT maintainers.
>> Node names should generic.
>>
>> ===
>>
>> My full u-boot is able to boot a Linux kernel for rk3066a.
>> Only when I give the command below it crashes:
>>
>> gpio status -a
>>
>> Could you confirm what other parts are effected?
>>
>> If it's boots then it's good enough for me and move forward, so please merge.(Kever)
>>
>> Driver fixes for u-boot depending on Linux DT changes is already very time consuming enough!
> 
> This reminded me of a patch that I never followed up with:
> 
> 	https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20220726162509.1304234-1-john@metanate.com/__;!!OOPJP91ZZw!lmqsst1SAzm_NO-LfRooukeMJOc5jP-Tqsm_DdWmyjnQc2VsP0D4TWJakw0qh5K-ENTe96HCtGp-MjKZtVrDM4Y8Tlg$
> 
> Can you test if that fixes `gpio status -a` for you?
> 

That's not enough I believe. You need CONFIG_DM_SEQ_ALIAS enabled, which 
isn't a given and we shouldn't rely on it. Also, there are currently no 
aliases for gpio controllers. And we shouldn't rely on u-boot.dtsi for 
those, because upstream might decide to have an alias gpio2 for gpio0 
for a board.

Cheers,
Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4
  2023-01-17 14:44   ` [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4 Johan Jonker
  2023-01-17 15:20     ` John Keeping
@ 2023-01-17 16:18     ` Quentin Schulz
  1 sibling, 0 replies; 6+ messages in thread
From: Quentin Schulz @ 2023-01-17 16:18 UTC (permalink / raw)
  To: Johan Jonker, kever.yang
  Cc: sjg, philipp.tomsich, u-boot, Heiko Stuebner,
	open list:ARM/Rockchip SoC support, Linus Walleij,
	Bartosz Golaszewski

Hi Johan,

On 1/17/23 15:44, Johan Jonker wrote:
> 
> 
> On 1/17/23 10:46, Quentin Schulz wrote:
>> Hi Johan,
>>
>> On 1/16/23 20:45, Johan Jonker wrote:
>>> Sync rk3066/rk3188 DT files from Linux.
>>> This is the state as of linux-next v6.2-rc4.
>>> New nfc node for MK808 rk3066a.
>>> CRU nodes now have a clock property.
>>> To prefend dtoc errors a fixed clock must also be
>>> included for tpl/spl in the rk3xxx-u-boot.dtsi file.
>>>
>>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>> ---
> 
> [..]
> 
>>> @@ -223,7 +224,7 @@
>>>            #size-cells = <1>;
>>>            ranges;
>>>
>>> -        gpio0: gpio0@2000a000 {
>>> +        gpio0: gpio@2000a000 {
>>>                compatible = "rockchip,rk3188-gpio-bank0";
>>>                reg = <0x2000a000 0x100>;
>>>                interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
>>> @@ -236,7 +237,7 @@
>>>                #interrupt-cells = <2>;
>>>            };
>>>
>>> -        gpio1: gpio1@2003c000 {
> 
>>> +        gpio1: gpio@2003c000 {
> 
> Hi,
> 
> LOL: I made that binding change on request from Linux DT maintainers.
> Node names should generic.
> 

Yup I know. I tried to use a 5.19 DTS on a 5.10 kernel while debugging 
some stuff and was quite surprised to not even be able to reach userspace.

> ===
> 
> My full u-boot is able to boot a Linux kernel for rk3066a.
> Only when I give the command below it crashes:
> 
> gpio status -a
> 
> Could you confirm what other parts are effected?
> 

Well... Anything using GPIOs I believe? So Devices with a phandle to a 
gpio from an SoC GPIO controller, or some platform code getting a GPIO 
udevice to interact with some GPIO. Pinctrl shouldn't be affected 
AFAICT, so that might be the reason why most things seems to have worked 
fine for you?

> If it's boots then it's good enough for me and move forward, so please merge.(Kever)
> 

Since there's only one board using it, I guess that's your/Kever';s 
call, but that's not the case for PX30/RK3399 for example... where we'll 
need a similar change (though yes, my PX30 board also boots fine without 
and currently has this new DTSI, so broken GPIO driver).

> Driver fixes for u-boot depending on Linux DT changes is already very time consuming enough!
> 
> ===
> 
> Using DT path or node name is wrong.
> 
> Comment by robh+dt:
> /sys/bus/platform/devices/ paths are not an ABI. I'll consider
> nodenames an ABI if a change is noticed, but not for sysfs path.
> 
> https://urldefense.com/v3/__https://lore.kernel.org/linux-rockchip/CAL_JsqJGx_MwX9ynMiCUm_xP3t6xWbrtiUi-F*2ceXqZ2Su5tA@mail.gmail.com/__;Kw!!OOPJP91ZZw!iA_hrQyQy4KmdSwe46spmhBqsGQRRG4X_39kp9-JtCChxwMUdG22ICVmKLZHaIogqzTRzCZuHJ_rZ1hw5urOjubS9xJrRg$
> 

Yeah well... We weren't using the sysfs path but still have an issue :)

> Make use of the properties and a compatible in a node instead.
> 

That we can agree on.

> ===
> A little test.
> From: rockchip_gpio_probe():
> 
> int main() {
> 	//char dev_name[] = "gpio1@2003c000";
> 	char dev_name[] = "gpio@2003c000";
> 	char priv_name[2];
> 	int priv_bank;
> 	char *end;
> 
> 	end = strrchr(dev_name, '@');
> 	priv_bank = trailing_strtoln(dev_name, end);
> 
> 	priv_name[0] = 'A' + priv_bank;
> 	priv_name[1] = 0;
> 
> 	printf("priv_bank: %d\n", priv_bank);
> 	printf("priv_name: %s\n", priv_name);
> 	return 0;
> }
> 
> // priv_bank: 1
> // priv_name: B
> 
> A change of node name gives:
> 
> // priv_bank: -1
> // priv_name: @
> 
> ===
> 
> Linux driver gpio-rockchip.c has an alias or else an increment id.
> 
> id = of_alias_get_id(np, "gpio");
> 	if (id < 0)
> 		id = gpio++;
> 
> Problem:
> Probe order is not guarantied.
> Aliases should go to board files and not in dtsi files anymore.
> 

And also, nothing forbids any board user to have gpio2= <&gpio0> in 
their aliases. We shouldn't rely on aliases for anything else than how 
it;s exposed to userspace I believe. Here it's a mistake anyways.

> ===
> 
> The current compatible string is generic, but these strings should normally be SoC orientated.
> 
> Proposal:
> 
> replace:
> compatible = "rockchip,gpio-bank";
> 
> by:
> compatible = "rockchip,rk3188-gpio-bank";
> 
> With that known we can lookup the reg address in a lookup table and it's name for u-boot.
> 
> 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 },
> 

Yes, that's an option too.

> ===
> 
> If needed I can help with changes to rockchip,gpio-bank.yaml
> 
> Let me know if it's useful or that you have an other solution as long as we get forward here.
> 

I believe Kever will want this to be upstreamed first in the kernel 
(though I think he doesn't mind if we sync stuff with linux-next, so 
that should make it faster to be merged in U-Boot).

I suggested:

rockchip,gpio-controller = <0>;
in gpio0,
rockchip,gpio-controller = <1>;
in gpio1, etc....

in the DT and reading this information from the driver directly. In 
which case, there's very little to do in the driver except getting this 
number and adding it to 'A'. We could also have a char there and not do 
this gymnastic with adding a number to the 'A' character.

Cheers,
Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4
  2023-01-17 15:58       ` Quentin Schulz
@ 2023-01-17 17:27         ` John Keeping
  2023-01-17 20:08           ` Johan Jonker
  0 siblings, 1 reply; 6+ messages in thread
From: John Keeping @ 2023-01-17 17:27 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Johan Jonker, kever.yang, sjg, philipp.tomsich, u-boot,
	Heiko Stuebner, open list:ARM/Rockchip SoC support,
	Linus Walleij, Bartosz Golaszewski

Hi Quentin,

On Tue, Jan 17, 2023 at 04:58:54PM +0100, Quentin Schulz wrote:
> On 1/17/23 16:20, John Keeping wrote:
> > Hi Johan,
> > 
> > On Tue, Jan 17, 2023 at 03:44:07PM +0100, Johan Jonker wrote:
> > > On 1/17/23 10:46, Quentin Schulz wrote:
> > > > On 1/16/23 20:45, Johan Jonker wrote:
> > > > > Sync rk3066/rk3188 DT files from Linux.
> > > > > This is the state as of linux-next v6.2-rc4.
> > > > > New nfc node for MK808 rk3066a.
> > > > > CRU nodes now have a clock property.
> > > > > To prefend dtoc errors a fixed clock must also be
> > > > > included for tpl/spl in the rk3xxx-u-boot.dtsi file.
> > > > > 
> > > > > Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> > > > > ---
> > > 
> > > [..]
> > > 
> > > > > @@ -223,7 +224,7 @@
> > > > >            #size-cells = <1>;
> > > > >            ranges;
> > > > > 
> > > > > -        gpio0: gpio0@2000a000 {
> > > > > +        gpio0: gpio@2000a000 {
> > > > >                compatible = "rockchip,rk3188-gpio-bank0";
> > > > >                reg = <0x2000a000 0x100>;
> > > > >                interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
> > > > > @@ -236,7 +237,7 @@
> > > > >                #interrupt-cells = <2>;
> > > > >            };
> > > > > 
> > > > > -        gpio1: gpio1@2003c000 {
> > > 
> > > > > +        gpio1: gpio@2003c000 {
> > > 
> > > Hi,
> > > 
> > > LOL: I made that binding change on request from Linux DT maintainers.
> > > Node names should generic.
> > > 
> > > ===
> > > 
> > > My full u-boot is able to boot a Linux kernel for rk3066a.
> > > Only when I give the command below it crashes:
> > > 
> > > gpio status -a
> > > 
> > > Could you confirm what other parts are effected?
> > > 
> > > If it's boots then it's good enough for me and move forward, so please merge.(Kever)
> > > 
> > > Driver fixes for u-boot depending on Linux DT changes is already very time consuming enough!
> > 
> > This reminded me of a patch that I never followed up with:
> > 
> > 	https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20220726162509.1304234-1-john@metanate.com/__;!!OOPJP91ZZw!lmqsst1SAzm_NO-LfRooukeMJOc5jP-Tqsm_DdWmyjnQc2VsP0D4TWJakw0qh5K-ENTe96HCtGp-MjKZtVrDM4Y8Tlg$
> > 
> > Can you test if that fixes `gpio status -a` for you?
> > 
> 
> That's not enough I believe. You need CONFIG_DM_SEQ_ALIAS enabled, which
> isn't a given and we shouldn't rely on it. Also, there are currently no
> aliases for gpio controllers. And we shouldn't rely on u-boot.dtsi for
> those, because upstream might decide to have an alias gpio2 for gpio0 for a
> board.

If the aliases are set up in a non-standard way, then I think changing
the name here may be the right thing to do.

Any GPIOs referenced in the device tree will be by handle so the name is
irrelevant.  GPIO names are only used by board code or by the `gpio`
command, both of which are tightly tied to the device tree anyway.

If a stable name for a GPIO is needed, then the gpio-line-names DT
property exists and is supported by Linux.  The names here are purely a
U-Boot construction and effectively part of a "userspace" interface.

Just using dev_seq() unconditionally as Simon suggested in response to
my patch seems like the right answer (and then there is no dependency on
CONFIG_DM_SEQ_ALIAS although boards can require it and use aliases if
necessary).


John

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4
  2023-01-17 17:27         ` John Keeping
@ 2023-01-17 20:08           ` Johan Jonker
  0 siblings, 0 replies; 6+ messages in thread
From: Johan Jonker @ 2023-01-17 20:08 UTC (permalink / raw)
  To: John Keeping, Quentin Schulz
  Cc: kever.yang, sjg, philipp.tomsich, u-boot, Heiko Stuebner,
	open list:ARM/Rockchip SoC support, Linus Walleij,
	Bartosz Golaszewski



On 1/17/23 18:27, John Keeping wrote:
> Hi Quentin,
> 
> On Tue, Jan 17, 2023 at 04:58:54PM +0100, Quentin Schulz wrote:
>> On 1/17/23 16:20, John Keeping wrote:
>>> Hi Johan,
>>>
>>> On Tue, Jan 17, 2023 at 03:44:07PM +0100, Johan Jonker wrote:
>>>> On 1/17/23 10:46, Quentin Schulz wrote:
>>>>> On 1/16/23 20:45, Johan Jonker wrote:
>>>>>> Sync rk3066/rk3188 DT files from Linux.
>>>>>> This is the state as of linux-next v6.2-rc4.
>>>>>> New nfc node for MK808 rk3066a.
>>>>>> CRU nodes now have a clock property.
>>>>>> To prefend dtoc errors a fixed clock must also be
>>>>>> included for tpl/spl in the rk3xxx-u-boot.dtsi file.
>>>>>>
>>>>>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>>>>> ---
>>>>
>>>> [..]
>>>>
>>>>>> @@ -223,7 +224,7 @@
>>>>>>            #size-cells = <1>;
>>>>>>            ranges;
>>>>>>
>>>>>> -        gpio0: gpio0@2000a000 {
>>>>>> +        gpio0: gpio@2000a000 {
>>>>>>                compatible = "rockchip,rk3188-gpio-bank0";
>>>>>>                reg = <0x2000a000 0x100>;
>>>>>>                interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
>>>>>> @@ -236,7 +237,7 @@
>>>>>>                #interrupt-cells = <2>;
>>>>>>            };
>>>>>>
>>>>>> -        gpio1: gpio1@2003c000 {
>>>>
>>>>>> +        gpio1: gpio@2003c000 {
>>>>
>>>> Hi,
>>>>
>>>> LOL: I made that binding change on request from Linux DT maintainers.
>>>> Node names should generic.
>>>>
>>>> ===
>>>>
>>>> My full u-boot is able to boot a Linux kernel for rk3066a.
>>>> Only when I give the command below it crashes:
>>>>
>>>> gpio status -a
>>>>
>>>> Could you confirm what other parts are effected?
>>>>
>>>> If it's boots then it's good enough for me and move forward, so please merge.(Kever)
>>>>
>>>> Driver fixes for u-boot depending on Linux DT changes is already very time consuming enough!
>>>
>>> This reminded me of a patch that I never followed up with:
>>>
>>> 	https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20220726162509.1304234-1-john@metanate.com/__;!!OOPJP91ZZw!lmqsst1SAzm_NO-LfRooukeMJOc5jP-Tqsm_DdWmyjnQc2VsP0D4TWJakw0qh5K-ENTe96HCtGp-MjKZtVrDM4Y8Tlg$
>>>
>>> Can you test if that fixes `gpio status -a` for you?
>>>
>>
>> That's not enough I believe. You need CONFIG_DM_SEQ_ALIAS enabled, which
>> isn't a given and we shouldn't rely on it. Also, there are currently no
>> aliases for gpio controllers. And we shouldn't rely on u-boot.dtsi for
>> those, because upstream might decide to have an alias gpio2 for gpio0 for a
>> board.
> 
> If the aliases are set up in a non-standard way, then I think changing
> the name here may be the right thing to do.
> 
> Any GPIOs referenced in the device tree will be by handle so the name is
> irrelevant.  GPIO names are only used by board code or by the `gpio`
> command, both of which are tightly tied to the device tree anyway.
> 
> If a stable name for a GPIO is needed, then the gpio-line-names DT
> property exists and is supported by Linux.  The names here are purely a
> U-Boot construction and effectively part of a "userspace" interface.
> 

> Just using dev_seq() unconditionally as Simon suggested in response to
> my patch seems like the right answer (and then there is no dependency on
> CONFIG_DM_SEQ_ALIAS although boards can require it and use aliases if
> necessary).

I tested a bit with rk3066a mk808 with number gap between 4 and 6.
That letter is not consistent F vs G in u-boot with and without aliases.

My idea is that the use of aliases is more of something of an additional thing that we not always can count on that is there.
DTOC processing for dt-plat.c only looks at the nodes it self. Finding your ID then comes from the properties inside that node.
(ie: compatible + reg or separate id property)
Others can have other better ideas of course. Let us know.

Johan

> 
> 
> John

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-17 20:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <48680512-0312-6342-ba0a-f2411463099a@gmail.com>
     [not found] ` <31883268-dedf-6e3c-8647-0b7955c1ff16@theobroma-systems.com>
2023-01-17 14:44   ` [PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4 Johan Jonker
2023-01-17 15:20     ` John Keeping
2023-01-17 15:58       ` Quentin Schulz
2023-01-17 17:27         ` John Keeping
2023-01-17 20:08           ` Johan Jonker
2023-01-17 16:18     ` Quentin Schulz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).