From: Andre Przywara Date: Tue, 19 Aug 2025 00:15:22 +0000 (+0100) Subject: phy: sun4i-usb: drop num_phys assumption X-Git-Tag: v6.18-rc1~62^2~35 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=75c21418beb92382dda164fdc2554610b49ed441;p=thirdparty%2Fkernel%2Flinux.git phy: sun4i-usb: drop num_phys assumption So far we set a number of expected Allwinner USB PHY instances for any given compatible string, and would fail if we do not find every PHY described properly in the DT (missing reset/PMU/clocks). This is somewhat redundant, as the DT only describes the resources for the implemented PHYs, but goes in line with being strict about firmware descriptions, and rather fixing things in the driver code, based on the compatible string. However this causes issues when we make a mistake, like we did recently on the A523: there are actually three USB PHYs, not two, as we assumed. Changing the number in the driver and the compatible string would cause all kinds of compatibility issues, both with older and newer DTs. To avoid problems with newer kernels and older or newer DTs, we can change the driver code to deduce the number of PHY instances from what's described in the DT. This has the added advantage of not requiring new compatible strings for new SoCs when just the number of PHYs change, which already happened and might occur again in the future. Drop the num_phys member from the config struct, and remove the fixed number of PHYs from each SoC's config description. Then enumerate the usb_reset properties for all of the maximum four PHY instances, and just stop once we cannot find such a property anymore. The binding describes the reset property as mandatory for each PHY instance, and each DT in the kernel tree matches exactly the num_phys value in the current driver code, so we can rely on that. Apart from being more future proof, this will solve the A523 mishap: Older DTs would just describe two PHYs, whereas newer ones would feature all three of them. In any case we would get a valid number, matching the other nodes in the DT. Older kernels would always enumerate two PHYs, which at least does not cause any regressions. Signed-off-by: Andre Przywara Link: https://lore.kernel.org/r/20250819001522.13011-1-andre.przywara@arm.com Signed-off-by: Vinod Koul --- diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c index 8873aed3a52a..59d38d88efb0 100644 --- a/drivers/phy/allwinner/phy-sun4i-usb.c +++ b/drivers/phy/allwinner/phy-sun4i-usb.c @@ -97,7 +97,6 @@ #define POLL_TIME msecs_to_jiffies(250) struct sun4i_usb_phy_cfg { - int num_phys; int hsic_index; u32 disc_thresh; u32 hci_phy_ctl_clear; @@ -115,6 +114,7 @@ struct sun4i_usb_phy_data { const struct sun4i_usb_phy_cfg *cfg; enum usb_dr_mode dr_mode; spinlock_t reg_lock; /* guard access to phyctl reg */ + int num_phys; struct sun4i_usb_phy { struct phy *phy; void __iomem *pmu; @@ -686,7 +686,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev, { struct sun4i_usb_phy_data *data = dev_get_drvdata(dev); - if (args->args[0] >= data->cfg->num_phys) + if (args->args[0] >= data->num_phys) return ERR_PTR(-ENODEV); if (data->cfg->missing_phys & BIT(args->args[0])) @@ -779,13 +779,22 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) return ret; } - for (i = 0; i < data->cfg->num_phys; i++) { + for (i = 0; i < MAX_PHYS; i++) { struct sun4i_usb_phy *phy = data->phys + i; char name[32]; if (data->cfg->missing_phys & BIT(i)) continue; + snprintf(name, sizeof(name), "usb%d_reset", i); + phy->reset = devm_reset_control_get(dev, name); + if (IS_ERR(phy->reset)) { + if (PTR_ERR(phy->reset) == -ENOENT) + break; + dev_err(dev, "failed to get reset %s\n", name); + return PTR_ERR(phy->reset); + } + snprintf(name, sizeof(name), "usb%d_vbus", i); phy->vbus = devm_regulator_get_optional(dev, name); if (IS_ERR(phy->vbus)) { @@ -828,13 +837,6 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) } } - snprintf(name, sizeof(name), "usb%d_reset", i); - phy->reset = devm_reset_control_get(dev, name); - if (IS_ERR(phy->reset)) { - dev_err(dev, "failed to get reset %s\n", name); - return PTR_ERR(phy->reset); - } - if (i || data->cfg->phy0_dual_route) { /* No pmu for musb */ snprintf(name, sizeof(name), "pmu%d", i); phy->pmu = devm_platform_ioremap_resource_byname(pdev, name); @@ -851,6 +853,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) phy->index = i; phy_set_drvdata(phy->phy, &data->phys[i]); } + data->num_phys = i; data->id_det_irq = gpiod_to_irq(data->id_det_gpio); if (data->id_det_irq > 0) { @@ -901,28 +904,24 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) } static const struct sun4i_usb_phy_cfg suniv_f1c100s_cfg = { - .num_phys = 1, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A10, .dedicated_clocks = true, }; static const struct sun4i_usb_phy_cfg sun4i_a10_cfg = { - .num_phys = 3, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A10, .dedicated_clocks = false, }; static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = { - .num_phys = 2, .disc_thresh = 2, .phyctl_offset = REG_PHYCTL_A10, .dedicated_clocks = false, }; static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = { - .num_phys = 3, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A10, .dedicated_clocks = true, @@ -930,14 +929,12 @@ static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = { }; static const struct sun4i_usb_phy_cfg sun7i_a20_cfg = { - .num_phys = 3, .disc_thresh = 2, .phyctl_offset = REG_PHYCTL_A10, .dedicated_clocks = false, }; static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = { - .num_phys = 2, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A10, .dedicated_clocks = true, @@ -945,7 +942,6 @@ static const struct sun4i_usb_phy_cfg sun8i_a23_cfg = { }; static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = { - .num_phys = 2, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, @@ -953,7 +949,6 @@ static const struct sun4i_usb_phy_cfg sun8i_a33_cfg = { }; static const struct sun4i_usb_phy_cfg sun8i_a83t_cfg = { - .num_phys = 3, .hsic_index = 2, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, @@ -961,7 +956,6 @@ static const struct sun4i_usb_phy_cfg sun8i_a83t_cfg = { }; static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = { - .num_phys = 4, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, @@ -970,7 +964,6 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = { }; static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = { - .num_phys = 3, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, @@ -979,7 +972,6 @@ static const struct sun4i_usb_phy_cfg sun8i_r40_cfg = { }; static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = { - .num_phys = 1, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, @@ -988,7 +980,6 @@ static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = { }; static const struct sun4i_usb_phy_cfg sun20i_d1_cfg = { - .num_phys = 2, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, .hci_phy_ctl_clear = PHY_CTL_SIDDQ, @@ -997,7 +988,6 @@ static const struct sun4i_usb_phy_cfg sun20i_d1_cfg = { }; static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = { - .num_phys = 2, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, @@ -1006,7 +996,6 @@ static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = { }; static const struct sun4i_usb_phy_cfg sun50i_h6_cfg = { - .num_phys = 4, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true, .phy0_dual_route = true, @@ -1015,7 +1004,6 @@ static const struct sun4i_usb_phy_cfg sun50i_h6_cfg = { }; static const struct sun4i_usb_phy_cfg sun50i_h616_cfg = { - .num_phys = 4, .disc_thresh = 3, .phyctl_offset = REG_PHYCTL_A33, .dedicated_clocks = true,