On 26/10/2022 20:53, Johan Jonker wrote:
> Add basic rk3128 support.
>
Thank you for your patch. There is something to discuss/improve.
> +#include <dt-bindings/clock/rk3128-cru.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +
> +/ {
> + compatible = "rockchip,rk3128";
> + interrupt-parent = <&gic>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + aliases {
> + gpio0 = &gpio0;
> + gpio1 = &gpio1;
> + gpio2 = &gpio2;
> + gpio3 = &gpio3;
> + i2c0 = &i2c0;
> + i2c1 = &i2c1;
> + i2c2 = &i2c2;
> + i2c3 = &i2c3;
> + spi0 = &spi0;
> + serial0 = &uart0;
> + serial1 = &uart1;
> + serial2 = &uart2;
Bus aliases are board specific and represent what is actually available
on headers/pins etc. These do not belong to SoC DTSI.
> + };
> +
> + arm-pmu {
> + compatible = "arm,cortex-a7-pmu";
> + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@f00 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0xf00>;
> + clock-latency = <40000>;
> + clocks = <&cru ARMCLK>;
> + operating-points = <
> + /* KHz uV */
> + 816000 1000000
> + >;
Why not operating-points-v2?
> + #cooling-cells = <2>; /* min followed by max */
> + };
> +
> + cpu1: cpu@f01 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0xf01>;
> + };
> +
> + cpu2: cpu@f02 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0xf02>;
> + };
> +
> + cpu3: cpu@f03 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a7";
> + reg = <0xf03>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv7-timer";
> + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> + arm,cpu-registers-not-fw-configured;
> + clock-frequency = <24000000>;
> + };
> +
> + xin24m: oscillator {
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "xin24m";
> + #clock-cells = <0>;
> + };
> +
> + pmu: syscon@100a0000 {
> + compatible = "rockchip,rk3128-pmu", "syscon", "simple-mfd";
> + reg = <0x100a0000 0x1000>;
> + };
> +
> + gic: interrupt-controller@10139000 {
> + compatible = "arm,cortex-a7-gic";
> + reg = <0x10139000 0x1000>,
> + <0x1013a000 0x1000>,
> + <0x1013c000 0x2000>,
> + <0x1013e000 0x2000>;
> + interrupts = <GIC_PPI 9 0xf04>;
f04 looks like a mask, so use standard defines for it.
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + #address-cells = <0>;
> + };
> +
> + usb_otg: usb@10180000 {
> + compatible = "rockchip,rk3128-usb", "rockchip,rk3066-usb", "snps,dwc2";
> + reg = <0x10180000 0x40000>;
> + interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru HCLK_OTG>;
> + clock-names = "otg";
> + dr_mode = "otg";
> + phys = <&usb2phy_otg>;
> + phy-names = "usb2-phy";
> + status = "disabled";
> + };
> +
> + usb_host_ehci: usb@101c0000 {
> + compatible = "generic-ehci";
> + reg = <0x101c0000 0x20000>;
> + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&usb2phy_host>;
> + phy-names = "usb";
> + status = "disabled";
> + };
> +
> + usb_host_ohci: usb@101e0000 {
> + compatible = "generic-ohci";
> + reg = <0x101e0000 0x20000>;
> + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&usb2phy_host>;
> + phy-names = "usb";
> + status = "disabled";
> + };
> +
> + sdmmc: mmc@10214000 {
> + compatible = "rockchip,rk3128-dw-mshc", "rockchip,rk3288-dw-mshc";
> + reg = <0x10214000 0x4000>;
> + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>,
> + <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>;
> + clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
> + dmas = <&pdma 10>;
> + dma-names = "rx-tx";
> + fifo-depth = <256>;
> + max-frequency = <150000000>;
> + resets = <&cru SRST_SDMMC>;
> + reset-names = "reset";
> + status = "disabled";
> + };
> +
> + sdio: mmc@10218000 {
> + compatible = "rockchip,rk3128-dw-mshc", "rockchip,rk3288-dw-mshc";
> + reg = <0x10218000 0x4000>;
> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru HCLK_SDIO>, <&cru SCLK_SDIO>,
> + <&cru SCLK_SDIO_DRV>, <&cru SCLK_SDIO_SAMPLE>;
> + clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
> + dmas = <&pdma 11>;
> + dma-names = "rx-tx";
> + fifo-depth = <256>;
> + max-frequency = <150000000>;
> + resets = <&cru SRST_SDIO>;
> + reset-names = "reset";
> + status = "disabled";
> + };
> +
> + emmc: mmc@1021c000 {
> + compatible = "rockchip,rk3128-dw-mshc", "rockchip,rk3288-dw-mshc";
> + reg = <0x1021c000 0x4000>;
> + interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>,
> + <&cru SCLK_EMMC_DRV>, <&cru SCLK_EMMC_SAMPLE>;
> + clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
> + dmas = <&pdma 12>;
> + dma-names = "rx-tx";
> + fifo-depth = <256>;
> + max-frequency = <150000000>;
> + resets = <&cru SRST_EMMC>;
> + reset-names = "reset";
> + status = "disabled";
> + };
> +
> + nfc: nand-controller@10500000 {
> + compatible = "rockchip,rk3128-nfc", "rockchip,rk2928-nfc";
> + reg = <0x10500000 0x4000>;
> + interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru HCLK_NANDC>, <&cru SCLK_NANDC>;
> + clock-names = "ahb", "nfc";
> + pinctrl-names = "default";
> + pinctrl-0 = <&flash_ale &flash_bus8 &flash_cle &flash_cs0
> + &flash_dqs &flash_rdn &flash_rdy &flash_wrn>;
> + status = "disabled";
> + };
> +
> + cru: clock-controller@20000000 {
> + compatible = "rockchip,rk3128-cru";
> + reg = <0x20000000 0x1000>;
> + clocks = <&xin24m>;
> + clock-names = "xin24m";
> + rockchip,grf = <&grf>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + assigned-clocks = <&cru PLL_GPLL>;
> + assigned-clock-rates = <594000000>;
> + };
> +
> + grf: syscon@20008000 {
> + compatible = "rockchip,rk3128-grf", "syscon", "simple-mfd";
> + reg = <0x20008000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + usb2phy: usb2phy@17c {
Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "rockchip,rk3128-usb2phy";
> + reg = <0x017c 0x0c>;
> + clocks = <&cru SCLK_OTGPHY0>;
> + clock-names = "phyclk";
> + clock-output-names = "usb480m_phy";
> + #clock-cells = <0>;
> + status = "disabled";
> +
Best regards,
Krzysztof
