[RFC PATCH 0/2] Baisc devicetree support for Amlogic C3

[RFC PATCH 0/2] Baisc devicetree support for Amlogic C3

[=Xianwei Zhao]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Amlogic C3 is an advanced edge AI processor designed for smart IP camera
applications.

Add ARCH_AMLIPC for the new series.

Add basic devicetree support for the C3 based Amlogic AW409 board,
which describes the following components: CPU, GIC, IRQ, Timer, UART.
It's capable of booting up into the serial console.

Xianwei Zhao (2):
  arm64: amlogic: add new ARCH_AMLIPC for IPC SoC
  arm64: dts: add support for C3 based Amlogic AW409

 arch/arm64/Kconfig.platforms                  | 12 +++
 arch/arm64/boot/dts/amlogic/Makefile          |  1 +
 .../amlogic/amlogic-c3-c302x-aw409-256m.dts   | 30 +++++++
 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi   | 87 +++++++++++++++++++
 arch/arm64/configs/defconfig                  |  2 +
 5 files changed, 132 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi


base-commit: ae68fb187b59bc8645974320808ab2d7c41b1833
-- 
2.37.1

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

[=Xianwei Zhao]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

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@amlogic.com>
---
 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"
+
+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
+	bool "Amlogic IPC Platforms"
+	help
+	  This enables support for the arm64 based Amlogic IPC SoCs
+	  such as the C302X, C308L
+endif
+
 config ARCH_MVEBU
 	bool "Marvell EBU SoC Family"
 	select ARMADA_AP806_SYSCON
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7790ee42c68a..f231bd1723fd 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -46,7 +46,9 @@ CONFIG_ARCH_LG1K=y
 CONFIG_ARCH_HISI=y
 CONFIG_ARCH_KEEMBAY=y
 CONFIG_ARCH_MEDIATEK=y
+CONFIG_ARCH_AMLOGIC=y
 CONFIG_ARCH_MESON=y
+CONFIG_ARCH_AMLIPC=y
 CONFIG_ARCH_MVEBU=y
 CONFIG_ARCH_NXP=y
 CONFIG_ARCH_LAYERSCAPE=y
-- 
2.37.1

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

[Krzysztof Kozlowski]

On 19/04/2023 09:38, =Xianwei Zhao wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> 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@amlogic.com>
> ---
>  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? There is already amlogic anyway.


Best regards,
Krzysztof

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

[Neil Armstrong]

Hi,

On 19/04/2023 09:38, =Xianwei Zhao wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> 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.

There's no strong reason to add a new ARCH here, apart willing to kill
the MESON name, but it's usual to keep a arch naming from the first
SoC family supported, it doesn't mean the new aren't supported or
whatever, it's only a simple config name.

I understand the new SoCs are a new family, but they still share a lot of
IPs and driver that are present and supported on the Meson SoCs.

Please concentrate on adding the DTs and compatibles, changing the
current config and driver names is low priority unless there's a really
strong reason.

It's perfectly ok to drop meson for new configs, new driver & new bindings,
and it's already the case for the A1 SPIFC driver and most new G12/SM1
compatibles.

Neil

> 
> No functional changes introduced.
> 
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> ---
>   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"
> +
> +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
> +	bool "Amlogic IPC Platforms"
> +	help
> +	  This enables support for the arm64 based Amlogic IPC SoCs
> +	  such as the C302X, C308L
> +endif
> +
>   config ARCH_MVEBU
>   	bool "Marvell EBU SoC Family"
>   	select ARMADA_AP806_SYSCON
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 7790ee42c68a..f231bd1723fd 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -46,7 +46,9 @@ CONFIG_ARCH_LG1K=y
>   CONFIG_ARCH_HISI=y
>   CONFIG_ARCH_KEEMBAY=y
>   CONFIG_ARCH_MEDIATEK=y
> +CONFIG_ARCH_AMLOGIC=y
>   CONFIG_ARCH_MESON=y
> +CONFIG_ARCH_AMLIPC=y
>   CONFIG_ARCH_MVEBU=y
>   CONFIG_ARCH_NXP=y
>   CONFIG_ARCH_LAYERSCAPE=y

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

