diff mbox series

arm: dts: omap: Apply am57xx-idk overlays to base dtbs

Message ID 20230911214609.2201040-1-robh@kernel.org
State New
Headers show
Series arm: dts: omap: Apply am57xx-idk overlays to base dtbs | expand

Commit Message

Rob Herring (Arm) Sept. 11, 2023, 9:46 p.m. UTC
DT overlays in tree need to be applied to a base DTB to validate they
apply, to run schema checks on them, and to catch any errors at compile
time.

Signed-off-by: Rob Herring <robh@kernel.org>
---
Note that I have no idea if this combination of overlays makes sense.
---
 arch/arm/boot/dts/ti/omap/Makefile | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Andrew Davis Sept. 22, 2023, 2:38 p.m. UTC | #1
On 9/12/23 1:59 AM, Tony Lindgren wrote:
> * Rob Herring <robh@kernel.org> [230911 21:46]:
>> DT overlays in tree need to be applied to a base DTB to validate they
>> apply, to run schema checks on them, and to catch any errors at compile
>> time.
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> Note that I have no idea if this combination of overlays makes sense.
> 

It does make sense, but it is only one of many valid combinations. I'm
guessing the goal here is just to make sure they all get applied in
at least one way so the scheme check runs. In that case this is fine
other than it might give the impression this is the only valid combinations.

Also now we end up with these odd `am57{1,2}x-idk-overlays.dtb` files
which also might confuse folks, I wonder if there is some way to
apply and check, but not ship/install these..

Andrew

> Adding Andrew to review this.
> 
> Regards,
> 
> Tony
> 
> ---
>>   arch/arm/boot/dts/ti/omap/Makefile | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/ti/omap/Makefile b/arch/arm/boot/dts/ti/omap/Makefile
>> index d2b590004fed..d0c76d09fe70 100644
>> --- a/arch/arm/boot/dts/ti/omap/Makefile
>> +++ b/arch/arm/boot/dts/ti/omap/Makefile
>> @@ -129,6 +129,11 @@ dtb-$(CONFIG_SOC_AM43XX) += \
>>   am57xx-evm-dtbs := am57xx-beagle-x15.dtb am57xx-evm.dtbo
>>   am57xx-evm-reva3-dtbs := am57xx-beagle-x15-revc.dtb am57xx-evm.dtbo
>>   
>> +am571x-idk-overlays-dtbs := am571x-idk.dtb \
>> +	am571x-idk-touchscreen.dtbo am57xx-idk-lcd-osd101t2587.dtbo
>> +am572x-idk-overlays-dtbs := am572x-idk.dtb \
>> +	am572x-idk-touchscreen.dtbo am57xx-idk-lcd-osd101t2045.dtbo
>> +
>>   dtb-$(CONFIG_SOC_DRA7XX) += \
>>   	am57xx-beagle-x15.dtb \
>>   	am57xx-beagle-x15-revb1.dtb \
>> @@ -145,6 +150,8 @@ dtb-$(CONFIG_SOC_DRA7XX) += \
>>   	am574x-idk.dtb \
>>   	am57xx-idk-lcd-osd101t2045.dtbo \
>>   	am57xx-idk-lcd-osd101t2587.dtbo \
>> +	am571x-idk-overlays.dtb \
>> +	am572x-idk-overlays.dtb \
>>   	dra7-evm.dtb \
>>   	dra72-evm.dtb \
>>   	dra72-evm-revc.dtb \
>> -- 
>> 2.40.1
>>
Rob Herring (Arm) Sept. 22, 2023, 4:23 p.m. UTC | #2
On Fri, Sep 22, 2023 at 9:38 AM Andrew Davis <afd@ti.com> wrote:
>
> On 9/12/23 1:59 AM, Tony Lindgren wrote:
> > * Rob Herring <robh@kernel.org> [230911 21:46]:
> >> DT overlays in tree need to be applied to a base DTB to validate they
> >> apply, to run schema checks on them, and to catch any errors at compile
> >> time.
> >>
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >> Note that I have no idea if this combination of overlays makes sense.
> >
>
> It does make sense, but it is only one of many valid combinations. I'm
> guessing the goal here is just to make sure they all get applied in
> at least one way so the scheme check runs. In that case this is fine
> other than it might give the impression this is the only valid combinations.

