linux-kernel - Re: [PATCH] usb: host: xhci-mtk: omit shared hcd if either root hub has no ports

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <02552cc60b3aae16b2c2e9717b42c800180e3a87.camel@mediatek.com>
Date:   Thu, 24 Nov 2022 10:38:00 +0000
From:   Chunfeng Yun (云春峰) 
        <Chunfeng.Yun@...iatek.com>
To:     "mathias.nyman@...ux.intel.com" <mathias.nyman@...ux.intel.com>,
        "mathias.nyman@...el.com" <mathias.nyman@...el.com>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Eddie Hung (洪正鑫) 
        <Eddie.Hung@...iatek.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: host: xhci-mtk: omit shared hcd if either root hub
 has no ports

On Wed, 2022-11-23 at 13:10 +0200, Mathias Nyman wrote:
> 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@...iatek.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
usb_remove_hcd() has handled NULL argument, no need check it here

> 	xhci->shared_hcd = NULL; // causes usb_put_hcd() issues if
> shared_hcd exists
This line shall be removed, I'll prepare a patch.

> 
> put_usb3_hcd:
>          usb_put_hcd(xhci->shared_hcd); // shared_hcd may be set NULL
> above
usb_put_hcd() has handled NULL argument, no need check it here

Thanks a lot

> 
> -Mathias
>