Message-ID: <fb9c9533-b72b-7e20-0ee2-cca6491ea2af@linaro.org>
Date:   Fri, 17 Feb 2023 09:38:38 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] ARM: dts: mvebu: Add support for Thecus N2350 board
Content-Language: en-US
To:     Tony Dinh <mibodhi@gmail.com>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Gregory Clement <gregory.clement@bootlin.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        Arnd Bergmann <arnd@arndb.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Olof Johansson <olof@lixom.net>,
        Rob Herring <robh+dt@kernel.org>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        linux-kernel@vger.kernel.org, soc@kernel.org
References: <20230216011046.3306-1-mibodhi@gmail.com>
 <b9af4410-2352-b5f1-0fa8-8fc481001065@linaro.org>
 <CAJaLiFypS2wtsj65D1WrSjsgDeN+jpUVYcbvOHdiYvneiCK_Aw@mail.gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <CAJaLiFypS2wtsj65D1WrSjsgDeN+jpUVYcbvOHdiYvneiCK_Aw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 16/02/2023 22:25, Tony Dinh wrote:
> Hi Andrew,
> Hi Krzysztof,
> 
> On Thu, Feb 16, 2023 at 12:31 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 16/02/2023 02:10, Tony Dinh wrote:
>>> Thecus N2350 is a NAS based on Marvell Armada 385 SoC.
>>>
>>> Board Specification:
>>>
>>> - Marvel MV88F6820 Dual Core at 1GHz
>>> - 1 GiB DDR4 RAM
>>> - 4MB Macronix mx25l3205d SPI flash
>>> - 512MB Hynix H27U4G8F2DTR-BC NAND flash
>>> - I2C
>>> - 2x USB 3.0
>>> - 1x GBE LAN port (PHY: Marvell 88E1510)
>>> - 2x SATA (hot swap slots)
>>> - 3x buttons
>>> - 10x LEDS
>>> - serial console
>>>
>>> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
>>> ---
>>>
>>>  arch/arm/boot/dts/Makefile                    |   1 +
>>>  arch/arm/boot/dts/armada-385-thecus-n2350.dts | 432 ++++++++++++++++++
>>>  2 files changed, 433 insertions(+)
>>>  create mode 100644 arch/arm/boot/dts/armada-385-thecus-n2350.dts
>>>
>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>> index d08a3c450..749790e6e 100644
>>> --- a/arch/arm/boot/dts/Makefile
>>> +++ b/arch/arm/boot/dts/Makefile
>>> @@ -1528,6 +1528,7 @@ dtb-$(CONFIG_MACH_ARMADA_38X) += \
>>>       armada-385-linksys-rango.dtb \
>>>       armada-385-linksys-shelby.dtb \
>>>       armada-385-synology-ds116.dtb \
>>> +     armada-385-thecus-n2350.dtb \
>>>       armada-385-turris-omnia.dtb \
>>>       armada-388-clearfog.dtb \
>>>       armada-388-clearfog-base.dtb \
>>> diff --git a/arch/arm/boot/dts/armada-385-thecus-n2350.dts b/arch/arm/boot/dts/armada-385-thecus-n2350.dts
>>> new file mode 100644
>>> index 000000000..38114d842
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/armada-385-thecus-n2350.dts
>>> @@ -0,0 +1,432 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Device Tree file for Thecus N2350 board
>>> + *
>>> + * Copyright (C) 2018-2023 Tony Dinh <mibodhi@gmail.com>
>>> + * Copyright (C) 2018 Manuel Jung <manuel.jung@hotmail.com>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include "armada-385.dtsi"
>>> +
>>> +/ {
>>> +     model = "Thecus N2350";
>>> +     compatible = "thecus,n2350", "marvell,armada385";
>>
>> Missing vendor prefix documentation. I wanted to say: "missing new
>> compatible documentation", but seems Marvell is missing these. Let's
>> start documenting at least new compatibles in YAML? Like Rob proposed
>> here for OMAP:
>> https://lore.kernel.org/all/20230209173534.GA539622-robh@kernel.org/
> 
> I was under the impression that the Documentation patch has to be a
> separate patch, as stated in
> Documentation/devicetree/bindings/submitting-patches.rst. Should I
> include documentation in this add-support patch?

Yes, it must be separate patch and must be first in the patchset using
new compatibles. Otherwise when we apply this, we get warnings of
undocumented compatibles.


Best regards,
Krzysztof