I only care that an overlay is applied to one base. You should care
about any combination a user might do in a bootloader because who
wants to debug a failure a) on a board and b) in the bootloader.

> Also now we end up with these odd `am57{1,2}x-idk-overlays.dtb` files
> which also might confuse folks, I wonder if there is some way to
> apply and check, but not ship/install these..

There's already a config option, CONFIG_OF_ALL_DTBS, which adds
$(dtb-) entries to the build. So if you have "dtb- +=
foo-overlays.dtb" it will only be built in that case. Note that they'd
probably get installed too, but who installs allyesconfig builds.

Rob
Tony Lindgren Oct. 7, 2023, 6:51 a.m. UTC | #3
* Rob Herring <robh@kernel.org> [230922 16:23]:
> On Fri, Sep 22, 2023 at 9:38 AM Andrew Davis <afd@ti.com> wrote:
> >
> > On 9/12/23 1:59 AM, Tony Lindgren wrote:
> > > * Rob Herring <robh@kernel.org> [230911 21:46]:
> > >> DT overlays in tree need to be applied to a base DTB to validate they
> > >> apply, to run schema checks on them, and to catch any errors at compile
> > >> time.
> > >>
> > >> Signed-off-by: Rob Herring <robh@kernel.org>
> > >> ---
> > >> Note that I have no idea if this combination of overlays makes sense.
> > >
> >
> > It does make sense, but it is only one of many valid combinations. I'm
> > guessing the goal here is just to make sure they all get applied in
> > at least one way so the scheme check runs. In that case this is fine
> > other than it might give the impression this is the only valid combinations.
> 
> I only care that an overlay is applied to one base. You should care
> about any combination a user might do in a bootloader because who
> wants to debug a failure a) on a board and b) in the bootloader.
> 
> > Also now we end up with these odd `am57{1,2}x-idk-overlays.dtb` files
> > which also might confuse folks, I wonder if there is some way to
> > apply and check, but not ship/install these..
> 
> There's already a config option, CONFIG_OF_ALL_DTBS, which adds
> $(dtb-) entries to the build. So if you have "dtb- +=
> foo-overlays.dtb" it will only be built in that case. Note that they'd
> probably get installed too, but who installs allyesconfig builds.

So what's the conclusion here? Is this safe to apply yes or no?

Regards,

Tony
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/ti/omap/Makefile b/arch/arm/boot/dts/ti/omap/Makefile
index d2b590004fed..d0c76d09fe70 100644
--- a/arch/arm/boot/dts/ti/omap/Makefile
+++ b/arch/arm/boot/dts/ti/omap/Makefile
@@ -129,6 +129,11 @@  dtb-$(CONFIG_SOC_AM43XX) += \
 am57xx-evm-dtbs := am57xx-beagle-x15.dtb am57xx-evm.dtbo
 am57xx-evm-reva3-dtbs := am57xx-beagle-x15-revc.dtb am57xx-evm.dtbo
 
+am571x-idk-overlays-dtbs := am571x-idk.dtb \
+	am571x-idk-touchscreen.dtbo am57xx-idk-lcd-osd101t2587.dtbo
+am572x-idk-overlays-dtbs := am572x-idk.dtb \
+	am572x-idk-touchscreen.dtbo am57xx-idk-lcd-osd101t2045.dtbo
+
 dtb-$(CONFIG_SOC_DRA7XX) += \
 	am57xx-beagle-x15.dtb \
 	am57xx-beagle-x15-revb1.dtb \
@@ -145,6 +150,8 @@  dtb-$(CONFIG_SOC_DRA7XX) += \
 	am574x-idk.dtb \
 	am57xx-idk-lcd-osd101t2045.dtbo \
 	am57xx-idk-lcd-osd101t2587.dtbo \
+	am571x-idk-overlays.dtb \
+	am572x-idk-overlays.dtb \
 	dra7-evm.dtb \
 	dra72-evm.dtb \
 	dra72-evm-revc.dtb \