diff mbox series

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

Message ID 20220307091923.9909-4-ychuang3@nuvoton.com (mailing list archive)
State Superseded
Headers show
Series Add initial support for MA35D1 SoC | expand

Commit Message

Jacky Huang March 7, 2022, 9:19 a.m. UTC
Add the initial device tree files for Nuvoton MA35D1 Soc.

Signed-off-by: Jacky Huang <ychuang3@nuvoton.com>
---
 arch/arm64/boot/dts/Makefile               |   1 +
 arch/arm64/boot/dts/nuvoton/Makefile       |   2 +
 arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts |  23 +++++
 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi    | 106 +++++++++++++++++++++
 4 files changed, 132 insertions(+)
 create mode 100644 arch/arm64/boot/dts/nuvoton/Makefile
 create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
 create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi

--
2.17.1

Comments

Krzysztof Kozlowski March 7, 2022, 10:25 a.m. UTC | #1
On 07/03/2022 10:19, Jacky Huang wrote:
> Add the initial device tree files for Nuvoton MA35D1 Soc.
> 
> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com>
> ---
>  arch/arm64/boot/dts/Makefile               |   1 +
>  arch/arm64/boot/dts/nuvoton/Makefile       |   2 +
>  arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts |  23 +++++
>  arch/arm64/boot/dts/nuvoton/ma35d1.dtsi    | 106 +++++++++++++++++++++
>  4 files changed, 132 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/nuvoton/Makefile
>  create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
>  create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> 
> 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.

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

> +       };

You need compatible and bindings.

> +
> +       memory@80000000 {
> +               device_type = "memory";
> +               reg = <0x00000000 0x80000000 0 0x10000000>;
> +       };
> +};
> +
> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> new file mode 100644
> index 000000000000..27adac4975c3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Nuvoton Technology Corp.
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> +
> +/ {
> +       compatible = "nuvoton,ma35d1";

Please run checkpatch. This compatible looks undocumented.

> +       interrupt-parent = <&gic>;
> +       #address-cells = <2>;
> +       #size-cells = <2>;
> +
> +       cpus {
> +               #address-cells = <2>;
> +               #size-cells = <0>;
> +               cpu-map {
> +                       cluster0 {
> +                               core0 {
> +                                       cpu = <&cpu0>;
> +                               };
> +                               core1 {
> +                                       cpu = <&cpu1>;
> +                               };
> +                       };
> +               };

Line break between each nodes, here and below.

> +               cpu0: cpu@0 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a35";
> +                       reg = <0x0 0x0>;
> +                       enable-method = "psci";
> +                       next-level-cache = <&L2_0>;
> +               };
> +               cpu1: cpu@1 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a35";
> +                       reg = <0x0 0x1>;
> +                       enable-method = "psci";
> +                       next-level-cache = <&L2_0>;
> +               };
> +               L2_0: l2-cache0 {
> +                       compatible = "cache";
> +                       cache-level = <2>;
> +               };
> +       };
> +
> +       psci {
> +               compatible = "arm,psci-0.2";
> +               method = "smc";
> +       };
> +
> +       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)>;
> +               clock-frequency = <12000000>;
> +       };
> +
> +       sys: system-controller@40460000 {
> +               compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";
> +               reg = <0x0 0x40460000 0x0 0x200>;
> +       };
> +
> +       reset: reset-controller {
> +               compatible = "nuvoton,ma35d1-reset";
> +               nuvoton,ma35d1-sys = <&sys>;
> +               #reset-cells = <1>;
> +       };
> +
> +       clk: clock-controller@40460200 {
> +               compatible = "nuvoton,ma35d1-clk";
> +               reg = <0x00000000 0x40460200 0x0 0x100>;
> +               #clock-cells = <1>;
> +               assigned-clocks = <&clk DDRPLL>,
> +                                 <&clk APLL>,
> +                                 <&clk EPLL>,
> +                                 <&clk VPLL>;
> +               assigned-clock-rates = <266000000>,
> +                                      <180000000>,
> +                                      <500000000>,
> +                                      <102000000>;
> +               clock-pll-mode = <1>, <0>, <0>, <0>;
> +       };
> +
> +       gic: interrupt-controller@50800000 {
> +               compatible = "arm,gic-400";
> +               #interrupt-cells = <3>;
> +               interrupt-parent = <&gic>;
> +               interrupt-controller;
> +               reg = <0x0 0x50801000 0 0x1000>,
> +                     <0x0 0x50802000 0 0x2000>,
> +                     <0x0 0x50804000 0 0x2000>,
> +                     <0x0 0x50806000 0 0x2000>;
> +               interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0x13) |
> +                                        IRQ_TYPE_LEVEL_HIGH)>;
> +       };
> +};
> --
> 2.17.1
> 
> ________________________________
> ________________________________
>  The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately.


