| Message ID | 20240718102637.3964232-1-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/6] usb: phy: mxs: Using regulator phy-3p0 | expand |
On Thu, Jul 18, 2024 at 06:26:32PM +0800, Xu Yang wrote: > It is one of PHY's power, and we need to enable it to keep signal > quality good, and pass eye diagram test. "Enable regulator 'phy-3p0' to pass eye diagram test since it improve signal qualilty." My question is why it just improve signal quality instead of make it work. It should not work if no power supply. > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > --- > drivers/usb/phy/phy-mxs-usb.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > index 920a32cd094d..42fcc8ad9492 100644 > --- a/drivers/usb/phy/phy-mxs-usb.c > +++ b/drivers/usb/phy/phy-mxs-usb.c > @@ -18,6 +18,7 @@ > #include <linux/regmap.h> > #include <linux/mfd/syscon.h> > #include <linux/iopoll.h> > +#include <linux/regulator/consumer.h> > > #define DRIVER_NAME "mxs_phy" > > @@ -204,6 +205,7 @@ struct mxs_phy { > int port_id; > u32 tx_reg_set; > u32 tx_reg_mask; > + struct regulator *phy_3p0; > }; > > static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy) > @@ -288,6 +290,16 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy) > if (ret) > goto disable_pll; > > + if (mxs_phy->phy_3p0) { > + ret = regulator_enable(mxs_phy->phy_3p0); > + if (ret) { > + dev_err(mxs_phy->phy.dev, > + "Failed to enable 3p0 regulator, ret=%d\n", > + ret); > + return ret; > + } > + } > + > /* Power up the PHY */ > writel(0, base + HW_USBPHY_PWD); > > @@ -448,6 +460,9 @@ static void mxs_phy_shutdown(struct usb_phy *phy) > if (is_imx7ulp_phy(mxs_phy)) > mxs_phy_pll_enable(phy->io_priv, false); > > + if (mxs_phy->phy_3p0) > + regulator_disable(mxs_phy->phy_3p0); > + > clk_disable_unprepare(mxs_phy->clk); > } > > @@ -789,6 +804,21 @@ static int mxs_phy_probe(struct platform_device *pdev) > mxs_phy->clk = clk; > mxs_phy->data = of_device_get_match_data(&pdev->dev); > > + mxs_phy->phy_3p0 = devm_regulator_get(&pdev->dev, "phy-3p0"); Does binding doc update? > + if (PTR_ERR(mxs_phy->phy_3p0) == -EPROBE_DEFER) { > + return -EPROBE_DEFER; > + } else if (PTR_ERR(mxs_phy->phy_3p0) == -ENODEV) { > + /* not exist */ > + mxs_phy->phy_3p0 = NULL; > + } else if (IS_ERR(mxs_phy->phy_3p0)) { > + dev_err(&pdev->dev, "Getting regulator error: %ld\n", > + PTR_ERR(mxs_phy->phy_3p0)); > + return PTR_ERR(mxs_phy->phy_3p0); > + } just need call dev_err_probe() > + > + if (mxs_phy->phy_3p0) > + regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000); > + > platform_set_drvdata(pdev, mxs_phy); > > device_set_wakeup_capable(&pdev->dev, true); > -- > 2.34.1 >
On Thu, Jul 18, 2024 at 10:42:33AM -0400, Frank Li wrote: > On Thu, Jul 18, 2024 at 06:26:32PM +0800, Xu Yang wrote: > > It is one of PHY's power, and we need to enable it to keep signal > > quality good, and pass eye diagram test. > > "Enable regulator 'phy-3p0' to pass eye diagram test since it improve signal > qualilty." > > My question is why it just improve signal quality instead of make it work. > It should not work if no power supply. I'm cleaning up these old local patch and I'm not very clear about the history. But I think this 3p0 is not the main power of phy, it's just one of phy's power. I can only get below info about this reg-3p0: 37.6.2 Regulator 3P0 Register (PMU_REG_3P0) "This register defines the control and status bits for the 3.0V regulator powered by the host USB VBUS pin." I've just tested it works well without this phy-3p0. I'll rewrite the commit message to avoid confusion. > > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > --- > > drivers/usb/phy/phy-mxs-usb.c | 30 ++++++++++++++++++++++++++++++ > > 1 file changed, 30 insertions(+) > > > > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c > > index 920a32cd094d..42fcc8ad9492 100644 > > --- a/drivers/usb/phy/phy-mxs-usb.c > > +++ b/drivers/usb/phy/phy-mxs-usb.c > > @@ -18,6 +18,7 @@ > > #include <linux/regmap.h> > > #include <linux/mfd/syscon.h> > > #include <linux/iopoll.h> > > +#include <linux/regulator/consumer.h> > > > > #define DRIVER_NAME "mxs_phy" > > > > @@ -204,6 +205,7 @@ struct mxs_phy { > > int port_id; > > u32 tx_reg_set; > > u32 tx_reg_mask; > > + struct regulator *phy_3p0; > > }; > > > > static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy) > > @@ -288,6 +290,16 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy) > > if (ret) > > goto disable_pll; > > > > + if (mxs_phy->phy_3p0) { > > + ret = regulator_enable(mxs_phy->phy_3p0); > > + if (ret) { > > + dev_err(mxs_phy->phy.dev, > > + "Failed to enable 3p0 regulator, ret=%d\n", > > + ret); > > + return ret; > > + } > > + } > > + > > /* Power up the PHY */ > > writel(0, base + HW_USBPHY_PWD); > > > > @@ -448,6 +460,9 @@ static void mxs_phy_shutdown(struct usb_phy *phy) > > if (is_imx7ulp_phy(mxs_phy)) > > mxs_phy_pll_enable(phy->io_priv, false); > > > > + if (mxs_phy->phy_3p0) > > + regulator_disable(mxs_phy->phy_3p0); > > + > > clk_disable_unprepare(mxs_phy->clk); > > } > > > > @@ -789,6 +804,21 @@ static int mxs_phy_probe(struct platform_device *pdev) > > mxs_phy->clk = clk; > > mxs_phy->data = of_device_get_match_data(&pdev->dev); > > > > + mxs_phy->phy_3p0 = devm_regulator_get(&pdev->dev, "phy-3p0"); > > Does binding doc update? Yes. > > > + if (PTR_ERR(mxs_phy->phy_3p0) == -EPROBE_DEFER) { > > + return -EPROBE_DEFER; > > + } else if (PTR_ERR(mxs_phy->phy_3p0) == -ENODEV) { > > + /* not exist */ > > + mxs_phy->phy_3p0 = NULL; > > + } else if (IS_ERR(mxs_phy->phy_3p0)) { > > + dev_err(&pdev->dev, "Getting regulator error: %ld\n", > > + PTR_ERR(mxs_phy->phy_3p0)); > > + return PTR_ERR(mxs_phy->phy_3p0); > > + } > > just need call dev_err_probe() Ok. Will change it. > > > + > > + if (mxs_phy->phy_3p0) > > + regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000); > > + > > platform_set_drvdata(pdev, mxs_phy); > > > > device_set_wakeup_capable(&pdev->dev, true); > > -- > > 2.34.1 > >
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c index 920a32cd094d..42fcc8ad9492 100644 --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/drivers/usb/phy/phy-mxs-usb.c @@ -18,6 +18,7 @@ #include <linux/regmap.h> #include <linux/mfd/syscon.h> #include <linux/iopoll.h> +#include <linux/regulator/consumer.h> #define DRIVER_NAME "mxs_phy" @@ -204,6 +205,7 @@ struct mxs_phy { int port_id; u32 tx_reg_set; u32 tx_reg_mask; + struct regulator *phy_3p0; }; static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy) @@ -288,6 +290,16 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy) if (ret) goto disable_pll; + if (mxs_phy->phy_3p0) { + ret = regulator_enable(mxs_phy->phy_3p0); + if (ret) { + dev_err(mxs_phy->phy.dev, + "Failed to enable 3p0 regulator, ret=%d\n", + ret); + return ret; + } + } + /* Power up the PHY */ writel(0, base + HW_USBPHY_PWD); @@ -448,6 +460,9 @@ static void mxs_phy_shutdown(struct usb_phy *phy) if (is_imx7ulp_phy(mxs_phy)) mxs_phy_pll_enable(phy->io_priv, false); + if (mxs_phy->phy_3p0) + regulator_disable(mxs_phy->phy_3p0); + clk_disable_unprepare(mxs_phy->clk); } @@ -789,6 +804,21 @@ static int mxs_phy_probe(struct platform_device *pdev) mxs_phy->clk = clk; mxs_phy->data = of_device_get_match_data(&pdev->dev); + mxs_phy->phy_3p0 = devm_regulator_get(&pdev->dev, "phy-3p0"); + if (PTR_ERR(mxs_phy->phy_3p0) == -EPROBE_DEFER) { + return -EPROBE_DEFER; + } else if (PTR_ERR(mxs_phy->phy_3p0) == -ENODEV) { + /* not exist */ + mxs_phy->phy_3p0 = NULL; + } else if (IS_ERR(mxs_phy->phy_3p0)) { + dev_err(&pdev->dev, "Getting regulator error: %ld\n", + PTR_ERR(mxs_phy->phy_3p0)); + return PTR_ERR(mxs_phy->phy_3p0); + } + + if (mxs_phy->phy_3p0) + regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000); + platform_set_drvdata(pdev, mxs_phy); device_set_wakeup_capable(&pdev->dev, true);
It is one of PHY's power, and we need to enable it to keep signal quality good, and pass eye diagram test. Signed-off-by: Xu Yang <xu.yang_2@nxp.com> --- drivers/usb/phy/phy-mxs-usb.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)