Re: [PATCH] usb: dwc3: gadget: clear gadget pointer after exit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

"Linyu Yuan (QUIC)" <quic_linyyuan@xxxxxxxxxxx> writes:
>> >> > > Why set this to NULL?  Who cares about this now?  What changed to
>> >> make
>> >> > > it required?
>> >> > It better to set to NULL for better understanding.
>> >>
>> >> Understanding of what?  What issue does this fix?  You need a reason to
>> >> make this, or any, kernel change.
>> >
>> > Sorry, let explain, for example, when do role switch, we can check it
>> > value to make sure it switch complete,
>> >
>> > If we do not set to NULL, it will be invalid.
>> 
>> Using this pointer as a role switch flag seems fragile, though.
> Hi Felipe,
>
> I update a v2 patch, no, I didn't use this pointer for role switch,
> just used to check role switch status, make sure it complete.

exactly, you're using it as a flag for role switch. But gadget pointer
being invalid is not very indication of that :-) It could be that right
now these two things correlate, but there's no guarantee. You end up
writing code that has possibility of failing in the future ;-)

>     when do role switch from device to host mode,
>     first disable device mode, currently there is no better way to
>     confirm when gadget was complete removed.

if ->udc_stop() finished running, you know for a fact that gadget was
completely removed ;-) If you want to be doubly sure, you can somehow
notify the UDC from the gadget_release() function, but that's a little
more invasive change

>     change device release function to clear gadget pointer,
>     this will confirm device stop complete when it is NULL.

And instead of checking for the pointer (which can be racy in some
occasions), perhaps tell the UDC about it with a callback or something
like that?

-- 
balbi



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux