On Sat, May 24, 2025 at 03:20:17PM +0100, Simon Glass wrote: > Hi Tom, > > On Fri, 23 May 2025 at 23:17, Tom Rini <tr...@konsulko.com> wrote: > > > > On Fri, May 23, 2025 at 09:23:15PM +0100, Simon Glass wrote: > > > Hi Mark, Tom, > > > > > > On Fri, 23 May 2025 at 18:12, Mark Kettenis <mark.kette...@xs4all.nl> > > > wrote: > > > > > > > > > Date: Fri, 23 May 2025 10:49:09 -0600 > > > > > From: Tom Rini <tr...@konsulko.com> > > > > > > > > > > On Fri, May 23, 2025 at 05:36:52PM +0100, Simon Glass wrote: > > > > > > Hi Heinrich, > > > > > > > > > > > > On Fri, May 23, 2025, 15:19 Heinrich Schuchardt > > > > > > <xypron.g...@gmx.de> wrote: > > > > > > > > > > > > > > On 23.05.25 15:06, Simon Glass wrote: > > > > > > > > Some functions provided in lib/efi_loader are actually useful > > > > > > > > for the > > > > > > > > app as well. This series refactors the Kconfig and directories > > > > > > > > a little > > > > > > > > so that code is easier to share. > > > > > > > > > > > > > > > > As a starting point, it moves some filename and device-path > > > > > > > > functions to > > > > > > > > the new directory. > > > > > > > > > > > > > > > > The next step would be to move device-path code over, but this > > > > > > > > will need > > > > > > > > some discussion. > > > > > > > > > > > > > > Hello Simon, > > > > > > > > > > > > > > Overall the ideas in this series look fine to me. But this series > > > > > > > does > > > > > > > not apply to origin/next. > > > > > > > > > > > > > > Applying: efi_loader: Separate device path into its own header > > > > > > > Patch failed at 0001 efi_loader: Separate device path into its > > > > > > > own header > > > > > > > error: patch failed: cmd/efidebug.c:8 > > > > > > > error: cmd/efidebug.c: patch does not apply > > > > > > > error: patch failed: include/efi_loader.h:967 > > > > > > > error: include/efi_loader.h: patch does not apply > > > > > > > error: patch failed: lib/efi_loader/efi_bootmgr.c:12 > > > > > > > error: lib/efi_loader/efi_bootmgr.c: patch does not apply > > > > > > > error: patch failed: lib/efi_loader/efi_device_path.c:10 > > > > > > > error: lib/efi_loader/efi_device_path.c: patch does not apply > > > > > > > error: patch failed: lib/efi_loader/efi_helper.c:6 > > > > > > > error: lib/efi_loader/efi_helper.c: patch does not apply > > > > > > > > > > > > > > Please, resend the series based on origin/next. > > > > > > > > > > > > > > Patches that are not based on upstream U-Boot cannot be reviewed > > > > > > > via > > > > > > > this mailing list. > > > > > > > > > > > > The app is quite behind in Tom's tree due to rejected series. > > > > > > > > > > It was not "Rejected", it was "Changes Requested", please stop > > > > > mis-representing things. > > > > > > > > > > > In fact > > > > > > the app is pretty limited on x86 and there is no Arm app at all. > > > > > > > > > > > > My current plan is to move forward and eventually Tom might take it > > > > > > via a pull request. > > > > > > > > > > > > Do you have any other ideas? > > > > > > > > > > > > Perhaps this is something we could put on the agenda for a future > > > > > > call. > > > > > > > > > > There's nothing to discuss in a future call as step one is "post > > > > > patches > > > > > against mainline". > > > > > > > > I agree with Tom here. I did look at some of the patches in this > > > > series and now that I've learnt it doesn't even apply on top of > > > > mainline it feels like I've wasted my time. > > > > > > There is a base commit in the cover letter, so you could look at that > > > if you like. > > > > Yes, it's on a commit that's not in mainline. No one wants to look at > > your personal tree. > > > > > > > I plan to apply this series at some point, so any review is not a > > > waste of time. I do respond to review feedback. > > > > And since it's not based on mainline, this is why people have told you > > they don't want to review your changes. Since you aren't working on > > mainline it's unclear if it will ever be in mainline. > > From my side I would suggest that you avoid bringing up this point on > each patch. It doesn't seem to be working towards resolution. I am > doing what I need to, for now, for the project to make progress.
No, you're hindering the project with your insistence on posting patches for your personal tree. I am trying to make it clear to the casual developers of the project that it is your personal tree and not mainline. > > Since this series in fact looks to be addressing some of the feedback > > that was provided to one of the series you applied yourself (the other > > being part of Casey's RFC), it would be much more effective to start on > > the next branch now with the leg work on factoring the common EFI things > > to be usable by EFI_APP and EFI_LOADER. This would lead to being able to > > rebase other work on top of it and reduce the delta between your tree > > and mainline. That's the only way it will get in to mainline. > > Yes I am acting on the feedback provided to Matthew's series and will > get there in the end. > > More generally, there is a substantial amount of work needed on the > EFI app and unfortunately this requires refactoring the EFI-loader > code to avoid duplication. Honestly, I'm not even sure that I want to > take that on, since my ability to get things into lib/efi_loader is > pretty limited and it is seldom an enjoyable experience. I am pleased > to see that the ANSI patch [1] is now in your tree, though, so thank > you for that. > > Re the app, I would very much like to send a PR at some point, but > there are other EFI patches not applied, so it is not trivial. > > I'm working with Heinrich on this and agreed with him that I'll take > another look... If you intend to get things in to mainline, you MUST stop posting things against your personal tree. You are making everything harder than it needs to be. You do not have some special exemption from the normal process. Iterative development on top of mainline, not some feature branch that branched off ages ago. And please note, *I* did not take the ANSI patch. Heinrich did. He and Ilias are the EFI_LOADER custodians. That's why it's in mainline. -- Tom
signature.asc
Description: PGP signature
