On Thu, May 15, 2025 at 09:26:42PM +0200, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 14 May 2025 at 23:58, Tom Rini <tr...@konsulko.com> wrote:
> >
> > On Sat, May 10, 2025 at 03:42:07PM +0200, Simon Glass wrote:
> >
> > > Accept a bloblist and control devicetree from a previous phase in
> > > registers 0 to 3, as documented in the Firmware Handoff
> > > specification[1].
> > >
> > > Note that there is currently a weak function called save_boot_params()
> > > which happens to save the same registers as are needed for this
> > > protocol. But it seems better to implement this feature as a first-class
> > > citizen, if for no other reason than that many boards have their own
> > > version of save_boot_params() which may not happen to save the required
> > > registers. A weak function is not a good basis for an API.
> > >
> > > [1] https://firmwarehandoff.github.io/firmware_handoff/main/index.html
> > >
> > > Signed-off-by: Simon Glass <s...@chromium.org>
> > > ---
> > > While some efforts were made to encourage Intel to adopt this
> > > specification, this has not yet been successful. However I do intend to
> > > use my proposed x86 handoff in U-Boot, once this series is in.
> > >
> > > Changes in v4:
> > > - Update commit message to indicate this can only be for ARM at present
> > > - Update commit message to mention why save_boot_params() is not used
> > >
> > > Changes in v3:
> > > - Add support for aarch64 also
> > > - Update registers to match the Firmware Handoff protocol
> > >
> > > Changes in v2:
> > > - Use three registers instead of two for the entry
> > >
> > >  arch/arm/cpu/armv7/start.S | 10 +++++++++-
> > >  arch/arm/cpu/armv8/start.S | 12 ++++++++++++
> > >  arch/arm/lib/crt0.S        |  6 ++++++
> > >  arch/arm/lib/crt0_64.S     |  6 ++++++
> > >  4 files changed, 33 insertions(+), 1 deletion(-)
> >
> > This is what makes me think you aren't actually working on my feedback
> > about taking what we have in-tree, now, rather than just forward porting
> > your old series and deleting the current code.
> 
> Well, yes, I sent this v3 series because I had thought you were
> willing to allow me to maintain bloblist. I certainly appreciate the
> work in this area, though.
> 
> >
> > We have save_boot_params doing exactly what it should today, including
> > saving off a standard passage and making what was saved available later,
> > in an intentional fashion. If, hypothetically, Pi needs to override
> > save_boot_param because a future new-firmware-blob does standard passage
> > but at run time we want to support both, that's a good thing.
> >
> > With commit ea3348ebc215 ('Merge patch series "Handoff bloblist from
> > previous boot stage"') we already implement the specification in [1]. It
> > is not a "which happens to". It is intentional.
> >
> > I guess the overall challenge here is that you want to do things which
> > are a superset of the firmware handoff spec itself?
> 
> Yes,, much of this series relates to putting some structure into
> things which are passed via standard passage, including validation.
> 
> I actually don't understand why you would be keen on using
> save_boot_params(), a weak function, used for board-specific features,
> to implement standard passage. We should build standard passage in as
> a first-class citizen. I certainly don't want to try to implement
> things that way on x86.

You seem to have misunderstood a lot of things and perhaps need to go
and re-read the feedback. Nothing about a weak function implies less
than "first class" and "maintain the code" does not equate "delete and
replace functional code with the same thing, but I wrote it". And if you
aren't going to do that, please don't bother posting a v4 either later
this week. I feel like I'm having zero progress with you taking my
feedback on anything at this point.

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to