| 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
| ||
|
Message-ID: <f45a4dcb-12e6-9a72-dbb3-7ec198ff2b1d@linaro.org>
Date: Thu, 27 Oct 2022 10:58:38 -0400
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Johan Jonker <jbx6244@...il.com>, kever.yang@...k-chips.com,
heiko@...ech.de
Cc: sjg@...omium.org, philipp.tomsich@...ll.eu,
zhangqing@...k-chips.com, hjc@...k-chips.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, daniel.lezcano@...aro.org,
tglx@...utronix.de, arnd@...db.de, olof@...om.net, soc@...nel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v1 3/4] ARM: dts: rockchip: add rk3128.dtsi
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@...a0000 {
> + compatible = "rockchip,rk3128-pmu", "syscon", "simple-mfd";
> + reg = <0x100a0000 0x1000>;
> + };
> +
> + gic: interrupt-controller@...39000 {
> + 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@...80000 {
> + 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@...c0000 {
> + 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@...e0000 {
> + compatible = "generic-ohci";
> + reg = <0x101e0000 0x20000>;
> + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&usb2phy_host>;
> + phy-names = "usb";
> + status = "disabled";
> + };
> +
> + sdmmc: mmc@...14000 {
> + 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@...18000 {
> + 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@...1c000 {
> + 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@...00000 {
> + 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@...00000 {
> + 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@...08000 {
> + 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
Powered by blists - more mailing lists