From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [92.198.50.35]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 8E13B1007D7 for ; Sat, 31 Jul 2010 04:58:53 +1000 (EST) Date: Fri, 30 Jul 2010 20:57:04 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: =?utf-8?Q?Micha=C5=82?= Nazarewicz Subject: Re: [PATCH RFC] usb gadget: introduce usb_gadget_probe_driver Message-ID: <20100730185704.GB25446@pengutronix.de> References: <1280501361-13410-1-git-send-email-u.kleine-koenig@pengutronix.de> <20100730152602.GA28042@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 In-Reply-To: Cc: Cliff Cai , Greg Kroah-Hartman , Dinh Nguyen , Tony Lindgren , Nicolas Ferre , linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org, Julia Lawall , Philipp Zabel , Mark Brown , Felipe Balbi , Andrea Gelmini , Robert Jarzmik , Fabien Chouteau , Dan Carpenter , David Brownell , Vladimir Zapolskiy , Sergei Shtylyov , Vincent Sanders , Marc Singer , =?iso-8859-1?Q?Andr=E9?= Goddard Rosa , Alan Stern , Sean MacLennan , Russell King , Tobias Klauser , Marc Kleine-Budde , Eirik Aanonsen , Mike Frysinger , Thomas Dahlmann , linux-geode@lists.infradead.org, Ben Dooks , Magnus Damm , Anton Vorontsov , Andrew Victor , linux-arm-kernel@lists.infradead.org, Anatolij Gustschin , Eric Miao , =?iso-8859-1?Q?N=E9meth_M=E1rton?= , Jiri Kosina , Yoshihiro Shimoda , linux-usb@vger.kernel.org, Harro Haan , FUJITA Tomonori , H Hartley Sweeten , Paul Mundt , Tejun Heo , Andrew Morton , Cory Maccarrone List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, Jul 30, 2010 at 06:08:23PM +0200, Michał Nazarewicz wrote: > On Fri, 30 Jul 2010 17:26:02 +0200, Uwe Kleine-König wrote: >> On Fri, Jul 30, 2010 at 05:16:46PM +0200, Michał Nazarewicz wrote: >>> On Fri, 30 Jul 2010 16:49:14 +0200, Uwe Kleine-König wrote: >>> >>>> This is like usb_gadget_register_driver with the only difference that it >>>> gets the bind function as parameter instead of using driver->bind. This >>>> allows fixing section mismatches like >>>> >>>> WARNING: drivers/usb/gadget/g_printer.o(.data+0xc): Section mismatch in >>>> reference from the variable printer_driver to the function >>>> .init.text:printer_bind() >>>> The variable printer_driver references >>>> the function __init printer_bind() >>>> >>>> by using usb_gadget_probe_driver with driver->bind = NULL. When all >>>> drivers are fixed to use the new function the bind member of struct >>>> usb_gadget_driver can go away. >>>> >>>> Signed-off-by: Uwe Kleine-König >>>> Cc: Michal Nazarewicz >>>> Cc: Greg Kroah-Hartman > > BTW. Dunno if there is a reason to put me on Cc and Greg will be in > Signed-off-by anyway. > >>>> --- >>>> Hello, >>>> >>>> there is an alternative patch in Greg's tree [1], but IMHO mine is >>>> better as it doesn't need __ref. >>>> >>>> Thoughts? >>> >>> Personally I don't see advantage of this over changing the __init to __ref. >>> Or am I missing something? I see your patch as an unnecessary API change. >>> The way I understand it, __ref was introduced exactly for cases like this >>> one where function is referenced from "normal" data but used only during >>> init. Could you try to clarify for me why you think your solution is >>> better then mine? > >> - Using __ref instead of __init moves all bind functions from .init.text >> to .text and so the code isn't freed after booting or module loading. >> (OK, you could fix this by marking the driver structs as __ref and >> keep __init for the bind functions.) > > I believe this to be untrue. __ref puts code in .ref.text which AFAIK is > freed. I'm pretty sure it's not freed. .ref.text is of course correct. >> - Using __ref might hide section mismatches. (Your patch hasn't this >> problem as the bind functions used to live in .init.text, so any >> reference to .init should be OK assuming that it was OK to live in >> .init.text in the first place. But when marking the driver structs >> this is an issue.) That's why I don't like __ref and said my patch >> were better as it doesn't make use of it. >> >> - The bind functions are only called at init time, so there is no need >> to save a pointer to it. > > OK, I see some merit in this approach. However, the same issue is with > usb_configuration and ?usb_composite_driver -- those should be changed > in the same way or it would defeat the purpose of the patch. I didn't skip these on purpose. I just stumbled over the gadget drivers first. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |