[PATCH v3 02/14] phy: qcom: add Qualcomm QUSB2 USB PHY driver

Caleb Connolly caleb.connolly at linaro.org
Wed Mar 20 14:47:00 CET 2024


Hi Sumit,

>> +static inline void qusb2_write_mask(void __iomem *base, u32 offset, u32 val,
>> +                                   u32 mask)
>> +{
>> +       u32 reg;
>> +
>> +       reg = readl(base + offset);
>> +       reg &= ~mask;
>> +       reg |= val & mask;
>> +       writel(reg, base + offset);
>> +
>> +       /* Ensure above write is completed */
>> +       readl(base + offset);
>> +}
> 
> Function unused?

Will drop
> 
>> +
>> +static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
>> +{
>> +       u32 reg;
>> +
>> +       reg = readl(base + offset);
>> +       reg |= val;
>> +       writel(reg, base + offset);
>> +
>> +       /* Ensure above write is completed */
>> +       readl(base + offset);
>> +}
>> +
>> +static inline void qusb2_clrbits(void __iomem *base, u32 offset, u32 val)
>> +{
>> +       u32 reg;
>> +
>> +       reg = readl(base + offset);
>> +       reg &= ~val;
>> +       writel(reg, base + offset);
>> +
>> +       /* Ensure above write is completed */
>> +       readl(base + offset);
>> +}
> 
> Any specific reason to not use setbits_le32() and clrbits_le32() her instead?

This is just copied from Linux. I tested with set/clrbits and it seems
to work ok... I'm a little worried this is a workaround for some obscure
bug but eh, I guess we'll just go without and see what happens.
> 
>> +
>> +static inline void qusb2_phy_configure(void __iomem *base,
>> +                                      const unsigned int *regs,
>> +                                      const struct qusb2_phy_init_tbl tbl[],
>> +                                      int num)
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < num; i++) {
>> +               if (tbl[i].in_layout)
>> +                       writel(tbl[i].val, base + regs[tbl[i].offset]);
>> +               else
>> +                       writel(tbl[i].val, base + tbl[i].offset);
>> +       }
>> +}
>> +
>> +static int qusb2phy_do_reset(struct qusb2_phy *qphy)
>> +{
>> +       int ret;
>> +
>> +       ret = reset_assert(&qphy->phy_rst);
>> +       if (ret)
>> +               return ret;
>> +
>> +       udelay(10);
>> +
>> +       ret = reset_deassert(&qphy->phy_rst);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return 0;
>> +}
>> +
>> +static int qusb2phy_power_on(struct phy *phy)
>> +{
>> +       struct qusb2_phy *qphy = dev_get_priv(phy->dev);
>> +       const struct qusb2_phy_cfg *cfg = qphy->cfg;
>> +       int ret;
>> +       u32 val;
>> +
>> +       ret = qusb2phy_do_reset(qphy);
>> +       if (ret)
>> +               return ret;
> 
> Do we really need to reset here? Wouldn't just
> reset_deassert(&qphy->phy_rst) be sufficient here instead?

The reset is not asserted by default, so de-asserting it would do
nothing. Yes we need to reset because we have no idea what state the
hardware is in, depending on what board and what bootloader you've come
from this phy could have already been powered up and configured before
we get to U-Boot.


> 
> -Sumit
> 
>> +
>> +       /* Disable the PHY */
>> +       qusb2_setbits(qphy->base, cfg->regs[QUSB2PHY_PORT_POWERDOWN],
>> +                     qphy->cfg->disable_ctrl);
>> +
>> +       if (cfg->has_pll_test) {
>> +               /* save reset value to override reference clock scheme later */
>> +               val = readl(qphy->base + QUSB2PHY_PLL_TEST);
>> +       }
>> +
>> +       qusb2_phy_configure(qphy->base, cfg->regs, cfg->tbl, cfg->tbl_num);
>> +
>> +       /* Enable the PHY */
>> +       qusb2_clrbits(qphy->base, cfg->regs[QUSB2PHY_PORT_POWERDOWN],
>> +                     POWER_DOWN);
>> +
>> +       /* Required to get phy pll lock successfully */
>> +       udelay(150);
>> +
>> +       if (cfg->has_pll_test) {
>> +               val |= CLK_REF_SEL;
>> +
>> +               writel(val, qphy->base + QUSB2PHY_PLL_TEST);
>> +
>> +               /* ensure above write is through */
>> +               readl(qphy->base + QUSB2PHY_PLL_TEST);
>> +       }
>> +
>> +       /* Required to get phy pll lock successfully */
>> +       udelay(100);
>> +
>> +       val = readb(qphy->base + cfg->regs[QUSB2PHY_PLL_STATUS]);
>> +       if (!(val & cfg->mask_core_ready)) {
>> +               pr_err("QUSB2PHY pll lock failed: status reg = %x\n", val);
>> +               ret = -EBUSY;
>> +               return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int qusb2phy_power_off(struct phy *phy)
>> +{
>> +       struct qusb2_phy *qphy = dev_get_priv(phy->dev);
>> +
>> +       /* Disable the PHY */
>> +       qusb2_setbits(qphy->base, qphy->cfg->regs[QUSB2PHY_PORT_POWERDOWN],
>> +                     qphy->cfg->disable_ctrl);
>> +
>> +       reset_assert(&qphy->phy_rst);
>> +
>> +       clk_disable(&qphy->cfg_ahb_clk);
>> +
>> +       return 0;
>> +}
>> +
>> +static int qusb2phy_clk_init(struct udevice *dev, struct qusb2_phy *qphy)
>> +{
>> +       int ret;
>> +
>> +       /* We ignore the ref clock as we currently lack a driver for rpmcc/rpmhcc where
>> +        * it usually comes from - we assume it's always on.
>> +        */
>> +       ret = clk_get_by_name(dev, "cfg_ahb", &qphy->cfg_ahb_clk);
>> +       if (ret == -ENOSYS || ret == -ENOENT)
>> +               return 0;
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = clk_enable(&qphy->cfg_ahb_clk);
>> +       if (ret) {
>> +               return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int qusb2phy_probe(struct udevice *dev)
>> +{
>> +       struct qusb2_phy *qphy = dev_get_priv(dev);
>> +       int ret;
>> +
>> +       qphy->base = (void __iomem *)dev_read_addr(dev);
>> +       if (IS_ERR(qphy->base))
>> +               return PTR_ERR(qphy->base);
>> +
>> +       ret = qusb2phy_clk_init(dev, qphy);
>> +       if (ret) {
>> +               printf("%s: Couldn't get clocks: %d\n", __func__, ret);
>> +               return ret;
>> +       }
>> +
>> +       ret = reset_get_by_index(dev, 0, &qphy->phy_rst);
>> +       if (ret) {
>> +               printf("%s: Couldn't get resets: %d\n", __func__, ret);
>> +               return ret;
>> +       }
>> +
>> +       qphy->cfg = (const struct qusb2_phy_cfg *)dev_get_driver_data(dev);
>> +       if (!qphy->cfg) {
>> +               printf("%s: Couldn't get driver data\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>> +       debug("%s success qusb phy cfg %p\n", __func__, qphy->cfg);
>> +       return 0;
>> +}
>> +
>> +static struct phy_ops qusb2phy_ops = {
>> +       .power_on = qusb2phy_power_on,
>> +       .power_off = qusb2phy_power_off,
>> +};
>> +
>> +static const struct udevice_id qusb2phy_ids[] = {
>> +       { .compatible = "qcom,qusb2-phy" },
>> +       { .compatible = "qcom,qcm2290-qusb2-phy",
>> +         .data = (ulong)&sm6115_phy_cfg },
>> +       { .compatible = "qcom,sm6115-qusb2-phy",
>> +         .data = (ulong)&sm6115_phy_cfg },
>> +       { .compatible = "qcom,qusb2-v2-phy", .data = (ulong)&qusb2_v2_phy_cfg },
>> +       {}
>> +};
>> +
>> +U_BOOT_DRIVER(qcom_qusb2_phy) = {
>> +       .name = "qcom-qusb2-phy",
>> +       .id = UCLASS_PHY,
>> +       .of_match = qusb2phy_ids,
>> +       .ops = &qusb2phy_ops,
>> +       .probe = qusb2phy_probe,
>> +       .priv_auto = sizeof(struct qusb2_phy),
>> +};
>>
>> --
>> 2.44.0
>>

-- 
// Caleb (they/them)


More information about the U-Boot mailing list