Hi Geert, Thanks for this great review, I appreciate the time spent on me! > Hi Julien, > > On Mon, 27 Jan 2025 at 10:39, Julien Massot <julien.massot@xxxxxxxxxxxxx> wrote: > > Add basic support for the MyIR Remi Pi (based on r9a07g044l2): > > - UART > > - i2c > > - emmc > > - USB host > > - HDMI output > > - Ethernet > > > > Signed-off-by: Julien Massot <julien.massot@xxxxxxxxxxxxx> > > Thanks for your patch! > > > --- /dev/null > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-remi-pi.dts > > @@ -0,0 +1,388 @@ > > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +/* > > + * Device Tree Source for the MYiR Remi Pi > > + * > > + * Copyright (C) 2022 MYiR Electronics Corp. > > + * Copyright (C) 2025 Collabora Ltd. > > + */ > > + > > +/dts-v1/; > > + > > +#include <dt-bindings/gpio/gpio.h> > > +#include <dt-bindings/pinctrl/rzg2l-pinctrl.h> > > + > > +#include "r9a07g044l2.dtsi" > > + > > +/ { > > + model = "MYIR Tech Limited Remi Pi MYB-YG2LX-REMI"; > > + compatible = "myir,remi-pi", "renesas,r9a07g044l2", "renesas,r9a07g044"; > > + > > + aliases { > > + ethernet0 = ð0; > > + ethernet1 = ð1; > > + > > + serial0 = &scif0; > > So serial0 is the CA55 console, OK. > > > + serial1 = &scif1; > > Connected to BT/WiFi, but not yet enabled below. Will drop, I want to support the BT/WiFi chip, but not that easy to enable with the muxes on the SDHI1. Let's keep that for the future. > > > > + serial2 = &scif2; > > This is wired to the Raspberri Pi expansion connector. > However, on Raspberri Pi, these pins are GPIOs. > While the Raspberri Pi UART_[TR]X pins are wired to SCIF4 here, > which is not yet enabled... > > > Ok then let be nice and try to be closer to the normal Raspberry Pi header: I will enable SCIF4, and remove SCIF2. > > > + serial3 = &scif3; > > Serial3 is the CA33 debug console, OK. Let disable this one to leave it available for a potential RTOS. > > > + > > + i2c0 = &i2c0; > > + i2c1 = &i2c1; > > + i2c2 = &i2c2; > > + i2c3 = &i2c3; > > + > > + mmc0 = &sdhi0; > > + mmc1 = &sdhi1; > > sdhi1 is not yet enabled, so please drop it. Right > > > + }; > > > + reg_5p0v: regulator-5p0v { > > + compatible = "regulator-fixed"; > > + regulator-name = "fixed-5.0V"; > > + regulator-min-microvolt = <5000000>; > > + regulator-max-microvolt = <5000000>; > > + }; > > + > > + reg_3p3v: regulator-3p3v { > > + compatible = "regulator-fixed"; > > + regulator-name = "fixed-3.3V"; > > + vin-supply = <®_5p0v>; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-always-on; > > + }; > > + > > + reg_1p8v: regulator-1p8v { > > + compatible = "regulator-fixed"; > > + regulator-name = "fixed-1.8V"; > > + vin-supply = <®_3p3v>; > > reg_5p0v, as 1.8V is generated from 5.0V, according to the schematics. Nice catch! will change > > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + regulator-always-on; > > + }; > > + > > + reg_1p1v: regulator-vdd-core { > > + compatible = "regulator-fixed"; > > + regulator-name = "fixed-1.1V"; > > + regulator-min-microvolt = <1100000>; > > + regulator-max-microvolt = <1100000>; > > + regulator-always-on; > > + }; > > > +ð0 { > > + pinctrl-0 = <ð0_pins>; > > + pinctrl-names = "default"; > > + phy-handle = <&phy0>; > > + phy-mode = "rgmii-id"; > > + status = "okay"; > > + > > + phy0: ethernet-phy@4 { > > + compatible = "ethernet-phy-ieee802.3-c22"; > > + reg = <4>; > > + interrupts-extended = <&pinctrl RZG2L_GPIO(44, 2) IRQ_TYPE_LEVEL_LOW>; > > Missing reset-gpios (P44_3). ok > > > + }; > > +}; > > + > > +ð1 { > > + pinctrl-0 = <ð1_pins>; > > + pinctrl-names = "default"; > > + phy-handle = <&phy1>; > > + phy-mode = "rgmii-id"; > > + status = "okay"; > > + > > + phy1: ethernet-phy@6 { > > + compatible = "ethernet-phy-ieee802.3-c22"; > > + reg = <6>; > > + interrupts-extended = <&pinctrl RZG2L_GPIO(43, 2) IRQ_TYPE_LEVEL_LOW>; > > Missing reset-gpios (P43_3). ok > > > + }; > > +}; > > > +&pinctrl { > > > + spi0_pins: spi0 { > > + pinmux = <RZG2L_PORT_PINMUX(47, 0, 5)>, /* CLK */ > > + <RZG2L_PORT_PINMUX(47, 1, 5)>, /* MOSI */ > > + <RZG2L_PORT_PINMUX(47, 2, 5)>, /* MISO */ > > + <RZG2L_PORT_PINMUX(47, 3, 5)>; /* Chip Enable*/ > > + }; > > Unused. will drop > > > + > > + eth0_pins: eth0 { > > Please sort pinctrl subnodes (alphabetically by node name). ok > > > + sdhi0_pins: sd0 { > > + sd0_data { > > + pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3", > > + "SD0_DATA4", "SD0_DATA5", "SD0_DATA6", "SD0_DATA7"; > > + power-source = <1800>; > > + }; > > + > > + sd0_ctrl { > > + pins = "SD0_CLK", "SD0_CMD"; > > + power-source = <1800>; > > + }; > > + > > + sd0_rst { > > + pins = "SD0_RST#"; > > + power-source = <1800>; > > + }; > > + }; > > + > > + sdhi0_pins_uhs: sd0_uhs { > > + sd0_data_uhs { > > + pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3", > > + "SD0_DATA4", "SD0_DATA5", "SD0_DATA6", "SD0_DATA7"; > > + power-source = <1800>; > > + }; > > + > > + sd0_ctrl_uhs { > > + pins = "SD0_CLK", "SD0_CMD"; > > + power-source = <1800>; > > + }; > > + > > + sd0_rst_uhs { > > + pins = "SD0_RST#"; > > + power-source = <1800>; > > + }; > > + }; > > sd0 and sd0_uhs are identical, so you can just always use the former, > and drop the latter. Ok > > > + > > + usb1_pins: usb1 { > > + pinmux = <RZG2L_PORT_PINMUX(42, 0, 1)>, /* VBUS */ > > + <RZG2L_PORT_PINMUX(42, 1, 1)>; /* OVC */ > > These two pins don't seem to be exposed on the SoM. > Perhaps this is wired on the SoM? Honestly I don't know :), I copy paste them from downstream but I'm not seeing anything in the schematics regarding these pins. Let drop them. > > > > + }; > > + > > + scif0_pins: scif0 { > > + pinmux = <RZG2L_PORT_PINMUX(38, 0, 1)>, /* TxD */ > > + <RZG2L_PORT_PINMUX(38, 1, 1)>; /* RxD */ > > + }; > > + > > + scif1_pins: scif1 { > > + pinmux = <RZG2L_PORT_PINMUX(40, 0, 1)>, /* TxD */ > > + <RZG2L_PORT_PINMUX(40, 1, 1)>; /* RxD */ > > + }; > > SCIF1 is connected to BT/WiFi, and the CTS/RTS pins should be included. > However, for now SCIF1 is not yet enabled, so please everything related to it. > > Will drop SCIF1, until I come with an idea to support BT/WIFI chip. > > > > +&usb2_phy1 { > > + pinctrl-0 = <&usb1_pins>; > > + pinctrl-names = "default"; > > + > > + status = "okay"; > > +}; > > + > > +&ehci1 { > > Please sort references to nodes (alphabetically). More below... ok > > > +&dsi { > > + status = "okay"; > > Please insert a blank line. ok > > > + ports { > > > +&i2c0 { > > + pinctrl-0 = <&i2c0_pins>; > > + pinctrl-names = "default"; > > + > > + clock-frequency = <400000>; > > + status = "okay"; > > + > > + hdmi-bridge@48 { > > + compatible = "lontium,lt8912b"; > > + reg = <0x48> ; > > + reset-gpios = <&pinctrl RZG2L_GPIO(42, 2) GPIO_ACTIVE_LOW>; > > Missing interrupts (P16_1), but that is not yet supported by the > bindings, so I guess it's fine to leave it out for now. I will keep it as is. Regards, -- Julien