Hi Manu,
On Thu, Sep 28, 2017 at 09:30:38AM +0530, Manu Gautam wrote:
> On 9/28/2017 12:46 AM, Jack Pham wrote:
> > On Wed, Sep 27, 2017 at 10:57:41AM -0700, Jack Pham wrote:
> >> 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 <mgautam@xxxxxxxxxxxxxx>
> >>> ---
> >>>
> >>> +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.
>
> Right. There is one more reason why qusb2 driver needs qscratch:
> - During runtime suspend, it has to check linestate to set correct polarity for dp/dm
> wakeup interrupt in order to detect disconnect/resume ion LS and FS/HS modes.
Ugh, oh yeah. The way I understand we did it in our downstream driver
is still to have the controller driver read the linestate but then pass
the information via additional set_mode() flags which the PHY driver
could use to correctly arm the interrupt trigger polarity.
An alternative would be to access a couple of the debug QUSB2PHY
registers that also provide a reading of the current UTMI linestate. The
HPG mentions them vaguely, and I can't remember if we tested that
interface or not. Assuming it works, would that be preferable to reading
a non-PHY register here?
> >> - 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.
>
> Yes, dwc3/drd.c currently deals with only EXTCON_USB_HOST. So, for platforms
> where role swap happens using only Vbus or single GPIO this should take care of.
>
>
> > 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
>
> However, I agree that more appropriate place for lane0-pwr-present and
> vbus override update is dwc3 glue driver. Since we don't have one right now,
>
> IMO once we have dwc3-qcom driver in place, this handling can be moved from
> PHY to glue driver. Until then we can use this approach to get USB device mode
> working on qcom platforms which are using dwc3-of-simple.c e.g. sdm820
> dragonboard.
Could that be done in this series too? IMO better to get it right in one
shot. Is this aimed for 4.15?
Jack
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
