On Sat, May 24, 2025 at 04:35:03PM +0100, Simon Glass wrote: > Hi Tom, > > On Sat, 24 May 2025 at 15:41, Tom Rini <tr...@konsulko.com> wrote: > > > > 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. > > So how about: > - let me speed up your CI
We last left this argument at you saying it was too much work to demonstrate speeding up CI on source.denx.de in u-boot-dm and me not being able to see that as a serious point because it's something like a dozen mouse clicks at most. > - accept some of the things that you've previously rejected That's probably going to continue to be a No because it's not like people flipped a coin and went with "Nope, lets reject this". People said you're wrong or going in the wrong direction and you disagree. > In general, I think it would really help if you could adopt a more > incremental approach to development. It would also help if we could > discuss things every now and then, instead of all this email. I don't know if you're being serious with this statement. I don't think you can be serious with this statement. Doing things incremental would mean doing them slowly, and in small reviewable chunks and 30-40-50 part series followed by more of the same. And there's a regularly scheduled community call every two weeks. I would really only ask that someone other than you or I on the call take the notes for any of these topics, just so it's clear later that any misrepresentation is accidental. -- Tom
signature.asc
Description: PGP signature
