| Message ID | 20210615110636.23403-15-andre.przywara@arm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
On 15-06-21, 12:06, 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; > + } no delay between deassert and assert... ?
On Mon, 21 Jun 2021 10:06:31 +0530 Vinod Koul <vkoul@kernel.org> wrote: Hi Vinod, thanks for having a look! > On 15-06-21, 12:06, 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; > > + } > > no delay between deassert and assert... ? Mmmh, not sure what you are after. This is just the clean-up path, when the deassert failed, and we tear down what was brought up before. And the assert is not for the same reset line that we tried to deassert anyway, if that is what you mean? Or do I miss something here? Cheers, Andre
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); + 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); + 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);
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(+)