Re: [PATCH 0/3] USB IMX Chipidea fix gpio vbus control

From: Peter Chen <peter.chen@nxp.com>
To: Marco Felsch <m.felsch@pengutronix.de>
Cc: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"jun.li@freescale.com" <jun.li@freescale.com>,
	"stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
	dl-linux-imx <linux-imx@nxp.com>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/3] USB IMX Chipidea fix gpio vbus control
Date: Thu, 27 Feb 2020 12:20:43 +0000	[thread overview]
Message-ID: <20200227122045.GB24071@b29397-desktop> (raw)
In-Reply-To: <20200227113539.gcx3nfwm2fbm3ukv@pengutronix.de>

On 20-02-27 12:35:39, Marco Felsch wrote:
> Hi Peter,
> 
> thanks for your fast reply :)
> 
> On 20-02-27 11:18, Peter Chen wrote:
> > On 20-02-27 11:42:09, Marco Felsch wrote:
> > > Hi,
> > > 
> > > the gpio-based port power is broken since commit [1,2]. I changed the
> > > core behaviour to to cleanup the code and avoid local workaround fixes.
> > 
> > Many i.mx series evk boards work well for gpio-based port power control,
> > what problem you have found, would you please list it detail?
> 
> Hm.. who could that work? I picked the important parts:
> 
> static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
> {
> 	...
> 
> 	if (priv->reg_vbus && enable != priv->enabled) {
> 
> 		...
> 
> 		if (enable)
> 			ret = regulator_enable(priv->reg_vbus);
> 		else
> 			ret = regulator_disable(priv->reg_vbus);
> 
> 		...
> 	}
> 
> 	...
> }
> 
> static int host_start(struct ci_hdrc *ci)
> {
> 	...
> 
> 	priv->reg_vbus = NULL;
> 
> 	if (ci->platdata->reg_vbus && !ci_otg_is_fsm_mode(ci)) {
> 		if (ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON) {
> 			ret = regulator_enable(ci->platdata->reg_vbus);
> 			if (ret) {
> 				dev_err(ci->dev,
> 				"Failed to enable vbus regulator, ret=%d\n",
> 									ret);
> 				goto put_hcd;
> 			}
> 		} else {
> 			priv->reg_vbus = ci->platdata->reg_vbus;
> 		}
> 	}
> 
> 	...
> }
> 
> Note, I'm using a imx6q which has the CI_HDRC_TURN_VBUS_EARLY_ON set.
> 

Do you have a VBUS regulator at your dts, and add it at controller's
node? See: arch/arm/boot/dts/imx6qdl-sabresd.dtsi as an example please?

If you have set CI_HDRC_TURN_VBUS_EARLY_ON, the VBUS is controlled by
chipidea driver, not by USB core through PORTSC.PP (ehci_ci_portpower).

-- 

Thanks,
Peter Chen