[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2c9ce2ed-c21d-496d-9563-237547142a29@oss.qualcomm.com>
Date: Tue, 24 Jun 2025 18:33:14 +0530
From: Krishna Kurapati <krishna.kurapati@....qualcomm.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Bjorn Andersson <bjorn.andersson@....qualcomm.com>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/4] usb: dwc3: core: Introduce glue callbacks for
flattened implementations
On 6/24/2025 4:54 AM, Thinh Nguyen wrote:
> On Tue, Jun 10, 2025, Krishna Kurapati wrote:
>> In certain situations like role switching, the glue layers need to be
>> informed of these events, so that they can take any necessary action.
>> But in non-flattened implementations, the glue drivers have no data on
>> when the core driver probe was successful post invoking of_platform_
>> populate. Now that the core driver supports flattened implementations
>> as well, introduce vendor callbacks that can be passed on from glue to
>> core before invoking dwc3_core_probe.
>>
>> Introduce callbacks to notify glue layer of role_switch and run_stop
>> changes. These can be used by flattened implementation of Qualcomm
>> glue layer to generate connect/disconnect events in controller during
>> cable connect and run stop modifications by udc in device mode.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@....qualcomm.com>
>> ---
>> drivers/usb/dwc3/core.c | 1 +
>> drivers/usb/dwc3/core.h | 26 ++++++++++++++++++++++++++
>> drivers/usb/dwc3/drd.c | 1 +
>> drivers/usb/dwc3/gadget.c | 1 +
>> 4 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 2bc775a747f2..c01b15e3710f 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -2351,6 +2351,7 @@ static int dwc3_probe(struct platform_device *pdev)
>> return -ENOMEM;
>>
>> dwc->dev = &pdev->dev;
>> + dwc->glue_ops = NULL;
>>
>> probe_data.dwc = dwc;
>> probe_data.res = res;
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index d5b985fa12f4..a803884be4ed 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -992,6 +992,17 @@ struct dwc3_scratchpad_array {
>> __le64 dma_adr[DWC3_MAX_HIBER_SCRATCHBUFS];
>> };
>>
>> +/*
>
> Let's keep consistent with the doc style /**
>
ACK.
>> + * struct dwc3_glue_ops - The ops indicate the notifications that
>> + * need to be passed on to glue layer
>> + * @notify_set_role: Notify glue of role switch notifications
>> + * @notify_run_stop: Notify run stop enable/disable information to glue
>> + */
>> +struct dwc3_glue_ops {
>> + void (*notify_set_role)(struct dwc3 *dwc, enum usb_role role);
>> + void (*notify_run_stop)(struct dwc3 *dwc, bool is_on);
>
> Use pre_ or prep_ prefix instead of notify_ indicating callbacks for
>glue driver to perform updates before set_role or run_stop.
ACK. Will change it accordingly.
>
>> +};
>> +
>> /**
>> * struct dwc3 - representation of our controller
>> * @drd_work: workqueue used for role swapping
>> @@ -1168,6 +1179,7 @@ struct dwc3_scratchpad_array {
>> * @wakeup_pending_funcs: Indicates whether any interface has requested for
>> * function wakeup in bitmap format where bit position
>> * represents interface_id.
>> + * @glue_ops: Vendor callbacks for flattened device implementations.
>> */
>> struct dwc3 {
>> struct work_struct drd_work;
>> @@ -1400,6 +1412,8 @@ struct dwc3 {
>> struct dentry *debug_root;
>> u32 gsbuscfg0_reqinfo;
>> u32 wakeup_pending_funcs;
>> +
>> + struct dwc3_glue_ops *glue_ops;
>
> Use const, and move this closer on top. Perhaps below gadget_driver.
>
ACK.
>> };
>>
>> #define INCRX_BURST_MODE 0
>> @@ -1614,6 +1628,18 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc);
>> int dwc3_core_soft_reset(struct dwc3 *dwc);
>> void dwc3_enable_susphy(struct dwc3 *dwc, bool enable);
>>
>> +static inline void dwc3_notify_set_role(struct dwc3 *dwc, enum usb_role role)
>> +{
>> + if (dwc->glue_ops && dwc->glue_ops->notify_set_role)
>> + dwc->glue_ops->notify_set_role(dwc, role);
>> +}
>> +
>> +static inline void dwc3_notify_run_stop(struct dwc3 *dwc, bool is_on)
>> +{
>> + if (dwc->glue_ops && dwc->glue_ops->notify_run_stop)
>> + dwc->glue_ops->notify_run_stop(dwc, is_on);
>> +}
>> +
>> #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
>> int dwc3_host_init(struct dwc3 *dwc);
>> void dwc3_host_exit(struct dwc3 *dwc);
>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>> index 7977860932b1..408551768a95 100644
>> --- a/drivers/usb/dwc3/drd.c
>> +++ b/drivers/usb/dwc3/drd.c
>> @@ -464,6 +464,7 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>> break;
>> }
>>
>> + dwc3_notify_set_role(dwc, role);
>
> This should be done in __dwc3_set_mode(). Perhaps right before setting
> PRTCAPDIR?
>
Qualcomm glue driver needs ROLE (device /host and none) information. Set
mode gives device and host mode information. So added set_role callback
to get cable disconnect information.
>> dwc3_set_mode(dwc, mode);
>> return 0;
>> }
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 321361288935..73bed11bccaf 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2641,6 +2641,7 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>> if (saved_config)
>> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>
>> + dwc3_notify_run_stop(dwc, is_on);
>
> This should be done right before writing to DCTL.run_stop.
>
ACK.
>> reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>> if (is_on) {
>> if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
>> --
>> 2.34.1
>>
>
Thanks for the review Thinh.
Regards,
Krishna,
Powered by blists - more mailing lists