Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\))
Subject: Re: [PATCH v3 8/8] ARM: dts: stm32: Add Octavo OSD32MP1-RED board
From:   Sean Nyekjaer <sean@geanix.com>
In-Reply-To: <219612e4-f306-9ce3-c75c-33195f1167f2@linaro.org>
Date:   Wed, 12 Jul 2023 11:17:42 +0200
Cc:     Ahmad Fatoum <a.fatoum@pengutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Arnd Bergmann <arnd@arndb.de>, Olof Johansson <olof@lixom.net>,
        soc@kernel.org, dantuguf14105@gmail.com,
        Olivier Moysan <olivier.moysan@foss.st.com>,
        devicetree@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <4AE6040B-5F3F-4CBE-9D2E-ABBD1B5605F6@geanix.com>
References: <20230712062954.2194505-1-sean@geanix.com>
 <20230712062954.2194505-8-sean@geanix.com>
 <219612e4-f306-9ce3-c75c-33195f1167f2@linaro.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Precedence: bulk


> On 12 Jul 2023, at 10.34, Krzysztof Kozlowski =
<krzysztof.kozlowski@linaro.org> wrote:
>=20
> On 12/07/2023 08:29, Sean Nyekjaer wrote:
>> Add support for the Octavo OSD32MP1-RED development board.
>>=20
>> General features:
>> - STM32MP157C
>> - 512MB DDR3
>> - CAN-FD
>> - HDMI
>> - USB-C OTG
>> - UART
>>=20
>=20
> ...
>=20
>> +
>> +&i2c1 {
>> + pinctrl-names =3D "default", "sleep";
>> + pinctrl-0 =3D <&i2c1_pins_a>;
>> + pinctrl-1 =3D <&i2c1_sleep_pins_a>;
>> + status =3D "okay";
>> + i2c-scl-rising-time-ns =3D <100>;
>> + i2c-scl-falling-time-ns =3D <7>;
>> + /delete-property/dmas;
>> + /delete-property/dma-names;
>=20
> You should explain it with comment, unless it is quite common for all
> STM32 boards to disable DMA for I2C...

Quite common for all STM32 boards, but I will add a comment anyway :)

>=20
>> +
>> + hdmi-transmitter@39 {
>> + compatible =3D "sil,sii9022";
>> + reg =3D <0x39>;
>> + reset-gpios =3D <&gpiog 0 GPIO_ACTIVE_LOW>;
>> + interrupts =3D <1 IRQ_TYPE_EDGE_FALLING>;
>> + interrupt-parent =3D <&gpiog>;
>> + pinctrl-names =3D "default", "sleep";
>> + pinctrl-0 =3D <&ltdc_pins_e>;
>> + pinctrl-1 =3D <&ltdc_sleep_pins_e>;
>> + status =3D "okay";
>=20
> Did anything disable this node?

No will remove.

>=20
>> +
>> + ports {
>> + #address-cells =3D <1>;
>> + #size-cells =3D <0>;
>> +
>> + port@0 {
>> + reg =3D <0>;
>> + sii9022_in: endpoint {
>> + remote-endpoint =3D <&ltdc_ep0_out>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg =3D <1>;
>> + sii9022_tx_endpoint: endpoint {
>> + remote-endpoint =3D <&i2s2_endpoint>;
>> + };
>> + };
>> + };
>> + };
>> +};
>> +
>> +&i2s2 {
>> + clocks =3D <&rcc SPI2>, <&rcc SPI2_K>, <&rcc CK_PER>, <&rcc =
PLL3_R>;
>> + clock-names =3D "pclk", "i2sclk", "x8k", "x11k";
>> + pinctrl-names =3D "default", "sleep";
>> + pinctrl-0 =3D <&i2s2_pins_b>;
>> + pinctrl-1 =3D <&i2s2_sleep_pins_b>;
>> + status =3D "okay";
>> +
>> + i2s2_port: port {
>> + i2s2_endpoint: endpoint {
>> + remote-endpoint =3D <&sii9022_tx_endpoint>;
>> + format =3D "i2s";
>> + mclk-fs =3D <256>;
>> + };
>> + };
>> +};
>> +
>> +&ltdc {
>> + status =3D "okay";
>> +
>> + port {
>> + ltdc_ep0_out: endpoint@0 {
>> + reg =3D <0>;
>> + remote-endpoint =3D <&sii9022_in>;
>> + };
>> + };
>> +};
>> +
>> +&m_can1 {
>> + pinctrl-names =3D "default", "sleep";
>> + pinctrl-0 =3D <&m_can1_pins_d>;
>> + pinctrl-1 =3D <&m_can1_sleep_pins_d>;
>> + status =3D "okay";
>> +};
>> +
>> +&pwr_regulators {
>> + vdd-supply =3D <&vdd>;
>> + vdd_3v3_usbfs-supply =3D <&vdd_usb>;
>> +};
>> +
>> +&rtc {
>> + status =3D "okay";
>> +};
>> +
>> +&sdmmc1 {
>> + pinctrl-names =3D "default", "opendrain", "sleep";
>> + pinctrl-0 =3D <&sdmmc1_b4_pins_a>;
>> + pinctrl-1 =3D <&sdmmc1_b4_od_pins_a>;
>> + pinctrl-2 =3D <&sdmmc1_b4_sleep_pins_a>;
>> + cd-gpios =3D <&gpioe 7 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
>> + disable-wp;
>> + st,neg-edge;
>> + bus-width =3D <4>;
>> + vmmc-supply =3D <&v3v3>;
>> + status =3D "okay";
>> +};
>> +
>> +&sdmmc2 {
>> + pinctrl-names =3D "default", "opendrain", "sleep";
>> + pinctrl-0 =3D <&sdmmc2_b4_pins_a &sdmmc2_d47_pins_d>;
>> + pinctrl-1 =3D <&sdmmc2_b4_od_pins_a>;
>> + pinctrl-2 =3D <&sdmmc2_b4_sleep_pins_a &sdmmc2_d47_sleep_pins_d>;
>> + non-removable;
>> + no-sd;
>> + no-sdio;
>> + st,neg-edge;
>> + bus-width =3D <8>;
>> + vmmc-supply =3D <&v3v3>;
>> + vqmmc-supply =3D <&vdd>;
>> + mmc-ddr-3_3v;
>> + status =3D "okay";
>> +};
>> +
>> +&uart4 {
>> + pinctrl-names =3D "default", "sleep", "idle";
>> + pinctrl-0 =3D <&uart4_pins_a>;
>> + pinctrl-1 =3D <&uart4_sleep_pins_a>;
>> + pinctrl-2 =3D <&uart4_idle_pins_a>;
>> + /delete-property/dmas;
>> + /delete-property/dma-names;
>=20
> Same concerns.

Thanks for the review Krzysztof.

/Sean

>=20
> Best regards,
> Krzysztof
>=20