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

From: Bhupesh Sharma <bhupesh.linux@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.sharma@linaro.org>
[code cleanup, align symbol names with Linux, switch to clk/reset_bulk APIs]
Signed-off-by: Caleb Connolly <caleb.connolly@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(+)

On Thu, 28 Mar 2024 at 23:29, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> From: Bhupesh Sharma <bhupesh.linux@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.sharma@linaro.org>
> [code cleanup, align symbol names with Linux, switch to clk/reset_bulk APIs]
> Signed-off-by: Caleb Connolly <caleb.connolly@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/Kconfig b/drivers/phy/qcom/Kconfig
> index 361dfb6e1126..b9fe608c2798 100644
> --- a/drivers/phy/qcom/Kconfig
> +++ b/drivers/phy/qcom/Kconfig
> @@ -18,8 +18,16 @@ config PHY_QCOM_QUSB2
>         help
>           Enable this to support the Super-Speed USB transceiver on various
>           Qualcomm chipsets.
>
> +config PHY_QCOM_USB_SNPS_FEMTO_V2
> +       tristate "Qualcomm SNPS FEMTO USB HS PHY v2"
> +       depends on PHY && ARCH_SNAPDRAGON
> +       help
> +         Enable this to support the Qualcomm Synopsys DesignWare Core 7nm
> +         High-Speed PHY driver. This driver supports the Hi-Speed PHY which
> +         is usually paired with Synopsys DWC3 USB IPs on MSM SOCs.
> +
>  config PHY_QCOM_USB_HS_28NM
>         tristate "Qualcomm 28nm High-Speed PHY"
>         depends on PHY && ARCH_SNAPDRAGON
>         help
> diff --git a/drivers/phy/qcom/Makefile b/drivers/phy/qcom/Makefile
> index f6af985666a4..5f4db4a53788 100644
> --- a/drivers/phy/qcom/Makefile
> +++ b/drivers/phy/qcom/Makefile
> @@ -1,5 +1,6 @@
>  obj-$(CONFIG_PHY_QCOM_IPQ4019_USB) += phy-qcom-ipq4019-usb.o
>  obj-$(CONFIG_MSM8916_USB_PHY) += msm8916-usbh-phy.o
>  obj-$(CONFIG_PHY_QCOM_QUSB2) += phy-qcom-qusb2.o
> +obj-$(CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2) += phy-qcom-snps-femto-v2.o
>  obj-$(CONFIG_PHY_QCOM_USB_HS_28NM) += phy-qcom-usb-hs-28nm.o
>  obj-$(CONFIG_PHY_QCOM_USB_SS) += phy-qcom-usb-ss.o
> 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
> @@ -0,0 +1,207 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2023 Bhupesh Sharma <bhupesh.sharma@linaro.org>
> + *
> + * Based on Linux driver
> + */
> +
> +#include <clk.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <dm/device_compat.h>
> +#include <dm/devres.h>
> +#include <generic-phy.h>
> +#include <malloc.h>
> +#include <reset.h>
> +
> +#include <asm/io.h>
> +#include <linux/bitops.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +
> +#define USB2_PHY_USB_PHY_UTMI_CTRL0 (0x3c)
> +#define SLEEPM BIT(0)
> +#define OPMODE_MASK GENMASK(4, 3)
> +#define OPMODE_NORMAL (0x00)
> +#define OPMODE_NONDRIVING BIT(3)
> +#define TERMSEL BIT(5)
> +
> +#define USB2_PHY_USB_PHY_UTMI_CTRL5 (0x50)
> +#define POR BIT(1)
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON0 (0x54)
> +#define SIDDQ BIT(2)
> +#define RETENABLEN BIT(3)
> +#define FSEL_MASK GENMASK(6, 4)
> +#define FSEL_DEFAULT (0x3 << 4)
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON1 (0x58)
> +#define VBUSVLDEXTSEL0 BIT(4)
> +#define PLLBTUNE BIT(5)
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2 (0x5c)
> +#define VREGBYPASS BIT(0)
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_CTRL1 (0x60)
> +#define VBUSVLDEXT0 BIT(0)
> +
> +#define USB2_PHY_USB_PHY_HS_PHY_CTRL2 (0x64)
> +#define USB2_AUTO_RESUME BIT(0)
> +#define USB2_SUSPEND_N BIT(2)
> +#define USB2_SUSPEND_N_SEL BIT(3)
> +
> +#define USB2_PHY_USB_PHY_CFG0 (0x94)
> +#define UTMI_PHY_DATAPATH_CTRL_OVERRIDE_EN BIT(0)
> +#define UTMI_PHY_CMN_CTRL_OVERRIDE_EN BIT(1)
> +
> +#define USB2_PHY_USB_PHY_REFCLK_CTRL (0xa0)
> +#define REFCLK_SEL_MASK GENMASK(1, 0)
> +#define REFCLK_SEL_DEFAULT (0x2 << 0)
> +
> +struct qcom_snps_hsphy {
> +       void __iomem *base;
> +       struct clk_bulk clks;
> +       struct reset_ctl_bulk resets;
> +};
> +
> +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()?

-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
>

Hi Sumit,

On 01/04/2024 06:46, Sumit Garg wrote:
> On Thu, 28 Mar 2024 at 23:29, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>> From: Bhupesh Sharma <bhupesh.linux@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.sharma@linaro.org>
>> [code cleanup, align symbol names with Linux, switch to clk/reset_bulk APIs]
>> Signed-off-by: Caleb Connolly <caleb.connolly@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
>>

On Tue, 2 Apr 2024 at 15:37, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Hi Sumit,
>
> On 01/04/2024 06:46, Sumit Garg wrote:
> > On Thu, 28 Mar 2024 at 23:29, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> >>
> >> From: Bhupesh Sharma <bhupesh.linux@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.sharma@linaro.org>
> >> [code cleanup, align symbol names with Linux, switch to clk/reset_bulk APIs]
> >> Signed-off-by: Caleb Connolly <caleb.connolly@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).

Strange, probably it's due to some strict device memory ordering
rules. If you can add some reasoning in comments for this API then
feel free to add:

Acked-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit