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

Kelvin Zhang
2023-04-27T09:25:04Z

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