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]
Message-ID: <02bfd468-5f92-4d06-bc90-f138c5153ee0@oss.qualcomm.com>
Date: Tue, 10 Jun 2025 22:10:33 +0530
From: Krishna Kurapati <krishna.kurapati@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Bjorn Andersson <bjorn.andersson@....qualcomm.com>,
        linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] usb: dwc3: qcom: Facilitate autosuspend during
 host mode



On 6/10/2025 4:30 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 10, 2025 at 02:43:56PM +0530, Krishna Kurapati wrote:
>> When in host mode, it is intended that the controller goes to suspend
>> state to save power and wait for interrupts from connected peripheral
>> to wake it up. This is particularly used in cases where a HID or Audio
>> device is connected. In such scenarios, the usb controller can enter
>> auto suspend and resume action after getting interrupts from the
>> connected device.
>>
>> Allow autosuspend for and xhci device and allow userspace to decide
>> whether to enable this functionality.
>>
>> a) Register to usb-core notifications in set_role vendor callback to
>> identify when root hubs are being created. Configure them to
>> use_autosuspend.
>>
>> b) Identify usb core notifications where the HCD is being added and
>> enable autosuspend for that particular xhci device.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@....qualcomm.com>
>> ---
>>   drivers/usb/dwc3/dwc3-qcom.c | 62 ++++++++++++++++++++++++++++++++----
>>   1 file changed, 56 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index d40b52e2ba01..17bbd5a06c08 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -95,6 +95,8 @@ struct dwc3_qcom {
>>   	 * internally by mutex lock.
>>   	 */
>>   	enum usb_role		current_role;
>> +
>> +	struct notifier_block	xhci_nb;
>>   };
>>   
>>   #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
>> @@ -647,6 +649,39 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>>   	return 0;
>>   }
>>   
>> +static int dwc3_xhci_event_notifier(struct notifier_block *nb,
>> +				    unsigned long event, void *ptr)
>> +{
>> +	struct dwc3_qcom  *qcom	= container_of(nb, struct dwc3_qcom, xhci_nb);
>> +	struct dwc3	  *dwc	= &qcom->dwc;
>> +	struct usb_bus	  *ubus	= ptr;
>> +	struct usb_hcd	  *hcd;
>> +
>> +	if (!dwc->xhci)
>> +		goto done;
>> +
>> +	hcd = platform_get_drvdata(dwc->xhci);
>> +	if (!hcd)
>> +		goto done;
>> +
>> +	if (event != USB_BUS_ADD)
>> +		goto done;
>> +
>> +	if (strcmp(dev_name(ubus->sysdev), dev_name(dwc->sysdev)) != 0)
>> +		goto done;
>> +
>> +	if (event == USB_BUS_ADD) {
>> +		/*
>> +		 * Identify instant of creation of primary hcd and
>> +		 * mark xhci as autosuspend capable at this point.
>> +		 */
>> +		pm_runtime_use_autosuspend(&dwc->xhci->dev);
> 
> This feels like an overkill, using notifiers to set autosuspend flag.
> Please extend platform data and/or other static code in order to set the
> flag on the created xHCI devices.
> 

Do you mean modifying pdev of xhci from dwc3/host.c ? Or adding the 
use_autosuspend call in xhci-plat.c ?

I thought adding this notifier would be a better way to identify when 
the xhci probe is in progress instead of touching pdev of xhci device 
from dwc3 layer.

Regards,
Krishna,

>> +	}
>> +
>> +done:
>> +	return NOTIFY_DONE;
>> +}
>> +
>>   static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
>>   {
>>   	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>> @@ -659,12 +694,22 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
>>   		return;
>>   	}
>>   
>> -	if (qcom->current_role == USB_ROLE_DEVICE &&
>> -	    next_role != USB_ROLE_DEVICE)
>> +	if (qcom->current_role == USB_ROLE_NONE) {
>> +		if (next_role == USB_ROLE_DEVICE) {
>> +			dwc3_qcom_vbus_override_enable(qcom, true);
>> +		} else if (next_role == USB_ROLE_HOST) {
>> +			qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
>> +			usb_register_notify(&qcom->xhci_nb);
>> +		}
>> +	} else if (qcom->current_role == USB_ROLE_DEVICE &&
>> +		   next_role != USB_ROLE_DEVICE) {
>>   		dwc3_qcom_vbus_override_enable(qcom, false);
>> -	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
>> -		 (next_role == USB_ROLE_DEVICE))
>> -		dwc3_qcom_vbus_override_enable(qcom, true);
>> +	} else if (qcom->current_role == USB_ROLE_HOST) {
>> +		if (next_role == USB_ROLE_NONE)
>> +			usb_unregister_notify(&qcom->xhci_nb);
>> +		else if (next_role == USB_ROLE_DEVICE)
>> +			dwc3_qcom_vbus_override_enable(qcom, true);
>> +	}
>>   
>>   	pm_runtime_mark_last_busy(qcom->dev);
>>   	pm_runtime_put_sync(qcom->dev);
>> @@ -774,6 +819,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   
>>   	if (qcom->mode == USB_DR_MODE_HOST) {
>>   		qcom->current_role = USB_ROLE_HOST;
>> +		qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
>> +		usb_register_notify(&qcom->xhci_nb);
>>   	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
>>   		qcom->current_role = USB_ROLE_DEVICE;
>>   		dwc3_qcom_vbus_override_enable(qcom, true);
>> @@ -794,7 +841,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   	ret = dwc3_core_probe(&probe_data);
>>   	if (ret)  {
>>   		ret = dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
>> -		goto clk_disable;
>> +		goto unregister_notify;
>>   	}
>>   
>>   	ret = dwc3_qcom_interconnect_init(qcom);
>> @@ -817,6 +864,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   	dwc3_qcom_interconnect_exit(qcom);
>>   remove_core:
>>   	dwc3_core_remove(&qcom->dwc);
>> +unregister_notify:
>> +	if (qcom->mode == USB_DR_MODE_HOST)
>> +		usb_unregister_notify(&qcom->xhci_nb);
>>   clk_disable:
>>   	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>>   
>> -- 
>> 2.34.1
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