linux-kernel - Re: [RFC PATCH v2 1/3] usb: support Samsung Exynos xHCI Controller

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 29 Dec 2022 11:25:58 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Daehwan Jung <dh10.jung@...sung.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Felipe Balbi <balbi@...nel.org>
Cc:     "open list:USB SUBSYSTEM" <linux-usb@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>, sc.suh@...sung.com,
        taehyun.cho@...sung.com, jh0801.jung@...sung.com,
        eomji.oh@...sung.com
Subject: Re: [RFC PATCH v2 1/3] usb: support Samsung Exynos xHCI Controller

On 29/12/2022 10:57, Daehwan Jung wrote:
> Currently, dwc3 invokes just xhci platform driver without any data.
> We add xhci node as child of dwc3 node in order to get data from
> device tree. It populates "xhci" child by name during initialization
> of host. This patch only effects if dwc3 node has a child named "xhci"
> not to disturb original path.
> 
> We add "samsung,exynos-xhci" compatible in xhci platform driver

Where? It is not documented.

> to support Exynos SOCs. 

That's so not true. You do nothing to support Exynos SoC here. Please
stop pasting incorrect and misleading commit msgs.

> We introduce roothub wakeup, which uses roothub
> as system wakeup source. It needs xhci platform driver to override
> roothub ops.

You did not explain why you introduced wakelocks...


(...)

>  	if (shared_hcd) {
>  		usb_remove_hcd(shared_hcd);
>  		xhci->shared_hcd = NULL;
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 79d7931c048a..693495054001 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -5502,6 +5502,10 @@ void xhci_init_driver(struct hc_driver *drv,
>  			drv->check_bandwidth = over->check_bandwidth;
>  		if (over->reset_bandwidth)
>  			drv->reset_bandwidth = over->reset_bandwidth;
> +		if (over->bus_suspend)
> +			drv->bus_suspend = over->bus_suspend;
> +		if (over->bus_resume)
> +			drv->bus_resume = over->bus_resume;
>  	}
>  }
>  EXPORT_SYMBOL_GPL(xhci_init_driver);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index c9f06c5e4e9d..cb9c54a6a22c 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1752,6 +1752,8 @@ struct xhci_hub {
>  struct xhci_hcd {
>  	struct usb_hcd *main_hcd;
>  	struct usb_hcd *shared_hcd;
> +	struct wakeup_source *main_wakelock;
> +	struct wakeup_source *shared_wakelock;

Drop wakelocks. This is not related to USB and not needed here. Do you
see anywhere else in core kernel code usage of the wakelocks?

You got this comment already, didn't you? So why you do not address it?

Best regards,
Krzysztof