[PATCH 2/9] scripts/modpost: add a exception for USB gadget drivers

[PATCH 2/9] scripts/modpost: add a exception for USB gadget drivers

[Sebastian Andrzej Siewior]

The "driver" struct for a gadget driver is named *_driver. On module
load, the gadget expects a UDC driver to be loaded and avaiable. If this
is not the case => -ENODEV and bye bye. That means that the gadget
driver is initialized immediately. The initialization process includes
calling ->bind() for the gadget driver. Therefore it is okay to put this
in __init. It will never be called again, we don't (yet) allow two
gadgets of the same kind to be ->bind() again.

Commit 07a18bd716 ("usb gadget: don't save bind callback in struct
usb_composite_driver") move the ->bind() member from the struct to an
argument.
I'm reverting this and adding this exception. For g_zero we save around
400bytes. I'm not having the conversion about how important it is to
save 400 bytes. I wait for configfs interface for the gadget subsystem.
This will remove the __init from the ->bind() because we will be able to
call ->bind() more than once and _after_ the init has been dropped.

Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 scripts/mod/modpost.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 68e9f5e..5e30776 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -920,6 +920,9 @@ static const char *head_sections[] = { ".head.text*", NULL };
 static const char *linker_symbols[] =
 	{ "__init_begin", "_sinittext", "_einittext", NULL };
 
+static const char *const driver_sym[] = { "*_driver", NULL };
+static const char *const gadget_bind[] = { "*_bind", NULL };
+
 enum mismatch {
 	TEXT_TO_ANY_INIT,
 	DATA_TO_ANY_INIT,
@@ -1129,6 +1132,13 @@ static int secref_whitelist(const struct sectioncheck *mismatch,
 	    match(fromsym, mismatch->symbol_white_list))
 		return 0;
 
+	/* Check for pattern 2a */
+	if (match(tosec, init_exit_sections) &&
+	    match(fromsec, data_sections) &&
+	    match(fromsym, driver_sym) &&
+	    match(tosym, gadget_bind))
+		return 0;
+
 	/* Check for pattern 3 */
 	if (match(fromsec, head_sections) &&
 	    match(tosec, init_sections))
-- 
1.7.10.4

Re: [PATCH 2/9] scripts/modpost: add a exception for USB gadget drivers

[Sam Ravnborg]

On Wed, Aug 15, 2012 at 09:59:12PM +0200, Sebastian Andrzej Siewior wrote:
> The "driver" struct for a gadget driver is named *_driver. On module
> load, the gadget expects a UDC driver to be loaded and avaiable. If this
> is not the case => -ENODEV and bye bye. That means that the gadget
> driver is initialized immediately. The initialization process includes
> calling ->bind() for the gadget driver. Therefore it is okay to put this
> in __init. It will never be called again, we don't (yet) allow two
> gadgets of the same kind to be ->bind() again.
> 
> Commit 07a18bd716 ("usb gadget: don't save bind callback in struct
> usb_composite_driver") move the ->bind() member from the struct to an
> argument.
> I'm reverting this and adding this exception. For g_zero we save around
> 400bytes. I'm not having the conversion about how important it is to
> save 400 bytes. I wait for configfs interface for the gadget subsystem.
> This will remove the __init from the ->bind() because we will be able to
> call ->bind() more than once and _after_ the init has been dropped.

Could you use the __ref / __refdata annotation?
This would be much cleaner than this hack in modpost.

	Sam

Re: [PATCH 2/9] scripts/modpost: add a exception for USB gadget drivers

[Sebastian Andrzej Siewior]

On 08/15/2012 10:14 PM, Sam Ravnborg wrote:
> Could you use the __ref / __refdata annotation?
> This would be much cleaner than this hack in modpost.

Hmmm.

$ git grep " __init " drivers/usb/gadget/ | grep -vE 
'[_-]udc\.c|net22..\.c|udc-core\.c|dummy_hcd' | wc -l
71

That is the number of annotations I would have to change temporary. I
would prefer to avoid it but if you say no then I guess I have to do it.

>
> 	Sam

Sebastian

Re: [PATCH 2/9] scripts/modpost: add a exception for USB gadget drivers

[ABRAHAM, KISHON VIJAY]

Hi Sebastian,

On Thu, Aug 16, 2012 at 1:29 AM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> The "driver" struct for a gadget driver is named *_driver. On module
> load, the gadget expects a UDC driver to be loaded and avaiable. If this
> is not the case => -ENODEV and bye bye. That means that the gadget
Will this be taken care of in your future series (with configfs?).
There might be cases when your gadget driver gets loaded before the
UDC driver.

Thanks
Kishon

Re: [PATCH 2/9] scripts/modpost: add a exception for USB gadget drivers

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On Thu, Aug 16, 2012 at 06:35:07PM +0530, ABRAHAM, KISHON VIJAY wrote:
> Hi Sebastian,
> 
> On Thu, Aug 16, 2012 at 1:29 AM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > The "driver" struct for a gadget driver is named *_driver. On module
> > load, the gadget expects a UDC driver to be loaded and avaiable. If this
> > is not the case => -ENODEV and bye bye. That means that the gadget
> Will this be taken care of in your future series (with configfs?).
> There might be cases when your gadget driver gets loaded before the
> UDC driver.

with configfs that won't be a problem because even though function
drivers are loaded, they won't bind to any controller until we tell them
to through configfs. IOW, functions will only bind to UDCs after we
assign them to a particular usb interface through configfs.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/9] scripts/modpost: add a exception for USB gadget drivers

[ABRAHAM, KISHON VIJAY]

On Thu, Aug 16, 2012 at 6:33 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Aug 16, 2012 at 06:35:07PM +0530, ABRAHAM, KISHON VIJAY wrote:
>> Hi Sebastian,
>>
>> On Thu, Aug 16, 2012 at 1:29 AM, Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de> wrote:
>> > The "driver" struct for a gadget driver is named *_driver. On module
>> > load, the gadget expects a UDC driver to be loaded and avaiable. If this
>> > is not the case => -ENODEV and bye bye. That means that the gadget
>> Will this be taken care of in your future series (with configfs?).
>> There might be cases when your gadget driver gets loaded before the
>> UDC driver.
>
> with configfs that won't be a problem because even though function
> drivers are loaded, they won't bind to any controller until we tell them
> to through configfs. IOW, functions will only bind to UDCs after we
> assign them to a particular usb interface through configfs.

Ok. Makes sense..

Thanks
Kishon