Re: [PATCH 08/11] MIPS: mobileye: Add EyeQ5 dtsi

From: Rob Herring
Date: Wed Oct 04 2023 - 12:42:08 EST

Next message: Stefano Garzarella: "Re: [PATCH net-next v2 00/12] vsock/virtio: continue MSG_ZEROCOPY support"
Previous message: Ingo Molnar: "Re: [PATCH 4/4] x86/percpu: Use C for percpu read/write accessors"
In reply to: Gregory CLEMENT: "[PATCH 08/11] MIPS: mobileye: Add EyeQ5 dtsi"
Next in thread: Gregory CLEMENT: "Re: [PATCH 08/11] MIPS: mobileye: Add EyeQ5 dtsi"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Oct 4, 2023 at 11:11 AM Gregory CLEMENT
<gregory.clement@xxxxxxxxxxx> wrote:
>
> Add a device tree include file for the Mobileye EyeQ5 SoC.
>
> Based on the work of Slava Samsonov <stanislav.samsonov@xxxxxxxxx>
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxx>
> ---
> arch/mips/boot/dts/Makefile | 1 +
> arch/mips/boot/dts/mobileye/Makefile | 4 +
> .../boot/dts/mobileye/eyeq5-fixed-clocks.dtsi | 315 ++++++++++++++++++
> arch/mips/boot/dts/mobileye/eyeq5.dtsi | 138 ++++++++
> 4 files changed, 458 insertions(+)
> create mode 100644 arch/mips/boot/dts/mobileye/Makefile
> create mode 100644 arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi
> create mode 100644 arch/mips/boot/dts/mobileye/eyeq5.dtsi
>
> diff --git a/arch/mips/boot/dts/Makefile b/arch/mips/boot/dts/Makefile
> index 928f38a79dff..edb8e8dee758 100644
> --- a/arch/mips/boot/dts/Makefile
> +++ b/arch/mips/boot/dts/Makefile
> @@ -8,6 +8,7 @@ subdir-$(CONFIG_LANTIQ) += lantiq
> subdir-$(CONFIG_MACH_LOONGSON64) += loongson
> subdir-$(CONFIG_SOC_VCOREIII) += mscc
> subdir-$(CONFIG_MIPS_MALTA) += mti
> +subdir-$(CONFIG_SOC_EYEQ5) += mobileye
> subdir-$(CONFIG_LEGACY_BOARD_SEAD3) += mti
> subdir-$(CONFIG_FIT_IMAGE_FDT_NI169445) += ni
> subdir-$(CONFIG_MACH_PIC32) += pic32
> diff --git a/arch/mips/boot/dts/mobileye/Makefile b/arch/mips/boot/dts/mobileye/Makefile
> new file mode 100644
> index 000000000000..99c4124fd4c0
> --- /dev/null
> +++ b/arch/mips/boot/dts/mobileye/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +# Copyright 2023 Mobileye Vision Technologies Ltd.
> +
> +obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))

You didn't add anything to 'dtb-y'. Did you test this?

Also, CONFIG_BUILTIN_DTB is supposed to be for legacy bootloaders
which don't understand DT. For a new SoC, fix the bootloader.

> diff --git a/arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi b/arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi
> new file mode 100644
> index 000000000000..a0066465ac8b
> --- /dev/null
> +++ b/arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi
> @@ -0,0 +1,315 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Copyright 2023 Mobileye Vision Technologies Ltd.
> + */

I assume these aren't all really fixed, but just 'I don't have a clock
driver yet'. That creates an ABI issue when you add the clock
driver(s). Just FYI.

