Re: [PATCH 2/3] usb: gadget: introduce UDC Class

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

 



* Michal Nazarewicz | 2011-06-01 01:30:12 [+0200]:
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.

On Wed, 01 Jun 2011 18:36:00 +0200, Sebastian Andrzej Siewior wrote:
no, you could rename the struct, for instance from  printer_driver to
printer_ops if it was all about shutting up the warning :)

Well, yes, that would be another solution.  I've never thought about doing
that though.

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.

You get a warning because you _may_ reference discarded data because
variable name ended with _driver. It behaves like driver why not move
it to __devinit? Still, it is a false positive if it is ensure that the
pointer is never reached.

Move what to __devinit? The structure with ops cannot be in __(dev)init
because all fields expect for bind are used after initialisation.  The
probe/register driver cannot be in __(dev)init because it may be used
after the UDC driver has been initialised (ie. if you insmod a gadget
driver) and moreover, FunctionFS can call it at any arbitrary time.

So, if my understanding is correct, the commit was a correct
implementation and made sense.  Feel free to correct me if I'm wrong.

If I look back at it, it looks like platform_driver_probe(). I don't like
it because breaks hotplug in general and and the win are few bytes.

How does it break hotplug?  Do gadget driver even support hotplug?  They
are initialised and registered at the moment module is inserted.

So where do we go with this?
The udc-core currently requires the udc to be loaded before the gadget.

Correct.

The gadget will return with -ENODEV if this is not the case.

Correct.

The difference is that earlier the udc would be loaded automaticly due
to required symbols.

If I'm not mistaken, that's still the case.  All gadget driers require the
usb_gadget_probe() function to be present.

So we have load the udc driver first, followed by the gadget.

Making possible to load them (udc & gadget) independently is nice I think.

You can load gadget whenever you want.  But yes, you cannot load udc after
the gadget.

Having "bind" as argument and its code in __init makes it impossible.
Also changing the gadget at runtime without rmmoding the gadget.
So what do you think?

Well, yes, but your patch does not make it possible anyway.

If you want to provide that functionality it deserves a separate patch.
Also, current gadgets assume that they are initialised and registered
when they are loaded so it will need some work.

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.

__devinit is discarded only if hotplot is disabled. There is no need to
mark everything __devinit or __init if the is required to remain during
the driver's lifetime.

Not every driver requires it.  Most don't.  Most are fine with having bind
callback in __init.  FunctionFS is an exception because it first needs to
read some data from user space.

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


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

  Powered by Linux