On 19.06.23 23:01, Paul Menzel wrote:
Am 19.06.23 um 22:31 schrieb Martin Roth:
Duplicated code between mainboards isn't a big issue in my opinion. It allows the boards to be customized without worrying about other companies' mainboards. We've tried to make mainboards as small as we can, and we can keep refactoring things out where it makes sense.
Are you talking in general or in this specific SRP boards. As stated by others, a lot of code was copy-pasted.
I don't see this. Maybe I'm too tired, but I only see about 500 lines in `archercity_crb/`. Maybe 300 of them being general boilerplate that one can't escape when adding a new board. There's an IIO header file (142 lines) that looks board specific.
The only exception seems to be the already mentioned mainboard_config_ iio() function which looks like something that belongs into platform code (it looks generic with only a few numbers hardcoded that could be added to a struct as well).
After all, if I'm not looking at the wrong branch, this looks cleaner than average for coreboot (wrt. copied code, I didn't look at the individual patches).
If some common code fits under the SoC, that's great, and I'm all for moving it there, but let's not force the burden of that refactoring work onto inexperienced mainboard maintainers. Doing so just encourages vendors to keep their mainboards in private repositories - the opposite of what we should be working for. Even if this means that it doesn't get refactored and gets a bit out-of-date, I find that preferable to making contributors (more) frustrated about getting their boards accepted.
I haven’t seen any frustration. Do you know more? I agree, nobody should be frustrated. The question has to be answered though, who is going to do that general maintenance work – I think Kyösti raised a similar question a few weeks back. I welcome all new contributors, but it has to be clear, that coreboot – especially for commercial offerings – needs and expects some kind of maintenance commitment. I am pretty sure, the vendors are going to understand the benefits. It just has to be communicated transparently.
I agree in general, but in this particular case it seems overzealous. We better save our energy until the next entire SoC code gets copied. Or for the review of these new boards. It seems like a good opportu- nity to teach people about coreboot.
Nico