Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 6 Feb 2018 16:50:41 +0100
From:   Sebastian Reichel <sebastian.reichel@collabora.co.uk>
To:     Peter Chen <hzpeterchen@gmail.com>
Cc:     Peter Chen <Peter.Chen@nxp.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        Ian Ray <ian.ray@ge.com>, Nandor Han <nandor.han@ge.com>,
        Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>,
        kernel@lists.collabora.co.uk
Subject: Re: [PATCH 2/2] usb: chipidea: imx: Fix ULPI on imx53
Message-ID: <20180206155041.objiu5j5kftmzp4q@earth.universe>
References: <20180124171439.10721-1-sebastian.reichel@collabora.co.uk>
 <20180124171439.10721-2-sebastian.reichel@collabora.co.uk>
 <20180129033315.GA20311@b29397-desktop>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="f37pcu7qnjouqver"
Content-Disposition: inline
In-Reply-To: <20180129033315.GA20311@b29397-desktop>
User-Agent: NeoMutt/20171215
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--f37pcu7qnjouqver
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Peter,

On Mon, Jan 29, 2018 at 11:33:15AM +0800, Peter Chen wrote:
> On Wed, Jan 24, 2018 at 06:14:39PM +0100, Sebastian Reichel wrote:
> > Traditionally, PORTSC should be set before initializing ULPI phys. But
> > setting PORTSC before powering on the phy results in a kernel freeze
> > on imx53 based GE PPD. As a workaround this initializes the phy early
> > in the imx platform code and disables phy power management from the
> > core.
> >=20
> > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >=20
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/=
ci_hdrc_imx.c
> > index de155c80eb70..e431c5aafe35 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -83,6 +83,7 @@ struct ci_hdrc_imx_data {
> >  	struct clk *clk;
> >  	struct imx_usbmisc_data *usbmisc_data;
> >  	bool supports_runtime_pm;
> > +	bool override_phy_control;
> >  	bool in_lpm;
> >  	/* SoC before i.mx6 (except imx23/imx28) needs three clks */
> >  	bool need_three_clks;
> > @@ -254,6 +255,7 @@ static int ci_hdrc_imx_probe(struct platform_device=
 *pdev)
> >  	int ret;
> >  	const struct of_device_id *of_id;
> >  	const struct ci_hdrc_imx_platform_flag *imx_platform_flag;
> > +	struct device_node *np =3D pdev->dev.of_node;
> > =20
> >  	of_id =3D of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
> >  	if (!of_id)
> > @@ -288,6 +290,14 @@ static int ci_hdrc_imx_probe(struct platform_devic=
e *pdev)
> >  	}
> > =20
> >  	pdata.usb_phy =3D data->phy;
> > +
> > +	if (of_device_is_compatible(np, "fsl,imx53-usb") && pdata.usb_phy &&
> > +	    of_usb_get_phy_mode(np) =3D=3D USBPHY_INTERFACE_MODE_ULPI) {
> > +		pdata.flags |=3D CI_HDRC_OVERRIDE_PHY_CONTROL;
> > +		data->override_phy_control =3D true;
> > +		usb_phy_init(pdata.usb_phy);
> > +	}
> > +
> >  	pdata.flags |=3D imx_platform_flag->flags;
> >  	if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
> >  		data->supports_runtime_pm =3D true;
> > @@ -341,6 +351,8 @@ static int ci_hdrc_imx_remove(struct platform_devic=
e *pdev)
> >  		pm_runtime_put_noidle(&pdev->dev);
> >  	}
> >  	ci_hdrc_remove_device(data->ci_pdev);
> > +	if (data->override_phy_control)
> > +		usb_phy_shutdown(data->phy);
> >  	imx_disable_unprepare_clks(&pdev->dev);
> > =20
>=20
> Sebastian, I have a question, do you have any USB or generic PHY drivers
> for ULPI bus, any power controls are needed for your ULPI peripheral?

The devicetree for GE PPD is available in the mainline kernel:

$ grep -A9 "usbphy[23] {" arch/arm/boot/dts/imx53-ppd.dts
	usbphy2: usbphy2 {
		compatible =3D "usb-nop-xceiv";
		reset-gpios =3D <&gpio4 4 GPIO_ACTIVE_LOW>;
		clock-names =3D "main_clk";
		clock-frequency =3D <24000000>;
		clocks =3D <&clks IMX5_CLK_CKO2>;
		assigned-clocks =3D <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
		assigned-clock-parents =3D <&clks IMX5_CLK_OSC>;
	};

	usbphy3: usbphy3 {
		compatible =3D "usb-nop-xceiv";
		reset-gpios =3D <&gpio2 19 GPIO_ACTIVE_LOW>;
		clock-names =3D "main_clk";

		clock-frequency =3D <24000000>;
		clocks =3D <&clks IMX5_CLK_CKO2>;
		assigned-clocks =3D <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>;
		assigned-clock-parents =3D <&clks IMX5_CLK_OSC>;
	};

So currently the machine only uses drivers/usb/phy/phy-generic.c. Both
USB phys are actually SMSC USB3315, which is also detected by the kernel:

root@csmon :~# cat /sys/bus/ulpi/devices/ci_hdrc.*.ulpi/uevent=20
DEVTYPE=3Dulpi_device
MODALIAS=3Dulpi:v0424p0006
DEVTYPE=3Dulpi_device
MODALIAS=3Dulpi:v0424p0006

So maybe drivers/usb/phy/phy-ulpi.c should be used, but I don't see
a simple way to do so and using the generic PHY works.

-- Sebastian

--f37pcu7qnjouqver
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE72YNB0Y/i3JqeVQT2O7X88g7+poFAlp5zssACgkQ2O7X88g7
+poeZBAAlVAXdTP0z0WIjJnmEnZWUWtIeZb6riM2k5aDYjtPjEs5Uwm0qk6V7dS6
0RCXES8Fphb+t0CcAue4r6KMaJducfjhaLC4j5rpVFw2I9+yf1VkPn7UfFyCncr9
9sLzSbX/z0Sn6MJATUi904PA5oeDn+bOj8SzpYq0gOFhO7VVoUKF6tu9uYA+StiQ
BDWmyHN//3G6BexmJFhfY5NGu9UUH1Fn9+QEiHX2SNrmAvYQzRZEp//9nNFLLaHm
NTKzxiC0GN5MRwNj3Wib5zJKMCrA2ZzRhWv4rFqLnNAPZ9s9AviPn5uXUebnjWMp
22whjlGJJg8bzxO1lFAoNQiXpurQk3PwqaT+EWE7DQ3XXkof+saY/bL0ARlrE9Z/
WH3tTIMzl9eLjDCDNsAcAlA/+lg2ok9UsU38tmqHe4tBMhQCAQmZaA/vDGtNxpO9
hdlCBAwCZouJkXv6yHkx/bfpR1YmvPjOk4n8JwcAhU0L3EYPx7q/UTJyCX0ao1+K
5COirzoBQV7SS2YUz3Qq8XZVXedb4ozgfC1SznOnIIMa+8OiWslvm3P/rkbs/7Ju
K8nA07COCFm/YBWGYQ6w4njVC3WEdDEoZk8SawX3ERtTvPZiH/qSer590JHp01mB
qtmSfHAYnxpF6yTEAE9hqleR2Lm8fjOdVABbKV5WA1HX4fk8Jv4=
=l+Js
-----END PGP SIGNATURE-----

--f37pcu7qnjouqver--