[Dmitry Rokosov]

On Wed, Apr 19, 2023 at 03:38:33PM +0800, =Xianwei Zhao wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> 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@amlogic.com>
> ---
>  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?

> +	bool "Amlogic IPC Platforms"
> +	help
> +	  This enables support for the arm64 based Amlogic IPC SoCs
> +	  such as the C302X, C308L
> +endif
> +
>  config ARCH_MVEBU
>  	bool "Marvell EBU SoC Family"
>  	select ARMADA_AP806_SYSCON
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 7790ee42c68a..f231bd1723fd 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -46,7 +46,9 @@ CONFIG_ARCH_LG1K=y
>  CONFIG_ARCH_HISI=y
>  CONFIG_ARCH_KEEMBAY=y
>  CONFIG_ARCH_MEDIATEK=y
> +CONFIG_ARCH_AMLOGIC=y
>  CONFIG_ARCH_MESON=y
> +CONFIG_ARCH_AMLIPC=y
>  CONFIG_ARCH_MVEBU=y
>  CONFIG_ARCH_NXP=y
>  CONFIG_ARCH_LAYERSCAPE=y
> -- 
> 2.37.1
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
Thank you,
Dmitry

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

[Neil Armstrong]

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@amlogic.com>
>>
>> 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@amlogic.com>
>> ---
>>   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 ?

Neil

> 
>> +	bool "Amlogic IPC Platforms"
>> +	help
>> +	  This enables support for the arm64 based Amlogic IPC SoCs
>> +	  such as the C302X, C308L
>> +endif
>> +
>>   config ARCH_MVEBU
>>   	bool "Marvell EBU SoC Family"
>>   	select ARMADA_AP806_SYSCON
>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>> index 7790ee42c68a..f231bd1723fd 100644
>> --- a/arch/arm64/configs/defconfig
>> +++ b/arch/arm64/configs/defconfig
>> @@ -46,7 +46,9 @@ CONFIG_ARCH_LG1K=y
>>   CONFIG_ARCH_HISI=y
>>   CONFIG_ARCH_KEEMBAY=y
>>   CONFIG_ARCH_MEDIATEK=y
>> +CONFIG_ARCH_AMLOGIC=y
>>   CONFIG_ARCH_MESON=y
>> +CONFIG_ARCH_AMLIPC=y
>>   CONFIG_ARCH_MVEBU=y
>>   CONFIG_ARCH_NXP=y
>>   CONFIG_ARCH_LAYERSCAPE=y
>> -- 
>> 2.37.1
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 

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

[Dmitry Rokosov]

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@amlogic.com>
> > > 
> > > 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@amlogic.com>
> > > ---
> > >   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.
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).

> > 
> > > +	bool "Amlogic IPC Platforms"
> > > +	help
> > > +	  This enables support for the arm64 based Amlogic IPC SoCs
> > > +	  such as the C302X, C308L
> > > +endif
> > > +
> > >   config ARCH_MVEBU
> > >   	bool "Marvell EBU SoC Family"
> > >   	select ARMADA_AP806_SYSCON
> > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > > index 7790ee42c68a..f231bd1723fd 100644
> > > --- a/arch/arm64/configs/defconfig
> > > +++ b/arch/arm64/configs/defconfig
> > > @@ -46,7 +46,9 @@ CONFIG_ARCH_LG1K=y
> > >   CONFIG_ARCH_HISI=y
> > >   CONFIG_ARCH_KEEMBAY=y
> > >   CONFIG_ARCH_MEDIATEK=y
> > > +CONFIG_ARCH_AMLOGIC=y
> > >   CONFIG_ARCH_MESON=y
> > > +CONFIG_ARCH_AMLIPC=y
> > >   CONFIG_ARCH_MVEBU=y
> > >   CONFIG_ARCH_NXP=y
> > >   CONFIG_ARCH_LAYERSCAPE=y
> > > -- 
> > > 2.37.1
> > > 
> > > 
> > > _______________________________________________
> > > linux-amlogic mailing list
> > > linux-amlogic@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-amlogic
> > 
> 