> Your cooperation is highly appreciated...

Cooperation seems futile... :)

Best regards,
Krzysztof
Jacky Huang April 6, 2022, 2:58 a.m. UTC | #2
On 2022/3/7 下午 06:25, Krzysztof Kozlowski wrote:
> On 07/03/2022 10:19, Jacky Huang wrote:
>> Add the initial device tree files for Nuvoton MA35D1 Soc.
>>
>> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com>
>> ---
>>   arch/arm64/boot/dts/Makefile               |   1 +
>>   arch/arm64/boot/dts/nuvoton/Makefile       |   2 +
>>   arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts |  23 +++++
>>   arch/arm64/boot/dts/nuvoton/ma35d1.dtsi    | 106 +++++++++++++++++++++
>>   4 files changed, 132 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/nuvoton/Makefile
>>   create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1-evb.dts
>>   create mode 100644 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
>>
>> 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, and 
add the
modification to this patch series.

config ARCH_MA35D1
     bool "Nuvoton MA35D1 SOC Family"
     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?

>> +
>> +       memory@80000000 {
>> +               device_type = "memory";
>> +               reg = <0x00000000 0x80000000 0 0x10000000>;
>> +       };
>> +};
>> +
>> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
>> new file mode 100644
>> index 000000000000..27adac4975c3
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
>> @@ -0,0 +1,106 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2022 Nuvoton Technology Corp.
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
>> +
>> +/ {
>> +       compatible = "nuvoton,ma35d1";
> Please run checkpatch. This compatible looks undocumented.

I will create a new binding file 
Documentation/devicetree/bindings/arm/nuvoton.yaml to this patch series.


>> +       interrupt-parent = <&gic>;
>> +       #address-cells = <2>;
>> +       #size-cells = <2>;
>> +
>> +       cpus {
>> +               #address-cells = <2>;
>> +               #size-cells = <0>;
>> +               cpu-map {
>> +                       cluster0 {
>> +                               core0 {
>> +                                       cpu = <&cpu0>;
>> +                               };
>> +                               core1 {
>> +                                       cpu = <&cpu1>;
>> +                               };
>> +                       };
>> +               };
> Line break between each nodes, here and below.

OK, I will fix it in the next version.

>
>> +               cpu0: cpu@0 {
>> +                       device_type = "cpu";
>> +                       compatible = "arm,cortex-a35";
>> +                       reg = <0x0 0x0>;
>> +                       enable-method = "psci";
>> +                       next-level-cache = <&L2_0>;
>> +               };
>> +               cpu1: cpu@1 {
>> +                       device_type = "cpu";
>> +                       compatible = "arm,cortex-a35";
>> +                       reg = <0x0 0x1>;
>> +                       enable-method = "psci";
>> +                       next-level-cache = <&L2_0>;
>> +               };
>> +               L2_0: l2-cache0 {
>> +                       compatible = "cache";
>> +                       cache-level = <2>;
>> +               };
>> +       };
>> +
>> +       psci {
>> +               compatible = "arm,psci-0.2";
>> +               method = "smc";
>> +       };
>> +
>> +       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)>;
>> +               clock-frequency = <12000000>;
>> +       };
>> +
>> +       sys: system-controller@40460000 {
>> +               compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";
>> +               reg = <0x0 0x40460000 0x0 0x200>;
>> +       };
>> +
>> +       reset: reset-controller {
>> +               compatible = "nuvoton,ma35d1-reset";
>> +               nuvoton,ma35d1-sys = <&sys>;
>> +               #reset-cells = <1>;
>> +       };
>> +
>> +       clk: clock-controller@40460200 {
>> +               compatible = "nuvoton,ma35d1-clk";
>> +               reg = <0x00000000 0x40460200 0x0 0x100>;
>> +               #clock-cells = <1>;
>> +               assigned-clocks = <&clk DDRPLL>,
>> +                                 <&clk APLL>,
>> +                                 <&clk EPLL>,
>> +                                 <&clk VPLL>;
>> +               assigned-clock-rates = <266000000>,
>> +                                      <180000000>,
>> +                                      <500000000>,
>> +                                      <102000000>;
>> +               clock-pll-mode = <1>, <0>, <0>, <0>;
>> +       };
>> +
>> +       gic: interrupt-controller@50800000 {
>> +               compatible = "arm,gic-400";
>> +               #interrupt-cells = <3>;
>> +               interrupt-parent = <&gic>;
>> +               interrupt-controller;
>> +               reg = <0x0 0x50801000 0 0x1000>,
>> +                     <0x0 0x50802000 0 0x2000>,
>> +                     <0x0 0x50804000 0 0x2000>,
>> +                     <0x0 0x50806000 0 0x2000>;
>> +               interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0x13) |
>> +                                        IRQ_TYPE_LEVEL_HIGH)>;
>> +       };
>> +};
>> --
>> 2.17.1
>>
>> ________________________________
>> ________________________________
>>   The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately.
>
>> Your cooperation is highly appreciated...
> Cooperation seems futile... :)
>
> Best regards,
> Krzysztof

