Re: [PATCH 3/3] arm64: dts: nuvoton: Add initial support for MA35D1

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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