[EXTERNAL] Re: [RFC PATCH 1/2 v1] arch: arm: Kconfig: Add config to use subnode's bootph property for binding drivers

[Moteen Shah]

Hello Simon,

On 07/03/25 10:25, Moteen Shah wrote:
>
> On 26/02/25 02:57, Simon Glass wrote:
>> Hi Moteen, On Thu, 13 Feb 2025 at 22: 05, Moteen Shah 
>> <m-shah@ ti. com> wrote: > > > On 13/02/25 19: 31, Simon Glass wrote: 
>> > > Hi Moteen, On Wed, 12 Feb 2025 at 02: 18, Moteen Shah > > 
>> <m-shah@ ti. com> wrote: >
>> ZjQcmQRYFpfptBannerStart
>> This message was sent from outside of Texas Instruments.
>> Do not click links or open attachments unless you recognize the 
>> source of this email and know the content is safe.
>> Report Suspicious
>> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!tDdkczlA_gRXoUoEULhq8lPl3ziRhLj9IlA3ZXDoziTij0dPBpn6-DgWRD6rC1BVPnBWST9bilaw10uU17sjumdmw40nkGdzZUT48cV7qkEDIlAys_tcmpU$> 
>>
>> ZjQcmQRYFpfptBannerEnd
>> Hi Moteen,
>>
>> On Thu, 13 Feb 2025 at 22:05, Moteen Shah <m-shah at ti.com> wrote:
>> >
>> >
>> > On 13/02/25 19:31, Simon Glass wrote:
>> > > Hi Moteen, On Wed, 12 Feb 2025 at 02: 18, Moteen Shah
>> > > <m-shah@ ti. com> wrote: > > Add a new config when set will traverse
>> > > through all the subnodes of > a given node scanning for bootph-all
>> > > property and propagate it to > all of
>> > > ZjQcmQRYFpfptBannerStart
>> > > This message was sent from outside of Texas Instruments.
>> > > Do not click links or open attachments unless you recognize the 
>> source
>> > > of this email and know the content is safe.
>> > > Report Suspicious
>> > > 
>> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!tDdkczlA_gRXoUgIvDspkOY_N58UTgTbK8UGS8zfYNsuu6kFHonhYC9p6QDyxO6fEM-jwDoJbz7g9IYZuow6CShwWbGJRkE39jUS3OLaz9G1-q9eGLvRc6M$>
>> > >
>> > > ZjQcmQRYFpfptBannerEnd
>> > > Hi Moteen,
>> > >
>> > > On Wed, 12 Feb 2025 at 02:18, Moteen Shah <m-shah at ti.com> wrote:
>> > > >
>> > > > Add a new config when set will traverse through all the 
>> subnodes of
>> > > > a given node scanning for bootph-all property and propagate it to
>> > > > all of its parent node up the hierarchy.
>> > > >
>> > > > Signed-off-by: Moteen Shah <m-shah at ti.com>
>> > > > ---
>> > > >  arch/arm/Kconfig | 11 +++++++++++
>> > > >  1 file changed, 11 insertions(+)
>> > > >
>> > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> > > > index 314916527c9..51fc952b0db 100644
>> > > > --- a/arch/arm/Kconfig
>> > > > +++ b/arch/arm/Kconfig
>> > > > @@ -805,6 +805,7 @@ config ARCH_K3
>> > > >         select FIT
>> > > >         select REGEX
>> > > >         select FIT_SIGNATURE if ARM64
>> > > > +       imply BIND_FROM_CHILD_BOOTPH
>> > > >         imply TI_SECURE_DEVICE
>> > > >
>> > > >  config ARCH_OMAP2PLUS
>> > > > @@ -2232,6 +2233,16 @@ config SYS_KWD_CONFIG
>> > > >           Path within the source directory to the kwbimage.cfg 
>> file to use
>> > > >           when packaging the U-Boot image for use.
>> > > >
>> > > > +
>> > > > +config BIND_FROM_CHILD_BOOTPH
>> > >
>> > > How about DM_F_STRICT_BOOTPH ?  or DM_F_CHILD_BOOTPH ?
>> >
>> > Yes, this should be more descriptive, will include this in v2.
>> >
>> > >
>> > > It indicates that it relates to driver model before relocation.
>> > >
>> > > This behaviour is actually required by the schema. I agree it should
>> > > be optional, but only due to its performance issues.
>> > >
>> > > A Binman solution would not have any performance issues.
>> >
>> > Will explore on this more, if you have some more pointers on this,
>> > then do let me know.
>>
>> As you requested on the call today:
>>
>> PrepareImagesAndDtbs() fiddles with the dtb so you can add some code 
>> there.
>>
>> My suggestion is to insert something after this chunk:
>>
>>      # Get the device tree ready by compiling it and copying the 
>> compiled
>>      # output into a file in our output directly. Then scan it for use
>>      # in binman.
>>      dtb_fname = fdt_util.EnsureCompiled(dtb_fname)
>>      fname = tools.get_output_filename('u-boot.dtb.out')
>>
>> here you can read in the file and modify it, e.g.:
>>
>> dtb = fdt.FdtScan(dtb_fname)
>> add_bootph_nodes(dtb)
>> dtb.Sync(True)
>> tools.write_file(fname, dtb.GetContents())
>>
>>      tools.write_file(fname, tools.read_file(dtb_fname))   # delete 
>> this line
>>      dtb = fdt.FdtScan(fname)
>>
>> It might need some tweaking. It will need some sort of test, see
>> ftest.py for that. As an example, testCompressSectionSize() reads back
>> the dtb to check it., so you can follow that You need to create a .dts
>> file in tools/binman/test, containing nodes other than 'binman'.
>
>
> Thanks for the pointers Simon. I'll start with the implementation soon 
> after checking the time delta
> we get with this patch but with direct binding rather than propagating 
> bootph-* properties.
>

I checked the time delta with the above mentioned approach, here are the 
results:

Board         Baseline   Previous delay  New delay
J7200-evm       2.2s       ~100 ms         ~60ms
J784s4-evm     2.7s        ~350 ms     300ms-350ms

The time delta is still significant, a binman solution should be the 
right approach to this, a binman based approach should be the right step 
I think. Will send a v2 based on it.


>
>> >
>> >
>> > Regards,
>> > Moteen
>> >
>> > >
>> > > > +       bool "Bind drivers from bootph* in subnode"
>> > > > +       depends on ARCH_K3
>> > > > +       help
>> > > > +        This config must be set to bind drivers in pre reloc 
>> stage whose
>> > > > +        compatible parent nodes are implicitly declared to be 
>> bound to
>> > > > +        their respective drivers by having bootph* property in 
>> one of
>> > > > +        their subnodes.
>> > > > +
>> > > >  source "arch/arm/mach-apple/Kconfig"
>> > > >
>> > > >  source "arch/arm/mach-aspeed/Kconfig"
>> > > > --
>> > > > 2.34.1
>> > > >
>> > >
>> > > Regards,
>> > > Simon
>>
>> Regards,
>> Simon
>
>
> Regards,
> Moteen


Regards,
Moteen