I am sorry. I have requested mail server manager to add exception rule.
The unpleasant official declarations are removed.

Thanks for your review.

Sincerely,

Jacky
Krzysztof Kozlowski April 6, 2022, 7:14 a.m. UTC | #3
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
Arnd Bergmann April 6, 2022, 7:43 a.m. UTC | #4
On Wed, Apr 6, 2022 at 4:58 AM Jacky Huang <ychuang3@nuvoton.com> wrote:
> On 2022/3/7 下午 06:25, Krzysztof Kozlowski wrote:
> > On 07/03/2022 10:19, Jacky Huang wrote:
>
> I would add the following to end of arch/arm64/Kconfig.platforms, and
> add the
> modification to this patch series.
>
> config ARCH_MA35D1
>      bool "Nuvoton MA35D1 SOC Family"
>      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.

Selecting SOC_BUS and the gpio/pinctrl stuff is ok, but please don't
select the video helpers from the platform config, those should not
be essential for building a kernel.

       Arnd
Jacky Huang April 6, 2022, 9:25 a.m. UTC | #5
On 2022/4/6 下午 03:14, Krzysztof Kozlowski wrote:
> 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.

OK, I will put it to the right place in alphanumeric order.
It should be between ARCH_MXC and ARCH_QCOM.

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

Yes, I would like to modify it as the following:

config ARCH_NUVOTON
     bool "Nuvoton SoC Family"
     select PINCTRL
     select PINCTRL_MA35D1
     select PM
     select GPIOLIB
     select SOC_BUS
     help
       This enables support for Nuvoton MA35D1 ARMv8 SoC.

(Currently, we have MA35D1 only in the support list for arm64 SoC.).

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

Thanks for your review.

Sincerely,
Jacky
Arnd Bergmann April 6, 2022, 9:40 a.m. UTC | #6
On Wed, Apr 6, 2022 at 11:25 AM Jacky Huang <ychuang3@nuvoton.com> wrote:
> On 2022/4/6 下午 03:14, Krzysztof Kozlowski wrote:
> > On 06/04/2022 04:58, Jacky Huang wrote:
> >> 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.
>
> Yes, I would like to modify it as the following:
>
> config ARCH_NUVOTON
>      bool "Nuvoton SoC Family"
>      select PINCTRL
>      select PINCTRL_MA35D1
>      select PM
>      select GPIOLIB
>      select SOC_BUS
>      help
>        This enables support for Nuvoton MA35D1 ARMv8 SoC.
>
> (Currently, we have MA35D1 only in the support list for arm64 SoC.).

You could reword this to "This enables support for Nuvoton ARMv8 SoCs
such as the MA35D1", to prevent this from getting stale, or repeatedly
updated when future SoCs are added.

Another change you can consider is to remove the 'select PINCTRL_MA35D1'
here and instead change the pinctrl Kconfig entry to

config PINCTRL_MA35D1
           bool "..."
           depends on ARCH_NUVOTON || COMPILE_TEST
           default ARCH_NUVOTON

That way you get it default-enabled when ARCH_NUVOTON is
turned on, or disabled in configurations without ARCH_NUVOTON,
but can make a more fine-grained selection for a particular SoC
if you get more than one such driver in the future.

        Arnd
Jacky Huang April 7, 2022, 4:07 a.m. UTC | #7
On 2022/4/6 下午 03:43, Arnd Bergmann wrote:
> On Wed, Apr 6, 2022 at 4:58 AM Jacky Huang <ychuang3@nuvoton.com> wrote:
>> On 2022/3/7 下午 06:25, Krzysztof Kozlowski wrote:
>>> On 07/03/2022 10:19, Jacky Huang wrote:
>> I would add the following to end of arch/arm64/Kconfig.platforms, and
>> add the
>> modification to this patch series.
>>
>> config ARCH_MA35D1
>>       bool "Nuvoton MA35D1 SOC Family"
>>       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.
> Selecting SOC_BUS and the gpio/pinctrl stuff is ok, but please don't
> select the video helpers from the platform config, those should not
> be essential for building a kernel.
>
>         Arnd

Yes, I will remove  VIDEOMODE_HELPERS and FB_MODE_HELPERS.


