On Tue, Sep 23, 2014 at 12:21:01PM +0200, Krzysztof Opasiak wrote: > > -----Original Message----- > > From: Tony Lindgren [mailto:tony@xxxxxxxxxxx] > > Sent: Monday, September 22, 2014 3:17 PM > > To: Krzysztof Opasiak > > Cc: 'Matt Porter'; linux-usb@xxxxxxxxxxxxxxx; Stanislaw Wadas; > > Andrzej Pietrasiewicz; Marek Szyprowski; Karol Lewandowski; > > philippedeswert@xxxxxxxxx > > Subject: Re: [PATCH] libusbg: Fix usbg_disable_gadget to actually > > clear the UDC > > > > * Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> [140922 01:07]: > > > Dear Tony, > > > > > > > -----Original Message----- > > > > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb- > > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Tony Lindgren > > > > Sent: Saturday, September 20, 2014 5:51 PM > > > > To: Matt Porter > > > > Cc: linux-usb@xxxxxxxxxxxxxxx > > > > Subject: [PATCH] libusbg: Fix usbg_disable_gadget to actually > > clear > > > > the UDC > > > > > > > > Currently usbg_disable_gadget() does not actually write > > anything > > > > to UDC to clear it and the configured UDC name stays there. > > > > > > > > > > No, udc name doesn't stay there due to O_TRUNC flag which is > > always used > > > for writing in usbg_write_string(). With this flag we don't need > > to > > > write new line to file because size of file is set to 0 while > > opening. > > > > > > Summing up: > > > > > > open("/sys/kernel/config/usb_gadget/g1/UDC", > > O_WRONLY|O_CREAT|O_TRUNC, > > > 0666) = 3 > > > close(3) = 0 > > > > > > causes unbind, so everything works fine. > > > > Hmm not clearing for me doing this afterwards: > > > > # cat /sys/kernel/config/usb_gadget/g1/UDC > > musb-hdrc.0.auto > > > > The UDC name stays there and won't get cleared. > > > > Am I missing something? > > Please forgive me, I have checked it once again and you are right. I > thought that truncate flag works on configfs in a similar way than on > others fs but I was wrong. This flag simply does nothing and you have > definitely found a bug. > > I was certain sure that this function works fine due to > gadget-vid-pid-remove example. In this simple program gadget is disabled > before removing. I had in my mind that it is impossible to modify a > gadget if it is bound to any udc. This example worked fine so I assumed > that usbg_disable_gadget() also works fine. I have look into configfs > composite gadget source and there I have found some surprise: > > /* > * ideally I would like to forbid to unlink functions while a > gadget is > * bound to an UDC. Since this isn't possible at the moment, we > simply > * force an unbind, the function is available here and then we > can > * remove the function. > */ > mutex_lock(&gi->lock); > if (gi->udc_name) > unregister_gadget(gi); > WARN_ON(gi->udc_name); > > This means that it is currently possible to remove function binding on > enabled gadget and it will cause unbind. This is why usbg_rm_gadget() > also worked fine without proper usbg_disable_gadget(). > > Summing up, Your patch fix an important bug. Its form is good for me. I > have checked it and it works fine. You may add: > > Reviewed-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> > > If it is going about Matt Porter activity, he is not responding for > mails or patches since April. I have github-fork of libusbg [1] with > latest source (my master is 35 commits ahead of libusbg/master and some > devel branches are even more). All patches which are intended for > libusbg (sent on a list or from pull requests) are merged there after > review. > > Please let me also notice that your patch has been also merged into my > master. > > Thank you for fixing this issue. Also verified that it fixes the issue here. Applied to master [1] along with all of the backlog, excluding the WIP usbg_udc patches. Thanks for everybody's patience. -Matt [1] https://github.com/libusbg/libusbg -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
