| 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
| ||
|
Date: Fri, 29 Dec 2023 16:47:59 +0100
From: Johan Jonker <jbx6244@...il.com>
To: Jonas Karlman <jonas@...boo.se>, heiko@...ech.de
Cc: robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
conor+dt@...nel.org, kever.yang@...k-chips.com, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] ARM: dts: rockchip: add gpio-ranges property to
gpio nodes
Hi Jonas,
On 12/28/23 17:30, Jonas Karlman wrote:
> Hi Johan,
>
> On 2023-12-27 19:33, Johan Jonker wrote:
>> Add a gpio-ranges property to Rockchip gpio nodes similar to
>> rk356x/rk3588 to be independent from aliases and probe order.
>>
>> Signed-off-by: Johan Jonker <jbx6244@...il.com>
>> Reviewed-by: Kever Yang <kever.yang@...k-chips.com>
>> ---
>>
>> Changed V3:
>> reword
>> rebase to new Rockchip directory
>> remove unknown properties
>> ---
>> arch/arm/boot/dts/rockchip/rk3036.dtsi | 3 +++
>> arch/arm/boot/dts/rockchip/rk3066a.dtsi | 6 ++++++
>> arch/arm/boot/dts/rockchip/rk3128.dtsi | 4 ++++
>> arch/arm/boot/dts/rockchip/rk3188.dtsi | 4 ++++
>> arch/arm/boot/dts/rockchip/rk322x.dtsi | 4 ++++
>> arch/arm/boot/dts/rockchip/rk3288.dtsi | 9 +++++++++
>> arch/arm/boot/dts/rockchip/rv1108.dtsi | 4 ++++
>> arch/arm/boot/dts/rockchip/rv1126.dtsi | 5 +++++
>> 8 files changed, 39 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/rockchip/rk3036.dtsi b/arch/arm/boot/dts/rockchip/rk3036.dtsi
>> index 2b00109bea6a..6e5028b6dbfa 100644
>> --- a/arch/arm/boot/dts/rockchip/rk3036.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rk3036.dtsi
>> @@ -593,6 +593,7 @@ gpio0: gpio@...7c000 {
>> clocks = <&cru PCLK_GPIO0>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -606,6 +607,7 @@ gpio1: gpio@...80000 {
>> clocks = <&cru PCLK_GPIO1>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -619,6 +621,7 @@ gpio2: gpio@...84000 {
>> clocks = <&cru PCLK_GPIO2>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> diff --git a/arch/arm/boot/dts/rockchip/rk3066a.dtsi b/arch/arm/boot/dts/rockchip/rk3066a.dtsi
>> index 30139f21de64..a4962b6b3f4c 100644
>> --- a/arch/arm/boot/dts/rockchip/rk3066a.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rk3066a.dtsi
>> @@ -285,6 +285,7 @@ gpio0: gpio@...34000 {
>> clocks = <&cru PCLK_GPIO0>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -298,6 +299,7 @@ gpio1: gpio@...3c000 {
>> clocks = <&cru PCLK_GPIO1>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -311,6 +313,7 @@ gpio2: gpio@...3e000 {
>> clocks = <&cru PCLK_GPIO2>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -324,6 +327,7 @@ gpio3: gpio@...80000 {
>> clocks = <&cru PCLK_GPIO3>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 96 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -337,6 +341,7 @@ gpio4: gpio@...84000 {
>> clocks = <&cru PCLK_GPIO4>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 128 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -350,6 +355,7 @@ gpio6: gpio@...0a000 {
>> clocks = <&cru PCLK_GPIO6>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 192 32>;
>
> It does not look like this matches what pins the pinctrl driver expose
> for rk3066a. Something like <&pinctrl 0 160 16> would probably be more
> correct.
See comment below.
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> diff --git a/arch/arm/boot/dts/rockchip/rk3128.dtsi b/arch/arm/boot/dts/rockchip/rk3128.dtsi
>> index e2264c40b924..d66fcf12032e 100644
>> --- a/arch/arm/boot/dts/rockchip/rk3128.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rk3128.dtsi
>> @@ -712,6 +712,7 @@ gpio0: gpio@...7c000 {
>> interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO0>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> @@ -723,6 +724,7 @@ gpio1: gpio@...80000 {
>> interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO1>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> @@ -734,6 +736,7 @@ gpio2: gpio@...84000 {
>> interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO2>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> @@ -745,6 +748,7 @@ gpio3: gpio@...88000 {
>> interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO3>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 96 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> diff --git a/arch/arm/boot/dts/rockchip/rk3188.dtsi b/arch/arm/boot/dts/rockchip/rk3188.dtsi
>> index 44b54af0bbf9..6677e4a10e5d 100644
>> --- a/arch/arm/boot/dts/rockchip/rk3188.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rk3188.dtsi
>> @@ -231,6 +231,7 @@ gpio0: gpio@...0a000 {
>> clocks = <&cru PCLK_GPIO0>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -244,6 +245,7 @@ gpio1: gpio@...3c000 {
>> clocks = <&cru PCLK_GPIO1>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -257,6 +259,7 @@ gpio2: gpio@...3e000 {
>> clocks = <&cru PCLK_GPIO2>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -270,6 +273,7 @@ gpio3: gpio@...80000 {
>> clocks = <&cru PCLK_GPIO3>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 96 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> diff --git a/arch/arm/boot/dts/rockchip/rk322x.dtsi b/arch/arm/boot/dts/rockchip/rk322x.dtsi
>> index 831561fc1814..0d4f9957b99b 100644
>> --- a/arch/arm/boot/dts/rockchip/rk322x.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rk322x.dtsi
>> @@ -959,6 +959,7 @@ gpio0: gpio@...10000 {
>> clocks = <&cru PCLK_GPIO0>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -972,6 +973,7 @@ gpio1: gpio@...20000 {
>> clocks = <&cru PCLK_GPIO1>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -985,6 +987,7 @@ gpio2: gpio@...30000 {
>> clocks = <&cru PCLK_GPIO2>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -998,6 +1001,7 @@ gpio3: gpio@...40000 {
>> clocks = <&cru PCLK_GPIO3>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 96 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> diff --git a/arch/arm/boot/dts/rockchip/rk3288.dtsi b/arch/arm/boot/dts/rockchip/rk3288.dtsi
>> index ead343dc3df1..c5550aae4ed8 100644
>> --- a/arch/arm/boot/dts/rockchip/rk3288.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rk3288.dtsi
>> @@ -1461,6 +1461,7 @@ gpio0: gpio@...50000 {
>> clocks = <&cru PCLK_GPIO0>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>
> The pinctrl driver for rk3288 only expose 24 pins for the first bank,
> correct range would probably be <&pinctrl 0 0 24> here,
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1474,6 +1475,7 @@ gpio1: gpio@...80000 {
>> clocks = <&cru PCLK_GPIO1>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>
> and correct range would probably be <&pinctrl 0 24 32> here,
Random gpio offset ranges for a gpio bank of the various Rockchip SoCs makes it impossible to easy link gpio to pinctrl and vica versa in a standard way without additional Rockchip specific node properties.
To keep things simple the gpio-range for a gpio bank must always be inside to same multiple of 32 for all Rockchip SoCs in relation to the pinctrl.
A "rockchip,pins" property always must have the same "gpio-ranges" set.
Johan
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1487,6 +1489,7 @@ gpio2: gpio@...90000 {
>> clocks = <&cru PCLK_GPIO2>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>
> and <&pinctrl 0 56 32> here,
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1500,6 +1503,7 @@ gpio3: gpio@...a0000 {
>> clocks = <&cru PCLK_GPIO3>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 96 32>;
>
> and <&pinctrl 0 88 32> here,
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1513,6 +1517,7 @@ gpio4: gpio@...b0000 {
>> clocks = <&cru PCLK_GPIO4>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 128 32>;
>
> and <&pinctrl 0 120 32> here,
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1526,6 +1531,7 @@ gpio5: gpio@...c0000 {
>> clocks = <&cru PCLK_GPIO5>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 160 32>;
>
> and <&pinctrl 0 152 32> here,
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1539,6 +1545,7 @@ gpio6: gpio@...d0000 {
>> clocks = <&cru PCLK_GPIO6>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 192 32>;
>
> and <&pinctrl 0 184 32> here,
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1552,6 +1559,7 @@ gpio7: gpio@...e0000 {
>> clocks = <&cru PCLK_GPIO7>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 224 32>;
>
> and <&pinctrl 0 216 32> here,
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -1565,6 +1573,7 @@ gpio8: gpio@...f0000 {
>> clocks = <&cru PCLK_GPIO8>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 256 32>;
>
> and bank num 8 only expose 16 pins, so <&pinctrl 0 248 16> here.
>
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> diff --git a/arch/arm/boot/dts/rockchip/rv1108.dtsi b/arch/arm/boot/dts/rockchip/rv1108.dtsi
>> index abf3006f0a84..d12b97ee7588 100644
>> --- a/arch/arm/boot/dts/rockchip/rv1108.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rv1108.dtsi
>> @@ -602,6 +602,7 @@ gpio0: gpio@...30000 {
>> clocks = <&cru PCLK_GPIO0_PMU>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -615,6 +616,7 @@ gpio1: gpio@...10000 {
>> clocks = <&cru PCLK_GPIO1>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -628,6 +630,7 @@ gpio2: gpio@...20000 {
>> clocks = <&cru PCLK_GPIO2>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> @@ -641,6 +644,7 @@ gpio3: gpio@...30000 {
>> clocks = <&cru PCLK_GPIO3>;
>>
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 96 32>;
>> #gpio-cells = <2>;
>>
>> interrupt-controller;
>> diff --git a/arch/arm/boot/dts/rockchip/rv1126.dtsi b/arch/arm/boot/dts/rockchip/rv1126.dtsi
>> index bb603cae13df..71d091af6395 100644
>> --- a/arch/arm/boot/dts/rockchip/rv1126.dtsi
>> +++ b/arch/arm/boot/dts/rockchip/rv1126.dtsi
>> @@ -569,6 +569,7 @@ gpio0: gpio@...60000 {
>> interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&pmucru PCLK_GPIO0>, <&pmucru DBCLK_GPIO0>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 0 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> @@ -580,6 +581,7 @@ gpio1: gpio@...20000 {
>> interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO1>, <&cru DBCLK_GPIO1>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 32 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> @@ -591,6 +593,7 @@ gpio2: gpio@...30000 {
>> interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO2>, <&cru DBCLK_GPIO2>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 64 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> @@ -602,6 +605,7 @@ gpio3: gpio@...40000 {
>> interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO3>, <&cru DBCLK_GPIO3>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 96 32>;
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> @@ -613,6 +617,7 @@ gpio4: gpio@...50000 {
>> interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru PCLK_GPIO4>, <&cru DBCLK_GPIO4>;
>> gpio-controller;
>> + gpio-ranges = <&pinctrl 0 128 32>;
>
> Bank num 4 only expose 2 pins, so should probably be <&pinctrl 0 128 2>.
>
> Regards,
> Jonas
>
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> --
>> 2.39.2
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
Powered by blists - more mailing lists