On Tue, 31 May 2011 19:23:14 +0200, Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
Michal Nazarewicz wrote:
* Call this in your gadget driver's module initialization function,
* to tell the underlying usb controller driver about your driver.
- * The @bind() function will be called to bind it to a gadget before
this
- * registration call returns. It's expected that the @bind()
function will
- * be in init sections.
+ * The @driver->bind() function will be called to bind it to a gadget
before
+ * this registration call returns. It's expected that the
@driver->bind()
+ * function will be in init sections.
*/
int usb_gadget_probe_driver(struct usb_gadget_driver *driver,
int (*bind)(struct usb_gadget *));
+int usb_gadget_register_driver(struct usb_gadget_driver *driver);
Wasn't the whole point of introducing usb_gadget_probe_driver() and
removing bind callback to avoid problems with section mismatches?
Adding it back the way it was will bring all those warnings back.
I've been looking at b0fca50f5 and I don't see how this fixes the issue.
It makes the warning go way but that is it. The point is that ->bind()
points to something that will be discarded after the driver is loaded.
This is also the case with ->probe() in struct pci_dev for instance[0].
Nobody removed the callback there.
So if you call usb_gadget_probe_driver() at post init time and
composite_bind() is __init then it points to the function which is
already discarded.
Yes, but that's a problem with the gadget. The point of the commit was
that
most of the gadget called usb_gadget_register() in __init but the structure
with callbacks (including bind) was not (and could not be) __initdata. To
fix a warning, gadgets would add __refdata to the structure.
The bind callback was called only by usb_gadget_register() though so it was
not needed to be kept with all the other callbacks. So what the commit
did was introduce the usb_gadget_probe_driver() which would take the
callback.
It is my understanding that if you call usb_gadget_probe_driver() in a
function that is not __init and pass a pointer to an __init bind callback,
you'll get a warning since the function references __init function.
Whereas, if you call usb_gadget_probe_driver() in an __init function
and pass an __init bind callback everything will be all right.
In fact, previously, if you declared the structure with the callback as
__refdata to silence the (false positive) warning and *then* convert
the setup function from __init to non-__init.
So, if my understanding is correct, the commit was a correct
implementation and made sense. Feel free to correct me if I'm wrong.
Instead, it appears to me, we should leave usb_gadget_probe_driver()
the way it is and convert all drivers in the same patch or make the
new usb_gadget_register_driver() take the bind callback. Personally,
I'd prefer the first approach as it seems *_probe_driver() matches
other similar functions' names.
I would like to keep the way it is. You have everything related to the
gadget in one struct and not in two places. This is the way every driver
/
device is registered today. Except platform_driver_probe() vs
platform_driver_register() where the former does not work with hotplug
or with devices/drivers appearing the expected order.
The udc driver can be compared to an I2C host and a gadget driver to
I2C device driver for instance. The "only" difference I see is that you
can only attach one gadget at a time.
If your only concern is section miss match I would let the warning
pop-up and fix it by moving the "probe" function of the gadget into
devinit.
You cannot do that because FunctionFS requires the probe function to be
present at all times.
This is something we should think about: Do we want a sysfs file where we
can switch the current active gadget or do want to change them always via
rmmod $old && modprobe $new.
I think that's another story. I thought about a composite gadget that
would
allow something like that. Similar to Android's composite but better. ;)
(Hope my employer does not read that last part. O_o )
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: mnazarewicz@xxxxxxxxxx>-----ooO--(_)--Ooo--
--
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
