On 2/3/2023 7:58 AM, Thinh Nguyen wrote:
On Thu, Feb 02, 2023, Thinh Nguyen wrote:
On Thu, Feb 02, 2023, Linyu Yuan wrote:
hi Thinh,
do you prefer below change ? will it be good for all cases ?
+static void dwc3_gadget_update_link_state(struct dwc3 *dwc,
+ const struct dwc3_event_devt *event)
+{
+ switch (event->type) {
+ case DWC3_DEVICE_EVENT_HIBER_REQ:
+ case DWC3_DEVICE_EVENT_LINK_STATUS_CHANGE:
+ case DWC3_DEVICE_EVENT_SUSPEND:
+ break;
+ default:
+ dwc->link_state = event->event_info & DWC3_LINK_STATE_MASK;
+ break;
+ }
+}
+
static void dwc3_gadget_interrupt(struct dwc3 *dwc,
const struct dwc3_event_devt *event)
{
+ dwc3_gadget_update_link_state(dwc3, event);
+
switch (event->type)
This would break the check in dwc3_gadget_suspend_interrupt(). However,
I'm actually not sure why we had that check in the beginning. I suppose
certain setup may trigger suspend event multiple time consecutively?
Actually, looking at it again, you're skipping updating the events
listed above and not for every event. So that should work. For some
reason, I thought you want to update the link_state for every new event.
the reason why suggest new change is because we found it also need
update link state in other case,
e.g. suspend -> reset ->conndone -> suspend
However, this would complicate the driver. Now we have to remember when
to set the link_state and when not to, and when to check the previous
link_state. On top of that, dwc->link_state may not reflect the current
state of the controller either, which makes it more confusing.
agree now it is complicate.
Perhaps we should not update dwc->link_state outside of link state
change event, and just track whether we called gadget_driver->suspend()
to call the correspond resume() on wakeup? It can be tracked with
dwc->gadget_driver_is_suspended.
could you help provide a change for this idea ?
Thanks,
Thinh
