On Thu, Jul 18, 2024 at 11:00:06AM -0400, Frank Li wrote:
> On Thu, Jul 18, 2024 at 06:26:33PM +0800, Xu Yang wrote:
> > Per IC engineer request, we need to keep USBPHY2's clk always on,
>
> "IP require keep keep USBPHY2's clk always on."
>
> Not personal request, even it is IC expert. It should base on the "fact"
> instead of personal's opinion.
Okay.
>
> > in this way, the USBPHY2 (PLL7) power can be controlled by
> > hardware suspend signal totally. It is benefit of USB remote wakeup
> > case which needs the resume signal be sent out as soon as
> > possible (without software interfere). Without this, we may see usb
> > remote wakeup issue since the host does not send resume in time.
>
> So USBPHY2 (PLL7) power can be controlled by suspend signal. USB remote
> wakeup needs resume signal be sent out as soon as possible to match
>
> "spec requirement" or some other requirement.
Will change.
>
> >
> > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> > ---
> > drivers/usb/phy/phy-mxs-usb.c | 36 ++++++++++++++++++++++++++++-------
> > 1 file changed, 29 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> > index 42fcc8ad9492..b6868cc22c1e 100644
> > --- a/drivers/usb/phy/phy-mxs-usb.c
> > +++ b/drivers/usb/phy/phy-mxs-usb.c
> > @@ -150,6 +150,16 @@
> > #define MXS_PHY_TX_D_CAL_MIN 79
> > #define MXS_PHY_TX_D_CAL_MAX 119
> >
> > +/*
> > + * At some versions, the PHY2's clock is controlled by hardware directly,
>
> It better declear which version, for example, which chip use if no version
> info in IP.
Okay, will add.
>
> > + * eg, according to PHY's suspend status. In these PHYs, we only need to
> > + * open the clock at the initialization and close it at its shutdown routine.
> > + * It will be benefit for remote wakeup case which needs to send resume
> > + * signal as soon as possible, and in this case, the resume signal can be sent
> > + * out without software interfere.
>
> These PHYs can send resume signal without software interfere if not gate
> clock.
Will change.
>
> > + */
> > +#define MXS_PHY_HARDWARE_CONTROL_PHY2_CLK BIT(4)
> > +
> > struct mxs_phy_data {
> > unsigned int flags;
> > };
> > @@ -161,12 +171,14 @@ static const struct mxs_phy_data imx23_phy_data = {
> > static const struct mxs_phy_data imx6q_phy_data = {
> > .flags = MXS_PHY_SENDING_SOF_TOO_FAST |
> > MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> > - MXS_PHY_NEED_IP_FIX,
> > + MXS_PHY_NEED_IP_FIX |
> > + MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
> > };
> >
> > static const struct mxs_phy_data imx6sl_phy_data = {
> > .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> > - MXS_PHY_NEED_IP_FIX,
> > + MXS_PHY_NEED_IP_FIX |
> > + MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
> > };
> >
> > static const struct mxs_phy_data vf610_phy_data = {
> > @@ -175,7 +187,8 @@ static const struct mxs_phy_data vf610_phy_data = {
> > };
> >
> > static const struct mxs_phy_data imx6sx_phy_data = {
> > - .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS,
> > + .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
> > + MXS_PHY_HARDWARE_CONTROL_PHY2_CLK,
> > };
> >
> > static const struct mxs_phy_data imx6ul_phy_data = {
> > @@ -206,6 +219,7 @@ struct mxs_phy {
> > u32 tx_reg_set;
> > u32 tx_reg_mask;
> > struct regulator *phy_3p0;
> > + bool hardware_control_phy2_clk;
>
> Needn't it. just check MXS_PHY_HARDWARE_CONTROL_PHY2_CLK flag is enough.
Okay.
>
> > };
> >
> > static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
> > @@ -518,12 +532,17 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
> > }
> > writel(BM_USBPHY_CTRL_CLKGATE,
> > x->io_priv + HW_USBPHY_CTRL_SET);
> > - clk_disable_unprepare(mxs_phy->clk);
> > + if (!(mxs_phy->port_id == 1 &&
> > + mxs_phy->hardware_control_phy2_clk))
> > + clk_disable_unprepare(mxs_phy->clk);
> > } else {
> > mxs_phy_clock_switch_delay();
> > - ret = clk_prepare_enable(mxs_phy->clk);
> > - if (ret)
> > - return ret;
> > + if (!(mxs_phy->port_id == 1 &&
> > + mxs_phy->hardware_control_phy2_clk)) {
> > + ret = clk_prepare_enable(mxs_phy->clk);
> > + if (ret)
> > + return ret;
> > + }
> > writel(BM_USBPHY_CTRL_CLKGATE,
> > x->io_priv + HW_USBPHY_CTRL_CLR);
> > writel(0, x->io_priv + HW_USBPHY_PWD);
> > @@ -819,6 +838,9 @@ static int mxs_phy_probe(struct platform_device *pdev)
> > if (mxs_phy->phy_3p0)
> > regulator_set_voltage(mxs_phy->phy_3p0, 3200000, 3200000);
> >
> > + if (mxs_phy->data->flags & MXS_PHY_HARDWARE_CONTROL_PHY2_CLK)
> > + mxs_phy->hardware_control_phy2_clk = true;
> > +
>
> Needn't it.
Okay. Will remove this.
>
> > platform_set_drvdata(pdev, mxs_phy);
> >
> > device_set_wakeup_capable(&pdev->dev, true);
> > --
> > 2.34.1
> >
