| Message ID | 20220803110053.22598-2-pali@kernel.org |
|---|---|
| State | Rejected |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | arm: mvebu: Cleanup u-boot,dm-pre-reloc code | expand |
On 03.08.22 13:00, Pali Rohár wrote: > For future changes it is needed for have spi0 device tree reference in > every mvebu soc dts file even when it is unused. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > arch/arm/dts/armada-7040.dtsi | 3 +++ > arch/arm/dts/armada-8040.dtsi | 3 +++ > arch/arm/dts/cn9130.dtsi | 3 +++ > 3 files changed, 9 insertions(+) You're changing 64-bit Armada files here, but in the cover letter you mention that this patchset only addresses 32-bit mvebu DT files. So why are you making these changes here? One more comment below... > diff --git a/arch/arm/dts/armada-7040.dtsi b/arch/arm/dts/armada-7040.dtsi > index 039d30c72a8c..1fb21c6bfde0 100644 > --- a/arch/arm/dts/armada-7040.dtsi > +++ b/arch/arm/dts/armada-7040.dtsi > @@ -63,3 +63,6 @@ > marvell,function = <3>; > }; > }; > + > +spi0: &cp0_spi1 { > +}; Why is spi0 aliased now to "cp0_spi1"? In the Kernel DT files I see this: spi0 = &spi0; spi1 = &cp0_spi0; spi2 = &cp0_spi1; ... spi0: spi@510600 { Thanks, Stefan > diff --git a/arch/arm/dts/armada-8040.dtsi b/arch/arm/dts/armada-8040.dtsi > index eec5fa277405..608ff0d97f95 100644 > --- a/arch/arm/dts/armada-8040.dtsi > +++ b/arch/arm/dts/armada-8040.dtsi > @@ -87,3 +87,6 @@ > marvell,function = <3>; > }; > }; > + > +spi0: &cp1_spi1 { > +}; > diff --git a/arch/arm/dts/cn9130.dtsi b/arch/arm/dts/cn9130.dtsi > index 68b767a70639..a200276f7a2e 100644 > --- a/arch/arm/dts/cn9130.dtsi > +++ b/arch/arm/dts/cn9130.dtsi > @@ -71,3 +71,6 @@ > marvell,function = <3>; > }; > }; > + > +spi0: &cp0_spi1 { > +}; Viele Grüße, Stefan Roese
On Thursday 04 August 2022 16:51:47 Stefan Roese wrote: > On 03.08.22 13:00, Pali Rohár wrote: > > For future changes it is needed for have spi0 device tree reference in > > every mvebu soc dts file even when it is unused. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > arch/arm/dts/armada-7040.dtsi | 3 +++ > > arch/arm/dts/armada-8040.dtsi | 3 +++ > > arch/arm/dts/cn9130.dtsi | 3 +++ > > 3 files changed, 9 insertions(+) > > You're changing 64-bit Armada files here, but in the cover letter you > mention that this patchset only addresses 32-bit mvebu DT files. So > why are you making these changes here? The main issue is that 64-bit Armada DTS files are compiled also during 32-bit Armada builds. And 32-bit Armada DTS files needs spi0 reference because it is used in 32-bit Armada section in mvebu-u-boot.dtsi file. I know it is broken build system if during compilation of 32-bit SoC are compiled also unrelated 64-bit Armada DTS files in 32-bit mode. But I do not see currently easier solution than just define "harmless" reference. During 64-bit Armada builds is content of mvebu-u-boot.dtsi skipped as there is a 32-bit guard. Proper way, of course, would be to fix build system, so none 64-bit file is compiled (and let unused) during 32-bit build. > One more comment below... > > > diff --git a/arch/arm/dts/armada-7040.dtsi b/arch/arm/dts/armada-7040.dtsi > > index 039d30c72a8c..1fb21c6bfde0 100644 > > --- a/arch/arm/dts/armada-7040.dtsi > > +++ b/arch/arm/dts/armada-7040.dtsi > > @@ -63,3 +63,6 @@ > > marvell,function = <3>; > > }; > > }; > > + > > +spi0: &cp0_spi1 { > > +}; > > Why is spi0 aliased now to "cp0_spi1"? Because cp0_spi1 is used in U-Boot 64-bit Armada DTS files as "spi0" alias. Really. > In the Kernel DT files I see > this: > > spi0 = &spi0; > spi1 = &cp0_spi0; > spi2 = &cp0_spi1; > ... > spi0: spi@510600 { And you bring another issue :-) U-Boot DTS files for 64-bit Armada SoCs (expects 3720) differs from kernel DTS files. They are incompatible and nobody fixed this issue yet. I fixed it only for 64-bit Armada 3720 as we are using and maintaining Armada 3720 boards. So I just do not know what to do with remaining 64-bit A7040/A8040/CN9030 SoCs. I just added simple harmless change which do not change behavior nor output of those boards. Either somebody has to start working on fixing U-Boot support and DTS files for those boards to be compatible with Linux kernel. Or those SoCs stays incompatible with Linux and their technical debt in U-Boot code just grow. Or you as maintainer can decide that they are unmaintained (and ready for removal?). I'm just not going to do this big cleanup for A7040/A8040/CN9030 SoCs too. > Thanks, > Stefan > > > diff --git a/arch/arm/dts/armada-8040.dtsi b/arch/arm/dts/armada-8040.dtsi > > index eec5fa277405..608ff0d97f95 100644 > > --- a/arch/arm/dts/armada-8040.dtsi > > +++ b/arch/arm/dts/armada-8040.dtsi > > @@ -87,3 +87,6 @@ > > marvell,function = <3>; > > }; > > }; > > + > > +spi0: &cp1_spi1 { > > +}; > > diff --git a/arch/arm/dts/cn9130.dtsi b/arch/arm/dts/cn9130.dtsi > > index 68b767a70639..a200276f7a2e 100644 > > --- a/arch/arm/dts/cn9130.dtsi > > +++ b/arch/arm/dts/cn9130.dtsi > > @@ -71,3 +71,6 @@ > > marvell,function = <3>; > > }; > > }; > > + > > +spi0: &cp0_spi1 { > > +}; > > Viele Grüße, > Stefan Roese > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
On 04.08.22 17:07, Pali Rohár wrote: > On Thursday 04 August 2022 16:51:47 Stefan Roese wrote: >> On 03.08.22 13:00, Pali Rohár wrote: >>> For future changes it is needed for have spi0 device tree reference in >>> every mvebu soc dts file even when it is unused. >>> >>> Signed-off-by: Pali Rohár <pali@kernel.org> >>> --- >>> arch/arm/dts/armada-7040.dtsi | 3 +++ >>> arch/arm/dts/armada-8040.dtsi | 3 +++ >>> arch/arm/dts/cn9130.dtsi | 3 +++ >>> 3 files changed, 9 insertions(+) >> >> You're changing 64-bit Armada files here, but in the cover letter you >> mention that this patchset only addresses 32-bit mvebu DT files. So >> why are you making these changes here? > > The main issue is that 64-bit Armada DTS files are compiled also during > 32-bit Armada builds. And 32-bit Armada DTS files needs spi0 reference > because it is used in 32-bit Armada section in mvebu-u-boot.dtsi file. > > I know it is broken build system if during compilation of 32-bit SoC are > compiled also unrelated 64-bit Armada DTS files in 32-bit mode. But I do > not see currently easier solution than just define "harmless" reference. > > During 64-bit Armada builds is content of mvebu-u-boot.dtsi skipped as > there is a 32-bit guard. > > Proper way, of course, would be to fix build system, so none 64-bit file > is compiled (and let unused) during 32-bit build. I see. Thanks for the explanation. >> One more comment below... >> >>> diff --git a/arch/arm/dts/armada-7040.dtsi b/arch/arm/dts/armada-7040.dtsi >>> index 039d30c72a8c..1fb21c6bfde0 100644 >>> --- a/arch/arm/dts/armada-7040.dtsi >>> +++ b/arch/arm/dts/armada-7040.dtsi >>> @@ -63,3 +63,6 @@ >>> marvell,function = <3>; >>> }; >>> }; >>> + >>> +spi0: &cp0_spi1 { >>> +}; >> >> Why is spi0 aliased now to "cp0_spi1"? > > Because cp0_spi1 is used in U-Boot 64-bit Armada DTS files as "spi0" > alias. Really. > >> In the Kernel DT files I see >> this: >> >> spi0 = &spi0; >> spi1 = &cp0_spi0; >> spi2 = &cp0_spi1; >> ... >> spi0: spi@510600 { > > And you bring another issue :-) U-Boot DTS files for 64-bit Armada SoCs > (expects 3720) differs from kernel DTS files. They are incompatible and > nobody fixed this issue yet. I fixed it only for 64-bit Armada 3720 as > we are using and maintaining Armada 3720 boards. Yes, I am aware of this. > So I just do not know what to do with remaining 64-bit > A7040/A8040/CN9030 SoCs. I just added simple harmless change which do > not change behavior nor output of those boards. > > Either somebody has to start working on fixing U-Boot support and DTS > files for those boards to be compatible with Linux kernel. Or those SoCs > stays incompatible with Linux and their technical debt in U-Boot code > just grow. Or you as maintainer can decide that they are unmaintained > (and ready for removal?). > > I'm just not going to do this big cleanup for A7040/A8040/CN9030 SoCs too. Understood. Let's keep the situation in this unfortunate state a while longer. Hopefully someone will find the time to work on this. If nothing changes in a longer period we should think again about this. Thanks, Stefan >> Thanks, >> Stefan >> >>> diff --git a/arch/arm/dts/armada-8040.dtsi b/arch/arm/dts/armada-8040.dtsi >>> index eec5fa277405..608ff0d97f95 100644 >>> --- a/arch/arm/dts/armada-8040.dtsi >>> +++ b/arch/arm/dts/armada-8040.dtsi >>> @@ -87,3 +87,6 @@ >>> marvell,function = <3>; >>> }; >>> }; >>> + >>> +spi0: &cp1_spi1 { >>> +}; >>> diff --git a/arch/arm/dts/cn9130.dtsi b/arch/arm/dts/cn9130.dtsi >>> index 68b767a70639..a200276f7a2e 100644 >>> --- a/arch/arm/dts/cn9130.dtsi >>> +++ b/arch/arm/dts/cn9130.dtsi >>> @@ -71,3 +71,6 @@ >>> marvell,function = <3>; >>> }; >>> }; >>> + >>> +spi0: &cp0_spi1 { >>> +}; >> >> Viele Grüße, >> Stefan Roese >> >> -- >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de Viele Grüße, Stefan Roese
On Friday 05 August 2022 12:25:55 Stefan Roese wrote: > On 04.08.22 17:07, Pali Rohár wrote: > > On Thursday 04 August 2022 16:51:47 Stefan Roese wrote: > > > On 03.08.22 13:00, Pali Rohár wrote: > > > > For future changes it is needed for have spi0 device tree reference in > > > > every mvebu soc dts file even when it is unused. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > --- > > > > arch/arm/dts/armada-7040.dtsi | 3 +++ > > > > arch/arm/dts/armada-8040.dtsi | 3 +++ > > > > arch/arm/dts/cn9130.dtsi | 3 +++ > > > > 3 files changed, 9 insertions(+) > > > > > > You're changing 64-bit Armada files here, but in the cover letter you > > > mention that this patchset only addresses 32-bit mvebu DT files. So > > > why are you making these changes here? > > > > The main issue is that 64-bit Armada DTS files are compiled also during > > 32-bit Armada builds. And 32-bit Armada DTS files needs spi0 reference > > because it is used in 32-bit Armada section in mvebu-u-boot.dtsi file. > > > > I know it is broken build system if during compilation of 32-bit SoC are > > compiled also unrelated 64-bit Armada DTS files in 32-bit mode. But I do > > not see currently easier solution than just define "harmless" reference. > > > > During 64-bit Armada builds is content of mvebu-u-boot.dtsi skipped as > > there is a 32-bit guard. > > > > Proper way, of course, would be to fix build system, so none 64-bit file > > is compiled (and let unused) during 32-bit build. > > I see. Thanks for the explanation. I played a bit with build system and have there alternative patch instead of patch 1/5 which "fixes" makefile build system. From 78ebf95fdf4223ce62cc537a57739ba3adbf556f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> Date: Fri, 5 Aug 2022 12:45:19 +0200 Subject: [PATCH] arm: mvebu: dts: Build only arch-compatible dts files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 64-bit Armada DTS files are not build correctly during compilation of 32-bit Armada boards and vice versa. So fix makefile build system to compile only those dts files which are compatible for the current build (64-bit Armada DTS files only for 64-bit builds and 32-bit Armada DTS files only for 32-bit builds). Signed-off-by: Pali Rohár <pali@kernel.org> --- arch/arm/dts/Makefile | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index ceaa39e4b4d6..7330121dbaba 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -232,12 +232,8 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \ tegra210-p2571.dtb \ tegra210-p3450-0000.dtb +ifdef CONFIG_ARMADA_32BIT dtb-$(CONFIG_ARCH_MVEBU) += \ - armada-3720-db.dtb \ - armada-3720-espressobin.dtb \ - armada-3720-turris-mox.dtb \ - armada-3720-eDPU.dtb \ - armada-3720-uDPU.dtb \ armada-375-db.dtb \ armada-385-atl-x530.dtb \ armada-385-atl-x530DP.dtb \ @@ -247,12 +243,6 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ armada-388-gp.dtb \ armada-388-helios4.dtb \ armada-38x-controlcenterdc.dtb \ - armada-7040-db-nand.dtb \ - armada-7040-db.dtb \ - armada-8040-clearfog-gt-8k.dtb \ - armada-8040-db.dtb \ - armada-8040-mcbin.dtb \ - armada-8040-puzzle-m801.dtb \ armada-xp-crs305-1g-4s.dtb \ armada-xp-crs305-1g-4s-bit.dtb \ armada-xp-crs326-24g-2s.dtb \ @@ -263,7 +253,20 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ armada-xp-gp.dtb \ armada-xp-maxbcm.dtb \ armada-xp-synology-ds414.dtb \ - armada-xp-theadorable.dtb \ + armada-xp-theadorable.dtb +else +dtb-$(CONFIG_ARCH_MVEBU) += \ + armada-3720-db.dtb \ + armada-3720-espressobin.dtb \ + armada-3720-turris-mox.dtb \ + armada-3720-eDPU.dtb \ + armada-3720-uDPU.dtb \ + armada-7040-db-nand.dtb \ + armada-7040-db.dtb \ + armada-8040-clearfog-gt-8k.dtb \ + armada-8040-db.dtb \ + armada-8040-mcbin.dtb \ + armada-8040-puzzle-m801.dtb \ cn9130-db-A.dtb \ cn9130-db-B.dtb \ cn9131-db-A.dtb \ @@ -272,6 +275,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ cn9132-db-B.dtb \ cn9130-crb-A.dtb \ cn9130-crb-B.dtb +endif dtb-$(CONFIG_ARCH_SYNQUACER) += synquacer-sc2a11-developerbox.dtb dtb-$(CONFIG_ARCH_UNIPHIER_LD11) += \
On Friday 05 August 2022 12:25:55 Stefan Roese wrote: > On 04.08.22 17:07, Pali Rohár wrote: > > And you bring another issue :-) U-Boot DTS files for 64-bit Armada SoCs > > (expects 3720) differs from kernel DTS files. They are incompatible and > > nobody fixed this issue yet. I fixed it only for 64-bit Armada 3720 as > > we are using and maintaining Armada 3720 boards. > > Yes, I am aware of this. > > > So I just do not know what to do with remaining 64-bit > > A7040/A8040/CN9030 SoCs. I just added simple harmless change which do > > not change behavior nor output of those boards. > > > > Either somebody has to start working on fixing U-Boot support and DTS > > files for those boards to be compatible with Linux kernel. Or those SoCs > > stays incompatible with Linux and their technical debt in U-Boot code > > just grow. Or you as maintainer can decide that they are unmaintained > > (and ready for removal?). > > > > I'm just not going to do this big cleanup for A7040/A8040/CN9030 SoCs too. > > Understood. Let's keep the situation in this unfortunate state a while > longer. Hopefully someone will find the time to work on this. If > nothing changes in a longer period we should think again about this. Ok!
On 05.08.22 12:48, Pali Rohár wrote: > On Friday 05 August 2022 12:25:55 Stefan Roese wrote: >> On 04.08.22 17:07, Pali Rohár wrote: >>> On Thursday 04 August 2022 16:51:47 Stefan Roese wrote: >>>> On 03.08.22 13:00, Pali Rohár wrote: >>>>> For future changes it is needed for have spi0 device tree reference in >>>>> every mvebu soc dts file even when it is unused. >>>>> >>>>> Signed-off-by: Pali Rohár <pali@kernel.org> >>>>> --- >>>>> arch/arm/dts/armada-7040.dtsi | 3 +++ >>>>> arch/arm/dts/armada-8040.dtsi | 3 +++ >>>>> arch/arm/dts/cn9130.dtsi | 3 +++ >>>>> 3 files changed, 9 insertions(+) >>>> >>>> You're changing 64-bit Armada files here, but in the cover letter you >>>> mention that this patchset only addresses 32-bit mvebu DT files. So >>>> why are you making these changes here? >>> >>> The main issue is that 64-bit Armada DTS files are compiled also during >>> 32-bit Armada builds. And 32-bit Armada DTS files needs spi0 reference >>> because it is used in 32-bit Armada section in mvebu-u-boot.dtsi file. >>> >>> I know it is broken build system if during compilation of 32-bit SoC are >>> compiled also unrelated 64-bit Armada DTS files in 32-bit mode. But I do >>> not see currently easier solution than just define "harmless" reference. >>> >>> During 64-bit Armada builds is content of mvebu-u-boot.dtsi skipped as >>> there is a 32-bit guard. >>> >>> Proper way, of course, would be to fix build system, so none 64-bit file >>> is compiled (and let unused) during 32-bit build. >> >> I see. Thanks for the explanation. > > I played a bit with build system and have there alternative patch > instead of patch 1/5 which "fixes" makefile build system. Thanks Pali. > From 78ebf95fdf4223ce62cc537a57739ba3adbf556f Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> > Date: Fri, 5 Aug 2022 12:45:19 +0200 > Subject: [PATCH] arm: mvebu: dts: Build only arch-compatible dts files > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > 64-bit Armada DTS files are not build correctly during compilation of > 32-bit Armada boards and vice versa. So fix makefile build system to > compile only those dts files which are compatible for the current build > (64-bit Armada DTS files only for 64-bit builds and 32-bit Armada DTS files > only for 32-bit builds). > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > arch/arm/dts/Makefile | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index ceaa39e4b4d6..7330121dbaba 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -232,12 +232,8 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \ > tegra210-p2571.dtb \ > tegra210-p3450-0000.dtb > > +ifdef CONFIG_ARMADA_32BIT > dtb-$(CONFIG_ARCH_MVEBU) += \ > - armada-3720-db.dtb \ > - armada-3720-espressobin.dtb \ > - armada-3720-turris-mox.dtb \ > - armada-3720-eDPU.dtb \ > - armada-3720-uDPU.dtb \ > armada-375-db.dtb \ > armada-385-atl-x530.dtb \ > armada-385-atl-x530DP.dtb \ > @@ -247,12 +243,6 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > armada-388-gp.dtb \ > armada-388-helios4.dtb \ > armada-38x-controlcenterdc.dtb \ > - armada-7040-db-nand.dtb \ > - armada-7040-db.dtb \ > - armada-8040-clearfog-gt-8k.dtb \ > - armada-8040-db.dtb \ > - armada-8040-mcbin.dtb \ > - armada-8040-puzzle-m801.dtb \ > armada-xp-crs305-1g-4s.dtb \ > armada-xp-crs305-1g-4s-bit.dtb \ > armada-xp-crs326-24g-2s.dtb \ > @@ -263,7 +253,20 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > armada-xp-gp.dtb \ > armada-xp-maxbcm.dtb \ > armada-xp-synology-ds414.dtb \ > - armada-xp-theadorable.dtb \ > + armada-xp-theadorable.dtb > +else > +dtb-$(CONFIG_ARCH_MVEBU) += \ > + armada-3720-db.dtb \ > + armada-3720-espressobin.dtb \ > + armada-3720-turris-mox.dtb \ > + armada-3720-eDPU.dtb \ > + armada-3720-uDPU.dtb \ > + armada-7040-db-nand.dtb \ > + armada-7040-db.dtb \ > + armada-8040-clearfog-gt-8k.dtb \ > + armada-8040-db.dtb \ > + armada-8040-mcbin.dtb \ > + armada-8040-puzzle-m801.dtb \ > cn9130-db-A.dtb \ > cn9130-db-B.dtb \ > cn9131-db-A.dtb \ > @@ -272,6 +275,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > cn9132-db-B.dtb \ > cn9130-crb-A.dtb \ > cn9130-crb-B.dtb > +endif > > dtb-$(CONFIG_ARCH_SYNQUACER) += synquacer-sc2a11-developerbox.dtb > dtb-$(CONFIG_ARCH_UNIPHIER_LD11) += \ Looks good. Thanks. Could you please send this as a normal patch to the list? I'll drop patch 1/5 from this series. Feel free to add my: Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
On Friday 05 August 2022 12:51:54 Stefan Roese wrote: > On 05.08.22 12:48, Pali Rohár wrote: > > On Friday 05 August 2022 12:25:55 Stefan Roese wrote: > > > On 04.08.22 17:07, Pali Rohár wrote: > > > > On Thursday 04 August 2022 16:51:47 Stefan Roese wrote: > > > > > On 03.08.22 13:00, Pali Rohár wrote: > > > > > > For future changes it is needed for have spi0 device tree reference in > > > > > > every mvebu soc dts file even when it is unused. > > > > > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > > --- > > > > > > arch/arm/dts/armada-7040.dtsi | 3 +++ > > > > > > arch/arm/dts/armada-8040.dtsi | 3 +++ > > > > > > arch/arm/dts/cn9130.dtsi | 3 +++ > > > > > > 3 files changed, 9 insertions(+) > > > > > > > > > > You're changing 64-bit Armada files here, but in the cover letter you > > > > > mention that this patchset only addresses 32-bit mvebu DT files. So > > > > > why are you making these changes here? > > > > > > > > The main issue is that 64-bit Armada DTS files are compiled also during > > > > 32-bit Armada builds. And 32-bit Armada DTS files needs spi0 reference > > > > because it is used in 32-bit Armada section in mvebu-u-boot.dtsi file. > > > > > > > > I know it is broken build system if during compilation of 32-bit SoC are > > > > compiled also unrelated 64-bit Armada DTS files in 32-bit mode. But I do > > > > not see currently easier solution than just define "harmless" reference. > > > > > > > > During 64-bit Armada builds is content of mvebu-u-boot.dtsi skipped as > > > > there is a 32-bit guard. > > > > > > > > Proper way, of course, would be to fix build system, so none 64-bit file > > > > is compiled (and let unused) during 32-bit build. > > > > > > I see. Thanks for the explanation. > > > > I played a bit with build system and have there alternative patch > > instead of patch 1/5 which "fixes" makefile build system. > > Thanks Pali. > > > From 78ebf95fdf4223ce62cc537a57739ba3adbf556f Mon Sep 17 00:00:00 2001 > > From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> > > Date: Fri, 5 Aug 2022 12:45:19 +0200 > > Subject: [PATCH] arm: mvebu: dts: Build only arch-compatible dts files > > MIME-Version: 1.0 > > Content-Type: text/plain; charset=UTF-8 > > Content-Transfer-Encoding: 8bit > > > > 64-bit Armada DTS files are not build correctly during compilation of > > 32-bit Armada boards and vice versa. So fix makefile build system to > > compile only those dts files which are compatible for the current build > > (64-bit Armada DTS files only for 64-bit builds and 32-bit Armada DTS files > > only for 32-bit builds). > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > arch/arm/dts/Makefile | 28 ++++++++++++++++------------ > > 1 file changed, 16 insertions(+), 12 deletions(-) > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > > index ceaa39e4b4d6..7330121dbaba 100644 > > --- a/arch/arm/dts/Makefile > > +++ b/arch/arm/dts/Makefile > > @@ -232,12 +232,8 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \ > > tegra210-p2571.dtb \ > > tegra210-p3450-0000.dtb > > +ifdef CONFIG_ARMADA_32BIT > > dtb-$(CONFIG_ARCH_MVEBU) += \ > > - armada-3720-db.dtb \ > > - armada-3720-espressobin.dtb \ > > - armada-3720-turris-mox.dtb \ > > - armada-3720-eDPU.dtb \ > > - armada-3720-uDPU.dtb \ > > armada-375-db.dtb \ > > armada-385-atl-x530.dtb \ > > armada-385-atl-x530DP.dtb \ > > @@ -247,12 +243,6 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > > armada-388-gp.dtb \ > > armada-388-helios4.dtb \ > > armada-38x-controlcenterdc.dtb \ > > - armada-7040-db-nand.dtb \ > > - armada-7040-db.dtb \ > > - armada-8040-clearfog-gt-8k.dtb \ > > - armada-8040-db.dtb \ > > - armada-8040-mcbin.dtb \ > > - armada-8040-puzzle-m801.dtb \ > > armada-xp-crs305-1g-4s.dtb \ > > armada-xp-crs305-1g-4s-bit.dtb \ > > armada-xp-crs326-24g-2s.dtb \ > > @@ -263,7 +253,20 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > > armada-xp-gp.dtb \ > > armada-xp-maxbcm.dtb \ > > armada-xp-synology-ds414.dtb \ > > - armada-xp-theadorable.dtb \ > > + armada-xp-theadorable.dtb > > +else > > +dtb-$(CONFIG_ARCH_MVEBU) += \ > > + armada-3720-db.dtb \ > > + armada-3720-espressobin.dtb \ > > + armada-3720-turris-mox.dtb \ > > + armada-3720-eDPU.dtb \ > > + armada-3720-uDPU.dtb \ > > + armada-7040-db-nand.dtb \ > > + armada-7040-db.dtb \ > > + armada-8040-clearfog-gt-8k.dtb \ > > + armada-8040-db.dtb \ > > + armada-8040-mcbin.dtb \ > > + armada-8040-puzzle-m801.dtb \ > > cn9130-db-A.dtb \ > > cn9130-db-B.dtb \ > > cn9131-db-A.dtb \ > > @@ -272,6 +275,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > > cn9132-db-B.dtb \ > > cn9130-crb-A.dtb \ > > cn9130-crb-B.dtb > > +endif > > dtb-$(CONFIG_ARCH_SYNQUACER) += synquacer-sc2a11-developerbox.dtb > > dtb-$(CONFIG_ARCH_UNIPHIER_LD11) += \ > > Looks good. Thanks. Could you please send this as a normal patch to the > list? I'll drop patch 1/5 from this series. Feel free to add my: > > Reviewed-by: Stefan Roese <sr@denx.de> > > Thanks, > Stefan Ok, if you like more this alternative patch, I will do it!
On Friday 05 August 2022 12:55:44 Pali Rohár wrote: > On Friday 05 August 2022 12:51:54 Stefan Roese wrote: > > On 05.08.22 12:48, Pali Rohár wrote: > > > On Friday 05 August 2022 12:25:55 Stefan Roese wrote: > > > > On 04.08.22 17:07, Pali Rohár wrote: > > > > > On Thursday 04 August 2022 16:51:47 Stefan Roese wrote: > > > > > > On 03.08.22 13:00, Pali Rohár wrote: > > > > > > > For future changes it is needed for have spi0 device tree reference in > > > > > > > every mvebu soc dts file even when it is unused. > > > > > > > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > > > --- > > > > > > > arch/arm/dts/armada-7040.dtsi | 3 +++ > > > > > > > arch/arm/dts/armada-8040.dtsi | 3 +++ > > > > > > > arch/arm/dts/cn9130.dtsi | 3 +++ > > > > > > > 3 files changed, 9 insertions(+) > > > > > > > > > > > > You're changing 64-bit Armada files here, but in the cover letter you > > > > > > mention that this patchset only addresses 32-bit mvebu DT files. So > > > > > > why are you making these changes here? > > > > > > > > > > The main issue is that 64-bit Armada DTS files are compiled also during > > > > > 32-bit Armada builds. And 32-bit Armada DTS files needs spi0 reference > > > > > because it is used in 32-bit Armada section in mvebu-u-boot.dtsi file. > > > > > > > > > > I know it is broken build system if during compilation of 32-bit SoC are > > > > > compiled also unrelated 64-bit Armada DTS files in 32-bit mode. But I do > > > > > not see currently easier solution than just define "harmless" reference. > > > > > > > > > > During 64-bit Armada builds is content of mvebu-u-boot.dtsi skipped as > > > > > there is a 32-bit guard. > > > > > > > > > > Proper way, of course, would be to fix build system, so none 64-bit file > > > > > is compiled (and let unused) during 32-bit build. > > > > > > > > I see. Thanks for the explanation. > > > > > > I played a bit with build system and have there alternative patch > > > instead of patch 1/5 which "fixes" makefile build system. > > > > Thanks Pali. > > > > > From 78ebf95fdf4223ce62cc537a57739ba3adbf556f Mon Sep 17 00:00:00 2001 > > > From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali@kernel.org> > > > Date: Fri, 5 Aug 2022 12:45:19 +0200 > > > Subject: [PATCH] arm: mvebu: dts: Build only arch-compatible dts files > > > MIME-Version: 1.0 > > > Content-Type: text/plain; charset=UTF-8 > > > Content-Transfer-Encoding: 8bit > > > > > > 64-bit Armada DTS files are not build correctly during compilation of > > > 32-bit Armada boards and vice versa. So fix makefile build system to > > > compile only those dts files which are compatible for the current build > > > (64-bit Armada DTS files only for 64-bit builds and 32-bit Armada DTS files > > > only for 32-bit builds). > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > --- > > > arch/arm/dts/Makefile | 28 ++++++++++++++++------------ > > > 1 file changed, 16 insertions(+), 12 deletions(-) > > > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > > > index ceaa39e4b4d6..7330121dbaba 100644 > > > --- a/arch/arm/dts/Makefile > > > +++ b/arch/arm/dts/Makefile > > > @@ -232,12 +232,8 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \ > > > tegra210-p2571.dtb \ > > > tegra210-p3450-0000.dtb > > > +ifdef CONFIG_ARMADA_32BIT > > > dtb-$(CONFIG_ARCH_MVEBU) += \ > > > - armada-3720-db.dtb \ > > > - armada-3720-espressobin.dtb \ > > > - armada-3720-turris-mox.dtb \ > > > - armada-3720-eDPU.dtb \ > > > - armada-3720-uDPU.dtb \ > > > armada-375-db.dtb \ > > > armada-385-atl-x530.dtb \ > > > armada-385-atl-x530DP.dtb \ > > > @@ -247,12 +243,6 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > > > armada-388-gp.dtb \ > > > armada-388-helios4.dtb \ > > > armada-38x-controlcenterdc.dtb \ > > > - armada-7040-db-nand.dtb \ > > > - armada-7040-db.dtb \ > > > - armada-8040-clearfog-gt-8k.dtb \ > > > - armada-8040-db.dtb \ > > > - armada-8040-mcbin.dtb \ > > > - armada-8040-puzzle-m801.dtb \ > > > armada-xp-crs305-1g-4s.dtb \ > > > armada-xp-crs305-1g-4s-bit.dtb \ > > > armada-xp-crs326-24g-2s.dtb \ > > > @@ -263,7 +253,20 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > > > armada-xp-gp.dtb \ > > > armada-xp-maxbcm.dtb \ > > > armada-xp-synology-ds414.dtb \ > > > - armada-xp-theadorable.dtb \ > > > + armada-xp-theadorable.dtb > > > +else > > > +dtb-$(CONFIG_ARCH_MVEBU) += \ > > > + armada-3720-db.dtb \ > > > + armada-3720-espressobin.dtb \ > > > + armada-3720-turris-mox.dtb \ > > > + armada-3720-eDPU.dtb \ > > > + armada-3720-uDPU.dtb \ > > > + armada-7040-db-nand.dtb \ > > > + armada-7040-db.dtb \ > > > + armada-8040-clearfog-gt-8k.dtb \ > > > + armada-8040-db.dtb \ > > > + armada-8040-mcbin.dtb \ > > > + armada-8040-puzzle-m801.dtb \ > > > cn9130-db-A.dtb \ > > > cn9130-db-B.dtb \ > > > cn9131-db-A.dtb \ > > > @@ -272,6 +275,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ > > > cn9132-db-B.dtb \ > > > cn9130-crb-A.dtb \ > > > cn9130-crb-B.dtb > > > +endif > > > dtb-$(CONFIG_ARCH_SYNQUACER) += synquacer-sc2a11-developerbox.dtb > > > dtb-$(CONFIG_ARCH_UNIPHIER_LD11) += \ > > > > Looks good. Thanks. Could you please send this as a normal patch to the > > list? I'll drop patch 1/5 from this series. Feel free to add my: > > > > Reviewed-by: Stefan Roese <sr@denx.de> > > > > Thanks, > > Stefan > > Ok, if you like more this alternative patch, I will do it! Here it is: https://patchwork.ozlabs.org/project/uboot/patch/20220805113725.28130-1-pali@kernel.org/
diff --git a/arch/arm/dts/armada-7040.dtsi b/arch/arm/dts/armada-7040.dtsi index 039d30c72a8c..1fb21c6bfde0 100644 --- a/arch/arm/dts/armada-7040.dtsi +++ b/arch/arm/dts/armada-7040.dtsi @@ -63,3 +63,6 @@ marvell,function = <3>; }; }; + +spi0: &cp0_spi1 { +}; diff --git a/arch/arm/dts/armada-8040.dtsi b/arch/arm/dts/armada-8040.dtsi index eec5fa277405..608ff0d97f95 100644 --- a/arch/arm/dts/armada-8040.dtsi +++ b/arch/arm/dts/armada-8040.dtsi @@ -87,3 +87,6 @@ marvell,function = <3>; }; }; + +spi0: &cp1_spi1 { +}; diff --git a/arch/arm/dts/cn9130.dtsi b/arch/arm/dts/cn9130.dtsi index 68b767a70639..a200276f7a2e 100644 --- a/arch/arm/dts/cn9130.dtsi +++ b/arch/arm/dts/cn9130.dtsi @@ -71,3 +71,6 @@ marvell,function = <3>; }; }; + +spi0: &cp0_spi1 { +};
For future changes it is needed for have spi0 device tree reference in every mvebu soc dts file even when it is unused. Signed-off-by: Pali Rohár <pali@kernel.org> --- arch/arm/dts/armada-7040.dtsi | 3 +++ arch/arm/dts/armada-8040.dtsi | 3 +++ arch/arm/dts/cn9130.dtsi | 3 +++ 3 files changed, 9 insertions(+)