Picking just a few points.. On Thu, Dec 12, 2024 at 07:04:54AM -0700, Simon Glass wrote: > Hi Ilias, > > On Sat, 7 Dec 2024 at 01:18, Ilias Apalodimas > <ilias.apalodi...@linaro.org> wrote: [snip] > > I haven't had time to look into that series, but I will. The problem > > with reverts like that is > > - I get the feeling you need some special code for some special > > ancient chromebooks that no one cares or uses apart from you. That's > > problematic overall. We can't just make a project look weird because > > some hardware is special. It's the hardware that has to adapt, not the > > other way around > > - Reverts need to have a *very solid* explanation. > > > > For example, this is your x86 revert patches > > https://lore.kernel.org/u-boot/20241112131830.576864-1-...@chromium.org/ > > and this is what actually got merged > > https://lore.kernel.org/u-boot/20241129170814.768438-1-ilias.apalodi...@linaro.org/ > > Which one would you rather read a year from now debugging a similar issue? > > Reverts are done to fix regressions. We don't need to justify a revert > that breaks something. It is just a revert. You can see how this > should be done at [1]. Tom applied it within a day, without any fuss, > as he should. No, this is not a core issue either.
You most certainly do need to justify a revert. More often than not,
reverting a change to fix one platform breaks another platform. See for
example the whole discussion around SPI-NOR changes that may or may not
remain in v2025.01 at this point because they enable some platforms and
broke others, and only just about now *may* have fixed the others. The
revert you note in [1] is another good example of how and who needs to
justify a revert. You wrote 2e9313179a84 ("global_data: Drop
spl_handoff") and so it's a lot easier for me to accept a revert from
you. I assume that you tested all of the cases and applied it. I'm
likely to apply
https://patchwork.ozlabs.org/project/uboot/patch/20241209161434.6563-2-clamo...@gmail.com/
as soon as Svyatoslav says it's ready or asks because that's also their
code. And if you want to get picky about it, I do wish your revert in
[1] was more detailed about what was broken afterall, assuming a rework
of the intended change is coming, so it's clear what was wrong, not just
that it was wrong. A revert (and git bisect) are great for finding a
problem, but only sometimes the *fix* for a problem.
[snip]
> > Now I don't know who Kevin and BoB
> > are and I wish them luck, but that's hardly a readable commit message
> > or will mean anything in a year from now.
>
> chromebook_kevin and chromebook_bob are a rk3399-based Chromebooks
>
> Again, the commit message is not a core issue. If you found yourself
> in a reflective mood, you might want to consider how I feel about 5-6
> platforms which I care about a lot, being permanently broken in Tom's
> tree.
Commit message are a core issue, because they need to be clear (and not
strictly concise) about what a change is for and what a platform
requires. We're a community and the written word, including commit
messages is how we tell everyone else things. Including ourselves a year
down the line so we can see why we did what we did.
--
Tom
signature.asc
Description: PGP signature