> +
> +/ {
> + /* Fixed clock */
> + pll_cpu: pll_cpu {

Don't use _ in node names.

> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1500000000>;
> + };
> +
> + pll_vdi: pll_vdi {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1280000000>;
> + };
> +
> + pll_per: pll_per {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <2000000000>;
> + };
> +
> + pll_ddr0: pll_ddr0 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1857210000>;
> + };
> +
> + pll_ddr1: pll_ddr1 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1857210000>;
> + };
> +
> +/* PLL_CPU derivatives */
> + occ_cpu: occ_cpu {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_cpu>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "occ_cpu";

Isn't the default name the node name? Drop these unless you really
have a need and they aren't redundant.

> + };
> + si_css0_ref_clk: si_css0_ref_clk { /* gate ClkRstGen_si_css0_ref */
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_cpu>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "si_css0_ref_clk";
> + };
> + cpc_clk: cpc_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&si_css0_ref_clk>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "cpc_clk";
> + };
> + core0_clk: core0_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&si_css0_ref_clk>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "core0_clk";
> + };
> + core1_clk: core1_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&si_css0_ref_clk>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "core1_clk";
> + };
> + core2_clk: core2_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&si_css0_ref_clk>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "core2_clk";
> + };
> + core3_clk: core3_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&si_css0_ref_clk>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "core3_clk";
> + };
> + cm_clk: cm_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&si_css0_ref_clk>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "cm_clk";
> + };
> + mem_clk: mem_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&si_css0_ref_clk>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "mem_clk";
> + };
> + occ_isram: occ_isram {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_cpu>;
> + #clock-cells = <0>;
> + clock-div = <2>;
> + clock-mult = <1>;
> + clock-output-names = "occ_isram";
> + };
> + isram_clk: isram_clk { /* gate ClkRstGen_isram */
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_isram>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "isram_clk";
> + };
> + occ_dbu: occ_dbu {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_cpu>;
> + #clock-cells = <0>;
> + clock-div = <10>;
> + clock-mult = <1>;
> + clock-output-names = "occ_dbu";
> + };
> + si_dbu_tp_pclk: si_dbu_tp_pclk { /* gate ClkRstGen_dbu */
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_dbu>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "si_dbu_tp_pclk";
> + };
> +/* PLL_VDI derivatives */
> + occ_vdi: occ_vdi {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_vdi>;
> + #clock-cells = <0>;
> + clock-div = <2>;
> + clock-mult = <1>;
> + clock-output-names = "occ_vdi";
> + };
> + vdi_clk: vdi_clk { /* gate ClkRstGen_vdi */
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_vdi>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "vdi_clk";
> + };
> + occ_can_ser: occ_can_ser {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_vdi>;
> + #clock-cells = <0>;
> + clock-div = <16>;
> + clock-mult = <1>;
> + clock-output-names = "occ_can_ser";
> + };
> + can_ser_clk: can_ser_clk { /* gate ClkRstGen_can_ser */
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_can_ser>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "can_ser_clk";
> + };
> + i2c_ser_clk: i2c_ser_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_vdi>;
> + #clock-cells = <0>;
> + clock-div = <20>;
> + clock-mult = <1>;
> + clock-output-names = "i2c_ser_clk";
> + };
> +/* PLL_PER derivatives */
> + occ_periph: occ_periph {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_per>;
> + #clock-cells = <0>;
> + clock-div = <16>;
> + clock-mult = <1>;
> + clock-output-names = "occ_periph";
> + };
> + periph_clk: periph_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_periph>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "periph_clk";
> + };
> + can_clk: can_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_periph>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "can_clk";
> + };
> + spi_clk: spi_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_periph>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "spi_clk";
> + };
> + uart_clk: uart_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_periph>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "uart_clk";
> + };
> + i2c_clk: i2c_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_periph>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "i2c_clk";
> + };
> + timer_clk: timer_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_periph>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "timer_clk";
> + };
> + gpio_clk: gpio_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_periph>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "gpio_clk";
> + };
> + emmc_sys_clk: emmc_sys_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_per>;
> + #clock-cells = <0>;
> + clock-div = <10>;
> + clock-mult = <1>;
> + clock-output-names = "emmc_sys_clk";
> + };
> + ccf_ctrl_clk: ccf_ctrl_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_per>;
> + #clock-cells = <0>;
> + clock-div = <4>;
> + clock-mult = <1>;
> + clock-output-names = "ccf_ctrl_clk";
> + };
> + occ_mjpeg_core: occ_mjpeg_core {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_per>;
> + #clock-cells = <0>;
> + clock-div = <2>;
> + clock-mult = <1>;
> + clock-output-names = "occ_mjpeg_core";
> + };
> + hsm_clk: hsm_clk { /* gate ClkRstGen_hsm */
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_mjpeg_core>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "hsm_clk";
> + };
> + mjpeg_core_clk: mjpeg_core_clk { /* gate ClkRstGen_mjpeg_gen */
> + compatible = "fixed-factor-clock";
> + clocks = <&occ_mjpeg_core>;
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <1>;
> + clock-output-names = "mjpeg_core_clk";
> + };
> + fcmu_a_clk: fcmu_a_clk {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_per>;
> + #clock-cells = <0>;
> + clock-div = <20>;
> + clock-mult = <1>;
> + clock-output-names = "fcmu_a_clk";
> + };
> + occ_pci_sys: occ_pci_sys {
> + compatible = "fixed-factor-clock";
> + clocks = <&pll_per>;
> + #clock-cells = <0>;
> + clock-div = <8>;
> + clock-mult = <1>;
> + clock-output-names = "occ_pci_sys";
> + };
> + pclk: pclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <250000000>; /* 250MHz */
> + };
> + tsu_clk: tsu_clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <125000000>; /* 125MHz */
> + };
> +};
> diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> new file mode 100644
> index 000000000000..0504c2fb3ad5
> --- /dev/null
> +++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0

