[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <4AE6040B-5F3F-4CBE-9D2E-ABBD1B5605F6@geanix.com>
Date: Wed, 12 Jul 2023 11:17:42 +0200
From: Sean Nyekjaer <sean@...nix.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Ahmad Fatoum <a.fatoum@...gutronix.de>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Arnd Bergmann <arnd@...db.de>, Olof Johansson <olof@...om.net>,
soc@...nel.org, dantuguf14105@...il.com,
Olivier Moysan <olivier.moysan@...s.st.com>,
devicetree@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 8/8] ARM: dts: stm32: Add Octavo OSD32MP1-RED board
> On 12 Jul 2023, at 10.34, Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:
>
> On 12/07/2023 08:29, Sean Nyekjaer wrote:
>> Add support for the Octavo OSD32MP1-RED development board.
>>
>> General features:
>> - STM32MP157C
>> - 512MB DDR3
>> - CAN-FD
>> - HDMI
>> - USB-C OTG
>> - UART
>>
>
> ...
>
>> +
>> +&i2c1 {
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&i2c1_pins_a>;
>> + pinctrl-1 = <&i2c1_sleep_pins_a>;
>> + status = "okay";
>> + i2c-scl-rising-time-ns = <100>;
>> + i2c-scl-falling-time-ns = <7>;
>> + /delete-property/dmas;
>> + /delete-property/dma-names;
>
> 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 :)
>
>> +
>> + hdmi-transmitter@39 {
>> + compatible = "sil,sii9022";
>> + reg = <0x39>;
>> + reset-gpios = <&gpiog 0 GPIO_ACTIVE_LOW>;
>> + interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
>> + interrupt-parent = <&gpiog>;
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <<dc_pins_e>;
>> + pinctrl-1 = <<dc_sleep_pins_e>;
>> + status = "okay";
>
> Did anything disable this node?
No will remove.
>
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + sii9022_in: endpoint {
>> + remote-endpoint = <<dc_ep0_out>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + sii9022_tx_endpoint: endpoint {
>> + remote-endpoint = <&i2s2_endpoint>;
>> + };
>> + };
>> + };
>> + };
>> +};
>> +
>> +&i2s2 {
>> + clocks = <&rcc SPI2>, <&rcc SPI2_K>, <&rcc CK_PER>, <&rcc PLL3_R>;
>> + clock-names = "pclk", "i2sclk", "x8k", "x11k";
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&i2s2_pins_b>;
>> + pinctrl-1 = <&i2s2_sleep_pins_b>;
>> + status = "okay";
>> +
>> + i2s2_port: port {
>> + i2s2_endpoint: endpoint {
>> + remote-endpoint = <&sii9022_tx_endpoint>;
>> + format = "i2s";
>> + mclk-fs = <256>;
>> + };
>> + };
>> +};
>> +
>> +<dc {
>> + status = "okay";
>> +
>> + port {
>> + ltdc_ep0_out: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&sii9022_in>;
>> + };
>> + };
>> +};
>> +
>> +&m_can1 {
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&m_can1_pins_d>;
>> + pinctrl-1 = <&m_can1_sleep_pins_d>;
>> + status = "okay";
>> +};
>> +
>> +&pwr_regulators {
>> + vdd-supply = <&vdd>;
>> + vdd_3v3_usbfs-supply = <&vdd_usb>;
>> +};
>> +
>> +&rtc {
>> + status = "okay";
>> +};
>> +
>> +&sdmmc1 {
>> + pinctrl-names = "default", "opendrain", "sleep";
>> + pinctrl-0 = <&sdmmc1_b4_pins_a>;
>> + pinctrl-1 = <&sdmmc1_b4_od_pins_a>;
>> + pinctrl-2 = <&sdmmc1_b4_sleep_pins_a>;
>> + cd-gpios = <&gpioe 7 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
>> + disable-wp;
>> + st,neg-edge;
>> + bus-width = <4>;
>> + vmmc-supply = <&v3v3>;
>> + status = "okay";
>> +};
>> +
>> +&sdmmc2 {
>> + pinctrl-names = "default", "opendrain", "sleep";
>> + pinctrl-0 = <&sdmmc2_b4_pins_a &sdmmc2_d47_pins_d>;
>> + pinctrl-1 = <&sdmmc2_b4_od_pins_a>;
>> + pinctrl-2 = <&sdmmc2_b4_sleep_pins_a &sdmmc2_d47_sleep_pins_d>;
>> + non-removable;
>> + no-sd;
>> + no-sdio;
>> + st,neg-edge;
>> + bus-width = <8>;
>> + vmmc-supply = <&v3v3>;
>> + vqmmc-supply = <&vdd>;
>> + mmc-ddr-3_3v;
>> + status = "okay";
>> +};
>> +
>> +&uart4 {
>> + pinctrl-names = "default", "sleep", "idle";
>> + pinctrl-0 = <&uart4_pins_a>;
>> + pinctrl-1 = <&uart4_sleep_pins_a>;
>> + pinctrl-2 = <&uart4_idle_pins_a>;
>> + /delete-property/dmas;
>> + /delete-property/dma-names;
>
> Same concerns.
Thanks for the review Krzysztof.
/Sean
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists