[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1944590.atdPhlSkOF@trenzalore>
Date: Mon, 19 Aug 2024 16:06:12 -0400
From: Detlev Casanova <detlev.casanova@...labora.com>
To: linux-kernel@...r.kernel.org, Johan Jonker <jbx6244@...dex.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
Andi Shyti <andi.shyti@...nel.org>, Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>, Lee Jones <lee@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>, Chris Morgan <macromorgan@...mail.com>,
Jonas Karlman <jonas@...boo.se>, Tim Lunn <tim@...thertop.org>,
Muhammed Efe Cetin <efectn@...tonmail.com>, Andy Yan <andyshrk@....com>,
Jagan Teki <jagan@...eble.ai>, Dragan Simic <dsimic@...jaro.org>,
Sebastian Reichel <sebastian.reichel@...labora.com>,
Shresth Prasad <shresthprasad7@...il.com>, Ondrej Jirman <megi@....cz>,
Weizhao Ouyang <weizhao.ouyang@....com>, Alexey Charkov <alchark@...il.com>,
Jimmy Hon <honyuenkwun@...il.com>, Finley Xiao <finley.xiao@...k-chips.com>,
Yifeng Zhao <yifeng.zhao@...k-chips.com>,
Elaine Zhang <zhangqing@...k-chips.com>, Liang Chen <cl@...k-chips.com>,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-i2c@...r.kernel.org,
linux-iio@...r.kernel.org, linux-serial@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH 09/10] arm64: dts: rockchip: Add rk3576 SoC base DT
Hi Johan,
On Thursday, 15 August 2024 05:30:25 EDT Johan Jonker wrote:
> Some comments below. Whenever useful.
>
> On 8/2/24 23:45, Detlev Casanova wrote:
> > This device tree contains all devices necessary for booting from network
> > or SD Card.
> >
> > It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and
> > SDHCI (everything necessary to boot Linux on this system on chip) as
> > well as Ethernet, I2C, SPI and OTP.
> >
> > Also add the necessary DT bindings for the SoC.
> >
> > Signed-off-by: Liang Chen <cl@...k-chips.com>
> > Signed-off-by: Finley Xiao <finley.xiao@...k-chips.com>
> > Signed-off-by: Yifeng Zhao <yifeng.zhao@...k-chips.com>
> > Signed-off-by: Elaine Zhang <zhangqing@...k-chips.com>
> > [rebase, squash and reword commit message]
> > Signed-off-by: Detlev Casanova <detlev.casanova@...labora.com>
> > ---
>
> [..]
>
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3576.dtsi new file mode 100644
> > index 0000000000000..00c4d2a153ced
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> [..]
>
> For uart0..uart11:
> > +
> > + uart1: serial@...10000 {
> > + compatible = "rockchip,rk3576-uart", "snps,dw-apb-
uart";
> > + reg = <0x0 0x27310000 0x0 0x100>;
> >
> > + interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
>
> "interrupts" are sort just like other properties. A mix of sort styles
> exists, so check all nodes.
Ok, so it should be sorted alphabetically with the following exceptions:
- 'compatible' and 'reg.*' on top
- "#.*" at the end, sorted
- "status" last.
Is that right ?
> > + clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
> > + clock-names = "baudclk", "apb_pclk";
> >
> > + reg-shift = <2>;
> > + reg-io-width = <4>;
>
> Move below "reg".
>
> > + dmas = <&dmac0 8>, <&dmac0 9>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&uart1m0_xfer>;
> > + status = "disabled";
> > + };
> > +
> > + pmu: power-management@...80000 {
[...]
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + clocks = <&cru ACLK_VOP>,
> > + <&cru HCLK_VOP>,
> > + <&cru HCLK_VOP_ROOT>;
> > + pm_qos = <&qos_vop_m0>,
> > + <&qos_vop_m1ro>;
> > +
> > + power-domain@...576_PD_USB {
>
> Since when is USB part of VOP?
> Recheck?
The TRM doesn't tell me anything, but If I don't put it as a child of VOP, it
just hangs when the kernel tries to shut it down.
[...]
> > +
> > + pinctrl: pinctrl {
> > + compatible = "rockchip,rk3576-pinctrl";
> > + rockchip,grf = <&ioc_grf>;
> > + rockchip,sys-grf = <&sys_grf>;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> >
> > + gpio0: gpio@...20000 {
>
> The use of gpio nodes as subnode of pinctrl is deprecated.
>
> patternProperties:
> "gpio@[0-9a-f]+$":
> type: object
>
> $ref: /schemas/gpio/rockchip,gpio-bank.yaml#
> deprecated: true
>
> unevaluatedProperties: false
I tried putting the gpio nodes out of the pinctrl node, they should work
because they already have a gpio-ranges field.
But unfortunately, that seem to break the pinctrl driver which hangs at some
point. Maybe some adaptations are needed to support this, or am I missing
something ?
> > + compatible = "rockchip,gpio-bank";
>
> When in use as separate node the compatible must be SoC related.
>
> Question for the maintainers: Extra entry to rockchip,gpio-bank.yaml ??
>
> > + reg = <0x0 0x27320000 0x0 0x200>;
> > + interrupts = <GIC_SPI 153
IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&cru PCLK_GPIO0>, <&cru
DBCLK_GPIO0>;
> > +
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges = <&pinctrl 0 0 32>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + };
> > +
> > + gpio1: gpio@...10000 {
> > +
> > + gpio2: gpio@...20000 {
> > +
> > + gpio3: gpio@...30000 {
> > +
> > + gpio4: gpio@...40000 {
> > + };
> > +};
> > +
> > +#include "rk3576-pinctrl.dtsi"
Regards,
Detlev
Powered by blists - more mailing lists