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