Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a384d15d-c1df-160c-030b-fddd5d965996@linux.intel.com>
Date:   Wed, 23 Nov 2022 13:10:08 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.2.2
To:     Chunfeng Yun <chunfeng.yun@mediatek.com>,
        Mathias Nyman <mathias.nyman@intel.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
        Eddie Hung <eddie.hung@mediatek.com>
References: <20221118110116.20165-1-chunfeng.yun@mediatek.com>
Content-Language: en-US
From:   Mathias Nyman <mathias.nyman@linux.intel.com>
Subject: Re: [PATCH] usb: host: xhci-mtk: omit shared hcd if either root hub
 has no ports
In-Reply-To: <20221118110116.20165-1-chunfeng.yun@mediatek.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 18.11.2022 13.01, Chunfeng Yun wrote:
> There is error log when add a usb3 root hub without ports:
> "hub 4-0:1.0: config failed, hub doesn't have any ports! (err -19)"
> 
> so omit the shared hcd if either of the root hubs has no ports, but
> usually there is no usb3 port.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>   drivers/usb/host/xhci-mtk.c | 72 +++++++++++++++++++++++--------------
>   1 file changed, 46 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 01705e559c42..cff3c4aea036 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -485,6 +485,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>   	const struct hc_driver *driver;
>   	struct xhci_hcd *xhci;
>   	struct resource *res;
> +	struct usb_hcd *usb3_hcd;
>   	struct usb_hcd *hcd;
>   	int ret = -ENODEV;
>   	int wakeup_irq;
> @@ -593,6 +594,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>   
>   	xhci = hcd_to_xhci(hcd);
>   	xhci->main_hcd = hcd;
> +	xhci->allow_single_roothub = 1;
>   
>   	/*
>   	 * imod_interval is the interrupt moderation value in nanoseconds.
> @@ -602,24 +604,29 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>   	xhci->imod_interval = 5000;
>   	device_property_read_u32(dev, "imod-interval-ns", &xhci->imod_interval);
>   
> -	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
> -			dev_name(dev), hcd);
> -	if (!xhci->shared_hcd) {
> -		ret = -ENOMEM;
> -		goto disable_device_wakeup;
> -	}
> -
>   	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>   	if (ret)
> -		goto put_usb3_hcd;
> +		goto disable_device_wakeup;
>   
> -	if (HCC_MAX_PSA(xhci->hcc_params) >= 4 &&
> +	if (!xhci_has_one_roothub(xhci)) {
> +		xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
> +							 dev_name(dev), hcd);
> +		if (!xhci->shared_hcd) {
> +			ret = -ENOMEM;
> +			goto dealloc_usb2_hcd;
> +		}
> +	}
> +
> +	usb3_hcd = xhci_get_usb3_hcd(xhci);
> +	if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4 &&
>   	    !(xhci->quirks & XHCI_BROKEN_STREAMS))
> -		xhci->shared_hcd->can_do_streams = 1;
> +		usb3_hcd->can_do_streams = 1;
>   
> -	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> -	if (ret)
> -		goto dealloc_usb2_hcd;
> +	if (xhci->shared_hcd) {
> +		ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
> +		if (ret)
> +			goto put_usb3_hcd;
> +	}
>   
>   	if (wakeup_irq > 0) {
>   		ret = dev_pm_set_dedicated_wake_irq_reverse(dev, wakeup_irq);
	
dev_pm_set_dedicated_wake_irq_reverse() can be called with just one hcd, if it fails
it will goto dealloc_usb3_hcd:

dealloc_usb3_hcd:
	usb_remove_hcd(xhci->shared_hcd);   // xhci->shared_hcd may be null
	xhci->shared_hcd = NULL; // causes usb_put_hcd() issues if shared_hcd exists

put_usb3_hcd:
         usb_put_hcd(xhci->shared_hcd); // shared_hcd may be set NULL above

-Mathias