Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
To:     pgwipeout@gmail.com
Cc:     devicetree@vger.kernel.org, heiko@sntech.de,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-rockchip@lists.infradead.org, robh+dt@kernel.org
References: <20210812204116.2303617-8-pgwipeout@gmail.com>
Subject: Re: [PATCH v2 7/8] arm64: dts: rockchip: add usb2 nodes to rk3568
 device tree
From:   Michael Riesch <michael.riesch@wolfvision.net>
Organization: WolfVision GmbH
Message-ID: <fecdda76-6eaf-9cf7-09d3-e2025ad1297b@wolfvision.net>
Date:   Thu, 14 Oct 2021 12:02:48 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
In-Reply-To: <20210812204116.2303617-8-pgwipeout@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZmNvOGdzay9jQUptU0Y3YzlZSFErU2tOWW9JRnY2bkNRZ21Say9jZ2RkNzZT?=
 =?utf-8?B?czY3c1NZYVhoazVLYU45cU5BU3pSNFhiM21xdlo2YkhjdE5WQ0xjZkpSSFFw?=
 =?utf-8?B?RGNTTjMwV2xpbldyRzZlUllSRXBnTlJMZDFPcjV1YWtkZ1VMWEhjNDlrR284?=
 =?utf-8?B?ajdGU0NaZkh6dHVIeU1vL0ROcXE5clQzb203UEw3Q1BxdDJ4OGdsa2tNYmhU?=
 =?utf-8?B?NzhXN2xibWhTN3RheGxOd2lGTzNHK3RLcUQwR3dTWHNGZFE4S1RGMXpDcXBJ?=
 =?utf-8?B?NVYrMWFwR01SSWlsSzZJcHdyaGtJL3hlRmpicnNSSmliMFBxR1pPdDZ2eDRZ?=
 =?utf-8?B?UlFpM3ZuTFI3SlRvMFNmYWRadjdFWUJnMUJHNnhHSm1MdUFCMy9KWG9OMFY2?=
 =?utf-8?B?UnhqZnNna05HMG5Yd3RBaU5tS3RlUk16dTFmT0xReWx5UWc0L3J1MU9pU2xl?=
 =?utf-8?B?TDQyVXU2MDBGQlA5NGZHM2FEWXJvclQ4VVhCS2luYzRGZXFuNWZxUnBqaWNy?=
 =?utf-8?B?RUdZYk1HWWRjUWtlWUxMMVFOV242dko5MUU5cndOa01nSzhpbkhwYy85eWgz?=
 =?utf-8?B?Zkw3aituU3ZtVEROeE55UkxyeEVvUC96K2pOSE5Kck81NVVjb1VMbFFoY2Iy?=
 =?utf-8?B?UDUwTG9Ka0RnQkxpWkRwZTNZQndodU1uSU1nMXhQeWdXbE9Ba2gxMXJpNzlG?=
 =?utf-8?B?Qk52K0F6VWo1dElMNzZBUm5tUGFWcVZVVExCQWtuOWF2ckx4VmVVODhtRmdp?=
 =?utf-8?B?TkRrUldUWVAxRHV4SmtVcGgvci8rT3hHTXRHVlhTeUQ3WFMrRkRESGE4VzRp?=
 =?utf-8?B?KzNUMENkU1FuREVGSHBjUGZqaCszWHA3b3E2RHFWVFQ5ZmdCQXpmSjBmOFNk?=
 =?utf-8?B?Zy8xS0FEdFVmMW9jVXZxQ21tbGxsV3lKNHd1ZGJMcXBWUkliUHc2Q29rNU5z?=
 =?utf-8?B?dW1TZ3NXeWw3SkcxNFZUOHcwQWlZV2thcmlxMkQyWDZ6cU4wRERrM3liamd4?=
 =?utf-8?B?cU1qV21LSzZXbjkwVHpvRFJuZGJ0T3ZtZFdqRUk2a2g5NXlLakJsdnQ2eXpK?=
 =?utf-8?B?SHhGaGFqQjVqRXcxRWtDbUloL2V3c0JHK2grdklTUXR1S1ovYmNZWmxSTi9l?=
 =?utf-8?B?ZnB4cHAyV0hoeVltTStITC8yZjliYUpxdHJIZm1qR3FPdUtSaHlpaklVbFdU?=
 =?utf-8?B?dWN6a0p6ZkpvRkhrNCtHbndZRWdaM05BdVFjcDlQT2E4Y3pnZlVnOUUzUGJZ?=
 =?utf-8?B?ZjBsK2hCWHhVVTBCZjRscWVMUlN4ZnF4MENiWVRKRTJ4bTNOSWFSUmY5eU5J?=
 =?utf-8?B?aEFIRUJ5TEVES1dpQy90NTdNQlYxL2F1c3RYZ2lmSWtKTkxiNG1GQkIxM2x4?=
 =?utf-8?B?VmRQa1c5YkE3MmtaNHNuYWJ2YzU0NmR6d0d5SHpOOXhtUkJCQzQ3MCtEVTcv?=
 =?utf-8?B?WVhtRjJzS2xrNEQ0ZnNudm1SOFh6QjI4RUVZTlcrdmIyRG5BcUFlNDJhMzQy?=
 =?utf-8?B?M0dxT29iajdpK0JoNWg2WmxLUFV2TjI4TzZIQjVUS3czZGtNZURPeVNUWXpJ?=
 =?utf-8?B?SGF5dnhlUHZIdEVxck9NQW52Q0QvUVdhdGhGWmdpaDkveTdSZFluTmNQMEZu?=
 =?utf-8?B?OFB2VVcyWnBFN3N3ZjJLcE8wMHRNbDA1azIrS2JPTjVvNXc1WlN2WXFuL3Zw?=
 =?utf-8?B?NFJHcmp1cXpVaXJMUEl1Q2ExcHZyTFFTaGZoVCthcy9IVlYxM2p2djZFeHdI?=
 =?utf-8?Q?QAIrL5Qgs2UDNQLPkxrrTDjOaPq09Oq87oYJIUq?=
