Re: [meta-ti][dunfell/master][PATCH V3] linux-ti-staging: Add extra dtc flags to support dtb overlays

["Denys Dmytriyenko" <denis@denix.org>]

On Tue, May 25, 2021 at 06:55:52AM -0500, Nishanth Menon wrote:
> On 21:51-20210524, Denys Dmytriyenko wrote:
> > On Mon, May 24, 2021 at 05:40:36PM -0500, Yogesh Siraswar via lists.yoctoproject.org wrote:
> > > Added KERNEL_DTB_OVERLAY_SUPPORT to individual MACHINE conf to
> > > compile dtbs with symbols. This fixes the issue where the dtbs
> > > are build without symbols causing issue with overlays.
> > > 
> > > Enabling overlay support increases the size of the dtb, hence
> > > it is done at a platform level rather than globally to allow custom
> > > boards disable it from local.conf to reduce size.
> > > 
> 
> ^^
> 
> > > Signed-off-by: Yogesh Siraswar <yogeshs@ti.com>
> > > ---
> > >  conf/machine/am335x-evm.conf                  | 2 ++
> > >  conf/machine/am335x-hs-evm.conf               | 2 ++
> > >  conf/machine/am437x-evm.conf                  | 2 ++
> > >  conf/machine/am437x-hs-evm.conf               | 2 ++
> > >  conf/machine/am57xx-evm.conf                  | 2 ++
> > >  conf/machine/am57xx-hs-evm.conf               | 3 +++
> > >  conf/machine/am65xx-evm.conf                  | 2 ++
> > >  conf/machine/am65xx-hs-evm.conf               | 2 ++
> > >  conf/machine/dra7xx-evm.conf                  | 2 ++
> > >  conf/machine/dra7xx-hs-evm.conf               | 3 +++
> > >  conf/machine/j7-evm.conf                      | 2 ++
> > >  conf/machine/j7-hs-evm.conf                   | 2 ++
> > >  conf/machine/j7200-evm.conf                   | 2 ++
> > >  conf/machine/k2g-evm.conf                     | 2 ++
> > >  conf/machine/k2g-hs-evm.conf                  | 2 ++
> > 
> > First question:
> > If you are weakly assigning KERNEL_DTB_OVERLAY_SUPPORT in most of the machine 
> > configs, should it instead be done in SoC include files, like ti33x.inc, 
> > k3.inc or even ti-soc.inc?
> 
> You would want the default behavior as default dtb, not the bloated
> symbol loaded overlay capable dtb, no?

Well, but from the list above the only machines missing are AM64 and legacy 
K2 HK/L/E. And since the assignment is weak (?=), it can still be changed 
from local.conf, as the commit message says.

Anyway, if you don't think making it a bit more generic for ease of 
maintenance is not worth it, then I'm fine with current approach.


> > > +
> > > +def get_extra_dtc_args(d):
> > > +    if d.getVar('KERNEL_DTB_OVERLAY_SUPPORT') == "1":
> > > +        return "DTC_FLAGS=-@"
> > > +    else:
> > > +        return ""
> > > +
> > > +EXTRA_DTC_ARGS += "${@get_extra_dtc_args(d)}"
> > 
> > Second question:
> > Are there any specific future plans for EXTRA_DTC_ARGS? As it seems to be 
> > an unused intermediary variable right now - you can directly append to 
> > KERNEL_EXTRA_ARGS here...
> 
> dtc changes are always incoming, does'nt make sense to keep that
> separated out?

Fair enough.

Acked-by: Denys Dmytriyenko <denys@konsulko.com>

-- 
Regards,
Denys Dmytriyenko <denis@denix.org>
PGP: 0x420902729A92C964 - https://denix.org/0x420902729A92C964
Fingerprint: 25FC E4A5 8A72 2F69 1186  6D76 4209 0272 9A92 C964