Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754072AbcCYRob (ORCPT ); Fri, 25 Mar 2016 13:44:31 -0400 Received: from mail-lf0-f50.google.com ([209.85.215.50]:36165 "EHLO mail-lf0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753321AbcCYRo3 (ORCPT ); Fri, 25 Mar 2016 13:44:29 -0400 From: Felipe Balbi To: Gregory CLEMENT Cc: Peter Griffin , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, mathias.nyman@intel.com, gregkh@linuxfoundation.org, lee.jones@linaro.org, linux-usb@vger.kernel.org, maxime.coquelin@st.com, patrice.chotard@st.com, stable@vger.kernel.org, yoshihiro.shimoda.uh@renesas.com, felipe.balbi@linux.intel.com Subject: Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value In-Reply-To: <87mvpmlc5e.fsf@free-electrons.com> References: <1458917188-28452-1-git-send-email-peter.griffin@linaro.org> <87mvpm8pfl.fsf@ti.com> <87mvpmlc5e.fsf@free-electrons.com> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/25.0.91.3 (x86_64-pc-linux-gnu) Date: Fri, 25 Mar 2016 19:44:22 +0200 Message-ID: <87io0a8nbd.fsf@ti.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6982 Lines: 215 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Gregory CLEMENT writes: >> Peter Griffin writes: >>> Otherwise generic-xhci and xhci-platform which have no data get wrongly >>> detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is(). >>> >>> This fixes a regression in v4.5 for STiH407 family SoC's which use the >>> synopsis dwc3 IP, whereby the disable_clk error path gets taken due to >>> wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd nev= er >>> gets added. >>> >>> I suspect this will also fix other dwc3 DT platforms such as Exynos, >>> although I've only tested on STih410 SoC. >>> >>> Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv") >>> Cc: stable@vger.kernel.org >>> Cc: gregory.clement@free-electrons.com >>> Cc: yoshihiro.shimoda.uh@renesas.com >>> Signed-off-by: Peter Griffin >>> --- >>> drivers/usb/host/xhci-plat.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h >>> index 5a2e2e3..529c3c4 100644 >>> --- a/drivers/usb/host/xhci-plat.h >>> +++ b/drivers/usb/host/xhci-plat.h >>> @@ -14,7 +14,7 @@ >>> #include "xhci.h" /* for hcd_to_xhci() */ >>>=20=20 >>> enum xhci_plat_type { >>> - XHCI_PLAT_TYPE_MARVELL_ARMADA, >>> + XHCI_PLAT_TYPE_MARVELL_ARMADA =3D 1, >>> XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2, >>> XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3, >> >> aren't these platforms using device tree ? Why aren't these just >> different compatible strings ? > > According to 4efb2f69411456d35051e9047c15157c9a5ba217 "usb: host: > xhci-plat: add struct xhci_plat_priv" : > > This patch adds struct xhci_plat_priv to simplify the code to match > platform specific variables. For now, this patch adds a member "type" in > the structure that's fine but the answer doesn't exactly match my question ;-) My point is that this enum shouldn't be necessary at all. We have compatible flags to make these checks instead. How about below ? (untested, uncompiled, yada yada yada). Note that we DON'T need this xhci_plat_type trickery, just need to be a little bit smarter about how we use driver_data: diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 1eefc988192d..1ea6c18b74f3 100644 =2D-- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -41,8 +41,9 @@ static void xhci_mvebu_mbus_config(void __iomem *base, } } =20 =2Dint xhci_mvebu_mbus_init_quirk(struct platform_device *pdev) +int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd) { + struct platform_device *pdev =3D to_platform_device(hcd->self.controller); struct resource *res; void __iomem *base; const struct mbus_dram_target_info *dram; diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 5c15e9bc5f7a..adb77c60a9ae 100644 =2D-- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -47,43 +47,56 @@ static void xhci_plat_quirks(struct device *dev, struct= xhci_hcd *xhci) xhci->quirks |=3D XHCI_PLAT; } =20 +static void xhci_priv_plat_start(struct usb_hcd *hcd) +{ + struct xhci_plat_priv *priv =3D hcd_to_xhci_priv(hcd); + + if (priv->plat_start) + priv->plat_start(hcd); +} + +static int xhci_priv_init_quirk(struct usb_hcd *hcd) +{ + struct xhci_plat_priv *priv =3D hcd_to_xhci_priv(hcd); + + if (!priv->init_quirk) + return 0; + + return priv->init_quirk(hcd); +} + /* called during probe() after chip reset completes */ static int xhci_plat_setup(struct usb_hcd *hcd) { int ret; =20 =2D if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) || =2D xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) { =2D ret =3D xhci_rcar_init_quirk(hcd); =2D if (ret) =2D return ret; =2D } + ret =3D xhci_priv_init_quirk(hcd); + if (ret) + return ret; =20 return xhci_gen_setup(hcd, xhci_plat_quirks); } =20 static int xhci_plat_start(struct usb_hcd *hcd) { =2D if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) || =2D xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) =2D xhci_rcar_start(hcd); =2D + xhci_priv_plat_start(hcd); return xhci_run(hcd); } =20 #ifdef CONFIG_OF static const struct xhci_plat_priv xhci_plat_marvell_armada =3D { =2D .type =3D XHCI_PLAT_TYPE_MARVELL_ARMADA, + .init_quirk =3D xhci_mvebu_mbus_init_quirk, }; =20 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 =3D { =2D .type =3D XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2, .firmware_name =3D XHCI_RCAR_FIRMWARE_NAME_V1, + .init_quirk =3D xhci_rcar_init_quirk, }; =20 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 =3D { =2D .type =3D XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3, .firmware_name =3D XHCI_RCAR_FIRMWARE_NAME_V2, + .init_quirk =3D xhci_rcar_init_quirk, + .plat_start =3D xhci_rcar_start, }; =20 static const struct of_device_id usb_xhci_of_match[] =3D { diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h index 5a2e2e3936c4..c4d565980832 100644 =2D-- a/drivers/usb/host/xhci-plat.h +++ b/drivers/usb/host/xhci-plat.h @@ -13,27 +13,13 @@ =20 #include "xhci.h" /* for hcd_to_xhci() */ =20 =2Denum xhci_plat_type { =2D XHCI_PLAT_TYPE_MARVELL_ARMADA, =2D XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2, =2D XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3, =2D}; =2D struct xhci_plat_priv { enum xhci_plat_type type; const char *firmware_name; + void (*plat_start)(struct usb_hcd *); + int (*init_quirk)(struct usb_hcd *); }; =20 #define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv) =20 =2Dstatic inline bool xhci_plat_type_is(struct usb_hcd *hcd, =2D enum xhci_plat_type type) =2D{ =2D struct xhci_plat_priv *priv =3D hcd_to_xhci_priv(hcd); =2D =2D if (priv && priv->type =3D=3D type) =2D return true; =2D else =2D return false; =2D} #endif /* _XHCI_PLAT_H */ ps: there might be bugs there, but it's a holiday and I really shouldn't be spending time on this right now ;-) Anyway, have fun testing. Let me know if it doesn't work. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJW9Xj2AAoJEIaOsuA1yqREl/gP/A7KnVpI6pzE4jUImEa39/xy feFSawQcu9RazDQI+SUzIm8rFZyPzZEU/z28YUpw3kNDCZePva4LCziREE5X5+oe /sybYNAl4NxVbJDpwommI+56mGOhMRxpOR9pQZjOfeq/e//woVJf/Ehp74853lZp v9w3nKfHhThSrv85mamjKAGiIfvpwsyp+zGAhGhd8fjotIvS/6Ygr238vrjOqVzF wUCqS0R3c62DOyinDRqpE5IaZeNZhUizxOF1nvQh7ENA+P/SNZb9jJlLDRcuZh1Y bWE5cBH1Px4C5kuzAkHYeqInS3tZdSPXfTre2cwygy2VxoMlRbPqHYzCWtx47Rgw HeiyUY/dqL4rRtsfZBsK0kALmRJNOHxWIJsHAu8m1icFOYZuThvc0C7Rfa9ZIG2v 4lsfgNouvvP2D/Gb5qGl5+aP7WHoPlsYHRWUqbH6rAM6T57aqIx4WDu0rC3IAgaz yw+7HRMzUJ1MLbVLGNpQa3UolHecPyyzCaYUmkJSTsTY2f0/NTJyZ3j4fZfuUV62 Sl8z03h/DKDz2Fd3O+Eue2s8bzltn5IuB7LiQdVT6i+boV46bskGU3Txe+tQwO6o WZ8e2mCalkmIim7xHSqmtVGjJ2E9DX+zGDRFSPRxlRDUnhtC7p/rDnWn0F2+4KGq DTDZWm5rQnBh5OQ6B01m =5YgX -----END PGP SIGNATURE----- --=-=-=--