X-OriginatorOrg: wolfvision.net
X-MS-Exchange-CrossTenant-Network-Message-Id: 6e5f1a8b-d446-437c-fb68-08d98ef9c822
X-MS-Exchange-CrossTenant-AuthSource: DBBPR08MB4523.eurprd08.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Oct 2021 10:02:50.7671
 (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: oXhYDuzbRswkUk1zjfy8ThnhRYVQsAYHDUzQ1joJnLHiQSiAWXspZ+urXIxSAkrwN8sSBa0bvKYJGsimJlOaxHF4ZomawQ2y4+8H7CAS9D8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR08MB5114
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Peter,

One cosmetic issue/question:

> [...]
> @@ -738,6 +792,50 @@ saradc: saradc@fe720000 {
>  		status = "disabled";
>  	};
>  
> +	usb2phy0: usb2-phy@fe8a0000 {
> +		compatible = "rockchip,rk3568-usb2phy";
> +		reg = <0x0 0xfe8a0000 0x0 0x10000>;
> +		clocks = <&pmucru CLK_USBPHY0_REF>;
> +		clock-names = "phyclk";
> +		clock-output-names = "clk_usbphy0_480m";
> +		interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>;
> +		rockchip,usbgrf = <&usb2phy0_grf>;
> +		#clock-cells = <0>;
> +		status = "disabled";
> +
> +		u2phy0_host: host-port {
> +			#phy-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		u2phy0_otg: otg-port {
> +			#phy-cells = <0>;
> +			status = "disabled";
> +		};

Would it make sense to name those usb2phy0_{host,otg}? This would clean
up the sorting in the dts files a bit. Otherwise, u2phy... and
usb2phy... would have to be separated by e.g. uart nodes.

> +	};
> +
> +	usb2phy1: usb2-phy@fe8b0000 {
> +		compatible = "rockchip,rk3568-usb2phy";
> +		reg = <0x0 0xfe8b0000 0x0 0x10000>;
> +		clocks = <&pmucru CLK_USBPHY1_REF>;
> +		clock-names = "phyclk";
> +		clock-output-names = "clk_usbphy1_480m";
> +		interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
> +		rockchip,usbgrf = <&usb2phy1_grf>;
> +		#clock-cells = <0>;
> +		status = "disabled";
> +
> +		u2phy1_host: host-port {
> +			#phy-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		u2phy1_otg: otg-port {
> +			#phy-cells = <0>;
> +			status = "disabled";
> +		};

Same here, of course.

> +	};
> +
> [...]

Best regards,
Michael