Re: [PATCH 3/7] phy: sun4i-usb: a523: add support for the USB2 PHY

[Andre Przywara <andre.przywara@arm.com>]

On Sat, 16 Aug 2025 16:46:56 +0800
iuncuim <iuncuim@gmail.com> 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 <iuncuim@gmail.com>
> 
> 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<x>" 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 <iuncuim@gmail.com>
> ---
>  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 },
>  	{ },