Re: [PATCH v3 35/42] ARM: dts: ep93xx: add ts7250 board

From: Nikita Shubin
Date: Mon Jul 24 2023 - 09:45:05 EST


Hello Krzysztof!

Thank you for checking so quickly!

On Fri, 2023-07-21 at 16:15 +0200, Krzysztof Kozlowski wrote:
> On 20/07/2023 13:29, Nikita Shubin via B4 Relay wrote:
> > From: Nikita Shubin <nikita.shubin@xxxxxxxxxxx>
> >
> > Add device tree file for Technologic Systems ts7250 board and
> > Liebherr bk3 board which have many in common, both are based on
> > ep9302 SoC variant.
> >
> > Signed-off-by: Nikita Shubin <nikita.shubin@xxxxxxxxxxx>
> > ---
> >  arch/arm/boot/dts/cirrus/Makefile          |   3 +
> >  arch/arm/boot/dts/cirrus/ep93xx-bk3.dts    | 126
> > +++++++++++++++++++++++++
> >  arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts | 145
> > +++++++++++++++++++++++++++++
> >  3 files changed, 274 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/cirrus/Makefile
> > b/arch/arm/boot/dts/cirrus/Makefile
> > index e944d3e2129d..211a7e2f2115 100644
> > --- a/arch/arm/boot/dts/cirrus/Makefile
> > +++ b/arch/arm/boot/dts/cirrus/Makefile
> > @@ -3,3 +3,6 @@ dtb-$(CONFIG_ARCH_CLPS711X) += \
> >         ep7211-edb7211.dtb
> >  dtb-$(CONFIG_ARCH_CLPS711X) += \
> >         ep7211-edb7211.dtb
> > +dtb-$(CONFIG_ARCH_EP93XX) += \
> > +       ep93xx-bk3.dtb \
> > +       ep93xx-ts7250.dtb
> > diff --git a/arch/arm/boot/dts/cirrus/ep93xx-bk3.dts
> > b/arch/arm/boot/dts/cirrus/ep93xx-bk3.dts
> > new file mode 100644
> > index 000000000000..91d76a1a8571
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/cirrus/ep93xx-bk3.dts
> > @@ -0,0 +1,126 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Device Tree file for Liebherr controller BK3.1 based on Cirrus
> > EP9302 SoC
> > + */
> > +/dts-v1/;
> > +#include "ep93xx.dtsi"
> > +
> > +/ {
> > +       model = "Liebherr controller BK3.1";
> > +       compatible = "liebherr,bk3", "cirrus,ep9301";
> > +       #address-cells = <1>;
> > +       #size-cells = <1>;
> > +
> > +       chosen {
> > +       };
> > +
> > +       memory@0 {
> > +               device_type = "memory";
> > +               /* should be set from ATAGS */
> > +               reg = <0x00000000 0x02000000>,
> > +                     <0x000530c0 0x01fdd000>;
> > +       };
> > +
> > +       nand-controller@60000000 {
> > +               compatible = "technologic,ts7200-nand";
> > +               reg = <0x60000000 0x8000000>;
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               nand@0 {
> > +                       reg = <0>;
> > +                       partitions {
> > +                               compatible = "fixed-partitions";
> > +                               #address-cells = <1>;
> > +                               #size-cells = <1>;
> > +
> > +                               partition@0 {
> > +                                       label = "System";
> > +                                       reg = <0x00000000
> > 0x01e00000>;
> > +                                       read-only;
> > +                               };
> > +
> > +                               partition@1e00000 {
> > +                                       label = "Data";
> > +                                       reg = <0x01e00000
> > 0x05f20000>;
> > +                               };
> > +
> > +                               partition@7d20000 {
> > +                                       label = "RedBoot";
> > +                                       reg = <0x07d20000
> > 0x002e0000>;
> > +                                       read-only;
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +
> > +       leds {
> > +               compatible = "gpio-leds";
> > +               led-0 {
> > +                       label = "grled";
> > +                       gpios = <&gpio4 0 GPIO_ACTIVE_HIGH>;
> > +                       linux,default-trigger = "heartbeat";
> > +                       function = LED_FUNCTION_HEARTBEAT;
> > +               };
> > +
> > +               led-1 {
> > +                       label = "rdled";
> > +                       gpios = <&gpio4 1 GPIO_ACTIVE_HIGH>;
> > +                       function = LED_FUNCTION_FAULT;
> > +               };
> > +       };
> > +};
> > +
> > +&eth0 {
> > +       phy-handle = <&phy0>;
> > +};
> > +
> > +&i2c0 {
> > +       status = "okay";
> > +};
> > +
> > +&i2s {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&i2s_on_ac97_pins>;
> > +       status = "okay";
> > +};
> > +
> > +&gpio1 {
> > +       /* PWM */
> > +       gpio-ranges = <&pinctrl 6 163 1>;
> > +};
> > +
> > +&gpio4 {
> > +       gpio-ranges = <&pinctrl 0 97 2>;
> > +       status = "okay";
> > +};
> > +
> > +&gpio6 {
> > +       gpio-ranges = <&pinctrl 0 87 2>;
> > +       status = "okay";
> > +};
> > +
> > +&gpio7 {
> > +       gpio-ranges = <&pinctrl 2 199 4>;
> > +       status = "okay";
> > +};
> > +
> > +&mdio0 {
> > +       phy0: ethernet-phy@1 {
> > +               reg = <1>;
> > +               device_type = "ethernet-phy";
> > +       };
> > +};
> > +
> > +&uart0 {
> > +       status = "okay";
> > +};
> > +
> > +&uart1 {
> > +       status = "okay";
> > +};
> > +
> > +&usb0 {
> > +       status = "okay";
> > +};
> > +
> > diff --git a/arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts
> > b/arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts
> > new file mode 100644
> > index 000000000000..625202f8cd25
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts
> > @@ -0,0 +1,145 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Device Tree file for Technologic Systems ts7250 board based on
> > Cirrus EP9302 SoC
> > + */
> > +/dts-v1/;
> > +#include "ep93xx.dtsi"
> > +#include <dt-bindings/dma/cirrus,ep93xx-dma.h>
> > +
> > +/ {
> > +       compatible = "technologic,ts7250", "cirrus,ep9301";
> > +       model = "TS-7250 SBC";
> > +       #address-cells = <1>;
> > +       #size-cells = <1>;
> > +
> > +       chosen {
> > +       };
> > +
> > +       memory@0 {
> > +               device_type = "memory";
> > +               /* should be set from ATAGS */
> > +               reg = <0x00000000 0x02000000>,
> > +                     <0x000530c0 0x01fdd000>;
> > +       };
> > +
> > +       nand-controller@60000000 {
>
> Where is this address? It does not work like that. If this is part of
> SoC, then should be in DTSI and part of soc node. If not, then it is
> some other bus which needs some description. Top-level is not a bus.
>

It's some kind of EBI, but it doesn't need a driver it is transparent
on ts7250, the logic is controlled through installed CPLD.

The EBI it self is a part of the SoC through:

https://elixir.bootlin.com/linux/v6.5-rc3/source/arch/arm/mach-ep93xx/soc.h#L35

EP93XX_CS0_PHYS_BASE_ASYNC to EP93XX_CS0_PHYS_BASE_SYNC.

So for ts7250 this includes:

- NAND
- m48t86
- watchdog

I don't even know how to represent it correctly, would "simple-bus"
with "ranges" defined suit here, so it will represent hierarchy but
won't do anything ?

> You should see errors when testing dtbs with W=1.

Strangely - i don't see any, but anyway the above will change.

>
>
> > +               compatible = "technologic,ts7200-nand";
> > +               reg = <0x60000000 0x8000000>;
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               nand@0 {
> > +                       reg = <0>;
> > +                       partitions {
> > +                               compatible = "fixed-partitions";
> > +                               #address-cells = <1>;
> > +                               #size-cells = <1>;
> > +
> > +                               partition@0 {
> > +                                       label = "TS-BOOTROM";
> > +                                       reg = <0x00000000
> > 0x00020000>;
> > +                                       read-only;
> > +                               };
> > +
> > +                               partition@20000 {
> > +                                       label = "Linux";
> > +                                       reg = <0x00020000
> > 0x07d00000>;
> > +                               };
> > +
> > +                               partition@7d20000 {
> > +                                       label = "RedBoot";
> > +                                       reg = <0x07d20000
> > 0x002e0000>;
> > +                                       read-only;
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +
> > +       rtc1: rtc@10800000 {
>
> Same problems
>
> > +               compatible = "st,m48t86";
> > +               reg = <0x10800000 0x1>,
> > +                     <0x11700000 0x1>;
> > +       };
> > +
> > +       watchdog1: watchdog@23800000 {
>
> Same problems
>
> > +               compatible = "technologic,ts7200-wdt";
> > +               reg = <0x23800000 0x01>,
> > +                     <0x23c00000 0x01>;
> > +               timeout-sec = <30>;
> > +       };
> > +
> > +       leds {
> > +               compatible = "gpio-leds";
> > +               led-0 {
> > +                       label = "grled";
> > +                       gpios = <&gpio4 0 GPIO_ACTIVE_HIGH>;
> > +                       linux,default-trigger = "heartbeat";
> > +                       function = LED_FUNCTION_HEARTBEAT;
> > +               };
> > +
> > +               led-1 {
> > +                       label = "rdled";
> > +                       gpios = <&gpio4 1 GPIO_ACTIVE_HIGH>;
> > +                       function = LED_FUNCTION_FAULT;
> > +               };
> > +       };
> > +};
> > +
> > +&eth0 {
> > +       phy-handle = <&phy0>;
> > +};
> > +
> > +&gpio1 {
> > +       /* PWM */
> > +       gpio-ranges = <&pinctrl 6 163 1>;
> > +};
> > +
> > +&gpio4 {
> > +       gpio-ranges = <&pinctrl 0 97 2>;
> > +       status = "okay";
> > +};
> > +
> > +&gpio6 {
> > +       gpio-ranges = <&pinctrl 0 87 2>;
> > +       status = "okay";
> > +};
> > +
> > +&gpio7 {
> > +       gpio-ranges = <&pinctrl 2 199 4>;
> > +       status = "okay";
> > +};
> > +
> > +&i2c0 {
> > +       status = "okay";
> > +};
> > +
> > +&spi0 {
> > +       cs-gpios = <&gpio5 2 GPIO_ACTIVE_HIGH>;
> > +       dmas = <&dma1 EP93XX_DMA_SSP>;
> > +       status = "okay";
> > +
> > +       tmp122_spi: tmp122@0 {
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
>
> > +               compatible = "ti,tmp122";
> > +               reg = <0>;
> > +               spi-max-frequency = <2000000>;
> > +       };
> > +};
> > +
>
>
> Best regards,
> Krzysztof
>