On Thu, 2018-09-27 at 19:26 +0300, Mathias Nyman wrote:
> Ensure that the shared_hcd pointer is valid when calling usb_put_hcd()
>
> The shared_hcd is removed and freed in xhci by first calling
> usb_remove_hcd(xhci->shared_hcd), and later
> usb_put_hcd(xhci->shared_hcd)
>
> Afer commit fe190ed0d602 ("xhci: Do not halt the host until both HCD have
> disconnected their devices.") the shared_hcd was never properly put as
> xhci->shared_hcd was set to NULL before usb_put_hcd(xhci->shared_hcd) was
> called.
>
> shared_hcd (USB3) is removed before primary hcd (USB2).
> While removing the primary hcd we might need to handle xhci interrupts
> to cleanly remove last USB2 devices, therefore we need to set
> xhci->shared_hcd to NULL before removing the primary hcd to let xhci
> interrupt handler know shared_hcd is no longer available.
>
> xhci-plat.c, xhci-histb.c and xhci-mtk first create both their hcd's before
> adding them. so to keep the correct reverse removal order use a temporary
> shared_hcd variable for them.
> For more details see commit 4ac53087d6d4 ("usb: xhci: plat: Create both
> HCDs before adding them")
>
> Fixes: fe190ed0d602 ("xhci: Do not halt the host until both HCD have
> disconnected their devices.")
> Cc: Joel Stanley <j...@jms.id.au>
> Cc: Chunfeng Yun <chunfeng....@mediatek.com>
> Cc: Thierry Reding <tred...@nvidia.com>
> Cc: Jianguo Sun <sunjiang...@huawei.com>
> Cc: <sta...@vger.kernel.org>
> Reported-by: Jack Pham <ja...@codeaurora.org>
> Signed-off-by: Mathias Nyman <mathias.ny...@linux.intel.com>
> ---
> drivers/usb/host/xhci-histb.c | 6 ++++--
> drivers/usb/host/xhci-mtk.c | 6 ++++--
> drivers/usb/host/xhci-pci.c | 1 +
> drivers/usb/host/xhci-plat.c | 6 ++++--
> drivers/usb/host/xhci-tegra.c | 1 +
> drivers/usb/host/xhci.c | 2 --
> 6 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
> index 27f0016..3c4abb5 100644
> --- a/drivers/usb/host/xhci-histb.c
> +++ b/drivers/usb/host/xhci-histb.c
> @@ -325,14 +325,16 @@ static int xhci_histb_remove(struct platform_device
> *dev)
> struct xhci_hcd_histb *histb = platform_get_drvdata(dev);
> struct usb_hcd *hcd = histb->hcd;
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> device_wakeup_disable(&dev->dev);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
>
> xhci_histb_host_disable(histb);
> usb_put_hcd(hcd);
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 71d0d33..60987c7 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -590,12 +590,14 @@ static int xhci_mtk_remove(struct platform_device *dev)
> struct xhci_hcd_mtk *mtk = platform_get_drvdata(dev);
> struct usb_hcd *hcd = mtk->hcd;
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> device_init_wakeup(&dev->dev, false);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
> usb_put_hcd(hcd);
> xhci_mtk_sch_exit(mtk);
> xhci_mtk_clks_disable(mtk);
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 51dd8e0..92fd6b6 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -356,6 +356,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
> if (xhci->shared_hcd) {
> usb_remove_hcd(xhci->shared_hcd);
> usb_put_hcd(xhci->shared_hcd);
> + xhci->shared_hcd = NULL;
> }
>
> /* Workaround for spurious wakeups at shutdown with HSW */
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 94e9392..e5da8ce 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -359,14 +359,16 @@ static int xhci_plat_remove(struct platform_device *dev)
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> struct clk *clk = xhci->clk;
> struct clk *reg_clk = xhci->reg_clk;
> + struct usb_hcd *shared_hcd = xhci->shared_hcd;
>
> xhci->xhc_state |= XHCI_STATE_REMOVING;
>
> - usb_remove_hcd(xhci->shared_hcd);
> + usb_remove_hcd(shared_hcd);
> + xhci->shared_hcd = NULL;
> usb_phy_shutdown(hcd->usb_phy);
>
> usb_remove_hcd(hcd);
> - usb_put_hcd(xhci->shared_hcd);
> + usb_put_hcd(shared_hcd);
>
> clk_disable_unprepare(clk);
> clk_disable_unprepare(reg_clk);
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 4b463e5..b1cce98 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1240,6 +1240,7 @@ static int tegra_xusb_remove(struct platform_device
> *pdev)
>
> usb_remove_hcd(xhci->shared_hcd);
> usb_put_hcd(xhci->shared_hcd);
> + xhci->shared_hcd = NULL;
> usb_remove_hcd(tegra->hcd);
> usb_put_hcd(tegra->hcd);
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 0420eef..c928dbb 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -719,8 +719,6 @@ static void xhci_stop(struct usb_hcd *hcd)
>
> /* Only halt host and free memory after both hcds are removed */
> if (!usb_hcd_is_primary_hcd(hcd)) {
> - /* usb core will free this hcd shortly, unset pointer */
> - xhci->shared_hcd = NULL;
> mutex_unlock(&xhci->mutex);
> return;
> }
Tested-by: Chunfeng Yun <chunfeng....@mediatek.com>
Thanks
