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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_Jsq+Pn=kWFL32Cit-vnyJg2pnap2TMn4LPVr9nTmyK-FrZw@mail.gmail.com>
Date:   Wed, 4 Oct 2023 11:41:48 -0500
From:   Rob Herring <robh+dt@...nel.org>
To:     Gregory CLEMENT <gregory.clement@...tlin.com>
Cc:     Paul Burton <paulburton@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-mips@...r.kernel.org,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Vladimir Kondratiev <vladimir.kondratiev@...el.com>,
        Tawfik Bayouk <tawfik.bayouk@...ileye.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Théo Lebrun <theo.lebrun@...tlin.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 08/11] MIPS: mobileye: Add EyeQ5 dtsi

On Wed, Oct 4, 2023 at 11:11 AM Gregory CLEMENT
<gregory.clement@...tlin.com> wrote:
>
> Add a device tree include file for the Mobileye EyeQ5 SoC.
>
> Based on the work of Slava Samsonov <stanislav.samsonov@...el.com>
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@...tlin.com>
> ---
>  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@...000000 {
> +                       reg = <0x8 0x04000000 0x0 0x1000000>;
> +               };
> +               shmem1_reserved: shmem@...000000 {
> +                       reg = <0x8 0x05000000 0x0 0x1000000>;
> +               };
> +               pci0_msi_reserved: pci0_msi@...000000 {
> +                       reg = <0x8 0x06000000 0x0 0x100000>;
> +               };
> +               pci1_msi_reserved: pci1_msi@...100000 {
> +                       reg = <0x8 0x06100000 0x0 0x100000>;
> +               };
> +
> +               mini_coredump0_reserved: mini_coredump0@...200000 {
> +                       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@...000 {
> +               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@...000 {
> +                       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@...000 {
> +                       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@...000 {
> +                       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@...000 {
> +                       compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> +                       reg = <0 0xe00000 0x0 0x400>;
> +                       reg-io-width = <4>;
> +               };
> +
> +       };
> +};
> --
> 2.40.1
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