Doesn't match eyeq5-fixed-clocks.dtsi

> +/*
> + * Copyright 2023 Mobileye Vision Technologies Ltd.
> + */
> +
> +#include <dt-bindings/interrupt-controller/mips-gic.h>
> +#include <dt-bindings/soc/mobileye,eyeq5.h>
> +
> +/memreserve/ 0x40000000 0xc0000000; /* DDR32 */
> +/memreserve/ 0x08000000 0x08000000; /* DDR_LOW */
> +
> +#include "eyeq5-fixed-clocks.dtsi"
> +
> +/* almost all GIC IRQs has the same characteristics. provide short form */

Maybe so, but I prefer not having 2 levels of lookup to figure out values.

> +#define GIC_IRQ(x) GIC_SHARED (x) IRQ_TYPE_LEVEL_HIGH
> +
> +/ {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "mti,i6500";
> + reg = <0>;
> + clocks = <&core0_clk>;
> + };
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> +/* These reserved memory regions are also defined in bootmanager
> + * for configuring inbound translation for BARS, don't change
> + * these without syncing with bootmanager
> + */

Indent with the rest of the node.

> + shmem0_reserved: shmem@804000000 {
> + reg = <0x8 0x04000000 0x0 0x1000000>;
> + };
> + shmem1_reserved: shmem@805000000 {
> + reg = <0x8 0x05000000 0x0 0x1000000>;
> + };
> + pci0_msi_reserved: pci0_msi@806000000 {
> + reg = <0x8 0x06000000 0x0 0x100000>;
> + };
> + pci1_msi_reserved: pci1_msi@806100000 {
> + reg = <0x8 0x06100000 0x0 0x100000>;
> + };
> +
> + mini_coredump0_reserved: mini_coredump0@806200000 {
> + reg = <0x8 0x06200000 0x0 0x100000>;
> + };
> + mhm_reserved_0: the_mhm_reserved_0@0 {
> + reg = <0x8 0x00000000 0x0 0x0000800>;
> + };
> + };
> +
> + aliases {
> + serial0 = &uart0;
> + serial1 = &uart1;
> + serial2 = &uart2;
> + };
> +
> + cpu_intc: interrupt-controller {
> + compatible = "mti,cpu-interrupt-controller";
> + interrupt-controller;
> + #address-cells = <0>;
> + #interrupt-cells = <1>;
> + };
> +
> + gic: interrupt-controller@140000 {
> + compatible = "mti,gic";
> + reg = <0x0 0x140000 0x0 0x20000>;
> + interrupt-controller;
> + #interrupt-cells = <3>;
> +
> + /*
> + * Declare the interrupt-parent even though the mti,gic
> + * binding doesn't require it, such that the kernel can
> + * figure out that cpu_intc is the root interrupt
> + * controller & should be probed first.
> + */
> + interrupt-parent = <&cpu_intc>;
> +
> + timer {
> + compatible = "mti,gic-timer";
> + interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
> + clocks = <&core0_clk>;
> + };
> + };
> +
> + soc: soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + compatible = "simple-bus";
> +
> + uart0: serial@800000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0 0x800000 0x0 0x1000>;
> + reg-io-width = <4>;
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_IRQ(NUM_INT_UART)>;
> + clocks = <&uart_clk>, <&occ_periph>;
> + clock-names = "uartclk", "apb_pclk";
> + };
> +
> + uart1: serial@900000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0 0x900000 0x0 0x1000>;
> + reg-io-width = <4>;
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_IRQ(NUM_INT_UART)>;
> + clocks = <&uart_clk>, <&occ_periph>;
> + clock-names = "uartclk", "apb_pclk";
> + };
> +
> + uart2: serial@a00000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0 0xa00000 0x0 0x1000>;
> + reg-io-width = <4>;
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_IRQ(NUM_INT_UART)>;
> + clocks = <&uart_clk>, <&occ_periph>;
> + clock-names = "uartclk", "apb_pclk";
> + };
> +
> + olb: olb@e00000 {
> + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> + reg = <0 0xe00000 0x0 0x400>;
> + reg-io-width = <4>;
> + };
> +
> + };
> +};
> --
> 2.40.1
>

Next message: Stefano Garzarella: "Re: [PATCH net-next v2 00/12] vsock/virtio: continue MSG_ZEROCOPY support"
Previous message: Ingo Molnar: "Re: [PATCH 4/4] x86/percpu: Use C for percpu read/write accessors"
In reply to: Gregory CLEMENT: "[PATCH 08/11] MIPS: mobileye: Add EyeQ5 dtsi"
Next in thread: Gregory CLEMENT: "Re: [PATCH 08/11] MIPS: mobileye: Add EyeQ5 dtsi"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]