Hi, Gregory CLEMENT <gregory.clement@xxxxxxxxxxxxxxxxxx> writes: >> Peter Griffin <peter.griffin@xxxxxxxxxx> 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 never >>> 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@xxxxxxxxxxxxxxx >>> Cc: gregory.clement@xxxxxxxxxxxxxxxxxx >>> Cc: yoshihiro.shimoda.uh@xxxxxxxxxxx >>> Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx> >>> --- >>> 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() */ >>> >>> enum xhci_plat_type { >>> - XHCI_PLAT_TYPE_MARVELL_ARMADA, >>> + XHCI_PLAT_TYPE_MARVELL_ARMADA = 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 --- 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, } } -int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev) +int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd) { + struct platform_device *pdev = 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 --- 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 |= XHCI_PLAT; } +static void xhci_priv_plat_start(struct usb_hcd *hcd) +{ + struct xhci_plat_priv *priv = 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 = 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; - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) || - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) { - ret = xhci_rcar_init_quirk(hcd); - if (ret) - return ret; - } + ret = xhci_priv_init_quirk(hcd); + if (ret) + return ret; return xhci_gen_setup(hcd, xhci_plat_quirks); } static int xhci_plat_start(struct usb_hcd *hcd) { - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) || - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) - xhci_rcar_start(hcd); - + xhci_priv_plat_start(hcd); return xhci_run(hcd); } #ifdef CONFIG_OF static const struct xhci_plat_priv xhci_plat_marvell_armada = { - .type = XHCI_PLAT_TYPE_MARVELL_ARMADA, + .init_quirk = xhci_mvebu_mbus_init_quirk, }; static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = { - .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2, .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1, + .init_quirk = xhci_rcar_init_quirk, }; static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = { - .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3, .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2, + .init_quirk = xhci_rcar_init_quirk, + .plat_start = xhci_rcar_start, }; static const struct of_device_id usb_xhci_of_match[] = { diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h index 5a2e2e3936c4..c4d565980832 100644 --- a/drivers/usb/host/xhci-plat.h +++ b/drivers/usb/host/xhci-plat.h @@ -13,27 +13,13 @@ #include "xhci.h" /* for hcd_to_xhci() */ -enum xhci_plat_type { - XHCI_PLAT_TYPE_MARVELL_ARMADA, - XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2, - XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3, -}; - 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 *); }; #define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv) -static inline bool xhci_plat_type_is(struct usb_hcd *hcd, - enum xhci_plat_type type) -{ - struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd); - - if (priv && priv->type == type) - return true; - else - return false; -} #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. -- balbi
Attachment:
signature.asc
Description: PGP signature