BALATON Zoltan <bala...@eik.bme.hu> writes: > On Tue, 21 Apr 2020, Markus Armbruster wrote: >> BALATON Zoltan <bala...@eik.bme.hu> writes: >>> On Mon, 20 Apr 2020, Markus Armbruster wrote: >>>> Requesting 32 or 64 MiB of RAM with the sam460ex machine type produces >>>> a useless warning: >>>> >>>> qemu-system-ppc: warning: Memory size is too small for SDRAM type, >>>> adjusting type >>> >>> Why is it useless? It lets user know there was a change so it could >>> help debugging for example. >> >> The memory type is chosen by QEMU, not the user. Why should QEMU warn >> the user when it chooses DDR, but not when it chooses DDR2? >> >>>> This is because sam460ex_init() asks spd_data_generate() for DDR2, >>>> which is impossible, so spd_data_generate() corrects it to DDR. >>> >>> This is correct and intended. The idea is that the board code should >>> not need to know about SPD data, all knowledge about that should be in >>> spd_data_genereate(). >> >> I challenge this idea. >> >> The kind of RAM module a board accepts is a property of the board. >> Modelling that in board code is sensible and easy. Attempting to model >> it in a one size fits all helper function is unlikely to work for all >> boards. >> >> Apparently some boards (including malta) need two banks, so your helper >> increases the number of banks from one to two, but only when that's >> possible without changing the type. >> >> What if another board needs one bank? Four? Two even if that requires >> changing the type? You'll end up with a bunch of flags to drive the >> helper's magic. Not yet because the helper has a grand total of *two* >> users, and much of its magic is used by neither, as demonstrated by >> PATCH 2. >> >> If you want magic, have a non-magic function that does exactly what it's >> told, and a magic one to tell it what to do. The non-magic one will be >> truly reusable. You can have any number of magic ones. Boards with >> sufficiently similar requirements can share a magic one. > > So far we have only sufficiently similar boards that can share the > only magic function. Not many boards use SPD data (these are mostly > needed for real board firmware so anything purely virtual don't model > it usually). The refactoring you propose could be needed if we had > more dissimilar boards but I think we could do that at that > time. Until then I've tried to make it simple for board code and put
Keeping things simple and just serve the needs you actually have is good. We're in a much better position to figure out how to best serve more complicated needs once we actually have them :) > all magic in one place instead of having separate implementation of > this in several boards. Maybe someone should try to convert the > remaining boards (MIPS Malta and ARM integratorcp) to see if any > refactoring is needed before doing those refactoring without checking > first what's needed. I did not try to convert those boards because I > cannot test them. That's fair. [...]