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@xxxxxxxxxxxxxx>
> > Signed-off-by: Finley Xiao <finley.xiao@xxxxxxxxxxxxxx>
> > Signed-off-by: Yifeng Zhao <yifeng.zhao@xxxxxxxxxxxxxx>
> > Signed-off-by: Elaine Zhang <zhangqing@xxxxxxxxxxxxxx>
> > [rebase, squash and reword commit message]
> > Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx>
> > ---
>
> [..]
>
> > 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@27310000 {
> > + 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@27380000 {
[...]
> > + #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@RK3576_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@27320000 {
>
> 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@2ae10000 {
> > +
> > + gpio2: gpio@2ae20000 {
> > +
> > + gpio3: gpio@2ae30000 {
> > +
> > + gpio4: gpio@2ae40000 {
> > + };
> > +};
> > +
> > +#include "rk3576-pinctrl.dtsi"
Regards,
Detlev