Best Regards,
Jacky
Jacky Huang April 7, 2022, 4:17 a.m. UTC | #8
On 2022/4/6 下午 05:40, Arnd Bergmann wrote:
> On Wed, Apr 6, 2022 at 11:25 AM Jacky Huang <ychuang3@nuvoton.com> wrote:
>> On 2022/4/6 下午 03:14, Krzysztof Kozlowski wrote:
>>> On 06/04/2022 04:58, Jacky Huang wrote:
>>>> 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.
>> Yes, I would like to modify it as the following:
>>
>> config ARCH_NUVOTON
>>       bool "Nuvoton SoC Family"
>>       select PINCTRL
>>       select PINCTRL_MA35D1
>>       select PM
>>       select GPIOLIB
>>       select SOC_BUS
>>       help
>>         This enables support for Nuvoton MA35D1 ARMv8 SoC.
>>
>> (Currently, we have MA35D1 only in the support list for arm64 SoC.).
> You could reword this to "This enables support for Nuvoton ARMv8 SoCs
> such as the MA35D1", to prevent this from getting stale, or repeatedly
> updated when future SoCs are added.
>
> Another change you can consider is to remove the 'select PINCTRL_MA35D1'
> here and instead change the pinctrl Kconfig entry to
>
> config PINCTRL_MA35D1
>             bool "..."
>             depends on ARCH_NUVOTON || COMPILE_TEST
>             default ARCH_NUVOTON
>
> That way you get it default-enabled when ARCH_NUVOTON is
> turned on, or disabled in configurations without ARCH_NUVOTON,
> but can make a more fine-grained selection for a particular SoC
> if you get more than one such driver in the future.
>
>          Arnd

Yes, it looks better.
So, I will modify it as:

config ARCH_NUVOTON
     bool "Nuvoton SoC Family"
     select PINCTRL
     select PM
     select GPIOLIB
     select SOC_BUS
     help
       This enables support for Nuvoton MA35D1 ARMv8 SoC such as MA35D1.

And move PINCTRL_MA35D1 to pinctrl Kconfig when we submit the pictrl 
driver.

Thanks you.

Jacky
diff mbox series

Patch

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
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";
+       };
+
+       memory@80000000 {
+               device_type = "memory";
+               reg = <0x00000000 0x80000000 0 0x10000000>;
+       };
+};
+
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
new file mode 100644
index 000000000000..27adac4975c3
--- /dev/null
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
@@ -0,0 +1,106 @@ 
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Nuvoton Technology Corp.
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+/ {
+       compatible = "nuvoton,ma35d1";
+       interrupt-parent = <&gic>;
+       #address-cells = <2>;
+       #size-cells = <2>;
+
+       cpus {
+               #address-cells = <2>;
+               #size-cells = <0>;
+               cpu-map {
+                       cluster0 {
+                               core0 {
+                                       cpu = <&cpu0>;
+                               };
+                               core1 {
+                                       cpu = <&cpu1>;
+                               };
+                       };
+               };
+               cpu0: cpu@0 {
+                       device_type = "cpu";
+                       compatible = "arm,cortex-a35";
+                       reg = <0x0 0x0>;
+                       enable-method = "psci";
+                       next-level-cache = <&L2_0>;
+               };
+               cpu1: cpu@1 {
+                       device_type = "cpu";
+                       compatible = "arm,cortex-a35";
+                       reg = <0x0 0x1>;
+                       enable-method = "psci";
+                       next-level-cache = <&L2_0>;
+               };
+               L2_0: l2-cache0 {
+                       compatible = "cache";
+                       cache-level = <2>;
+               };
+       };
+
+       psci {
+               compatible = "arm,psci-0.2";
+               method = "smc";
+       };
+
+       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)>;
+               clock-frequency = <12000000>;
+       };
+
+       sys: system-controller@40460000 {
+               compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";
+               reg = <0x0 0x40460000 0x0 0x200>;
+       };
+
+       reset: reset-controller {
+               compatible = "nuvoton,ma35d1-reset";
+               nuvoton,ma35d1-sys = <&sys>;
+               #reset-cells = <1>;
+       };
+
+       clk: clock-controller@40460200 {
+               compatible = "nuvoton,ma35d1-clk";
+               reg = <0x00000000 0x40460200 0x0 0x100>;
+               #clock-cells = <1>;
+               assigned-clocks = <&clk DDRPLL>,
+                                 <&clk APLL>,
+                                 <&clk EPLL>,
+                                 <&clk VPLL>;
+               assigned-clock-rates = <266000000>,
+                                      <180000000>,
+                                      <500000000>,
+                                      <102000000>;
+               clock-pll-mode = <1>, <0>, <0>, <0>;
+       };
+
+       gic: interrupt-controller@50800000 {
+               compatible = "arm,gic-400";
+               #interrupt-cells = <3>;
+               interrupt-parent = <&gic>;
+               interrupt-controller;
+               reg = <0x0 0x50801000 0 0x1000>,
+                     <0x0 0x50802000 0 0x2000>,
+                     <0x0 0x50804000 0 0x2000>,
+                     <0x0 0x50806000 0 0x2000>;
+               interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0x13) |
+                                        IRQ_TYPE_LEVEL_HIGH)>;
+       };
+};