On 11.02.2008 03:42, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
system is not there to set code internal only variables. That's what
arch/x86 cpu dependent include files are there for.
Actually, this is one of the good aspects of v2: setting
subarch-dependant variables dependant on subarch and/or mainboard.
We start reveiling too much information in the configuration process,
implying this is something that could be changed on a per-board basis.
In reality it is just a part of the code that we source out now. The
CAR code for a given platform should know how to get CAR working on
Same could be said for serial, yet we offer configurability (in the dts)
for serial iobase even on a mainboard level.
My prognosis is we will end up with different settings
like we had in v2.
How so? The code I committed selects an invisible Kconfig variable based
on the subarch alone. A mainboard porter will never see the variable nor
does he need to know it exists.
And in 6 months noone will know anymore why we have
settings. In v2 you need a larger CAR area if you want to enable some
features triggered by other CONFIG variables. So there's a dependency,
but it is up to the user to manually resolve it. Just enabling those
features will not work anymore, because you need to know that you also
have to increase CAR.
And manual resolution is prone to failure because the user needs to know
the dependencies. That's why having that stuff in Kconfig in v3 makes
things easier. Besides that, code which just fails to build (or even
worse, fails to run) without giving a user-understandable error message
is just broken. My CAR changes in v2 and v3 fixed some of the most
glaring problems in that area for CAR.
The next step is to pack all kinds of logic into the
that does not belong there. In v2 we even calculated the offsets of
normal and fallback in the config file. You enable a small piece of
code on the Xth board using a given mainboard and things will fall apart.
I think CAR_BASE and CAR_SIZE should be a property encapsulated in the
CAR code component. If some component needs to change those values,
they should provide a "reason" rather than just values. As the
complexity of v3 increases, the CAR component can then locally look at
all the "reasons" influencing required CAR size and react upon that.
Locally. Not globally. Kconfig stuff is global. Global is bad.
Oh, CARBASE must be global anyway because printk needs to know it for
the buffer address calculation. That would make CARBASE a perfect fit
How do you
define "code internal only variables"? I think variables with
arch-dependent value (as opposed to arch-dependent existence) should be
set by Kconfig. There is no reason to have stuff like LX_NUM_CACHELINES
in Kconfig because it doesn't even exist on other subarches.
CONFIG stuff should be feature driven, not code driven. A feature is
something like the "printk log buffer". If you want that, you might
need to increase the CAR size. The code needs to know that, and change
the CAR values accordingly.
CONSOLE_LOGLEVEL is code driven, yet nobody is arguing for its removal.
Otherwise the fragility of the code is increased by
the fragility of
the Kconfig mechanism. If someone thinks it is necessary to adjust
CAR_BASE and CAR_SIZE via Kconfig, the logic of the approach is
broken. It means the code needs fixing. Not hotfixing.
CARBASE can not be adjusted with any config interface. It is a hidden
variable. I still fail to see where the logic of the approach is broken.
There is indeed no reason for LX_NUM_CACHELINES in
Keeping it there is as broken as your last change.
That's not a configuration aspekt. You can't configure your board to
have more cache lines.
Packing all this stuff into Kconfig looks very developer friendly at
first, while it is going to betray us in the same way it did and does
in v2. It reduces the testability and reusability of the code while
implicating that everything got more flexible.
How does it reduce testability and usability over lots of #ifdefs?
That seems to be a bad model for us. The wikipedia page says 3% of the
total effort are spent on design review. If we really did that, nobody
would have had the time to question my design to have CARBASE in Kconfig.