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

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

 




> -----Original Message-----
> From: Tony Lindgren [mailto:tony@xxxxxxxxxxx]
> Sent: Tuesday, September 23, 2014 4:34 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> [140923 03:22]:
> > > -----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.
> 
> Oh OK maybe he's busy with other things then. I'll give your branch
> a try then.
> 
> > Please let me also notice that your patch has been also merged
> into my
> > master.
> 
> OK thanks.
> 
> > Thank you for fixing this issue.
> 
> No problem, thanks for queueing the pending patches.
> 
> Is there an example somewhere that completely clears any configured
> gadget from /sys/config/?
> 

Yes, please check my master branch.

There is a set of functions: usbg_rm_*() which can be used to remove
selected entity. The most convenient usage for gadget is to use
usbg_rm_gadget() with USBG_RM_RECURSE flag which works quite like rm -rf
*.

There is also an example called gadget-vid-pid-remove which removes all
gadgets based on vid and pid. Currently this example is hardcoded to
remove gadget with vid 0x1d6b and pid 0x0104 (those are used along
almost all examples) but you may easily edit it to take vid pid or
gadget name from command line.

Moreover there is also a command line tool called gt [1] to manipulate
usb gadgets but it's in very initial state (only command line parsing is
ready). This tool would do the thing in a future but currently this
functionality is not implemented.


--
Best regards,
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@xxxxxxxxxxx





--
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