[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a1922840-75dc-ad19-d001-9718d73891a2@linaro.org>
Date: Fri, 15 Sep 2023 13:24:52 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: nikita.shubin@...uefel.me, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Alexander Sverdlin <alexander.sverdlin@...il.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH v4 34/42] ARM: dts: add Cirrus EP93XX SoC .dtsi
On 15/09/2023 10:11, Nikita Shubin via B4 Relay wrote:
> From: Nikita Shubin <nikita.shubin@...uefel.me>
>
> Add support for Cirrus Logic EP93XX SoC's family.
>
> Co-developed-by: Alexander Sverdlin <alexander.sverdlin@...il.com>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...il.com>
> Signed-off-by: Nikita Shubin <nikita.shubin@...uefel.me>
> ---
> arch/arm/boot/dts/cirrus/ep93xx.dtsi | 454 +++++++++++++++++++++++++++++++++++
> 1 file changed, 454 insertions(+)
>
> diff --git a/arch/arm/boot/dts/cirrus/ep93xx.dtsi b/arch/arm/boot/dts/cirrus/ep93xx.dtsi
> new file mode 100644
> index 000000000000..11ca6d6a190d
> --- /dev/null
> +++ b/arch/arm/boot/dts/cirrus/ep93xx.dtsi
> @@ -0,0 +1,454 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree file for Cirrus Logic systems EP93XX SoC
> + */
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/clock/cirrus,ep9301-clk.h>
> +/ {
> + soc: soc {
> + compatible = "simple-bus";
> + ranges;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + syscon: syscon@...30000 {
> + compatible = "cirrus,ep9301-syscon",
> + "syscon", "simple-mfd";
Odd indentation. Please align with opening "
> + reg = <0x80930000 0x1000>;
> +
> + eclk: clock-controller {
> + compatible = "cirrus,ep9301-clk";
> + #clock-cells = <1>;
> + clocks = <&xtali>;
> + };
> +
> + pinctrl: pinctrl {
> + compatible = "cirrus,ep9301-pinctrl";
> +
> + spi_default_pins: pins-spi {
> + function = "spi";
> + groups = "ssp";
Please include one such entry in your bindings example, so it will be
validated. It would also help you in review - you would not get a
comment that pinctrl node is not used :/
> + };
> +
> + ac97_default_pins: pins-ac97 {
> + function = "ac97";
> + groups = "ac97";
> + };
> +
> + i2s_on_ssp_pins: pins-i2sonssp {
> + function = "i2s";
> + groups = "i2s_on_ssp";
> + };
> +
> + i2s_on_ac97_pins: pins-i2sonac97 {
> + function = "i2s";
> + groups = "i2s_on_ac97";
> + };
> +
> + gpio1_default_pins: pins-gpio1 {
> + function = "gpio";
> + groups = "gpio1agrp";
> + };
> +
> + pwm1_default_pins: pins-pwm1 {
> + function = "pwm";
> + groups = "pwm1";
> + };
> +
> + gpio2_default_pins: pins-gpio2 {
> + function = "gpio";
> + groups = "gpio2agrp";
> + };
> +
> + gpio3_default_pins: pins-gpio3 {
> + function = "gpio";
> + groups = "gpio3agrp";
> + };
> +
> + keypad_default_pins: pins-keypad {
> + function = "keypad";
> + groups = "keypadgrp";
> + };
> +
> + gpio4_default_pins: pins-gpio4 {
> + function = "gpio";
> + groups = "gpio4agrp";
> + };
> +
> + gpio6_default_pins: pins-gpio6 {
> + function = "gpio";
> + groups = "gpio6agrp";
> + };
> +
> + gpio7_default_pins: pins-gpio7 {
> + function = "gpio";
> + groups = "gpio7agrp";
> + };
> +
> + ide_default_pins: pins-ide {
> + function = "pata";
> + groups = "idegrp";
> + };
> +
> + lcd_on_dram0_pins: pins-rasteronsdram0 {
> + function = "lcd";
> + groups = "rasteronsdram0grp";
> + };
> +
> + lcd_on_dram3_pins: pins-rasteronsdram3 {
> + function = "lcd";
> + groups = "rasteronsdram3grp";
> + };
> + };
> +
> + reboot {
> + compatible = "cirrus,ep9301-reboot";
> + };
> + };
> +
> + adc: adc@...00000 {
> + compatible = "cirrus,ep9301-adc";
> + reg = <0x80900000 0x28>;
> + clocks = <&eclk EP93XX_CLK_ADC>;
> + interrupt-parent = <&vic0>;
> + interrupts = <30>;
> + status = "disabled";
> + };
> +
> + /*
> + * The EP93XX expansion bus is a set of up to 7 each up to 16MB
> + * windows in the 256MB space from 0x50000000 to 0x5fffffff.
> + * But since we don't require to setup it in any way, we can
> + * represent it as a simple-bus.
> + */
> + ebi: bus@...80000 {
> + compatible = "simple-bus";
> + reg = <0x80080000 0x20>;
> + native-endian;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + };
> +
> + dma0: dma-controller@...00000 {
> + compatible = "cirrus,ep9301-dma-m2p";
> + reg = <0x80000000 0x0040>,
> + <0x80000040 0x0040>,
> + <0x80000080 0x0040>,
> + <0x800000c0 0x0040>,
> + <0x80000240 0x0040>,
> + <0x80000200 0x0040>,
> + <0x800002c0 0x0040>,
> + <0x80000280 0x0040>,
> + <0x80000340 0x0040>,
> + <0x80000300 0x0040>;
> + clocks = <&eclk EP93XX_CLK_M2P0>,
> + <&eclk EP93XX_CLK_M2P1>,
> + <&eclk EP93XX_CLK_M2P2>,
> + <&eclk EP93XX_CLK_M2P3>,
> + <&eclk EP93XX_CLK_M2P4>,
> + <&eclk EP93XX_CLK_M2P5>,
> + <&eclk EP93XX_CLK_M2P6>,
> + <&eclk EP93XX_CLK_M2P7>,
> + <&eclk EP93XX_CLK_M2P8>,
> + <&eclk EP93XX_CLK_M2P9>;
> + clock-names = "m2p0", "m2p1",
> + "m2p2", "m2p3",
> + "m2p4", "m2p5",
> + "m2p6", "m2p7",
> + "m2p8", "m2p9";
> + interrupt-parent = <&vic0>;
> + interrupts = <7>, <8>, <9>, <10>, <11>,
> + <12>, <13>, <14>, <15>, <16>;
> + #dma-cells = <1>;
> + };
> +
> + dma1: dma-controller@...00100 {
> + compatible = "cirrus,ep9301-dma-m2m";
> + reg = <0x80000100 0x0040>,
> + <0x80000140 0x0040>;
> + clocks = <&eclk EP93XX_CLK_M2M0>,
> + <&eclk EP93XX_CLK_M2M1>;
> + clock-names = "m2m0", "m2m1";
> + interrupt-parent = <&vic0>;
> + interrupts = <17>, <18>;
> + #dma-cells = <1>;
> + };
> +
> + eth0: ethernet@...10000 {
> + compatible = "cirrus,ep9301-eth";
> + reg = <0x80010000 0x10000>;
> + interrupt-parent = <&vic1>;
> + interrupts = <7>;
> + mdio0: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> + gpio0: gpio@...40000 {
> + compatible = "cirrus,ep9301-gpio";
> + reg = <0x80840000 0x04>,
> + <0x80840010 0x04>,
> + <0x80840090 0x1c>;
> + reg-names = "data", "dir", "intr";
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + interrupt-parent = <&vic1>;
> + interrupts = <27>;
> + };
> +
> + gpio1: gpio@...40004 {
> + compatible = "cirrus,ep9301-gpio";
> + reg = <0x80840004 0x04>,
> + <0x80840014 0x04>,
> + <0x808400ac 0x1c>;
> + reg-names = "data", "dir", "intr";
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + interrupt-parent = <&vic1>;
> + interrupts = <27>;
> + };
> +
> + gpio2: gpio@...40008 {
> + compatible = "cirrus,ep9301-gpio";
> + reg = <0x80840008 0x04>,
> + <0x80840018 0x04>;
> + reg-names = "data", "dir";
> + gpio-controller;
> + #gpio-cells = <2>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&gpio2_default_pins>;
> + status = "disabled";
Why this node is disabled? Any dependencies/resources missing? Similar
questions to other gpio nodes.
Best regards,
Krzysztof
Powered by blists - more mailing lists