Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Peter Chen <peter.chen@nxp.com>
To:     =?iso-8859-1?Q?Pali_Roh=E1r?= <pali@kernel.org>
CC:     Mathias Nyman <mathias.nyman@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jun Li <jun.li@nxp.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb: host: xhci-plat: fix support for XHCI_SKIP_PHY_INIT
 quirk
Thread-Topic: [PATCH] usb: host: xhci-plat: fix support for XHCI_SKIP_PHY_INIT
 quirk
Thread-Index: AQHW16tInodopkLEv0Oy1XwtiA3jeaoCYPUAgAB36YCAAEUggIAAwSsA
Date:   Wed, 23 Dec 2020 01:02:43 +0000
Message-ID: <20201223010214.GA18398@b29397-desktop>
References: <20201221150903.26630-1-pali@kernel.org>
 <20201222021416.GB4706@b29397-desktop> <20201222092327.jq5b7g4bffeccq3o@pali>
 <20201222133051.jfvrgkcdhjnldz3z@pali>
In-Reply-To: <20201222133051.jfvrgkcdhjnldz3z@pali>
Accept-Language: en-US
Content-Language: en-US
user-agent: Mutt/1.9.4 (2018-02-28)
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <AE1B4FFCE038BE4CB4ACBFBFAA11914B@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e38425c-6355-452d-921a-08d8a6de7449
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Dec 2020 01:02:43.6067
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Q86W/lIHmJd0MiXkNLVJwyj1P6vfyPpGaQjh43Dew70EvQqsnsN1LhUuGkh1va2JufNXharNrF6i1ROfwbJ42w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR04MB5657
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 20-12-22 14:30:51, Pali Roh=E1r wrote:
> On Tuesday 22 December 2020 10:23:27 Pali Roh=E1r wrote:
> > On Tuesday 22 December 2020 02:14:45 Peter Chen wrote:
> > > On 20-12-21 16:09:03, Pali Roh=E1r wrote:
> > > > Currently init_quirk callbacks for xhci platform drivers are called
> > > > xhci_plat_setup() function which is called after chip reset complet=
es.
> > > > It happens in the middle of the usb_add_hcd() function.
> > > >=20
> > > > But XHCI_SKIP_PHY_INIT quirk is checked in the xhci_plat_probe() fu=
nction
> > > > prior calling usb_add_hcd() function. Therefore this XHCI_SKIP_PHY_=
INIT
> > > > currently does nothing as prior xhci_plat_setup() it is not set.
> > > >=20
> > > > Quirk XHCI_SKIP_PHY_INIT is only setting hcd->skip_phy_initializati=
on value
> > > > which really needs to be set prior calling usb_add_hcd() as this fu=
nction
> > > > at its beginning skips PHY init if this member is set.
> > > >=20
> > > > This patch fixes implementation of the XHCI_SKIP_PHY_INIT quirk by =
calling
> > > > init_quirk callbacks (via xhci_priv_init_quirk()) prior checking if
> > > > XHCI_SKIP_PHY_INIT is set.
> > > >=20
> > > > Fixes: f768e718911e0 ("usb: host: xhci-plat: add priv quirk for ski=
p PHY initialization")
> > > > Signed-off-by: Pali Roh=E1r <pali@kernel.org>

Hi Pali,

I know your case, you need to choose XHCI_SKIP_PHY_INIT quirk
conditionally, but you may can't add Fixes tag at your patch
since your issue was existed before my patch.

Peter


> > > > ---
> > > >  drivers/usb/host/xhci-plat.c | 14 +++++++-------
> > > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > >=20
> > > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-p=
lat.c
> > > > index 4d34f6005381..58704c5b002b 100644
> > > > --- a/drivers/usb/host/xhci-plat.c
> > > > +++ b/drivers/usb/host/xhci-plat.c
> > > > @@ -89,13 +89,6 @@ static void xhci_plat_quirks(struct device *dev,=
 struct xhci_hcd *xhci)
> > > >  /* called during probe() after chip reset completes */
> > > >  static int xhci_plat_setup(struct usb_hcd *hcd)
> > > >  {
> > > > -	int ret;
> > > > -
> > > > -
> > > > -	ret =3D xhci_priv_init_quirk(hcd);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -
> > > >  	return xhci_gen_setup(hcd, xhci_plat_quirks);
> > > >  }
> > > > =20
> > > > @@ -330,6 +323,13 @@ static int xhci_plat_probe(struct platform_dev=
ice *pdev)
> > > > =20
> > > >  	hcd->tpl_support =3D of_usb_host_tpl_support(sysdev->of_node);
> > > >  	xhci->shared_hcd->tpl_support =3D hcd->tpl_support;
> > > > +
> > > > +	if (priv) {
> > > > +		ret =3D xhci_priv_init_quirk(hcd);
> > > > +		if (ret)
> > > > +			goto disable_usb_phy;
> > > > +	}
> > > > +
> > > >  	if (priv && (priv->quirks & XHCI_SKIP_PHY_INIT))
> > > >  		hcd->skip_phy_initialization =3D 1;
> > > > =20
> > >=20
> > > Hi Pali,
> > >=20
> > > What's problem you have met? In structure xhci_plat_priv, the quirks =
are
> > > defined at .quirks entry which is got at below code. .init_quirk is t=
he
> > > routine if special initializations are needed.
> >=20
> > Hello!
> >=20
> > I'm talking about .init_quirk. And if XHCI_SKIP_PHY_INIT quirk is set i=
n
> > this function then has no effect.
>=20
> Ok, this patch is not enough, I will send V2.
>=20
> > I'm working currently on patch for xhci mvebu which conditionally enabl=
e
> > or disable XHCI_SKIP_PHY_INIT quirk (it is going to fix existing
> > regression since v5.1 kernel) and without this patch XHCI_SKIP_PHY_INIT
> > quirk from the init_quirk does not work.
>=20
> And now I have tested V2 with my mvebu regression fix. I will send it to
> mailing list for review.
>=20
> > > 	if (pdev->dev.of_node)
> > > 		priv_match =3D of_device_get_match_data(&pdev->dev);
> > > 	else
> > > 		priv_match =3D dev_get_platdata(&pdev->dev);
> > >=20
> > > 	if (priv_match) {
> > > 		priv =3D hcd_to_xhci_priv(hcd);
> > > 		/* Just copy data for now */
> > > 		*priv =3D *priv_match;
> > > 	}
> > > --=20
> > >=20
> > > Thanks,
> > > Peter Chen

--=20

Thanks,
Peter Chen=