On Wed, Sep 27, 2017 at 10:57:41AM -0700, Jack Pham wrote:
> Hi Manu,
>
> On Wed, Sep 27, 2017 at 02:29:10PM +0530, Manu Gautam wrote:
> > VBUS signal coming from PHY must be asserted in device for
> > controller to start operation or assert pull-up. For some
> > platforms where VBUS line is not connected to PHY there is
> > HS_PHY_CTRL register in QSCRATCH wrapper that can be used
> > by software to override VBUS signal going to controller.
> >
> > Signed-off-by: Manu Gautam <mgau...@codeaurora.org>
> > ---
> >
> > +static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
> > +{
> > + struct qusb2_phy *qphy = phy_get_drvdata(phy);
> > +
> > + qphy->mode = mode;
> > +
> > + /* Update VBUS override in qscratch register */
> > + if (qphy->qscratch_base) {
> > + if (mode == PHY_MODE_USB_DEVICE)
> > + qusb2_setbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> > + UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
> > + else
> > + qusb2_clrbits(qphy->qscratch_base, QSCRATCH_HS_PHY_CTRL,
> > + UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
>
> Wouldn't this be better off handled in the controller glue driver? Two
> reasons I think this patch is unattractive:
>
> - qscratch_base is part of the controller's register space. Your later
> patch 16/17 ("phy: qcom-qmp: Override lane0_power_present signal in
> device mode") does a similar thing and hence both drivers have to
> ioremap() the same register resource while at the same time avoiding
> request_mem_region() (called by devm_ioremap_resource) to allow it to
> be mapped in both places.
>
> - VBUS override bit becomes asserted simply because the mode is changed
> to device mode but this is irrespective of the actual VBUS state. This
> could break some test setups which perform a logical disconnect by
> switching off/on VBUS while leaving data lines connected. Controller
> would go merrily along thinking it is still attached to the host.
>
> Instead maybe this could be tied to EXTCON_USB handling in the glue
> driver; though it would need to be an additional notifier on top of
> dwc3/drd.c which already handles extcon for host/device mode.
That is to say, we'd probably need to split out dwc3-qcom from
dwc3-of-simple.c into its own driver (again) in order to add this.
Jack
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
