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
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 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 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
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
