From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8ACB026281; Sat, 16 Aug 2025 13:34:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755351250; cv=none; b=ZxYd0IxZDD04l4yNwAW/EreuYo8FIuBc27PskK7baE2ethkXnWLsEDQRwjS6phkJ6dRr5e1Bsf5LJVHGqOXcQI2+NSZkVMUJWFDUiedAAI/G475EdksjjsE8cuTMnlmwlkFsFTCen0u/+izVVqbzzUaJWM0Truwjwbs+CT3zeB0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755351250; c=relaxed/simple; bh=sQQoLhpMijiAs0tCXeUL1nZEAmraz+fwhbqr5RfrM9c=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=darLSJSOU24t8fUV4kBG3mUHLlN0i2zOVU+LrXIU2aGoEbGtrdAiQCPmn19dOWtCirlYZjN3UY8kBKAhPvWExLEQHM59r1rCjHqIF9bVnypWlgAgYS4qHitGrRBR4QguwIJMuwfohCsarXc+h/e2dfgmXhMiwSOc/9TO76RbXeo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 940551691; Sat, 16 Aug 2025 06:33:59 -0700 (PDT) Received: from minigeek.lan (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 874CD3F5A1; Sat, 16 Aug 2025 06:34:05 -0700 (PDT) Date: Sat, 16 Aug 2025 14:31:17 +0100 From: Andre Przywara To: iuncuim Cc: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Michael Turquette , Stephen Boyd , Vinod Koul , Kishon Vijay Abraham I , Philipp Zabel , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-phy@lists.infradead.org, linux-clk@vger.kernel.org, linux-sunxi@lists.linux.dev Subject: Re: [PATCH 3/7] phy: sun4i-usb: a523: add support for the USB2 PHY Message-ID: <20250816143117.4b86ecc6@minigeek.lan> In-Reply-To: <20250816084700.569524-4-iuncuim@gmail.com> References: <20250816084700.569524-1-iuncuim@gmail.com> <20250816084700.569524-4-iuncuim@gmail.com> Organization: Arm Ltd. X-Mailer: Claws Mail 4.2.0 (GTK 3.24.31; x86_64-slackware-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Sat, 16 Aug 2025 16:46:56 +0800 iuncuim wrote: Hi Mikhail, many thanks for posting these patches, that's a nice surprise to see this moving forward! I think there are some issues with this series, but we can surely iron those out. > From: Mikhail Kalashnikov > > Previously, USB PHY was compatible with D1 and did not require > separatedata options. But now we need to add a third PHY, which makes it > incompatible. The third PHY is used together with USB3/PCIe combophy with > DWC3 controller. It is really used together? I think on IRC we lately established that the USB2.0 and USB3.0 parts are somewhat independent, and I think the Radxa board wires PHY1 to the D-/D+ pins of the USB3.0 socket, instead of PHY2. Regardless: this change would be a compatibility break: When a new kernel with this change sees an old DT, it would bail out, because it doesn't find the resources for the third PHY in the DT. What I'd suggest instead is to drop the hardcoded assumption about the number of PHYs, and instead just enumerate all "pmu" cells in reg-names. That would work with both old and new DTs, and then we can also keep the fallback compatible name in the DT, to allow new DTs on older kernels (for instance a U-Boot provided DT on a stable kernel). As an added benefit, it would make future enablement easier, as we are more likely to find a matching compatible string, even when the number of PHYs differ. Cheers, Andre > In the BSP code, the third PHY requires a separate glue > driver, but it seems that it is not needed. > > Signed-off-by: Mikhail Kalashnikov > --- > drivers/phy/allwinner/phy-sun4i-usb.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c > index 8873aed3a..bb79339f2 100644 > --- a/drivers/phy/allwinner/phy-sun4i-usb.c > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c > @@ -1025,6 +1025,15 @@ static const struct sun4i_usb_phy_cfg sun50i_h616_cfg = { > .siddq_in_base = true, > }; > > +static const struct sun4i_usb_phy_cfg sun55i_a523_cfg = { > + .num_phys = 3, > + .phyctl_offset = REG_PHYCTL_A33, > + .dedicated_clocks = true, > + .hci_phy_ctl_clear = PHY_CTL_SIDDQ, > + .phy0_dual_route = true, > + .siddq_in_base = true, > +}; > + > static const struct of_device_id sun4i_usb_phy_of_match[] = { > { .compatible = "allwinner,sun4i-a10-usb-phy", .data = &sun4i_a10_cfg }, > { .compatible = "allwinner,sun5i-a13-usb-phy", .data = &sun5i_a13_cfg }, > @@ -1041,6 +1050,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = { > .data = &sun50i_a64_cfg}, > { .compatible = "allwinner,sun50i-h6-usb-phy", .data = &sun50i_h6_cfg }, > { .compatible = "allwinner,sun50i-h616-usb-phy", .data = &sun50i_h616_cfg }, > + { .compatible = "allwinner,sun55i-a523-usb-phy", .data = &sun55i_a523_cfg }, > { .compatible = "allwinner,suniv-f1c100s-usb-phy", > .data = &suniv_f1c100s_cfg }, > { },