Messages in this thread | | | Subject | Re: [PATCH v10 14/18] phy: sun4i-usb: Introduce port2 SIDDQ quirk | From | Samuel Holland <> | Date | Tue, 22 Feb 2022 21:57:07 -0600 |
| |
On 2/11/22 6:26 AM, Andre Przywara wrote: > At least the Allwinner H616 SoC requires a weird quirk to make most > USB PHYs work: Only port2 works out of the box, but all other ports > need some help from this port2 to work correctly: The CLK_BUS_PHY2 and > RST_USB_PHY2 clock and reset need to be enabled, and the SIDDQ bit in > the PMU PHY control register needs to be cleared. For this register to > be accessible, CLK_BUS_ECHI2 needs to be ungated. Don't ask .... > > Instead of disguising this as some generic feature, do exactly that > in our PHY init: > If the quirk bit is set, and we initialise a PHY other than PHY2, ungate > this one special clock, and clear the SIDDQ bit. We can pull in the > other required clocks via the DT. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/phy/allwinner/phy-sun4i-usb.c | 59 +++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c > index 126ef74d013c..316ef5fca831 100644 > --- a/drivers/phy/allwinner/phy-sun4i-usb.c > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c > @@ -120,6 +120,7 @@ struct sun4i_usb_phy_cfg { > u8 phyctl_offset; > bool dedicated_clocks; > bool phy0_dual_route; > + bool needs_phy2_siddq; > int missing_phys; > }; > > @@ -289,6 +290,50 @@ static int sun4i_usb_phy_init(struct phy *_phy) > return ret; > } > > + /* Some PHYs on some SoCs need the help of PHY2 to work. */ > + if (data->cfg->needs_phy2_siddq && phy->index != 2) { > + struct sun4i_usb_phy *phy2 = &data->phys[2]; > + > + ret = clk_prepare_enable(phy2->clk); > + if (ret) { > + reset_control_assert(phy->reset); > + clk_disable_unprepare(phy->clk2); > + clk_disable_unprepare(phy->clk); > + return ret; > + } > + > + ret = reset_control_deassert(phy2->reset); > + if (ret) { > + clk_disable_unprepare(phy2->clk); > + reset_control_assert(phy->reset); > + clk_disable_unprepare(phy->clk2); > + clk_disable_unprepare(phy->clk); > + return ret; > + } > + > + /* > + * This extra clock is just needed to access the > + * REG_HCI_PHY_CTL PMU register for PHY2. > + */ > + ret = clk_prepare_enable(phy2->clk2); > + if (ret) { > + reset_control_assert(phy2->reset); > + clk_disable_unprepare(phy2->clk); > + reset_control_assert(phy->reset); > + clk_disable_unprepare(phy->clk2); > + clk_disable_unprepare(phy->clk);
This is quite a lot of duplication. Please consider using goto for the error path.
> + return ret; > + } > + > + if (phy2->pmu && data->cfg->hci_phy_ctl_clear) { > + val = readl(phy2->pmu + REG_HCI_PHY_CTL); > + val &= ~data->cfg->hci_phy_ctl_clear; > + writel(val, phy2->pmu + REG_HCI_PHY_CTL); > + } > + > + clk_disable_unprepare(phy->clk2); > + } > + > if (phy->pmu && data->cfg->hci_phy_ctl_clear) { > val = readl(phy->pmu + REG_HCI_PHY_CTL); > val &= ~data->cfg->hci_phy_ctl_clear; > @@ -354,6 +399,13 @@ static int sun4i_usb_phy_exit(struct phy *_phy) > data->phy0_init = false; > } > > + if (data->cfg->needs_phy2_siddq && phy->index != 2) { > + struct sun4i_usb_phy *phy2 = &data->phys[2]; > + > + clk_disable_unprepare(phy2->clk); > + reset_control_assert(phy2->reset); > + } > + > sun4i_usb_phy_passby(phy, 0); > reset_control_assert(phy->reset); > clk_disable_unprepare(phy->clk2); > @@ -785,6 +837,13 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev) > dev_err(dev, "failed to get clock %s\n", name); > return PTR_ERR(phy->clk2); > } > + } else { > + snprintf(name, sizeof(name), "pmu%d_clk", i); > + phy->clk2 = devm_clk_get_optional(dev, name);
This clock is not documented anywhere in the binding.
Regards, Samuel
> + if (IS_ERR(phy->clk2)) { > + dev_err(dev, "failed to get clock %s\n", name); > + return PTR_ERR(phy->clk2); > + } > } > > snprintf(name, sizeof(name), "usb%d_reset", i); >
| |