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.apalodimas@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-sjg@chromium.org/ and this is what actually got merged https://lore.kernel.org/u-boot/20241129170814.768438-1-ilias.apalodimas@lina... 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... 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.