-- 
Thank you,
Dmitry

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

[Neil Armstrong]

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@amlogic.com>
>>>>
>>>> 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@amlogic.com>
>>>> ---
>>>>    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

> 
>>>
>>>> +	bool "Amlogic IPC Platforms"
>>>> +	help
>>>> +	  This enables support for the arm64 based Amlogic IPC SoCs
>>>> +	  such as the C302X, C308L
>>>> +endif
>>>> +
>>>>    config ARCH_MVEBU
>>>>    	bool "Marvell EBU SoC Family"
>>>>    	select ARMADA_AP806_SYSCON
>>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>>>> index 7790ee42c68a..f231bd1723fd 100644
>>>> --- a/arch/arm64/configs/defconfig
>>>> +++ b/arch/arm64/configs/defconfig
>>>> @@ -46,7 +46,9 @@ CONFIG_ARCH_LG1K=y
>>>>    CONFIG_ARCH_HISI=y
>>>>    CONFIG_ARCH_KEEMBAY=y
>>>>    CONFIG_ARCH_MEDIATEK=y
>>>> +CONFIG_ARCH_AMLOGIC=y
>>>>    CONFIG_ARCH_MESON=y
>>>> +CONFIG_ARCH_AMLIPC=y
>>>>    CONFIG_ARCH_MVEBU=y
>>>>    CONFIG_ARCH_NXP=y
>>>>    CONFIG_ARCH_LAYERSCAPE=y
>>>> -- 
>>>> 2.37.1
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-amlogic mailing list
>>>> linux-amlogic@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>>>
>>
> 

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@amlogic.com>
> > > > > 
> > > > > 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@amlogic.com>
> > > > > ---
> > > > >    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

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

[Kelvin Zhang]

On 2023/4/20 01:00, Dmitry Rokosov wrote:
> [ EXTERNAL EMAIL ]
>
> 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@amlogic.com>
>>>>>>
>>>>>> 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@amlogic.com>
>>>>>> ---
>>>>>>     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.
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,
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.
>> 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.
>> 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.
Proper names play an important role in understanding the code, right?
>> 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
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Neil Armstrong]

Hi Kelvin,

On 20/04/2023 10:43, Kelvin Zhang wrote:
> On 2023/4/20 01:00, Dmitry Rokosov wrote:
>> [ EXTERNAL EMAIL ]
>>
>> 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@amlogic.com>
>>>>>>>
>>>>>>> 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@amlogic.com>
>>>>>>> ---
>>>>>>>     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.
> 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,
> 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.

Thanks for sharing such details, we are all happy to see Amlogic's
commitment to upstream of these SoC families.

But as I explained, this ARCH_MESON doesn't define the SoC type
but badly defines the Amlogic SoCs support.

>>> 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?

Yes, so the right thing to do is to move to ARCH_AMLOGIC

>>>> 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.

The goal of mainline Linux is to build as much as possible and
be modular at runtime, the only supported configuration is arch/arm64/configs/defconfig
and adding a new config to differentiate an Application type
doesn't make sense.

>>> 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.
> Proper names play an important role in understanding the code, right?

Yes, but stable config names also play an important role for the
users for mainline, and there's a big number of users relying
on the stable naming for all SoCs starting from Meson6.

So if you really want to get rid of the ARCH_MESON, migrating to
ARCH_AMLOGIC is the right thing to do, but it involves doing
a treewide migration and there's no easy way to do this and make
sure the users still manages to build for other Amlogic platforms.

So as the Amlogic ARM/ARM64 SoC support maintainer I must make sure
breakage don't happens, and so far I don't see how achieve that.

Now, you can post a RFC so we can discuss on something.

Neil

>>> 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
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Kelvin Zhang]

