On Wed, Jun 19, 2019 at 10:55 PM Felipe Balbi <balbi@xxxxxxxxxx> wrote: > > > Hi, > > Lianwei Wang <lianwei.wang@xxxxxxxxx> writes: > > > On Tue, Jun 18, 2019 at 11:21 PM Felipe Balbi <balbi@xxxxxxxxxx> wrote: > >> > >> > >> Hi, > >> > >> Lianwei Wang <lianwei.wang@xxxxxxxxx> writes: > >> >> Lianwei Wang <lianwei.wang@xxxxxxxxx> writes: > >> >> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@xxxxxxxxxx> wrote: > >> >> >> > >> >> >> Lianwei Wang <lianwei.wang@xxxxxxxxx> writes: > >> >> >> > >> >> >> > The udc and gadget device will be deleted when udc device is > >> >> >> > disconnected and the related function will be unbind with it. > >> >> >> > > >> >> >> > But if the configfs is not deleted, then the function object > >> >> >> > will be kept and the bound status is kept true. > >> >> >> > > >> >> >> > Then after udc device is connected again and a new udc and > >> >> >> > gadget objects will be created and passed to bind interface. > >> >> >> > But because the bound is still true, the new gadget is not > >> >> >> > updated to netdev and a previous freed gadget will be used > >> >> >> > in netdev after bind. > >> >> >> > > >> >> >> > To fix this using after freed issue, always set the gadget > >> >> >> > object to netdev in bind interface. > >> >> >> > > >> >> >> > Signed-off-by: Lianwei Wang <lianwei.wang@xxxxxxxxx> > >> >> >> > >> >> >> I can't actually understand what's the problem here. The gadget is not > >> >> >> deleted when we disconnect the cable. > >> >> >> > >> >> >> -- > >> >> >> balbi > >> >> > > >> >> > The issue was observed with a dual-role capable USB controller (e.g. Intel > >> >> > XHCI controller), which has the ability to switch role between host and device > >> >> > mode. The gadget is deleted when we switch role to device mode from host > >> >> > mode. See below log: > >> >> > # echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4) > >> >> > >> >> oh, so you're using a modified tree :-) Then we can't really help. > >> >> > >> >> > [ 41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode > >> >> > [ 41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS > >> >> > [ 41.187420] dwc3 dwc3.0.auto: dwc3_resume_common > >> >> > [ 41.191192] usb 1-1: USB disconnect, device number 3 > >> >> > [ 41.191284] usb 1-1.1: USB disconnect, device number 4 > >> >> > [ 41.218958] usb 1-1.5: USB disconnect, device number 5 > >> >> > [ 41.238117] android_work: sent uevent USB_STATE=CONFIGURED > >> >> > [ 41.240572] android_work: sent uevent USB_STATE=DISCONNECTED > >> >> > >> >> What is this android_work. That doesn't exist upstream. > >> >> > >> >> > [ 41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget' > >> >> > [ 41.263413] configfs-gadget gadget: unbind function 'Function FS > >> >> > Gadget'/ffff8801db049e38 > >> >> > [ 41.263969] configfs-gadget gadget: unbind function > >> >> > 'cdc_network'/ffff8801d8897400 > >> >> > [ 41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF > >> >> > [ 41.720957] dabr_udc deleted > >> >> > [ 41.721097] dabridge 1-5 deleted > >> >> > > >> >> > The UDC and gadget will be deleted after switch role to device mode. > >> >> > And they will be > >> >> > created as new object when switching back to host mode. At this time > >> >> > the bind in function > >> >> > driver (e.g. f_ncm) will not set the new gadget. > >> >> > > >> >> > For kernel 4.19+, the role switch command will be: > >> >> > echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role > >> >> > > >> >> > The latest Intel role switch kernel driver can be found here: > >> >> > https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c > >> >> > >> >> Right, please test against v5.2-rc5 and show me the problem on that > >> >> kernel. I can't apply patches for problems that may not even exist in > >> >> upstream, sorry. > >> >> > >> >> -- > >> >> balbi > >> > > >> > The issue exist in main line kernel, but I can not test it with > >> > v5.2-rc5 kernel. I tested it with 4.19 kernel, > >> > >> which of the v4.19? > >> > >> > which, for the usb gadget part, has almost the same code as v5.2. It > >> > is 100% reproducible with dual role > >> > USB controller or by removing UDC hardware. Take f_ncm for example, > >> > the use case as follows: > >> > >> Keep in mind that the way android handles dual-role is completely > >> different from what we have upstream. > >> > > Right, Our xchi controller support dual role and normally it works in host mode > > and the other device, e.g. phone can connect to our system as a gadget device. > > It works in the same way as PC. > > > >> > 1. USB controller is in host mode, f_ncm and UDC is configured in configfs. > >> > - The ncm is instanced and alloced when "functions/ncm.usb0" is > >> > created and it will be freed > >> > when those files are delted from configfs. > >> > > >> > 2. enable the gadget and bind it to this ncm function. > >> > - For the first time running, ncm_opts->bound is 0 and > >> > gether_set_gadget is called to set the > >> > gadget. The bound is set to 1 then. > >> > > >> > 3. If the UDC is disconnected from bus, then the UDC and its gadget is > >> > >> what do you mean by "disconnected from the bus"? Removing the cable > >> (aka, disconnect) will only cause the ->disconnect() callback to be > >> called. It will not result in the UDC being freed. Is this, perhaps, > >> something specific to android? > >> > > Our UDC is removable so we can remove the UDC from our system and the > > udc and gadget will be deleted after that. > > > > And our xhci support dual role mode and we can switch role between host and > > device mode. In host mode, the UDC will be connected to xhci host controller > > and the other device can connect to UDC. In device mode, it will work > > as a device > > and can connect to other host computer. > > > >> > deleted. But because the > >> > ncm.usb0 is still there, ncm object is not freed and > >> > ncm_opts->bound is still set. > >> > There are two ways to disconnect the UDC hardware. One is for dual > >> > role host controller by switch > >> > host controller role to device mode. Another way is by removing > >> > the UDC hardware from bus, both > >> > will generate an usb device disconnect event to UDC driver to > >> > delete udc and gadget. > >> > >> not true, unless I misunderstand what you mean. Disconnect will generate > >> a disconnect interrupt in most UDCs (except for dummy) and the > >> ->disconnect() callback will be called. Nothing will be freed. > >> > > As I mentioned above, the disconnect means the UDC is disconnected. > > > >> > 4. Now the bound of ncm is still set and gadget is deleted due to udc > >> > disconnected. And if we connect > >> > the udc device again, then it will create new udc and gadget and > >> > bind to ncm again. But because > >> > bound is already set, the new gadget is not set to gether > >> > (gether_register_netdev not called). > >> > > >> > Not sure if this is clear to you. Please review the scenario and the patch. > >> > >> This sounds a little like it's android-specific. Is your platform using > >> dwc3? Can you capture tracepoints of the failure? ftrace_dump_on_oops > >> will help getting the actual tracepoints in this case. > >> > >> cheers > >> > > It is not android specific. For the dual role controller, in device > > mode the dwc3 will > > work as gadget device and can connect to PC as device. In host mode, the dwc3 > > does not work, instead the removable UDC will be connected to our system as > > gadget device. > > > >> -- > >> balbi > > > > And the key point is that if the UDC can be disconnected as deleted? > > The answer is > > yes for below two cases: > > - The UDC is removable > > - It is a dual role controller and can switch between host mode and > > device mode. In host > > mode it will work as a xhci host controller. And in device mode it > > works as dwc3 gadget > > device. > > > > In both way we see that the udc is disconnected and deleted in our system. > > > > config USB_ROLES_INTEL_XHCI > > tristate "Intel XHCI USB Role Switch" > > depends on ACPI && X86 > > help > > Driver for the internal USB role switch for switching the USB data > > lines between the xHCI host controller and the dwc3 gadget controller > > found on various Intel SoCs. > > > > To compile the driver as a module, choose M here: the module will > > be called intel-xhci-usb-role-switch. > > > > The issue should be 100% reproducible with that support Intel XHCI USB > > role switch, or > > the udc can be removabled. > > what's specific to your case is *how* you implemented dual-role. None of > that is upstream and I have no means of verifying that what you claim is > true. > > Sorry > > -- > balbi That's Ok, Maybe I'm misunderstanding how upstream kernel works. The issue is happened on Intel hardware which should be support by upstream kernel. I did not implement any dual role, it is from upstream. And the UDC driver is working as the same mechanism as upstream too. The upstream kernel support remove/disconnect UDC device. I try to explain it clearly but unfortunately... my failure. If we look at the code, we have remove function defined in udc driver, and which will finally delete UDC (usb_del_gadget_udc). Could you tell me how the upstream kernel guarantee that the remove and usb_del_gadget_udc function is never called? Thanks, Lianwei