Re: [RFC PATCH 1/2] arm64: amlogic: add new ARCH_AMLIPC for IPC SoC

[Dmitry Rokosov]

On Wed, Apr 19, 2023 at 06:25:07PM +0200, Neil Armstrong wrote:
> On 19/04/2023 18:04, Dmitry Rokosov wrote:
> > On Wed, Apr 19, 2023 at 03:43:12PM +0200, Neil Armstrong wrote:
> > > On 19/04/2023 15:14, Dmitry Rokosov wrote:
> > > > On Wed, Apr 19, 2023 at 03:38:33PM +0800, =Xianwei Zhao wrote:
> > > > > From: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
> > > > > 
> > > > > The C series SoCs are designed for smart IP camera
> > > > > applications, which does not belong to Meson series.
> > > > > So, Add ARCH_AMLIPC for the new series.
> > > > > 
> > > > > There are now multiple amlogic SoC seies supported, so group them under
> > > > > their own menu. we can easily add new platforms there in the future.
> > > > > Introduce ARCH_AMLOGIC to cover all Amlogic SoC series.
> > > > > 
> > > > > No functional changes introduced.
> > > > > 
> > > > > Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
> > > > > ---
> > > > >    arch/arm64/Kconfig.platforms | 12 ++++++++++++
> > > > >    arch/arm64/configs/defconfig |  2 ++
> > > > >    2 files changed, 14 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > > > > index 89a0b13b058d..bfbc817eef8f 100644
> > > > > --- a/arch/arm64/Kconfig.platforms
> > > > > +++ b/arch/arm64/Kconfig.platforms
> > > > > @@ -162,12 +162,24 @@ config ARCH_MEDIATEK
> > > > >    	  This enables support for MediaTek MT27xx, MT65xx, MT76xx
> > > > >    	  & MT81xx ARMv8 SoCs
> > > > > +menuconfig ARCH_AMLOGIC
> > > > > +	bool "NXP SoC support"
> > > > 
> > > > NXP? Did you mean "Amlogic"?
> > > > 
> > > > > +
> > > > > +if ARCH_AMLOGIC
> > > > > +
> > > > >    config ARCH_MESON
> > > > >    	bool "Amlogic Platforms"
> > > > >    	help
> > > > >    	  This enables support for the arm64 based Amlogic SoCs
> > > > >    	  such as the s905, S905X/D, S912, A113X/D or S905X/D2
> > > > > +config ARCH_AMLIPC
> > > > 
> > > > Do we really need a different ARCH for Amlogic IPC?
> > > > I can imagine that it's not the Meson architecture at all.
> > > > But maybe a better solution is just to rename ARCH_MESON to ARCH_AMLOGIC?
> > > 
> > > It should be changed treewide, and is it worth it ?
> > 
> > As far as I understand, the A1 and S4 families are not fully compatible
> > with the Meson architecture, and we haven't provided additional ARCH_*
> > for them.
> 
> The GXBB, GXL/GXM, G12A, G12B & SM1 are also not fully compatible,
> but they lie under the "MESON" umbrella which covers SoC since the
> Meson6 architecture. It's a facility to include/exclude Amlogic
> drivers/DT, nothing else.
> 
> If you compare it to BCM or NXP, it's different situation, the
> different ARCH_* actually targets totally different SoCs from
> completely different Business Units or from companies acquisitions.
> 
> We should have named it ARCH_AMLOGIC since the beginning, but we
> can't change history.
> 
> > In my opinion, it's a good time to split the Meson architecture into
> > proper subsets, or rename it treewide (maybe only config option
> > ARCH_MESON => ARCH_AMLOGIC).
> 
> MESON is only a codename to differentiate from other SoC vendors
> because Amlogic used it as a codename for a long time.
> Compare this to Allwinner's "sunxi" or Qualcomm's "msm".
> 
> This config has no functional mean, it's only a config namespace.
> 
> Renaming it would need renaming it in all subsystems Kconfig/Makefiles
> and will certainly break builds with custom kernel configs
> in various publicly used builds like Armbian, meta-meson, LibreELEC,
> Debian, Suse, ...
> 
> So it's pointless to change, and even add a different one since
> it's not a family differentiator since the Kernel is modular
> and works around DT to determine which drivers to probe.
> 
> Neil
> 

Thank you for the detailed explanation; it makes sense!
Actually, I disagree with creating a separate ARCH without first reworking
all of its subsets - that's why I started this discussion.
Now, I see that you share my perspective and believe that C3
should be added to the ARCH_MESON subset, so I have no objections.

[...]

-- 
Thank you,
Dmitry