Re: [PATCH v5 03/16] phy: qcom: Add SNPS femto v2 USB HS phy

[Caleb Connolly]

Hi Sumit,

On 01/04/2024 06:46, Sumit Garg wrote:
On Thu, 28 Mar 2024 at 23:29, Caleb Connolly <caleb.conno...@linaro.org> wrote:

From: Bhupesh Sharma <bhupesh.li...@gmail.com>

Some Qualcomm SoCs newer than SDM845 feature a so-called "7nm phy"
driver, notable the SM8250 SoC which will gain U-Boot support in
upcoming patches.

Introduce a driver based on the Linux driver.

Signed-off-by: Bhupesh Sharma <bhupesh.sha...@linaro.org>
[code cleanup, align symbol names with Linux, switch to clk/reset_bulk APIs]
Signed-off-by: Caleb Connolly <caleb.conno...@linaro.org>
---
  drivers/phy/qcom/Kconfig                  |   8 ++
  drivers/phy/qcom/Makefile                 |   1 +
  drivers/phy/qcom/phy-qcom-snps-femto-v2.c | 207 ++++++++++++++++++++++++++++++
  3 files changed, 216 insertions(+)

...
diff --git a/drivers/phy/qcom/phy-qcom-snps-femto-v2.c 
b/drivers/phy/qcom/phy-qcom-snps-femto-v2.c
new file mode 100644
index 000000000000..58eb01972402
--- /dev/null
+++ b/drivers/phy/qcom/phy-qcom-snps-femto-v2.c
...
+static inline void qcom_snps_hsphy_write_mask(void __iomem *base, u32 offset,
+                                             u32 mask, u32 val)
+{
+       u32 reg;
+
+       reg = readl_relaxed(base + offset);
+
+       reg &= ~mask;
+       reg |= val & mask;
+       writel_relaxed(reg, base + offset);
+
+       /* Ensure above write is completed */
+       readl_relaxed(base + offset);

It looks like you have missed addressing comments related to this API.
Again, why do we need this special handling in U-Boot? Why not just
clrsetbits_le32()?

I have tried to find more info about this, but from what I can tell it's 
important (my guess is that it's a weird bus arbitration thing with the 
QMP phy's).

If I replace this with clrsetbits_le32() then my SM8250 board crashdumps 
while probing USB. So it is needed (and I'm wondering now if I ought to 
switch back to this for the dwc3 qcom glue as well).

-Sumit

+}
+
+static int qcom_snps_hsphy_usb_init(struct phy *phy)
+{
+       struct qcom_snps_hsphy *priv = dev_get_priv(phy->dev);
+
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_CFG0,
+                                  UTMI_PHY_CMN_CTRL_OVERRIDE_EN,
+                                  UTMI_PHY_CMN_CTRL_OVERRIDE_EN);
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_UTMI_CTRL5, POR,
+                                  POR);
+       qcom_snps_hsphy_write_mask(priv->base,
+                                  USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON0, 
FSEL_MASK, 0);
+       qcom_snps_hsphy_write_mask(priv->base,
+                                  USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON1,
+                                  PLLBTUNE, PLLBTUNE);
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_REFCLK_CTRL,
+                                  REFCLK_SEL_DEFAULT, REFCLK_SEL_MASK);
+       qcom_snps_hsphy_write_mask(priv->base,
+                                  USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON1,
+                                  VBUSVLDEXTSEL0, VBUSVLDEXTSEL0);
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_HS_PHY_CTRL1,
+                                  VBUSVLDEXT0, VBUSVLDEXT0);
+
+       qcom_snps_hsphy_write_mask(priv->base,
+                                  USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2,
+                                  VREGBYPASS, VREGBYPASS);
+
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_HS_PHY_CTRL2,
+                                  USB2_SUSPEND_N_SEL | USB2_SUSPEND_N,
+                                  USB2_SUSPEND_N_SEL | USB2_SUSPEND_N);
+
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_UTMI_CTRL0,
+                                  SLEEPM, SLEEPM);
+
+       qcom_snps_hsphy_write_mask(
+               priv->base, USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON0, SIDDQ, 0);
+
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_UTMI_CTRL5, POR,
+                                  0);
+
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_HS_PHY_CTRL2,
+                                  USB2_SUSPEND_N_SEL, 0);
+
+       qcom_snps_hsphy_write_mask(priv->base, USB2_PHY_USB_PHY_CFG0,
+                                  UTMI_PHY_CMN_CTRL_OVERRIDE_EN, 0);
+
+       return 0;
+}
+
+static int qcom_snps_hsphy_power_on(struct phy *phy)
+{
+       struct qcom_snps_hsphy *priv = dev_get_priv(phy->dev);
+       int ret;
+
+       clk_enable_bulk(&priv->clks);
+
+       ret = reset_deassert_bulk(&priv->resets);
+       if (ret)
+               return ret;
+
+       ret = qcom_snps_hsphy_usb_init(phy);
+       if (ret)
+               return ret;
+
+       return 0;
+}
+
+static int qcom_snps_hsphy_power_off(struct phy *phy)
+{
+       struct qcom_snps_hsphy *priv = dev_get_priv(phy->dev);
+
+       reset_assert_bulk(&priv->resets);
+       clk_disable_bulk(&priv->clks);
+
+       return 0;
+}
+
+static int qcom_snps_hsphy_phy_probe(struct udevice *dev)
+{
+       struct qcom_snps_hsphy *priv = dev_get_priv(dev);
+       int ret;
+
+       priv->base = dev_read_addr_ptr(dev);
+       if (IS_ERR(priv->base))
+               return PTR_ERR(priv->base);
+
+       ret = clk_get_bulk(dev, &priv->clks);
+       if (ret < 0 && ret != -ENOENT) {
+               printf("%s: Failed to get clocks %d\n", __func__, ret);
+               return ret;
+       }
+
+       ret = reset_get_bulk(dev, &priv->resets);
+       if (ret < 0) {
+               printf("failed to get resets, ret = %d\n", ret);
+               return ret;
+       }
+
+       clk_enable_bulk(&priv->clks);
+       reset_deassert_bulk(&priv->resets);
+
+       return 0;
+}
+
+static struct phy_ops qcom_snps_hsphy_phy_ops = {
+       .power_on = qcom_snps_hsphy_power_on,
+       .power_off = qcom_snps_hsphy_power_off,
+};
+
+static const struct udevice_id qcom_snps_hsphy_phy_ids[] = {
+       { .compatible = "qcom,sm8150-usb-hs-phy" },
+       { .compatible = "qcom,usb-snps-hs-5nm-phy" },
+       { .compatible = "qcom,usb-snps-hs-7nm-phy" },
+       { .compatible = "qcom,usb-snps-femto-v2-phy" },
+       {}
+};
+
+U_BOOT_DRIVER(qcom_usb_qcom_snps_hsphy) = {
+       .name = "qcom-snps-hsphy",
+       .id = UCLASS_PHY,
+       .of_match = qcom_snps_hsphy_phy_ids,
+       .ops = &qcom_snps_hsphy_phy_ops,
+       .probe = qcom_snps_hsphy_phy_probe,
+       .priv_auto = sizeof(struct qcom_snps_hsphy),
+};

--
2.44.0


--
// Caleb (they/them)