Re: [PATCH 3/3] arm64: dts: nuvoton: Add initial support for MA35D1
From: Krzysztof Kozlowski
Date: Wed Apr 06 2022 - 06:49:44 EST
On 06/04/2022 04:58, Jacky Huang wrote:
>>> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
>>> index 639e01a4d855..28e01442094f 100644
>>> --- a/arch/arm64/boot/dts/Makefile
>>> +++ b/arch/arm64/boot/dts/Makefile
>>> @@ -30,3 +30,4 @@ subdir-y += synaptics
>>> subdir-y += ti
>>> subdir-y += toshiba
>>> subdir-y += xilinx
>>> +subdir-y += nuvoton
>>> diff --git a/arch/arm64/boot/dts/nuvoton/Makefile b/arch/arm64/boot/dts/nuvoton/Makefile
>>> new file mode 100644
>>> index 000000000000..e1e0c466bf5e
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/nuvoton/Makefile
>>> @@ -0,0 +1,2 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +dtb-$(CONFIG_ARCH_NUVOTON) += ma35d1-evb.dtb
>> ARCH_NUVOTON does not exist.
>
> I would add the following to end of arch/arm64/Kconfig.platforms,
Don't add things at the end of files but rather in respective place
without messing the order.
> and
> add the
> modification to this patch series.
>
> config ARCH_MA35D1
> bool "Nuvoton MA35D1 SOC Family"
We do not add options for specific SoCs, but for entire families, so
ARCH_NUVOTON is correct.
> select PINCTRL
> select PINCTRL_MA35D1
> select PM
> select GPIOLIB
> select SOC_BUS
> select VIDEOMODE_HELPERS
> select FB_MODE_HELPERS
> help
> This enables support for Nuvoton MA35D1 SOC Family.
>
>
>>> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
>>> new file mode 100644
>>> index 000000000000..38e4f734da0f
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
>>> @@ -0,0 +1,23 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Device Tree Source for MA35D1 Evaluation Board (EVB)
>>> + *
>>> + * Copyright (C) 2021 Nuvoton Technology Corp.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include "ma35d1.dtsi"
>>> +
>>> +/ {
>>> + model = "Nuvoton MA35D1-EVB";
>>> +
>>> + chosen {
>>> + bootargs = "console=ttyS0,115200n8";
>> No bootargs. "chosen", please.
>
> OK, I would modify it as:
>
> chosen {
> stdout-path = "serial0:115200n8";
> };
>
>
>>> + };
>> You need compatible and bindings.
>
> I will add the compatible here
> compatible = "nuvoton,ma35d1-evb", "nuvoton,ma35d1"
>
> And, I should create a new binding file
> Documentation/devicetree/bindings/arm/nuvoton.yaml to this patch series.
> And the property would be:
>
> properties:
> compatible:
> description: Nuvoton MA35D1-EVB
> items:
> - const: nuvoton,ma35d1-evb
> - const: nuvoton,ma35d1
>
>
> Is it OK?
Yes
Best regards,
Krzysztof