[PATCH v1] arm: dts: rockchip: sync rk3066/rk3188 DT files from linux-next v6.2-rc4
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 --- arch/arm/dts/rk3066a-mk808.dts | 27 ++++++++++++++++++++++++++- arch/arm/dts/rk3066a.dtsi | 3 ++- arch/arm/dts/rk3188-radxarock.dts | 24 +++++++++++++----------- arch/arm/dts/rk3188.dtsi | 27 ++++++++++++++++----------- arch/arm/dts/rk3xxx-u-boot.dtsi | 4 ++++ arch/arm/dts/rk3xxx.dtsi | 9 +++++++-- 6 files changed, 68 insertions(+), 26 deletions(-)
diff --git a/arch/arm/dts/rk3066a-mk808.dts b/arch/arm/dts/rk3066a-mk808.dts index 667d57a4..06790f05 100644 --- a/arch/arm/dts/rk3066a-mk808.dts +++ b/arch/arm/dts/rk3066a-mk808.dts @@ -32,7 +32,7 @@ keyup-threshold-microvolt = <2500000>; poll-interval = <100>;
- recovery { + button-recovery { label = "recovery"; linux,code = <KEY_VENDOR>; press-threshold-microvolt = <0>; @@ -157,7 +157,32 @@ pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_bus4>; pinctrl-names = "default"; vmmc-supply = <&vcc_wifi>; + #address-cells = <1>; + #size-cells = <0>; status = "okay"; + + brcmf: wifi@1 { + compatible = "brcm,bcm4329-fmac"; + reg = <1>; + }; +}; + +&nfc { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + nand@0 { + reg = <0>; + label = "rk-nand"; + nand-bus-width = <8>; + nand-ecc-mode = "hw"; + nand-ecc-step-size = <1024>; + nand-ecc-strength = <40>; + nand-is-boot-medium; + rockchip,boot-blks = <8>; + rockchip,boot-ecc-strength = <24>; + }; };
&pinctrl { diff --git a/arch/arm/dts/rk3066a.dtsi b/arch/arm/dts/rk3066a.dtsi index c25b9695..de9915d9 100644 --- a/arch/arm/dts/rk3066a.dtsi +++ b/arch/arm/dts/rk3066a.dtsi @@ -202,8 +202,9 @@ cru: clock-controller@20000000 { compatible = "rockchip,rk3066a-cru"; reg = <0x20000000 0x1000>; + clocks = <&xin24m>; + clock-names = "xin24m"; rockchip,grf = <&grf>; - #clock-cells = <1>; #reset-cells = <1>; assigned-clocks = <&cru PLL_CPLL>, <&cru PLL_GPLL>, diff --git a/arch/arm/dts/rk3188-radxarock.dts b/arch/arm/dts/rk3188-radxarock.dts index e7138a4a..118deacd 100644 --- a/arch/arm/dts/rk3188-radxarock.dts +++ b/arch/arm/dts/rk3188-radxarock.dts @@ -6,7 +6,6 @@ /dts-v1/; #include <dt-bindings/input/input.h> #include "rk3188.dtsi" -#include "rk3188-radxarock-u-boot.dtsi"
/ { model = "Radxa Rock"; @@ -25,7 +24,7 @@ compatible = "gpio-keys"; autorepeat;
- power { + key-power { gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>; linux,code = <KEY_POWER>; label = "GPIO Key Power"; @@ -72,7 +71,7 @@ #sound-dai-cells = <0>; };
- ir_recv: gpio-ir-receiver { + ir_recv: ir-receiver { compatible = "gpio-ir-receiver"; gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>; pinctrl-names = "default"; @@ -127,18 +126,21 @@ };
&emac { - status = "okay"; - + phy = <&phy0>; + phy-supply = <&vcc_rmii>; pinctrl-names = "default"; pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&phy_int>; + status = "okay";
- phy = <&phy0>; - phy-supply = <&vcc_rmii>; + mdio { + #address-cells = <1>; + #size-cells = <0>;
- phy0: ethernet-phy@0 { - reg = <0>; - interrupt-parent = <&gpio3>; - interrupts = <RK_PD2 IRQ_TYPE_LEVEL_LOW>; + phy0: ethernet-phy@0 { + reg = <0>; + interrupt-parent = <&gpio3>; + interrupts = <RK_PD2 IRQ_TYPE_LEVEL_LOW>; + }; }; };
diff --git a/arch/arm/dts/rk3188.dtsi b/arch/arm/dts/rk3188.dtsi index 9a80f83a..44b54af0 100644 --- a/arch/arm/dts/rk3188.dtsi +++ b/arch/arm/dts/rk3188.dtsi @@ -54,7 +54,7 @@ }; };
- cpu0_opp_table: opp_table0 { + cpu0_opp_table: opp-table-0 { compatible = "operating-points-v2"; opp-shared;
@@ -195,8 +195,9 @@ cru: clock-controller@20000000 { compatible = "rockchip,rk3188-cru"; reg = <0x20000000 0x1000>; + clocks = <&xin24m>; + clock-names = "xin24m"; rockchip,grf = <&grf>; - #clock-cells = <1>; #reset-cells = <1>; }; @@ -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 { compatible = "rockchip,gpio-bank"; reg = <0x2003c000 0x100>; interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>; @@ -249,7 +250,7 @@ #interrupt-cells = <2>; };
- gpio2: gpio2@2003e000 { + gpio2: gpio@2003e000 { compatible = "rockchip,gpio-bank"; reg = <0x2003e000 0x100>; interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; @@ -262,7 +263,7 @@ #interrupt-cells = <2>; };
- gpio3: gpio3@20080000 { + gpio3: gpio@20080000 { compatible = "rockchip,gpio-bank"; reg = <0x20080000 0x100>; interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>; @@ -275,15 +276,15 @@ #interrupt-cells = <2>; };
- pcfg_pull_up: pcfg_pull_up { + pcfg_pull_up: pcfg-pull-up { bias-pull-up; };
- pcfg_pull_down: pcfg_pull_down { + pcfg_pull_down: pcfg-pull-down { bias-pull-down; };
- pcfg_pull_none: pcfg_pull_none { + pcfg_pull_none: pcfg-pull-none { bias-disable; };
@@ -378,7 +379,7 @@ rockchip,pins = <2 RK_PD3 1 &pcfg_pull_none>; };
- lcdc1_rgb24: ldcd1-rgb24 { + lcdc1_rgb24: lcdc1-rgb24 { rockchip,pins = <2 RK_PA0 1 &pcfg_pull_none>, <2 RK_PA1 1 &pcfg_pull_none>, <2 RK_PA2 1 &pcfg_pull_none>, @@ -606,7 +607,6 @@
&global_timer { interrupts = <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_EDGE_RISING)>; - status = "disabled"; };
&local_timer { @@ -641,6 +641,11 @@ &grf { compatible = "rockchip,rk3188-grf", "syscon", "simple-mfd";
+ io_domains: io-domains { + compatible = "rockchip,rk3188-io-voltage-domain"; + status = "disabled"; + }; + usbphy: usbphy { compatible = "rockchip,rk3188-usb-phy"; #address-cells = <1>; diff --git a/arch/arm/dts/rk3xxx-u-boot.dtsi b/arch/arm/dts/rk3xxx-u-boot.dtsi index e67432fb..c77d1fae 100644 --- a/arch/arm/dts/rk3xxx-u-boot.dtsi +++ b/arch/arm/dts/rk3xxx-u-boot.dtsi @@ -33,3 +33,7 @@ &uart2 { clock-frequency = <24000000>; }; + +&xin24m { + u-boot,dm-pre-reloc; +}; diff --git a/arch/arm/dts/rk3xxx.dtsi b/arch/arm/dts/rk3xxx.dtsi index 616a828e..cb4e42ed 100644 --- a/arch/arm/dts/rk3xxx.dtsi +++ b/arch/arm/dts/rk3xxx.dtsi @@ -76,6 +76,13 @@ reg = <0x1013c200 0x20>; interrupts = <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>; clocks = <&cru CORE_PERI>; + status = "disabled"; + /* The clock source and the sched_clock provided by the arm_global_timer + * on Rockchip rk3066a/rk3188 are quite unstable because their rates + * depend on the CPU frequency. + * Keep the arm_global_timer disabled in order to have the + * DW_APB_TIMER (rk3066a) or ROCKCHIP_TIMER (rk3188) selected by default. + */ };
local_timer: local-timer@1013c600 { @@ -186,8 +193,6 @@ compatible = "snps,arc-emac"; reg = <0x10204000 0x3c>; interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>; - #address-cells = <1>; - #size-cells = <0>;
rockchip,grf = <&grf>;
-- 2.20.1
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
arch/arm/dts/rk3066a-mk808.dts | 27 ++++++++++++++++++++++++++- arch/arm/dts/rk3066a.dtsi | 3 ++- arch/arm/dts/rk3188-radxarock.dts | 24 +++++++++++++----------- arch/arm/dts/rk3188.dtsi | 27 ++++++++++++++++----------- arch/arm/dts/rk3xxx-u-boot.dtsi | 4 ++++ arch/arm/dts/rk3xxx.dtsi | 9 +++++++-- 6 files changed, 68 insertions(+), 26 deletions(-)
diff --git a/arch/arm/dts/rk3066a-mk808.dts b/arch/arm/dts/rk3066a-mk808.dts index 667d57a4..06790f05 100644 --- a/arch/arm/dts/rk3066a-mk808.dts +++ b/arch/arm/dts/rk3066a-mk808.dts @@ -32,7 +32,7 @@ keyup-threshold-microvolt = <2500000>; poll-interval = <100>;
recovery {
button-recovery { label = "recovery"; linux,code = <KEY_VENDOR>; press-threshold-microvolt = <0>;@@ -157,7 +157,32 @@ pinctrl-0 = <&sd1_clk &sd1_cmd &sd1_bus4>; pinctrl-names = "default"; vmmc-supply = <&vcc_wifi>;
- #address-cells = <1>;
- #size-cells = <0>; status = "okay";
- brcmf: wifi@1 {
compatible = "brcm,bcm4329-fmac";reg = <1>;- };
+};
+&nfc {
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
nand@0 {
reg = <0>;label = "rk-nand";nand-bus-width = <8>;nand-ecc-mode = "hw";nand-ecc-step-size = <1024>;nand-ecc-strength = <40>;nand-is-boot-medium;rockchip,boot-blks = <8>;rockchip,boot-ecc-strength = <24>;}; };
&pinctrl {
diff --git a/arch/arm/dts/rk3066a.dtsi b/arch/arm/dts/rk3066a.dtsi index c25b9695..de9915d9 100644 --- a/arch/arm/dts/rk3066a.dtsi +++ b/arch/arm/dts/rk3066a.dtsi @@ -202,8 +202,9 @@ cru: clock-controller@20000000 { compatible = "rockchip,rk3066a-cru"; reg = <0x20000000 0x1000>;
clocks = <&xin24m>;clock-names = "xin24m";
- #clock-cells = <1>; #reset-cells = <1>; assigned-clocks = <&cru PLL_CPLL>, <&cru PLL_GPLL>,
diff --git a/arch/arm/dts/rk3188-radxarock.dts b/arch/arm/dts/rk3188-radxarock.dts index e7138a4a..118deacd 100644 --- a/arch/arm/dts/rk3188-radxarock.dts +++ b/arch/arm/dts/rk3188-radxarock.dts @@ -6,7 +6,6 @@ /dts-v1/; #include <dt-bindings/input/input.h> #include "rk3188.dtsi" -#include "rk3188-radxarock-u-boot.dtsi"
/ { model = "Radxa Rock"; @@ -25,7 +24,7 @@ compatible = "gpio-keys"; autorepeat;
power {
key-power { gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>; linux,code = <KEY_POWER>; label = "GPIO Key Power";@@ -72,7 +71,7 @@ #sound-dai-cells = <0>; };
- ir_recv: gpio-ir-receiver {
- ir_recv: ir-receiver { compatible = "gpio-ir-receiver"; gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>; pinctrl-names = "default";
@@ -127,18 +126,21 @@ };
&emac {
- status = "okay";
- phy = <&phy0>;
- phy-supply = <&vcc_rmii>; pinctrl-names = "default"; pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&phy_int>;
- status = "okay";
- phy = <&phy0>;
- phy-supply = <&vcc_rmii>;
- mdio {
#address-cells = <1>;#size-cells = <0>;
- phy0: ethernet-phy@0 {
reg = <0>;interrupt-parent = <&gpio3>;interrupts = <RK_PD2 IRQ_TYPE_LEVEL_LOW>;
phy0: ethernet-phy@0 {reg = <0>;interrupt-parent = <&gpio3>;interrupts = <RK_PD2 IRQ_TYPE_LEVEL_LOW>;};diff --git a/arch/arm/dts/rk3188.dtsi b/arch/arm/dts/rk3188.dtsi index 9a80f83a..44b54af0 100644 --- a/arch/arm/dts/rk3188.dtsi +++ b/arch/arm/dts/rk3188.dtsi @@ -54,7 +54,7 @@ }; };
- cpu0_opp_table: opp_table0 {
- cpu0_opp_table: opp-table-0 { compatible = "operating-points-v2"; opp-shared;
@@ -195,8 +195,9 @@ cru: clock-controller@20000000 { compatible = "rockchip,rk3188-cru"; reg = <0x20000000 0x1000>;
clocks = <&xin24m>;clock-names = "xin24m";
- #clock-cells = <1>; #reset-cells = <1>; };
@@ -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 {
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@theobrom... and its answers.
Cheers, Quentin
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-...
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@theobrom... and its answers.
Cheers, Quentin
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
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.13...
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
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.13...
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
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.13...
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
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_JsqJG...
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
participants (3)
-
Johan Jonker -
John Keeping -
Quentin Schulz