Re: [PATCH V2 14/14] ARM: dts: stm32: Split AV96 into DHCOR SoM and AV96 board
Hi Marek On 3/31/20 2:48 AM, Marek Vasut wrote: > The AV96 is in fact an assembly of DH Electronics DHCOR SoM on top > of an AV96 reference board. Split the DTs to reflect that and make > sure to DHCOR SoM can be reused on other boards easily. > > It is also highly recommended to configure the board for the DHCOM > make stm32mp15_dhcom_basic_defconfig > make DEVICE_TREE=stm32mp15xx-dhcor-avenger96 > as that permits reusing the board code for the DH components, like > accessing and reading out the ethernet MAC from EEPROM. > > Signed-off-by: Marek Vasut > Cc: Patrick Delaunay > Cc: Patrice Chotard Reviewed-by: Patrice Chotard Thanks > --- > V2: No change > --- > arch/arm/dts/Makefile | 3 +- > arch/arm/dts/stm32mp157a-avenger96.dts| 421 +- > .../stm32mp15xx-dhcor-avenger96-u-boot.dtsi | 80 > arch/arm/dts/stm32mp15xx-dhcor-avenger96.dts | 211 + > ...oot.dtsi => stm32mp15xx-dhcor-u-boot.dtsi} | 79 +--- > arch/arm/dts/stm32mp15xx-dhcor.dtsi | 231 ++ > doc/board/st/stm32mp1.rst | 8 +- > 7 files changed, 535 insertions(+), 498 deletions(-) > create mode 100644 arch/arm/dts/stm32mp15xx-dhcor-avenger96-u-boot.dtsi > create mode 100644 arch/arm/dts/stm32mp15xx-dhcor-avenger96.dts > rename arch/arm/dts/{stm32mp157a-avenger96-u-boot.dtsi => > stm32mp15xx-dhcor-u-boot.dtsi} (75%) > create mode 100644 arch/arm/dts/stm32mp15xx-dhcor.dtsi > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index 9c593b2c98..2564f790de 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -884,7 +884,8 @@ dtb-$(CONFIG_STM32MP15x) += \ > stm32mp157c-dk2.dtb \ > stm32mp157c-ed1.dtb \ > stm32mp157c-ev1.dtb \ > - stm32mp15xx-dhcom-pdk2.dtb > + stm32mp15xx-dhcom-pdk2.dtb \ > + stm32mp15xx-dhcor-avenger96.dtb > > dtb-$(CONFIG_SOC_K3_AM6) += k3-am654-base-board.dtb > k3-am654-r5-base-board.dtb > dtb-$(CONFIG_SOC_K3_J721E) += k3-j721e-common-proc-board.dtb \ > diff --git a/arch/arm/dts/stm32mp157a-avenger96.dts > b/arch/arm/dts/stm32mp157a-avenger96.dts > index 4fa20bc31d..9c165104fb 100644 > --- a/arch/arm/dts/stm32mp157a-avenger96.dts > +++ b/arch/arm/dts/stm32mp157a-avenger96.dts > @@ -4,422 +4,5 @@ > * Author: Manivannan Sadhasivam > */ > > -/dts-v1/; > - > -#include "stm32mp157c.dtsi" > -#include "stm32mp157xac-pinctrl.dtsi" > -#include > -#include > - > -/ { > - model = "Arrow Electronics STM32MP157A Avenger96 board"; > - compatible = "arrow,stm32mp157a-avenger96", "st,stm32mp157"; > - > - aliases { > - ethernet0 = ðernet0; > - mmc0 = &sdmmc1; > - serial0 = &uart4; > - serial1 = &uart7; > - spi0 = &qspi; > - }; > - > - chosen { > - stdout-path = "serial0:115200n8"; > - }; > - > - memory@c000 { > - device_type = "memory"; > - reg = <0xc000 0x4000>; > - }; > - > - led { > - compatible = "gpio-leds"; > - led1 { > - label = "green:user1"; > - gpios = <&gpioz 7 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "heartbeat"; > - default-state = "off"; > - }; > - > - led2 { > - label = "green:user2"; > - gpios = <&gpiof 3 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "mmc0"; > - default-state = "off"; > - }; > - > - led3 { > - label = "green:user3"; > - gpios = <&gpiog 0 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "mmc1"; > - default-state = "off"; > - }; > - > - led4 { > - label = "green:user3"; > - gpios = <&gpiog 1 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "none"; > - default-state = "off"; > - panic-indicator; > - }; > - > - led5 { > - label = "yellow:wifi"; > - gpios = <&gpioz 3 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "phy0tx"; > - default-state = "off"; > - }; > - > - led6 { > - label = "blue:bt"; > - gpios = <&gpioz 6 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "bluetooth-power"; > - default-state = "off"; > - }; > - }; > - > - sd_switch: regulator-sd_switch { > - compatible = "regulator-gpio"; > - regulator-name = "sd_switch"; > - regulator-min-microvolt = <180>; > - regulator-max-microvolt = <290>; > - regulator-type = "voltage"; > - regulator-always-on
RE: [PATCH V2 14/14] ARM: dts: stm32: Split AV96 into DHCOR SoM and AV96 board
Hi, > From: Marek Vasut > Sent: mardi 31 mars 2020 02:49 > > The AV96 is in fact an assembly of DH Electronics DHCOR SoM on top of an > AV96 reference board. Split the DTs to reflect that and make sure to DHCOR SoM > can be reused on other boards easily. > > It is also highly recommended to configure the board for the DHCOM make > stm32mp15_dhcom_basic_defconfig make DEVICE_TREE=stm32mp15xx-dhcor- > avenger96 > as that permits reusing the board code for the DH components, like accessing > and > reading out the ethernet MAC from EEPROM. Recommended or mandatory... For my point of view - board/st/stm32mp1 manage the ST board (STM32MP15x-DKX and STM32MP15x-EV1) Can be used as starting point for customer new board - board/dhelectronics/dh_stm32mp1 manage the board based on DHCOR SoM or can be a starting point of SoM user For AV96, the first upstream was directly based on ST board, but I agree : it is clearly not a perfect solution (MAC address issue for example) => support on this board should be in dh_stm32mp1 board or create a new board AV96 based on it. And I need to continue to move in st/common the part common for all the STM32MP157 boards > Signed-off-by: Marek Vasut > Cc: Patrick Delaunay > Cc: Patrice Chotard > --- > V2: No change > --- > arch/arm/dts/Makefile | 3 +- > arch/arm/dts/stm32mp157a-avenger96.dts| 421 +- > .../stm32mp15xx-dhcor-avenger96-u-boot.dtsi | 80 > arch/arm/dts/stm32mp15xx-dhcor-avenger96.dts | 211 + ...oot.dtsi => > stm32mp15xx-dhcor-u-boot.dtsi} | 79 +--- > arch/arm/dts/stm32mp15xx-dhcor.dtsi | 231 ++ > doc/board/st/stm32mp1.rst | 8 +- > 7 files changed, 535 insertions(+), 498 deletions(-) create mode 100644 > arch/arm/dts/stm32mp15xx-dhcor-avenger96-u-boot.dtsi > create mode 100644 arch/arm/dts/stm32mp15xx-dhcor-avenger96.dts > rename arch/arm/dts/{stm32mp157a-avenger96-u-boot.dtsi => stm32mp15xx- > dhcor-u-boot.dtsi} (75%) create mode 100644 arch/arm/dts/stm32mp15xx- > dhcor.dtsi > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index > 9c593b2c98..2564f790de 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -884,7 +884,8 @@ dtb-$(CONFIG_STM32MP15x) += \ > stm32mp157c-dk2.dtb \ > stm32mp157c-ed1.dtb \ > stm32mp157c-ev1.dtb \ > - stm32mp15xx-dhcom-pdk2.dtb > + stm32mp15xx-dhcom-pdk2.dtb \ > + stm32mp15xx-dhcor-avenger96.dtb Force device tree support for each target ? Avoid to mix incompatible device tree and defconfig dtb-$(TARGET_ST_STM32MP15x) += \ stm32mp157a-dk1.dtb \ stm32mp157c-dk2.dtb \ stm32mp157c-ed1.dtb \ stm32mp157c-ev1.dtb dtb-$(TARGET_DH_STM32MP1_PDK2) += \ stm32mp15xx-dhcom-pdk2.dtb \ stm32mp15xx-dhcor-avenger96.dtb > dtb-$(CONFIG_SOC_K3_AM6) += k3-am654-base-board.dtb k3-am654-r5-base- > board.dtb > dtb-$(CONFIG_SOC_K3_J721E) += k3-j721e-common-proc-board.dtb \ diff --git > a/arch/arm/dts/stm32mp157a-avenger96.dts b/arch/arm/dts/stm32mp157a- > avenger96.dts > index 4fa20bc31d..9c165104fb 100644 > --- a/arch/arm/dts/stm32mp157a-avenger96.dts > +++ b/arch/arm/dts/stm32mp157a-avenger96.dts > @@ -4,422 +4,5 @@ > * Author: Manivannan Sadhasivam > */ > > -/dts-v1/; > - > -#include "stm32mp157c.dtsi" > -#include "stm32mp157xac-pinctrl.dtsi" > -#include > -#include > - > -/ { > - model = "Arrow Electronics STM32MP157A Avenger96 board"; > - compatible = "arrow,stm32mp157a-avenger96", "st,stm32mp157"; > - > - aliases { > - ethernet0 = ðernet0; > - mmc0 = &sdmmc1; > - serial0 = &uart4; > - serial1 = &uart7; > - spi0 = &qspi; > - }; > - > - chosen { > - stdout-path = "serial0:115200n8"; > - }; > - > - memory@c000 { > - device_type = "memory"; > - reg = <0xc000 0x4000>; > - }; > - > - led { > - compatible = "gpio-leds"; > - led1 { > - label = "green:user1"; > - gpios = <&gpioz 7 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "heartbeat"; > - default-state = "off"; > - }; > - > - led2 { > - label = "green:user2"; > - gpios = <&gpiof 3 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "mmc0"; > - default-state = "off"; > - }; > - > - led3 { > - label = "green:user3"; > - gpios = <&gpiog 0 GPIO_ACTIVE_HIGH>; > - linux,default-trigger = "mmc1"; > - default-state = "off"; > - }; > - > - led4 { > - label = "green:user3"; > - gpios = <&gpiog 1 GPIO_ACTIVE_HIGH>; > -
Re: [PATCH V2 14/14] ARM: dts: stm32: Split AV96 into DHCOR SoM and AV96 board
On 3/31/20 4:59 PM, Patrick DELAUNAY wrote: > Hi, Hi, >> It is also highly recommended to configure the board for the DHCOM make >> stm32mp15_dhcom_basic_defconfig make DEVICE_TREE=stm32mp15xx-dhcor- >> avenger96 >> as that permits reusing the board code for the DH components, like accessing >> and >> reading out the ethernet MAC from EEPROM. > > Recommended or mandatory... Both work, the later provides more complete solution. > For my point of view > > - board/st/stm32mp1 manage the ST board (STM32MP15x-DKX and STM32MP15x-EV1) > Can be used as starting point for customer > new board > > - board/dhelectronics/dh_stm32mp1 manage the board based on DHCOR SoM or can > be a starting point of SoM user > > For AV96, the first upstream was directly based on ST board, but I agree : it > is clearly not a perfect solution (MAC address issue for example) Yes indeed. > => support on this board should be in dh_stm32mp1 board or create a new board > AV96 based on it. > > And I need to continue to move in st/common the part common for all the > STM32MP157 boards [...] >> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index >> 9c593b2c98..2564f790de 100644 >> --- a/arch/arm/dts/Makefile >> +++ b/arch/arm/dts/Makefile >> @@ -884,7 +884,8 @@ dtb-$(CONFIG_STM32MP15x) += \ >> stm32mp157c-dk2.dtb \ >> stm32mp157c-ed1.dtb \ >> stm32mp157c-ev1.dtb \ >> -stm32mp15xx-dhcom-pdk2.dtb >> +stm32mp15xx-dhcom-pdk2.dtb \ >> +stm32mp15xx-dhcor-avenger96.dtb > > Force device tree support for each target ? > Avoid to mix incompatible device tree and defconfig > > dtb-$(TARGET_ST_STM32MP15x) += \ > stm32mp157a-dk1.dtb \ > stm32mp157c-dk2.dtb \ > stm32mp157c-ed1.dtb \ > stm32mp157c-ev1.dtb > > dtb-$(TARGET_DH_STM32MP1_PDK2) += \ > stm32mp15xx-dhcom-pdk2.dtb \ > stm32mp15xx-dhcor-avenger96.dtb You probably want to build all DTs for STM32MP1 when building STM32MP1 platforms ? [...] (please, learn to trim the responses in email) >> -&usbphyc { >> -status = "okay"; >> -}; >> - >> -&usbphyc_port0 { >> -phy-supply = <&vdd_usb>; >> -}; >> - >> -&usbphyc_port1 { >> -phy-supply = <&vdd_usb>; >> -}; > > When this file will be removed ? why kept this file. > I propose to completely remove this file (no device tree for same board) Backward compatibility, I'd keep it in for a release or two. But if removing it is fine, then so be it. >> +/* This is kept for backward compatibility and will be removed */ >> +#include "stm32mp15xx-dhcor-avenger96.dts" > > Missing u-boot file to avoid issue.. > > +#include " stm32mp15xx-dhcor-avenger96-u-boot.dtsi" That's actually included via the avenger96.dts, so should be OK. Although I am not real happy with that. [...] >> Build Procedure >> --- >> @@ -229,7 +229,7 @@ Build Procedure >> >> # export KBUILD_OUTPUT=stm32mp15_basic >> # make stm32mp15_basic_defconfig >> - # make DEVICE_TREE=stm32mp157a-avenger96 all >> + # make DEVICE_TREE=stm32mp15xx-dhcor-avenger96 all >> >> 6. Output files > > Reference could be removed if support if done by > stm32mp15_dhcom_basic_defconfig / dh_stm32mp1 board See my first question about the defconfig.
RE: [PATCH V2 14/14] ARM: dts: stm32: Split AV96 into DHCOR SoM and AV96 board
Hi, > From: Marek Vasut > Sent: mardi 31 mars 2020 18:38 > > On 3/31/20 4:59 PM, Patrick DELAUNAY wrote: > > Hi, > > Hi, > > >> It is also highly recommended to configure the board for the DHCOM > >> make stm32mp15_dhcom_basic_defconfig make > >> DEVICE_TREE=stm32mp15xx-dhcor- > >> avenger96 > >> as that permits reusing the board code for the DH components, like > >> accessing and reading out the ethernet MAC from EEPROM. > > > > Recommended or mandatory... > > Both work, the later provides more complete solution. Ok, today both work. [...] > >> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index > >> 9c593b2c98..2564f790de 100644 > >> --- a/arch/arm/dts/Makefile > >> +++ b/arch/arm/dts/Makefile > >> @@ -884,7 +884,8 @@ dtb-$(CONFIG_STM32MP15x) += \ > >>stm32mp157c-dk2.dtb \ > >>stm32mp157c-ed1.dtb \ > >>stm32mp157c-ev1.dtb \ > >> - stm32mp15xx-dhcom-pdk2.dtb > >> + stm32mp15xx-dhcom-pdk2.dtb \ > >> + stm32mp15xx-dhcor-avenger96.dtb > > > > Force device tree support for each target ? > > Avoid to mix incompatible device tree and defconfig > > > > dtb-$(TARGET_ST_STM32MP15x) += \ > > stm32mp157a-dk1.dtb \ > > stm32mp157c-dk2.dtb \ > > stm32mp157c-ed1.dtb \ > > stm32mp157c-ev1.dtb > > > > dtb-$(TARGET_DH_STM32MP1_PDK2) += \ > > stm32mp15xx-dhcom-pdk2.dtb \ > > stm32mp15xx-dhcor-avenger96.dtb > > You probably want to build all DTs for STM32MP1 when building STM32MP1 > platforms ? I use buildman to compile all the stm32mp15x target / defconfig, so for me it is not mandatory. In his makefile, the 2 strategy exist dtb-$(CONFIG_ARCH_) dtb-$(CONFIG_TARGET_) I have no clear preference I just highlight that using TARGET avoid bad configuration between defconfig and associated device tree. > [...] > > (please, learn to trim the responses in email) Yes sorry. Life is lifelong learning. > >> -&usbphyc { > >> - status = "okay"; > >> -}; > >> - > >> -&usbphyc_port0 { > >> - phy-supply = <&vdd_usb>; > >> -}; > >> - > >> -&usbphyc_port1 { > >> - phy-supply = <&vdd_usb>; > >> -}; > > > > When this file will be removed ? why kept this file. > > I propose to completely remove this file (no device tree for same > > board) > > Backward compatibility, I'd keep it in for a release or two. > But if removing it is fine, then so be it. Yes I prefer. Except if maintainers of this file have a other opinion. > >> +/* This is kept for backward compatibility and will be removed */ > >> +#include "stm32mp15xx-dhcor-avenger96.dts" > > > > Missing u-boot file to avoid issue.. > > > > +#include " stm32mp15xx-dhcor-avenger96-u-boot.dtsi" > > That's actually included via the avenger96.dts, so should be OK. > Although I am not real happy with that. Ok, I miss that. I agree, it is unexpected. Regards Patrick
Re: [PATCH V2 14/14] ARM: dts: stm32: Split AV96 into DHCOR SoM and AV96 board
On 4/1/20 10:37 AM, Patrick DELAUNAY wrote: > Hi, [...] >>> When this file will be removed ? why kept this file. >>> I propose to completely remove this file (no device tree for same >>> board) >> >> Backward compatibility, I'd keep it in for a release or two. >> But if removing it is fine, then so be it. > > Yes I prefer. > Except if maintainers of this file have a other opinion. OK +/* This is kept for backward compatibility and will be removed */ +#include "stm32mp15xx-dhcor-avenger96.dts" >>> >>> Missing u-boot file to avoid issue.. >>> >>> +#include " stm32mp15xx-dhcor-avenger96-u-boot.dtsi" >> >> That's actually included via the avenger96.dts, so should be OK. >> Although I am not real happy with that. > > Ok, I miss that. > I agree, it is unexpected. It's a bit weird, I think this should be added to the list of things to improve in the next cycle. -- Best regards, Marek Vasut