Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver

Simon Glass
2023-01-23T16:32:39Z

Hi Sudeep,

On Fri, 20 Jan 2023 at 04:17, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Thu, Jan 19, 2023 at 10:22:07AM -0700, Simon Glass wrote:
> > Hi Sudeep,
> >
> > On Thu, 19 Jan 2023 at 10:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Thu, Jan 19, 2023 at 11:57:44AM -0500, Tom Rini wrote:
> > > >
> > > > But it's also true that at run-time, within U-Boot, we can modify the
> > > > device tree we have, with live tree yes? So, the whole series in
> > > > question here can be done without modifying the base DT and getting in
> > > > to the further discussions that doing so entails. The assertion is that
> > > > the software discoverable bus here is sufficient to not need DT, so, OK,
> > > > lets go.
> > >
> > > OK, may be I am not up-to-date on the U-Boot. IIUC, the modifications
> > > done in the DT by U-Boot is mostly for consumption by the next stage
> > > loader/OS and not for self-consumption. But if it is for self consumption,
> > > then good. It helps especially for the subnodes(as Simon referred) or the
> > > partitions that can be discovered at run-time using FF-A interface.
> >
> > It's really just dodging the issue though, because you need a
> > compatible string and you might as well add it to the DT in the source
> > as do it at runtime.
> >
>
> Well that is one of the argument assuming DT node is a must. But adding
> DT node requirement when it is not needed can be seen an issue itself if
> one has to play devil's advocate here.
>
> > >
> > > As mentioned I am not again DT, it is just not needed and especially
> > > for subnodes it could result in inconsistency b/w what is in DT and
> > > what the firmware provides. As mentioned in previous response, having a
> > > simple node that Simon provided as example earlier is fine by me if that
> > > is the only option to make progress as I just feel it is redundant and
> > > one can say not scalable(but that is debatable again 😄).
> >
> > Gosh, how many of these things are you going to add? I believe the
> > inconsistency argument is dealt with by the bind/probe explanation. It
> > may be redundant in Linux but I doubt it would hurt there either.
> >
>
> Not really. Currently the number of partitions is static and all of them
> are bundled into on or few binaries. But we could have dynamic partitions
> in the future. More over it is pain to update the DT for each possible
> configuration especially during development where you want to experiment
> things around. Having to deal with the DT for every small change in the
> config is just annoying. Yes one could have universal list of partitions
> and defer it to be dealt at probe/bind, but not sure if it is possible
> to have such a universal list during development. We may have it handy
> for production but we also want to ease development here.
>
> In short, I have had quite a lot of issues with DT and firmware
> inconsistency due to duplication of same data at the beginning and the
> things diverged. So I am simply not ready to go back there and I am sure
> quite a few in the kernel community have their fingers bitten because of
> such inconsistency created by *unnecessary static addition of data* to
> the DT. So, let me be clear, I wouldn't use the information from the
> DT if I can get more accurate one dynamically from the firmware in the
> kernel driver.

I'd like to see DT defined across firmware and OS, not just be a Linux
thing. It is a better approach than having little fiefdoms everywhere
with their own config mechanisms.

It seems that you have lots of build-time config in the ARM
components. I suggest looking at how you can move this to runtime
config, if you are not planning to upstream the functionality to
U-Boot.

Regards,
Simon