On Sat, May 24, 2025 at 03:31:08PM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 15 May 2025 at 21:54, Tom Rini <tr...@konsulko.com> wrote:
> >
> > 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.
> 
> Are you completely set on the idea of a weak function here?

Am I completely set on following the general architecture and design we
have and not making needless changes? Again, you seem to have either
misread not not understood feedback and are intent on doing what you
want anyhow.

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to