lkml.org 
[lkml]   [2017]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode
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@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.
>
> 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

\
 
 \ /
  Last update: 2017-09-28 18:54    [W:0.055 / U:1.232 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site