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

[Kelvin Zhang]

On 2023/4/20 20:39, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
>
> On 20/04/2023 10:43, Kelvin Zhang wrote:
> > > > > > > > +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.
> > GXBB, GXL/GXM, G12A, G12B , SM1 and S4 belong to media box.
> > So, "MESON" represents the media box series.
> > Up to now, "MESON" works well for all existing chips except A1 and AXG.
> > > > 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.
> > Firstly, the new C series is totally different from previous MESON series.
> >   From the perspective of application, the new C series is designed for
> > smart IP camera applications,
> > while MESON series is designed for hybrid OTT/ IP Set Top Box  and
> > high-end media box applications.
> >   From the perspective of architecture, the new C series integrates the
> > sensor interface, image signal processing unit, Dewarp, video encoder,
> > neural networking processing unit,

These new drivers are still under development.

> > which MESON series does not and will never have.
> > Secondly, there are C1 and C2 besides C3.
> > Moreover, more other series are on the way, such as T series.
> > If we always stick to "MESON", people will get more and more confused.
> > Therefore, I think it is the right time to add ARCH_AMLIPC.
>
> Your DTS uses compatibles from meson, so I would argue that it is still
> part of the same architecture.
So far, this patch only contains basic peripherals, which are reused IPs.

> Anyway, this is just config symbol, so it does not matter. There will be
> no confusion and no problem of keeping it ARCH_MESON.
>
>
> > > > We should have named it ARCH_AMLOGIC since the beginning, but we
> > > > can't change history.
> > Shouldn't we deserve a chance to make it right?
> > > > > 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, ...
> > Let's get back to ARCH_AMLIPC.
> > We just need to add ARCH_AMLIPC in the necessary subsystems
> > Kconfig/Makefile.
> > This change will keep the existing MESON related code,  and will neither
> > involve renaming nor break any builds.
>
> It is also not necessary and not justified. We do not have multiple
> top-level subarchs for one architecture. We had such talks already and
> there was no consensus to change it.

Got it.
Thanks!
> Best regards,
> Krzysztof