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