Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jernej =?utf-8?B?xaBrcmFiZWM=?= <jernej.skrabec@gmail.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Renze Nicolai <renze@rnplus.nl>
Cc:     linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Chen-Yu Tsai <wens@csie.org>,
        Samuel Holland <samuel@sholland.org>
Subject: Re: [PATCH v2] arm64: allwinner: a64: add device tree for SoPine with clusterboard
Date:   Mon, 07 Nov 2022 22:39:50 +0100
Message-ID: <3166097.AJdgDx1Vlc@kista>
In-Reply-To: <c2523181268451aa04a2474a2cf5bedb@rnplus.nl>
References: <b3b8320f0d2b54a0557331977582d05c@rnplus.nl> <334c5f69-2f25-aeb1-b87f-500cf2a6c205@linaro.org> <c2523181268451aa04a2474a2cf5bedb@rnplus.nl>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

Dne ponedeljek, 07. november 2022 ob 21:55:04 CET je Renze Nicolai napisal(a):
> Hello Krzysztof,
> 
> Thank you for your feedback! I've fixed the indentation and I've added
> the compatible and model elements.

You didn't document compatible in bindings as instructed by Krzysztof. That 
should be in a patch preceding this one.

Comments like this, which are not part of commit message, should be either in 
cover letter or right below --- line, so they are ignored when patch is 
applied. While above comment can be considered as changelog of sort, it's 
preferred to write it as list, point by point, in area which is ignored when 
applying patch.

I guess you don't use "git send-email" to send your patches? New versions 
should not be send in reply to old ones.

> 
> Greetings,
> Renze Nicolai
> 
> 
> The Pine64 clusterboard needs a to have the tx delay changed to
> 500ps for ethernet to work. This solution was described by multiple
> people on the Pine64 forum years ago and nobody has submitted it.

When you're adding new board, commit message should describe new board either 
by stating features or telling which board is similar and what are the 
differences. Something like that clusterboard is almost the same as baseboard 
except ethernet TX delay, which is 500 ps. Omit second part of second 
statement, it's not relevant.

Best regards,
Jernej

> 
> Add a device tree for SoPine with the clusterboard.
> 
> Signed-off-by: Renze Nicolai <renze@rnplus.nl>
> ---
>   arch/arm64/boot/dts/allwinner/Makefile           |  1 +
>   .../allwinner/sun50i-a64-sopine-clusterboard.dts | 16 ++++++++++++++++
>   2 files changed, 17 insertions(+)
>   create mode 100644
> arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-clusterboard.dts
> 
> diff --git a/arch/arm64/boot/dts/allwinner/Makefile
> b/arch/arm64/boot/dts/allwinner/Makefile
> index 6a96494a2e0a..15b011d1aaea 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -15,6 +15,7 @@ dtb-$(CONFIG_ARCH_SUNXI) +=
> sun50i-a64-pinephone-1.2.dtb
>   dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pinetab.dtb
>   dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pinetab-early-adopter.dtb
>   dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-baseboard.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-clusterboard.dtb
>   dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-teres-i.dtb
>   dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a100-allwinner-perf1.dtb
>   dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-bananapi-m2-plus.dtb
> diff --git
> a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-clusterboard.dts
> b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-clusterboard.dts
> new file mode 100644
> index 000000000000..f75325d43d76
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-clusterboard.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (c) 2022 Renze Nicolai <renze@rnplus.nl>
> +
> +/dts-v1/;
> +
> +#include "sun50i-a64-sopine-baseboard.dts"
> +
> +/ {
> +	model = "SoPine with clusterboard";
> +	compatible = "pine64,sopine-clusterboard", "pine64,sopine",
> +		     "allwinner,sun50i-a64";
> +};
> +
> +&emac {
> +	allwinner,tx-delay-ps = <500>;
> +};
> --
> 2.38.1