Re: [PATCH] libusbg: Fix usbg_disable_gadget to actually clear the UDC

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

 



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




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

  Powered by Linux