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;
Message-ID: <1afb1586-ebd9-6b99-580b-531e8ba7df32@wolfvision.net>
Date:   Mon, 28 Nov 2022 07:52:39 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCHv4 6/7] arm64: dts: rockchip: Add rock-5a board
Content-Language: en-US
To:     Sebastian Reichel <sebastian.reichel@collabora.com>,
        Heiko Stuebner <heiko@sntech.de>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Christopher Obbard <chris.obbard@collabora.com>,
        Benjamin Gaignard <benjamin.gaignard@collabora.com>,
        linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel@collabora.com
References: <20221124144928.35381-1-sebastian.reichel@collabora.com>
 <20221124144928.35381-7-sebastian.reichel@collabora.com>
From:   Michael Riesch <michael.riesch@wolfvision.net>
In-Reply-To: <20221124144928.35381-7-sebastian.reichel@collabora.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dzRGdDM4K1E0VjVtNG5tOU9lUXN5QkJnRkJOWExMa2h5VVJQWlFLdUhkNUxG?=
 =?utf-8?B?ZEdpNzF0SFFZMGFCQXFVM29KdEdxNVYrcURxRUpyV0lUcEs0Q0lkUTEvUmlu?=
 =?utf-8?B?ekFYckNJMURjZnRoS0k2M1d3VUFidnpMaE04NUZyMXZQRVlQK2w0KzMya3VV?=
 =?utf-8?B?aU1FQ0VkSDhhVUdSTUptOHNiYVN2U1JjZW1OL2xHaVU5ZGpRc0FreC9MNVZS?=
 =?utf-8?B?MFJkQ0tmV01ybDNyYW1jcmd2R2t1OTU5bWJkNnlVdEVIdTZyTEg4TlFOTjIx?=
 =?utf-8?B?d3FOMkNqdWtqUVNrMEEvVjZxblNmWUpIWXlqWmJrNWowdm9LTHVSUXlodXF3?=
 =?utf-8?B?NmNESjkxWlZDcmZyQzQ2bFc1T3VVdUtvenRqQ3hjcWtBRGhGYnBvWnluOVFC?=
 =?utf-8?B?eWZrUEY5UDhVZGwzNE5CbHNOWEFUMXArbWtaZzFxZUt0TGY1K0U3L1RhVzZF?=
 =?utf-8?B?ZUxsOGdyNnlKTThYNXI2emh0bEtneTFHS1ZZcGFIUXk0cWpxZDZUN1U0K04r?=
 =?utf-8?B?L1BkWTluKzZZa0lpendYQ2tNbEduLzR3eS9PWHZjYlg4MzQ2b0J4c0pRcWNQ?=
 =?utf-8?B?WW5qWElQdDl4MWYyeGZGYlNLMElSRkJobjk3ZHgwVFZFQlV0MEhvUTRKSzV2?=
 =?utf-8?B?cWlnUGI1ejYzN3ZpNld5Ymc5ZytkRXdubjZqa3ZGdnIxM2JqVk1uUkRhSkZJ?=
 =?utf-8?B?aFkvd2w2bHJxdHRVL3drT1NKU0NVSGYrZ3prb29RSUZZQit0SmU4RFYrNjNG?=
 =?utf-8?B?QS9JbldsdFZWQm1yN0NaSjZtQVA1V2ZxZ2FWY3l1TnZvK0MxUTEvaGJ3Tnk3?=
 =?utf-8?B?Ujg2NGZ0Kzk1dWpia1hMcHBncW5jeGpydzEwK3EwVVFnZjk0cEZ6Y3VnQzNy?=
 =?utf-8?B?aVAvRFZ0eHAya2t5eXdVSm52Y2hrdnRBZGxTV2ZRWEpaT0lWSTBOZmphSG5X?=
 =?utf-8?B?OStPdTN5d0NLK2JzZGh3Yy93c0szTUVURGp2VXBLZnM4M0NXMEF5WWxSM0hM?=
 =?utf-8?B?VHErREhiNGFtcGpOWmFOU2haZUhvTnFjb2NVeXZYWFlIbDBmRVBBaENXUTRQ?=
 =?utf-8?B?QUc0dkZjK1hYditZdlpGejVCTkZYM1BqWnNObzZGYnBwdFJqU2U4c2NMdXdV?=
 =?utf-8?B?Vm1tWjR6bU02MWEvUys3MHZNSmpnZysxT0tNaHBJclJDVVpIaS9oaEpjR0lJ?=
 =?utf-8?B?WGVVOXRsT3MvSng0SUlVOG4zRXNMamljaDZkL2hoeEhaVmRGRThhZCtkSHBt?=
 =?utf-8?B?a1MvRmw3RWZKYTY1UmRnZENkeVJjYThSdTJOMVJyVDRMYjJiSlZ0N2hyTnpz?=
 =?utf-8?B?MHdSRnp6L0FBWkJTRnRwcWR3TVF1ZVRMUnpYSk1WbTlHYkRTZlR5enlzUVZ5?=
 =?utf-8?B?UWwvZ3lrVkp5bzVvS29lcnFUZWNnYXpLajk2Q2t5bU45YkE1U2twMHhKQkpN?=
 =?utf-8?B?ejBTbE1XQVNTZllCRmkwWlg4a0t6dzRGQUd0OW5VVVpxaitld2FxWWVOSGVB?=
 =?utf-8?B?aHl2Rm9Mak9QaDB2M0d5MlZXWm9HYzJjVWYyUjh1Wk00bWpwWnVXZmN1S1l0?=
 =?utf-8?B?MTgxSXR4eGVmUmRKMlBjUndydGJmRy9qMGJVcW5aUDZwb2Nxa2xNUjc4Q0Vn?=
 =?utf-8?B?U2YyTFFjcTZNRVRpTXdvYU9vUjhnVnRvbTRHUENBNW9qcjBKb1ZGOUMrdHRy?=
 =?utf-8?B?aUQvUFFzUU9tUUNyTWE0Sk8wSDV2THhuZGRJUUZGYVdEVlJIYlMvUVJyaU1h?=
 =?utf-8?B?RGJLZHZyZVhvNDcxR1ljUzczVE1zVE04aittMGpjT3lxaWRCZ3dnOXc5bWQ3?=
 =?utf-8?B?ZWFSdlZ0SHo5WW8zUXkvMzRZc3VOR09wNkhPeE9xRjdHdVNua1VKTTFLZDdO?=
 =?utf-8?B?MFpHZDcybDJRcURjU0hKcXBieFNRN2tycmtnM2hlSTRzdVlaVEJnZUM5VmZw?=
 =?utf-8?B?a0g2T2J2RVlqL3ZNWmRmc1dHM2xpYjJDbFpvR25uclhXaWVhNWdhbnZyT1NE?=
 =?utf-8?B?VitTaFpDOTdmSXlCTzBVWFE4RXhPdWxFQis5NklmOUdpODBTdnl4QTN1ZGVQ?=
 =?utf-8?B?QjJyQkVLVks2N3p2L2hrTUJJVHk5UEtlY1E0WnRJOWlUbVVjamlSWXBqV0FH?=
 =?utf-8?B?YUlpeE5vN29XcE9wTDRZQTZvMUdhZWpaMGFaS3YyWXFJZm9RZ3BmbHNtNmRJ?=
 =?utf-8?B?M2hzcDFsTWFnZjhyMHgwdXBPMW5wZldkTzJKQU9laElFNnp5RE9PK2xiYXI3?=
 =?utf-8?B?V3d1WFMwTTJvb2theFlVVW9HR093PT0=?=
