[PATCH] drivers: firmware: ti_sci: Add DM_FLAG_PRE_RELOC to driver

[Manorit Chawdhry]

Hi Simon,

On 08:08-20241219, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 18 Dec 2024 at 10:38, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Wed, Dec 18, 2024 at 10:38:37AM +0530, Manorit Chawdhry wrote:
> > > Hi Tom,
> > >
> > > On 08:53-20241217, Tom Rini wrote:
> > > > On Tue, Dec 17, 2024 at 02:24:37PM +0530, Manorit Chawdhry wrote:
> > > >
> > > > > Currently the driver relies on bootph flag to probe it during PRE_RELOC
> > > > > stage but with the upcoming cleanup of v6.13, we don't have the bootph
> > > > > property in the parent nodes anymore and ti_sci driver being one of the
> > > > > parent nodes required during SPL stage would end up hampering the probe
> > > > > model [0].
> > > > >
> > > > > Add DM_FLAG_PRE_RELOC to ti_sci driver for mitigating this issue.
> > > >
> > > >
> > > > Can you please expand on why with v6.13 we don't have the flag anymore?
> > > > Thanks.
> > >
> > > So the idea was that following series [0] would help us cleanup all the
> > > parent node bootph-* tags as putting a bootph-* in child node implies
> > > that it applies to parent nodes as well, though even if the parent node
> > > gets packed after the following series, the parent node still don't get
> > > probed as from my understanding, we need bootph-* tag to probe the
> > > driver as well but the phase tag doesn't get propagated as such.
> > >
> > > So while working in Linux, some comments related to this was raised [1]
> > > and after confirming the behaviour with Simon [2], the cleanup was done
> > > in Linux and meanwhile this way was found to probe the driver so that
> > > even after cleaning up the parent bootph-* tags we can continue.
> > >
> > > I don't think it's a hack also as this driver is required pre-relocation
> > > and this is another way to probe it pre-relocation but the only thing is
> > > that since the cleanup is done right now, we would kind of need it to be
> > > booting in some platforms ( that were fully reliant on Linux DT and
> > > don't have -u-boot override for this particular node with bootph-* )
> > > before 6.13 DT gets synced up.
> > >
> > > [0]: https://lore.kernel.org/u-boot/20231217163627.2339802-1-sjg@chromium.org/
> > > [1]: https://lore.kernel.org/all/20240709084857.nf7c57mi6miajeau@uda0497581/#t
> > > [2]: https://lore.kernel.org/u-boot/CAFLszTjr7bJFPnyVx7rTdFB3AFBM_2DVAZU8FZnLBE7ep5ejXw@mail.gmail.com/
> >
> > Thanks. Did you end up filing an issue about this? And can you now
> > perhaps push to make doing the fdtgrep work a priority? There's going to
> > be a lot of other cases like this to resolve I suspect.
> 
> fdtgrep is done, the confusion is that there are two [0] references in
> this chain. 
> 

Ah yeah, sorry for the confusion. 

> The one you want is:
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/21
> 
> For SPL and U-Boot proper, all is well. But for pre-relocation, U-Boot
> doesn't actually bind parents even if their children have a bootph
> property, as I understand it. I believe that is what needs to be
> fixed?

So I see that when we jump from our SPL stage to U-boot proper and end
up in board_init_f with the common sequence, we end up in
dm_probe_devices.

#0  dm_probe_devices (dev=0x80478030, pre_reloc_only=pre_reloc_only at entry=true) drivers/core/root.c:294
#1  0x0000000080834f9c in dm_scan (pre_reloc_only=true) drivers/core/root.c:348
#2  dm_init_and_scan (pre_reloc_only=<optimized out>) drivers/core/root.c:361
#3  0x000000008089393c in initcall_run_list (init_sequence=init_sequence at entry=0x808b9dc0 <init_sequence_f>) lib/initcall.c:75
#4  0x0000000080828908 in board_init_f (boot_flags=<optimized out>) common/board_f.c:1026
#5  0x000000008080252c in _main () arch/arm/lib/crt0_64.S:100
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

dm_probe_devices act differently with and without this patch even though
there is no DT change and child node has the bootph. During traversal,
in working case we go to "system-controller at 44083000" ( parent node of
power-controller ) from "bus at 28380000" [5] but in failing case we don't
[6] ( DT model can be found at [7] )

Since the dm_probe_devices act differently, while going to serial_init
we end up not finding the driver for probing the clock and power domains
( children of system-controller ) and fail.

Though am not sure as to why it's not going to that node during
dm_probe_devices in the failing case, suspecting something around [8]
but not sure. Let me know if it helps Simon.

[5]: https://gist.github.com/manorit2001/a9b82b40b6c32643fb10d523cab9da70#file-with-the-dm_flag_pre_reloc-patch-L26-L48
[6]: https://gist.github.com/manorit2001/a9b82b40b6c32643fb10d523cab9da70#file-without-the-patch-L25-L39
[7]: https://github.com/torvalds/linux/blob/v6.13-rc3/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-mcu-wakeup-common.dtsi#L9
[8]: https://github.com/u-boot/u-boot/blob/078542c346347406cfacdec8adeac66ae6758880/drivers/core/root.c#L297-L301

Regards,
Manorit

> 
> Regards,
> SImon