linux-kernel - Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in device mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170927191643.GC12812@usblab-sd-06.qualcomm.com>
Date:   Wed, 27 Sep 2017 12:16:43 -0700
From:   Jack Pham <jackp@...eaurora.org>
To:     Manu Gautam <mgautam@...eaurora.org>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Felipe Balbi <balbi@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-usb@...r.kernel.org,
        Vivek Gautam <vivek.gautam@...eaurora.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        "open list:GENERIC PHY FRAMEWORK" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 14/17] phy: qcom-qusb2: Set vbus sw-override signal in
 device mode

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 <mgautam@...eaurora.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