On 2023/4/20 20:18, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi Kelvin,
> 
> On 20/04/2023 10:43, Kelvin Zhang wrote:
>> On 2023/4/20 01:00, Dmitry Rokosov wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> 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@amlogic.com>
>>>>>>>>
>>>>>>>> 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@amlogic.com>
>>>>>>>> ---
>>>>>>>>     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.
>> 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,
>> 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.
> 
> Thanks for sharing such details, we are all happy to see Amlogic's
> commitment to upstream of these SoC families.
> 
> But as I explained, this ARCH_MESON doesn't define the SoC type
> but badly defines the Amlogic SoCs support.
> 
>>>> 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?
> 
> Yes, so the right thing to do is to move to ARCH_AMLOGIC
> 
>>>>> 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.
> 
> The goal of mainline Linux is to build as much as possible and
> be modular at runtime, the only supported configuration is 
> arch/arm64/configs/defconfig
> and adding a new config to differentiate an Application type
> doesn't make sense.
> 
>>>> 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.
>> Proper names play an important role in understanding the code, right?
> 
> Yes, but stable config names also play an important role for the
> users for mainline, and there's a big number of users relying
> on the stable naming for all SoCs starting from Meson6.
> 
> So if you really want to get rid of the ARCH_MESON, migrating to
> ARCH_AMLOGIC is the right thing to do, but it involves doing
> a treewide migration and there's no easy way to do this and make
> sure the users still manages to build for other Amlogic platforms.
> 
> So as the Amlogic ARM/ARM64 SoC support maintainer I must make sure
> breakage don't happens, and so far I don't see how achieve that.
> 
> Now, you can post a RFC so we can discuss on something.

OK. Let's continue with ARCH_MESON.

> 
> Neil
> 
>>>> 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
>>>
>>> _______________________________________________
>>> linux-amlogic mailing list
>>> linux-amlogic@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 

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

[Krzysztof Kozlowski]

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,
> 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.

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.

Best regards,
Krzysztof

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
> 

[RFC PATCH 2/2] arm64: dts: add support for C3 based Amlogic AW409

[=Xianwei Zhao]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Amlogic C3 is an advanced edge AI processor designed for smart IP camera
applications.

Add basic support for the C3 based Amlogic AW409 board, which describes
the following components: CPU, GIC, IRQ, Timer, UART. It's capable of
booting up into the serial console.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/Makefile          |  1 +
 .../amlogic/amlogic-c3-c302x-aw409-256m.dts   | 30 +++++++
 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi   | 87 +++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi

diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
index cd1c5b04890a..d2b5d0d750bc 100644
--- a/arch/arm64/boot/dts/amlogic/Makefile
+++ b/arch/arm64/boot/dts/amlogic/Makefile
@@ -74,3 +74,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
 dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
 dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air-gbit.dtb
 dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air.dtb
+dtb-$(CONFIG_ARCH_AMLIPC) += amlogic-c3-c302x-aw409-256m.dtb
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
new file mode 100644
index 000000000000..38ca98a32181
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "amlogic-c3.dtsi"
+
+/ {
+	model = "Amlogic C302 aw409 Development Board";
+	compatible = "amlogic,aw409", "amlogic,c3";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	aliases {
+		serial0 = &uart_B;
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x10000000>;
+	};
+
+};
+
+&uart_B {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
new file mode 100644
index 000000000000..c69072ac57f5
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x0 0x1>;
+			enable-method = "psci";
+		};
+
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	psci {
+		compatible = "arm,psci-1.0";
+		method = "smc";
+	};
+
+	xtal: xtal-clk {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "xtal";
+		#clock-cells = <0>;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		gic: interrupt-controller@fff01000 {
+			compatible = "arm,gic-400";
+			#interrupt-cells = <3>;
+			#address-cells = <0>;
+			interrupt-controller;
+			reg = <0x0 0xfff01000 0 0x1000>,
+			      <0x0 0xfff02000 0 0x2000>,
+			      <0x0 0xfff04000 0 0x2000>,
+			      <0x0 0xfff06000 0 0x2000>;
+			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+		};
+
+		apb4: apb4@fe000000 {
+			compatible = "simple-bus";
+			reg = <0x0 0xfe000000 0x0 0x480000>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
+
+			uart_B: serial@7a000 {
+				compatible = "amlogic,meson-g12a-uart";
+				reg = <0x0 0x7a000 0x0 0x18>;
+				interrupts = <GIC_SPI 169 IRQ_TYPE_EDGE_RISING>;
+				status = "disabled";
+				clocks = <&xtal>, <&xtal>, <&xtal>;
+				clock-names = "xtal", "pclk", "baud";
+			};
+
+		};
+	};
+};
-- 
2.37.1

