Re: [Xen-devel] [PATCH v7 08/12] arm: add ALL, QEMU, Rcar3 and MPSoC configs

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 28/07/18 00:30, Stefano Stabellini wrote:
> On Fri, 27 Jul 2018, Julien Grall wrote:
> > Hi Stefano,
> > Sorry for the top posting.
> >
> > I think Andrii made a good point. With your new code MPSOC will get built on 
> > Arm 32 bit as well.
> >
> > This was not the case before this patch.
> >
> > So I would like at least that to be fixed before any commit.
> OK, this is a problem. I'll fix it.
>
> Looking into it I think there is a way to solve it that doesn't require
> duplication, and not even the introduction of ALL_32 and ALL_64: we just
> need to add "if ARM_64" for the ARM64 platforms and "if ARM_32" for the
> ARM32 platforms.
>
> So today we would have (the "if ARM_64" is new compared to this patch):
>
>    config ALL
>        bool "All Platforms"
>        select MPSOC_PLATFORM if ARM_64
>
> In the furure, assuming that we had EXYNOS5 and OMAP5 options, it would become:
>
>    config ALL
>        bool "All Platforms"
>        select MPSOC_PLATFORM if ARM_64
>        select EXYNOS5 if ARM_32
>        select OMAP5 if ARM_32
>
> What do you think?
I think I would prefer the suggestion I made on Andrii's answer:

choice
   config ALL_PLAT
      bool "All Platforms"
      "...."
endchoice

config ALL_PLAT_64
    default (ALL && ARM_64)

config ALL_PLAT_32
    default (ALL_PLAT && ARM_32)

config MPSOC_PLATFORM
    bool
    default (ALL_PLAT_64 || MPSOC)

This would avoid a long list in "ALL_PLATFORMS".

Also, I just noticed that 'ALL' will become CONFIG_ALL. This is slightly 
too vague as a name. It would be better to include "PLAT" in it.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel