Re: [PATCH v2 25/35] nds32: Build infrastructure

[Arnd Bergmann]

On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@xxxxxxxxx> wrote:
> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@xxxxxxxx>:
>> On Wed, Nov 29, 2017 at 12:39 PM, Greentime Hu <green.hu@xxxxxxxxx> wrote:
>>>
>>> How about this?
>>>
>>> choice
>>>         prompt "CPU type"
>>>         default CPU_N13
>>> config CPU_N15
>>>         bool "AndesCore N15"
>>>         select CPU_CACHE_NONALIASING
>>> config CPU_N13
>>>         bool "AndesCore N13"
>>>         select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> config CPU_N10
>>>         bool "AndesCore N10"
>>>         select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> config CPU_D15
>>>         bool "AndesCore D15"
>>>         select CPU_CACHE_NONALIASING
>>>         select HWZOL
>>> config CPU_D10
>>>         bool "AndesCore D10"
>>>         select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> endchoice
>>
>> With a 'choice' statement this works, but I would consider that
>> suboptimal for another reason: now you cannot build a kernel that
>> works on e.g. both N13 and N15.
>>
>> This is what we had on ARM a long time ago and on MIPS not so long
>> ago, but it's really a burden for testing and distribution once you get
>> support for more than a handful of machines supported in the mainline
>> kernel: If each CPU core is mutually incompatible with the other ones,
>> this means you likely end up having one defconfig per CPU core,
>> or even one defconfig per SoC or board.
>>
>> I would always try to get the largest amount of hardware to work
>> in the same kernel concurrently.
>>
>> One way of of this would be to define the "CPU type" as the minimum
>> supported CPU, e.g. selecting D15 would result in a kernel that
>> only works on D15, while selecting N15 would work on both N15 and
>> D15, and selecting D10 would work on both D10 and D15.
>>
>
> Hi, Arnd:
>
> Maybe we should keep the original implementation for this reason.
> The default value of CPU_CACHE_NONALIASING and ANDES_PAGE_SIZE_8KB is
> available for all CPU types for now.
> User can use these configs built kernel to boot on almost all nds32 CPUs.
>
> It might be a little bit weird if we config CPU_N10 but run on a N13 CPU.
> This might confuse our users.

I think it really depends on how much flexibility you want to give to users.
The way I suggested first was to allow selecting an arbitrary combination
of CPUs, and then let Kconfig derive the correct set of optimization flags
and other options that work with those. That is probably the easiest for
the users, but can be tricky to get right for all combinations.

When you put them in a sorted list like I mentioned for simplicity, you
could reduce the confusion by naming them differently, e.g.
CONFIG_CPU_N10_OR_NEWER.

Having only the CPU_CACHE_NONALIASING option is fine if you
never need to make any other decisions based on the CPU core
type, but then the help text should describe specifically which cases
are affected (N10/N13/D13 with 4K page size), and you can decide to
hide the option and make it always-on when using 8K page size.

       Arnd