| Message ID | 20230524143631.42471-1-francesco@dolcini.it |
|---|---|
| Headers | show |
| Series | Add Toradex Verdin AM62 | expand |
On Wed, May 24, 2023 at 04:36:29PM +0200, Francesco Dolcini wrote: > From: Francesco Dolcini <francesco.dolcini@toradex.com> > > This patch adds the device tree to support Toradex Verdin AM62 a > computer on module which can be used on different carrier boards > and the Toradex Verdin Development Board carrier board. > > The module consists of an TI AM62 family SoC (either AM623 or AM625), a > TPS65219 PMIC, a Gigabit Ethernet PHY, 512MB to 2GB of LPDDR4 RAM, an > eMMC, a TLA2024 ADC, an I2C EEPROM, an RX8130 RTC, and optional Parallel > RGB to MIPI DSI bridge plus an optional Bluetooth/Wi-Fi module. > > Anything that is not self-contained on the module is disabled by > default. > > So far there is no display nor USB role switch supported, apart of that > all the other functionalities are fine. > > Link: https://developer.toradex.com/hardware/verdin-som-family/modules/verdin-am62/ > Link: https://www.toradex.com/computer-on-modules/verdin-arm-family/ti-am62 > Link: https://www.toradex.com/products/carrier-board/verdin-development-board-kit > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > --- > arch/arm64/boot/dts/ti/Makefile | 2 + > .../arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi | 233 +++ > .../boot/dts/ti/k3-am62-verdin-nonwifi.dtsi | 16 + > .../boot/dts/ti/k3-am62-verdin-wifi.dtsi | 36 + > arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi | 1400 +++++++++++++++++ > .../dts/ti/k3-am625-verdin-nonwifi-dev.dts | 19 + > .../boot/dts/ti/k3-am625-verdin-wifi-dev.dts | 19 + > 7 files changed, 1725 insertions(+) > create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi > create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-nonwifi.dtsi > create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi > create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi > create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-nonwifi-dev.dts > create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-wifi-dev.dts > <snip> > +/* Verdin UART_1, connector X50 through RS485 transceiver. */ > +&main_uart1 { > + linux,rs485-enabled-at-boot-time; > + /* > + * The 8250 OMAP driver interprets rs485-rts-active-high and its > + * ioctl equivalent as driving RTS low on send. > + */ > + rs485-rts-active-high; After some additional investigation this (rs485-rts-active-high) can be removed, I'll send a v2 before the end of the week with just this change unless I get some more feedback in the meantime. Francesco
On 16:36-20230524, Francesco Dolcini wrote: [...] > diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi > new file mode 100644 > index 000000000000..e138b1c8ed14 > --- /dev/null > +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi > @@ -0,0 +1,233 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > +/* > + * Copyright 2023 Toradex Please also add appropriate product links to dts/dtsi > + */ > + > +/ { > + sound_card: sound-card { > + compatible = "simple-audio-card"; > + simple-audio-card,bitclock-master = <&dailink_master>; > + simple-audio-card,format = "i2s"; > + simple-audio-card,frame-master = <&dailink_master>; > + simple-audio-card,name = "verdin-nau8822"; > + simple-audio-card,routing = > + "Headphones", "LHP", > + "Headphones", "RHP", > + "Speaker", "LSPK", > + "Speaker", "RSPK", > + "Line Out", "AUXOUT1", > + "Line Out", "AUXOUT2", > + "LAUX", "Line In", > + "RAUX", "Line In", > + "LMICP", "Mic In", > + "RMICP", "Mic In"; > + simple-audio-card,widgets = > + "Headphones", "Headphones", > + "Line Out", "Line Out", > + "Speaker", "Speaker", > + "Microphone", "Mic In", > + "Line", "Line In"; > + > + dailink_master: simple-audio-card,codec { > + clocks = <&k3_clks 157 10>; > + sound-dai = <&nau8822_1a>; > + }; > + > + simple-audio-card,cpu { > + sound-dai = <&mcasp0>; > + }; > + }; > +}; > + > +/* Verdin ETHs */ > +&cpsw3g { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_rgmii1 &pinctrl_rgmii2>; here and elsewhere: pinctrl-0 = <&pinctrl_rgmii1>, <&pinctrl_rgmii2>; > + status = "okay"; > +}; [...] > + /* EEPROM */ > + eeprom@57 { > + compatible = "st,24c02", "atmel,24c02"; checkpatch warns: DT compatible string "st,24c02" appears un-documented > + reg = <0x57>; > + pagesize = <16>; > + }; > +}; > + > +/* Verdin I2C_2_DSI */ > +&main_i2c2 { > + status = "okay"; Here and few other dtsis: you should set status along with pinmux. [...] > + > +/* Verdin UART_2 */ > +&wkup_uart0 { > + /* FIXME: WKUP UART0 is used by DM firmware */ > + status = "reserved"; If you do configure this in R5 SPL, you'd want to add the pinmux as well. > +}; > diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi > new file mode 100644 > index 000000000000..289db1666fc0 > --- /dev/null > +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi > @@ -0,0 +1,36 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > +/* > + * Copyright 2023 Toradex > + */ > + > +/ { > + wifi_pwrseq: wifi-pwrseq { > + compatible = "mmc-pwrseq-simple"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_wifi_en>; > + reset-gpios = <&main_gpio0 22 GPIO_ACTIVE_LOW>; > + }; > +}; > + > + Drop extra EoLs. [...] > diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi > new file mode 100644 > index 000000000000..2e7cb607df45 > --- /dev/null > +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi > @@ -0,0 +1,1400 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT Assuming you really intent this instead of purely GPL-2.0 [...] > +/* MDIO, shared by Verdin ETH_1 (On-module PHY) and Verdin ETH_2_RGMII */ > +&cpsw3g_mdio { > + assigned-clocks = <&k3_clks 157 20>; > + assigned-clock-parents = <&k3_clks 157 22>; > + assigned-clock-rates = <25000000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_eth_clock &pinctrl_mdio>; > + status = "disabled"; > + > + cpsw3g_phy0: ethernet-phy@0 { > + compatible = "ethernet-phy-id2000.a231"; Check binding - we don't include any compatibles that dont have yaml conversion done (pinctrl is the only exception). [...] > +/* TODO: Verdin DSI_1 / TIDSS */ Drop the TODO. [...]
Hi On 5/30/2023 5:40 PM, Nishanth Menon wrote: > On 16:36-20230524, Francesco Dolcini wrote: > [...] >> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi >> new file mode 100644 >> index 000000000000..e138b1c8ed14 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi >> @@ -0,0 +1,233 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT >> +/* >> + * Copyright 2023 Toradex > > Please also add appropriate product links to dts/dtsi > >> + */ >> + >> +/ { >> + sound_card: sound-card { >> + compatible = "simple-audio-card"; >> + simple-audio-card,bitclock-master = <&dailink_master>; >> + simple-audio-card,format = "i2s"; >> + simple-audio-card,frame-master = <&dailink_master>; >> + simple-audio-card,name = "verdin-nau8822"; >> + simple-audio-card,routing = >> + "Headphones", "LHP", >> + "Headphones", "RHP", >> + "Speaker", "LSPK", >> + "Speaker", "RSPK", >> + "Line Out", "AUXOUT1", >> + "Line Out", "AUXOUT2", >> + "LAUX", "Line In", >> + "RAUX", "Line In", >> + "LMICP", "Mic In", >> + "RMICP", "Mic In"; >> + simple-audio-card,widgets = >> + "Headphones", "Headphones", >> + "Line Out", "Line Out", >> + "Speaker", "Speaker", >> + "Microphone", "Mic In", >> + "Line", "Line In"; >> + >> + dailink_master: simple-audio-card,codec { >> + clocks = <&k3_clks 157 10>; >> + sound-dai = <&nau8822_1a>; >> + }; >> + >> + simple-audio-card,cpu { >> + sound-dai = <&mcasp0>; >> + }; >> + }; >> +}; >> + >> +/* Verdin ETHs */ >> +&cpsw3g { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_rgmii1 &pinctrl_rgmii2>; > > here and elsewhere: > pinctrl-0 = <&pinctrl_rgmii1>, <&pinctrl_rgmii2>; > > >> + status = "okay"; >> +}; > [...] >> + /* EEPROM */ >> + eeprom@57 { >> + compatible = "st,24c02", "atmel,24c02"; > > checkpatch warns: DT compatible string "st,24c02" appears un-documented > Checkpatch now seems outdated in favor of dtbs_check. DT schemas use regex for compatibles and thus simple grep in Doucmentation/devicetree-bindings/ doesn't help. Eg: In this case: Documentation/devicetree/bindings/eeprom/at24.yaml has the regex to cover the compatibles >> + reg = <0x57>; >> + pagesize = <16>; >> + }; >> +}; >> + >> +/* Verdin I2C_2_DSI */ >> +&main_i2c2 { >> + status = "okay"; > > Here and few other dtsis: > you should set status along with pinmux. > > [...] >> + >> +/* Verdin UART_2 */ >> +&wkup_uart0 { >> + /* FIXME: WKUP UART0 is used by DM firmware */ >> + status = "reserved"; > > If you do configure this in R5 SPL, you'd want to add the pinmux as > well. > >> +}; >> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi >> new file mode 100644 >> index 000000000000..289db1666fc0 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi >> @@ -0,0 +1,36 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT >> +/* >> + * Copyright 2023 Toradex >> + */ >> + >> +/ { >> + wifi_pwrseq: wifi-pwrseq { >> + compatible = "mmc-pwrseq-simple"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_wifi_en>; >> + reset-gpios = <&main_gpio0 22 GPIO_ACTIVE_LOW>; >> + }; >> +}; >> + >> + > > Drop extra EoLs. > > [...] > >> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi >> new file mode 100644 >> index 000000000000..2e7cb607df45 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi >> @@ -0,0 +1,1400 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > > Assuming you really intent this instead of purely GPL-2.0 > > [...] > >> +/* MDIO, shared by Verdin ETH_1 (On-module PHY) and Verdin ETH_2_RGMII */ >> +&cpsw3g_mdio { >> + assigned-clocks = <&k3_clks 157 20>; >> + assigned-clock-parents = <&k3_clks 157 22>; >> + assigned-clock-rates = <25000000>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_eth_clock &pinctrl_mdio>; >> + status = "disabled"; >> + >> + cpsw3g_phy0: ethernet-phy@0 { >> + compatible = "ethernet-phy-id2000.a231"; > > Check binding - we don't include any compatibles that dont have yaml > conversion done (pinctrl is the only exception). Same here ;) Documentation/devicetree/bindings/net/ethernet-phy.yaml covers it > > [...] > >> +/* TODO: Verdin DSI_1 / TIDSS */ > Drop the TODO. > > [...] >
On Tue, May 30, 2023 at 07:10:44AM -0500, Nishanth Menon wrote: > On 16:36-20230524, Francesco Dolcini wrote: > > +/* Verdin I2C_2_DSI */ > > +&main_i2c2 { > > + status = "okay"; > > Here and few other dtsis: > you should set status along with pinmux. This is already done in the SoM dtsi, same applies to the other comment you have on this pinmux topic. To rephrase what's hopefully is already written in the commit message/series description, or at least it was in my intention. The system is modular, with multiple SoM variant and multiple carrier boards. Standard interfaces are defined at the family level, e.g. already in the SoM, in the carrier board DT file peripherals are just enabled, the pinmux is already defined in the common som.dtsi [1][2][3] files and the carrier board just use those unless there is some kind of non-standard deviation. This prevents duplication and simplify writing device tree file for board that use standard Verdin family interfaces. This should be visible looking at this series in which 3 different boards (Dev, Yavia and Dahlia) are added. All of that is clearly defined in our datasheets and publicly available documentation. Francesco [1] arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi [2] arch/arm64/boot/dts/ti/k3-am62-verdin-nonwifi.dtsi [3] arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi
On 18:36-20230530, Francesco Dolcini wrote: > On Tue, May 30, 2023 at 07:10:44AM -0500, Nishanth Menon wrote: > > On 16:36-20230524, Francesco Dolcini wrote: > > > +/* Verdin I2C_2_DSI */ > > > +&main_i2c2 { > > > + status = "okay"; > > > > Here and few other dtsis: > > you should set status along with pinmux. > This is already done in the SoM dtsi, same applies to the other comment > you have on this pinmux topic. > > To rephrase what's hopefully is already written in the commit > message/series description, or at least it was in my intention. > > The system is modular, with multiple SoM variant and multiple carrier > boards. Standard interfaces are defined at the family level, e.g. > already in the SoM, in the carrier board DT file peripherals are just > enabled, the pinmux is already defined in the common som.dtsi [1][2][3] > files and the carrier board just use those unless there is some kind of > non-standard deviation. > > This prevents duplication and simplify writing device tree file for board > that use standard Verdin family interfaces. This should be visible > looking at this series in which 3 different boards (Dev, Yavia and > Dahlia) are added. It helps clarity if the node is marked "okay" when all the necessary properties required for operation (in this case pinmux) is enabled. I don't see a big change as a result. Just stops people from hunting for where pinmux is actually done.
On Tue, May 30, 2023 at 11:53:51AM -0500, Nishanth Menon wrote: > On 18:36-20230530, Francesco Dolcini wrote: > > On Tue, May 30, 2023 at 07:10:44AM -0500, Nishanth Menon wrote: > > > On 16:36-20230524, Francesco Dolcini wrote: > > > > +/* Verdin I2C_2_DSI */ > > > > +&main_i2c2 { > > > > + status = "okay"; > > > > > > Here and few other dtsis: > > > you should set status along with pinmux. > > This is already done in the SoM dtsi, same applies to the other comment > > you have on this pinmux topic. > > > > To rephrase what's hopefully is already written in the commit > > message/series description, or at least it was in my intention. > > > > The system is modular, with multiple SoM variant and multiple carrier > > boards. Standard interfaces are defined at the family level, e.g. > > already in the SoM, in the carrier board DT file peripherals are just > > enabled, the pinmux is already defined in the common som.dtsi [1][2][3] > > files and the carrier board just use those unless there is some kind of > > non-standard deviation. > > > > This prevents duplication and simplify writing device tree file for board > > that use standard Verdin family interfaces. This should be visible > > looking at this series in which 3 different boards (Dev, Yavia and > > Dahlia) are added. > > It helps clarity if the node is marked "okay" when all the necessary > properties required for operation (in this case pinmux) is enabled. I > don't see a big change as a result. Just stops people from hunting for > where pinmux is actually done. I would disagree here, I would prefer to keep this as it is. Of course, doing the change you request here is trivial, just some copy paste. The pinctrl is not all the necessary properties, you still need go hunting on the dtsi includes hierarchy to see that everything is there. And I think this is just fine, we do this just to avoid duplicating common stuff. What you get for sure is information duplication, with all the interfaces that are enabled getting the same pinctrl on multiple files. Just on this series you have 3 carrier boards, we have 1 more we should just send and they all share mostly the same pinmux. ... And the Verdin AM62 is really a very brand new product. From my point of view I would also lose some clarity, since the current structure, at least to some extent, helps understanding when a carrier board is deviating from the family specification. I hope this explanation gives some more context. Francesco
From: Francesco Dolcini <francesco.dolcini@toradex.com> This series adds support for the Toradex Verdin AM62 SoM which can be used on different carrier boards (Verdin Development Board, Dahlia and Yavia). The module consists of an TI AM62 family SoC (either AM623 or AM625), a TPS65219 PMIC, a Gigabit Ethernet PHY, 512MB to 2GB of LPDDR4 RAM, an eMMC, a TLA2024 ADC, an I2C EEPROM, an RX8130 RTC, and optional Parallel RGB to MIPI DSI bridge plus an optional Bluetooth/Wi-Fi module. Link: https://www.toradex.com/computer-on-modules/verdin-arm-family/ti-am62 Francesco Dolcini (5): dt-bindings: arm: ti: add toradex,verdin-am62 et al. arm64: defconfig: enable drivers for Verdin AM62 arm64: dts: ti: add verdin am62 arm64: dts: ti: add verdin am62 dahlia arm64: dts: ti: add verdin am62 yavia .../devicetree/bindings/arm/ti/k3.yaml | 20 + arch/arm64/boot/dts/ti/Makefile | 6 + .../boot/dts/ti/k3-am62-verdin-dahlia.dtsi | 214 +++ .../arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi | 233 +++ .../boot/dts/ti/k3-am62-verdin-nonwifi.dtsi | 16 + .../boot/dts/ti/k3-am62-verdin-wifi.dtsi | 36 + .../boot/dts/ti/k3-am62-verdin-yavia.dtsi | 202 +++ arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi | 1400 +++++++++++++++++ .../dts/ti/k3-am625-verdin-nonwifi-dahlia.dts | 19 + .../dts/ti/k3-am625-verdin-nonwifi-dev.dts | 19 + .../dts/ti/k3-am625-verdin-nonwifi-yavia.dts | 19 + .../dts/ti/k3-am625-verdin-wifi-dahlia.dts | 19 + .../boot/dts/ti/k3-am625-verdin-wifi-dev.dts | 19 + .../dts/ti/k3-am625-verdin-wifi-yavia.dts | 19 + arch/arm64/configs/defconfig | 3 + 15 files changed, 2244 insertions(+) create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-dahlia.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-dev.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-nonwifi.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-wifi.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin-yavia.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-nonwifi-dahlia.dts create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-nonwifi-dev.dts create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-nonwifi-yavia.dts create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-wifi-dahlia.dts create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-wifi-dev.dts create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-wifi-yavia.dts