Re: [PATCH 16/21] usb: chipidea: msm: Restore wrapper settings after reset

[Stephen Boyd <stephen.boyd@linaro.org>]

Quoting Peter Chen (2016-06-29 01:26:48)
> On Sun, Jun 26, 2016 at 12:28:33AM -0700, Stephen Boyd wrote:
> > When the RESET bit is set in the USBCMD register it resets quite
> > @@ -21,11 +23,22 @@
> >  #define HS_PHY_SEC_CTRL                      0x0278
> >  # define HS_PHY_DIG_CLAMP_N          BIT(16)
> >  
> > +#define HS_PHY_GENCONFIG             0x009c
> > +# define HS_PHY_TXFIFO_IDLE_FORCE_DIS        BIT(4)
> > +
> > +#define HS_PHY_GENCONFIG_2           0x00a0
> > +# define HS_PHY_SESS_VLD_CTRL_EN     BIT(7)
> > +# define HS_PHY_ULPI_TX_PKT_EN_CLR_FIX       BIT(19)
> > +
> > +#define HSPHY_SESS_VLD_CTRL          BIT(25)
> > +
> 
> Keep alignment please.

I take it this means it should look like:

#define HS_PHY_GENCONFIG
#define HS_PHY_TXFIFO_IDLE_FORCE_DIS

?

> > @@ -141,6 +172,13 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> >       if (!base)
> >               return -ENOMEM;
> >  
> > +     ci->vbus_edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
> > +     if (IS_ERR(ci->vbus_edev)) {
> > +             if (PTR_ERR(ci->vbus_edev) != -ENODEV)
> > +                     return PTR_ERR(ci->vbus_edev);
> > +             ci->vbus_edev = NULL;
> > +     }
> > +
> 
> Why not using ci->platdata->vbus_extcon directly?

Because ci->platdata->vbus_extcon is assigned after the child platform
driver probes, and we have no idea when that will happen from the
ci_hdrc_msm driver probe. If we try after ci_hdrc_add_device() we'll
race with the driver probe and only get the pointer sometimes.

> 
> >       reset_control_assert(reset);
> >       usleep_range(10000, 12000);
> >       reset_control_deassert(reset);
> > @@ -157,6 +195,14 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
> >       if (ret)
> >               goto err_mux;
> >  
> > +     ulpi_node = of_find_node_by_name(pdev->dev.of_node, "ulpi");
> > +     if (ulpi_node) {
> > +             phy_node = of_get_next_available_child(ulpi_node, NULL);
> > +             ci->hsic = of_device_is_compatible(phy_node, "qcom,usb-hsic-phy");
> > +             of_node_put(phy_node);
> > +     }
> > +     of_node_put(ulpi_node);
> > +
> 
> Just confirm with you that ci->platdata->phy_mode is not enough?

Right. The phy_mode is never set to HSIC. It's always ULPI.