Re: [RFC PATCH 2/2] arm64: dts: add support for C3 based Amlogic AW409

[Krzysztof Kozlowski]

On 19/04/2023 09:38, =Xianwei Zhao wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> Amlogic C3 is an advanced edge AI processor designed for smart IP camera
> applications.
> 
> Add basic support for the C3 based Amlogic AW409 board, which describes
> the following components: CPU, GIC, IRQ, Timer, UART. It's capable of
> booting up into the serial console.
> 
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> ---
>  arch/arm64/boot/dts/amlogic/Makefile          |  1 +
>  .../amlogic/amlogic-c3-c302x-aw409-256m.dts   | 30 +++++++
>  arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi   | 87 +++++++++++++++++++
>  3 files changed, 118 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>  create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
> 
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
> index cd1c5b04890a..d2b5d0d750bc 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -74,3 +74,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air-gbit.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air.dtb
> +dtb-$(CONFIG_ARCH_AMLIPC) += amlogic-c3-c302x-aw409-256m.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
> new file mode 100644
> index 000000000000..38ca98a32181
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "amlogic-c3.dtsi"
> +
> +/ {
> +	model = "Amlogic C302 aw409 Development Board";
> +	compatible = "amlogic,aw409", "amlogic,c3";

Undocumented compatibles. Run checkpatch and fix all warnings. Anyway, I
am not sure this warrants separate architecture. Isn't C3 standard Meson
SoC?

> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	aliases {
> +		serial0 = &uart_B;
> +	};
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x0 0x0 0x0 0x10000000>;
> +	};
> +

Drop stray blank lines.

> +};
> +
> +&uart_B {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
> new file mode 100644
> index 000000000000..c69072ac57f5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a35";
> +			reg = <0x0 0x0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a35";
> +			reg = <0x0 0x1>;
> +			enable-method = "psci";
> +		};
> +

Everywhere...

> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0";
> +		method = "smc";
> +	};
> +
> +	xtal: xtal-clk {
> +		compatible = "fixed-clock";
> +		clock-frequency = <24000000>;
> +		clock-output-names = "xtal";
> +		#clock-cells = <0>;
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		gic: interrupt-controller@fff01000 {
> +			compatible = "arm,gic-400";
> +			#interrupt-cells = <3>;
> +			#address-cells = <0>;
> +			interrupt-controller;
> +			reg = <0x0 0xfff01000 0 0x1000>,
> +			      <0x0 0xfff02000 0 0x2000>,
> +			      <0x0 0xfff04000 0 0x2000>,
> +			      <0x0 0xfff06000 0 0x2000>;
> +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> +		};
> +
> +		apb4: apb4@fe000000 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +			compatible = "simple-bus";
> +			reg = <0x0 0xfe000000 0x0 0x480000>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
> +
> +			uart_B: serial@7a000 {

lowercase for labels

> +				compatible = "amlogic,meson-g12a-uart";

So this is just meson? Drop new ARCH, no need for it. Anyway, you need
SoC specific compatible.

Best regards,
Krzysztof

Re: [RFC PATCH 2/2] arm64: dts: add support for C3 based Amlogic AW409

[xianwei.zhao]

Hi Krzysztof,
             Thank you for your reply.

On 2023/4/19 15:48, Krzysztof Kozlowski wrote:
> [你通常不会收到来自 krzysztof.kozlowski@linaro.org 的电子邮件。请访问 https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要]
> 
> [ EXTERNAL EMAIL ]
> 
> On 19/04/2023 09:38, =Xianwei Zhao wrote:
>> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
>>
>> Amlogic C3 is an advanced edge AI processor designed for smart IP camera
>> applications.
>>
>> Add basic support for the C3 based Amlogic AW409 board, which describes
>> the following components: CPU, GIC, IRQ, Timer, UART. It's capable of
>> booting up into the serial console.
>>
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
>> ---
>>   arch/arm64/boot/dts/amlogic/Makefile          |  1 +
>>   .../amlogic/amlogic-c3-c302x-aw409-256m.dts   | 30 +++++++
>>   arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi   | 87 +++++++++++++++++++
>>   3 files changed, 118 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>>   create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
>> index cd1c5b04890a..d2b5d0d750bc 100644
>> --- a/arch/arm64/boot/dts/amlogic/Makefile
>> +++ b/arch/arm64/boot/dts/amlogic/Makefile
>> @@ -74,3 +74,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air-gbit.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air.dtb
>> +dtb-$(CONFIG_ARCH_AMLIPC) += amlogic-c3-c302x-aw409-256m.dtb
>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>> new file mode 100644
>> index 000000000000..38ca98a32181
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>> @@ -0,0 +1,30 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "amlogic-c3.dtsi"
>> +
>> +/ {
>> +     model = "Amlogic C302 aw409 Development Board";
>> +     compatible = "amlogic,aw409", "amlogic,c3";
> 
> Undocumented compatibles. Run checkpatch and fix all warnings. Anyway, I
> am not sure this warrants separate architecture. Isn't C3 standard Meson
> SoC?
> 
The patch related to bindings is committed, see: 
https://lore.kernel.org/all/20230407102704.1055152-1-kelvin.zhang@amlogic.com/
>> +     interrupt-parent = <&gic>;
>> +     #address-cells = <2>;
>> +     #size-cells = <2>;
>> +
>> +     aliases {
>> +             serial0 = &uart_B;
>> +     };
>> +
>> +     memory@0 {
>> +             device_type = "memory";
>> +             reg = <0x0 0x0 0x0 0x10000000>;
>> +     };
>> +
> 
> Drop stray blank lines.
> 
will do
>> +};
>> +
>> +&uart_B {
>> +     status = "okay";
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
>> new file mode 100644
>> index 000000000000..c69072ac57f5
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
>> @@ -0,0 +1,87 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +
>> +/ {
>> +     cpus {
>> +             #address-cells = <2>;
>> +             #size-cells = <0>;
>> +
>> +             cpu0: cpu@0 {
>> +                     device_type = "cpu";
>> +                     compatible = "arm,cortex-a35";
>> +                     reg = <0x0 0x0>;
>> +                     enable-method = "psci";
>> +             };
>> +
>> +             cpu1: cpu@1 {
>> +                     device_type = "cpu";
>> +                     compatible = "arm,cortex-a35";
>> +                     reg = <0x0 0x1>;
>> +                     enable-method = "psci";
>> +             };
>> +
> 
> Everywhere...
> 
will do
>> +     };
>> +
>> +     timer {
>> +             compatible = "arm,armv8-timer";
>> +             interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +                          <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +                          <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +                          <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> +     };
>> +
>> +     psci {
>> +             compatible = "arm,psci-1.0";
>> +             method = "smc";
>> +     };
>> +
>> +     xtal: xtal-clk {
>> +             compatible = "fixed-clock";
>> +             clock-frequency = <24000000>;
>> +             clock-output-names = "xtal";
>> +             #clock-cells = <0>;
>> +     };
>> +
>> +     soc {
>> +             compatible = "simple-bus";
>> +             #address-cells = <2>;
>> +             #size-cells = <2>;
>> +             ranges;
>> +
>> +             gic: interrupt-controller@fff01000 {
>> +                     compatible = "arm,gic-400";
>> +                     #interrupt-cells = <3>;
>> +                     #address-cells = <0>;
>> +                     interrupt-controller;
>> +                     reg = <0x0 0xfff01000 0 0x1000>,
>> +                           <0x0 0xfff02000 0 0x2000>,
>> +                           <0x0 0xfff04000 0 0x2000>,
>> +                           <0x0 0xfff06000 0 0x2000>;
>> +                     interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>> +             };
>> +
>> +             apb4: apb4@fe000000 {
> 
> Node names should be generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
will do
>> +                     compatible = "simple-bus";
>> +                     reg = <0x0 0xfe000000 0x0 0x480000>;
>> +                     #address-cells = <2>;
>> +                     #size-cells = <2>;
>> +                     ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
>> +
>> +                     uart_B: serial@7a000 {
> 
> lowercase for labels
> 
will do
>> +                             compatible = "amlogic,meson-g12a-uart";
> 
> So this is just meson? Drop new ARCH, no need for it. Anyway, you need
> SoC specific compatible.
> 
> Best regards,
> Krzysztof
> 

Re: [RFC PATCH 2/2] arm64: dts: add support for C3 based Amlogic AW409

[Krzysztof Kozlowski]

On 27/04/2023 10:54, xianwei.zhao wrote:
> Hi Krzysztof,
> 
>          Thank you for your reply.
> 
> On 2023/4/19 15:48, Krzysztof Kozlowski wrote:
>> [你通常不会收到来自krzysztof.kozlowski@linaro.org  的电子邮件。请访问https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要]
>>
>> [ EXTERNAL EMAIL ]
>>
>> On 19/04/2023 09:38, =Xianwei Zhao wrote:
>>> From: Xianwei Zhao<xianwei.zhao@amlogic.com>
>>>
>>> Amlogic C3 is an advanced edge AI processor designed for smart IP camera
>>> applications.
>>>
>>> Add basic support for the C3 based Amlogic AW409 board, which describes
>>> the following components: CPU, GIC, IRQ, Timer, UART. It's capable of
>>> booting up into the serial console.
>>>
>>> Signed-off-by: Xianwei Zhao<xianwei.zhao@amlogic.com>
>>> ---
>>>   arch/arm64/boot/dts/amlogic/Makefile          |  1 +
>>>   .../amlogic/amlogic-c3-c302x-aw409-256m.dts   | 30 +++++++
>>>   arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi   | 87 +++++++++++++++++++
>>>   3 files changed, 118 insertions(+)
>>>   create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>>>   create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
>>> index cd1c5b04890a..d2b5d0d750bc 100644
>>> --- a/arch/arm64/boot/dts/amlogic/Makefile
>>> +++ b/arch/arm64/boot/dts/amlogic/Makefile
>>> @@ -74,3 +74,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air-gbit.dtb
>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air.dtb
>>> +dtb-$(CONFIG_ARCH_AMLIPC) += amlogic-c3-c302x-aw409-256m.dtb
>>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>>> new file mode 100644
>>> index 000000000000..38ca98a32181
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>>> @@ -0,0 +1,30 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "amlogic-c3.dtsi"
>>> +
>>> +/ {
>>> +     model = "Amlogic C302 aw409 Development Board";
>>> +     compatible = "amlogic,aw409", "amlogic,c3";
>> Undocumented compatibles. Run checkpatch and fix all warnings. Anyway, I
>> am not sure this warrants separate architecture. Isn't C3 standard Meson
>> SoC?
> The patch related to bindings is committed, see: 
> https://lore.kernel.org/all/20230407102704.1055152-1-kelvin.zhang@amlogic.com/

And how can we know this? There are no bindings in next, so you are
supposed to sent these together. At minimum link to the bindings patch.

Otherwise you will keep getting review comments.

Best regards,
Krzysztof

Re: [RFC PATCH 2/2] arm64: dts: add support for C3 based Amlogic AW409

[Neil Armstrong]

On 19/04/2023 09:38, =Xianwei Zhao wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> Amlogic C3 is an advanced edge AI processor designed for smart IP camera
> applications.
> 
> Add basic support for the C3 based Amlogic AW409 board, which describes
> the following components: CPU, GIC, IRQ, Timer, UART. It's capable of
> booting up into the serial console.
> 
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> ---
>   arch/arm64/boot/dts/amlogic/Makefile          |  1 +
>   .../amlogic/amlogic-c3-c302x-aw409-256m.dts   | 30 +++++++
>   arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi   | 87 +++++++++++++++++++
>   3 files changed, 118 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>   create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
> 
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
> index cd1c5b04890a..d2b5d0d750bc 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -74,3 +74,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air-gbit.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air.dtb
> +dtb-$(CONFIG_ARCH_AMLIPC) += amlogic-c3-c302x-aw409-256m.dtb

It's pointless to only build the c3 DTs here.

Neil

> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
> new file mode 100644
> index 000000000000..38ca98a32181
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "amlogic-c3.dtsi"
> +
> +/ {
> +	model = "Amlogic C302 aw409 Development Board";
> +	compatible = "amlogic,aw409", "amlogic,c3";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	aliases {
> +		serial0 = &uart_B;
> +	};
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x0 0x0 0x0 0x10000000>;
> +	};
> +
> +};
> +
> +&uart_B {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
> new file mode 100644
> index 000000000000..c69072ac57f5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a35";
> +			reg = <0x0 0x0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a35";
> +			reg = <0x0 0x1>;
> +			enable-method = "psci";
> +		};
> +
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0";
> +		method = "smc";
> +	};
> +
> +	xtal: xtal-clk {
> +		compatible = "fixed-clock";
> +		clock-frequency = <24000000>;
> +		clock-output-names = "xtal";
> +		#clock-cells = <0>;
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		gic: interrupt-controller@fff01000 {
> +			compatible = "arm,gic-400";
> +			#interrupt-cells = <3>;
> +			#address-cells = <0>;
> +			interrupt-controller;
> +			reg = <0x0 0xfff01000 0 0x1000>,
> +			      <0x0 0xfff02000 0 0x2000>,
> +			      <0x0 0xfff04000 0 0x2000>,
> +			      <0x0 0xfff06000 0 0x2000>;
> +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> +		};
> +
> +		apb4: apb4@fe000000 {
> +			compatible = "simple-bus";
> +			reg = <0x0 0xfe000000 0x0 0x480000>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
> +
> +			uart_B: serial@7a000 {
> +				compatible = "amlogic,meson-g12a-uart";
> +				reg = <0x0 0x7a000 0x0 0x18>;
> +				interrupts = <GIC_SPI 169 IRQ_TYPE_EDGE_RISING>;
> +				status = "disabled";
> +				clocks = <&xtal>, <&xtal>, <&xtal>;
> +				clock-names = "xtal", "pclk", "baud";
> +			};
> +
> +		};
> +	};
> +};

Re: [RFC PATCH 0/2] Baisc devicetree support for Amlogic C3

[Dmitry Rokosov]

Please replace "Baisc" with "Basic" in the subject, it's a mistyping.

On Wed, Apr 19, 2023 at 03:38:32PM +0800, =Xianwei Zhao wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> Amlogic C3 is an advanced edge AI processor designed for smart IP camera
> applications.
> 
> Add ARCH_AMLIPC for the new series.
> 
> Add basic devicetree support for the C3 based Amlogic AW409 board,
> which describes the following components: CPU, GIC, IRQ, Timer, UART.
> It's capable of booting up into the serial console.
> 
> Xianwei Zhao (2):
>   arm64: amlogic: add new ARCH_AMLIPC for IPC SoC
>   arm64: dts: add support for C3 based Amlogic AW409
> 
>  arch/arm64/Kconfig.platforms                  | 12 +++
>  arch/arm64/boot/dts/amlogic/Makefile          |  1 +
>  .../amlogic/amlogic-c3-c302x-aw409-256m.dts   | 30 +++++++
>  arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi   | 87 +++++++++++++++++++
>  arch/arm64/configs/defconfig                  |  2 +
>  5 files changed, 132 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3-c302x-aw409-256m.dts
>  create mode 100644 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
> 
> 
> base-commit: ae68fb187b59bc8645974320808ab2d7c41b1833
> -- 
> 2.37.1
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
Thank you,
Dmitry