X-OriginatorOrg: wolfvision.net
X-MS-Exchange-CrossTenant-Network-Message-Id: 089ae183-5e17-4867-9464-08dad10d256f
X-MS-Exchange-CrossTenant-AuthSource: DU0PR08MB9155.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Nov 2022 06:52:42.0446
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: e94ec9da-9183-471e-83b3-51baa8eb804f
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ktjqUIb5yxM5swgOhM1S7X5r1IJFq4jPo6REN4w5e56M6K4J8NoNeLkupV2Zmq2naLacNxRjR9Q5uYKAqjnmPINIBHKsq8POKhQilbHDpz8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR08MB6079
X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Sebastian,

On 11/24/22 15:49, Sebastian Reichel wrote:
> Add board file for the RK3588s Rock 5A board. While the hardware
> offers plenty of peripherals and connectivity this basic implementation
> just handles things required to access eMMC, UART and Ethernet (i.e.
> enough to successfully boot Linux).
> 
> Tested-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Isn't your Signed-off-by: tag supposed to come first? (This question
also holds for patches 1, 2 (your S-o-B before the Reviewed-by) and 4 of
this series.

> ---
>  arch/arm64/boot/dts/rockchip/Makefile         |  1 +
>  .../boot/dts/rockchip/rk3588s-rock-5a.dts     | 63 +++++++++++++++++++
>  2 files changed, 64 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index 12ed53de11eb..31fa55750a0f 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -73,3 +73,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-rock-3a.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-rock-5a.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> new file mode 100644
> index 000000000000..b9d1ade62b4c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include "rk3588s.dtsi"
> +
> +/ {
> +	model = "Radxa Rock 5A Board";
> +	compatible = "radxa,rock-5a", "rockchip,rk3588s";
> +
> +	aliases {
> +		mmc1 = &sdhci;
> +		serial2 = &uart2;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial2:1500000n8";
> +	};
> +};
> +
> +&gmac1 {
> +	clock_in_out = "output";
> +	phy-handle = <&rgmii_phy1>;
> +	phy-mode = "rgmii-rxid";
> +	pinctrl-0 = <&gmac1_miim
> +		     &gmac1_tx_bus2
> +		     &gmac1_rx_bus2
> +		     &gmac1_rgmii_clk
> +		     &gmac1_rgmii_bus>;
> +	pinctrl-names = "default";
> +	rx_delay = <0x3e>;

Documentation says that the MAC should not add an RX delay in the
"rgmii-rxid" case, but it is specified. Is this really necessary?

> +	tx_delay = <0x3a>;
> +	status = "okay";
> +};
> +
> +&mdio1 {
> +	rgmii_phy1: ethernet-phy@1 {
> +		/* RTL8211F */
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x1>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;

No pinctrl required for this reset pin?

With this addressed:

Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>

Thanks and best regards,
Michael

> +	};
> +};
> +
> +&sdhci {
> +	bus-width = <8>;
> +	no-sdio;
> +	no-sd;
> +	non-removable;
> +	max-frequency = <200000000>;
> +	mmc-hs400-1_8v;
> +	mmc-hs400-enhanced-strobe;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	pinctrl-0 = <&uart2m0_xfer>;
> +	status = "okay";
> +};