On 8/9/22 18:36, Tom Rini wrote:
[...]
It's tricky to say when to "select" or just "default y if .." or "imply"
a given option. It's not only a matter of "can you disable X and the
system is functional" but "would you normally want to". There are
certainly system bring-up cases and similar where you want to disable
L2CC because you're tracking down something. But it's really a feature
of the chip and expected to work and something we want enabled, and the
scope of when it would be disabled is very very narrow. Looking back at
the patch itself, the config.h files that enabled this are almost all a
SoC-common file (ti_am335x_common.h should have been setting this I bet,
something else for the TODO pile for me), so my thought is that it
should be a select'd option, but if there's strong opinion that it's
useful to make it easy to turn off when needed, imply'd instead by the
various ARCH's in question instead.
That said, thinking about the missing dependency I listed above, that's
how to disable L2 when needed, so perhaps select SYS_L2_PL310 if
!SYS_L2CACHE_OFF under the various ARCH's is the right way.
Uh, I didn't realize we also had this SYS_L2CACHE_OFF .
Yeah, and it's also one of the odd "Enable this to turn oFF ..."
options, but I think in this case, that makes the most sense. Note that
SYS_L2CACHE_OFF defaults to 'n' so do assume we have an L2 cache, and
also the option is ARM-specific.
In that case, the SYS_L2_PL310 selects the L2CC driver to be compiled in and
that should likely be per-arch select indeed. And then SYS_L2CACHE_OFF
should be 'default' or 'imply', so the user can configure it for the various
hardware bring up cases ?
Yeah, I think we're in agreement here.
Yes
