Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate

  

 
Linux kernel RSS feed   Index | Next | Previous | View Threaded


hartleys at visionengravers

May 11, 2012, 10:07 AM

Post #1 of 13 (154 views)
Permalink
[PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate
Introduce the module_comedi_pci_driver macro, and the
associated register/unregister functions, which is a
convenience macro for comedi pci driver modules similar
to module_platform_driver. It is intended to be used by
drivers where the init/exit section does nothing but
register/unregister the comedi driver and associated pci
driver. By using this macro it is possible to eliminate
a few lines of boilerplate code per comedi pci driver.

Also, when registering the pci driver check for failure
and unregister the comedi driver.

Signed-off-by: H Hartley Sweeten <hsweeten [at] visionengravers>
Cc: Ian Abbott <abbotti [at] mev>
Cc: Mori Hess <fmhess [at] users>
Cc: Greg Kroah-Hartman <gregkh [at] linuxfoundation>

---

diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
index 8828609..92a1199 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -310,6 +310,25 @@ int comedi_driver_unregister(struct comedi_driver *);
module_driver(__comedi_driver, comedi_driver_register, \
comedi_driver_unregister)

+struct pci_driver;
+
+int comedi_pci_driver_register(struct comedi_driver *, struct pci_driver *);
+void comedi_pci_driver_unregister(struct comedi_driver *, struct pci_driver *);
+
+/**
+ * module_comedi_pci_driver() - Helper macro for registering a comedi PCI driver
+ * @__comedi_driver: comedi_driver struct
+ * @__pci_driver: pci_driver struct
+ *
+ * Helper macro for comedi PCI drivers which do not do anything special
+ * in module init/exit. This eliminates a lot of boilerplate. Each
+ * module may only use this macro once, and calling it replaces
+ * module_init() and module_exit()
+ */
+#define module_comedi_pci_driver(__comedi_driver, __pci_driver) \
+ module_driver(__comedi_driver, comedi_pci_driver_register, \
+ comedi_pci_driver_unregister, &(__pci_driver))
+
void init_polling(void);
void cleanup_polling(void);
void start_polling(struct comedi_device *);
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 49681a1..7071109 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -950,6 +950,34 @@ void comedi_pci_auto_unconfig(struct pci_dev *pcidev)
}
EXPORT_SYMBOL_GPL(comedi_pci_auto_unconfig);

+int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
+ struct pci_driver *pci_driver)
+{
+ int ret;
+
+ ret = comedi_driver_register(comedi_driver);
+ if (ret < 0)
+ return ret;
+
+ pci_driver->name = comedi_driver->driver_name;
+ ret = pci_register_driver(pci_driver);
+ if (ret < 0) {
+ comedi_driver_unregister(comedi_driver);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(comedi_pci_driver_register);
+
+void comedi_pci_driver_unregister(struct comedi_driver *comedi_driver,
+ struct pci_driver *pci_driver)
+{
+ pci_unregister_driver(pci_driver);
+ comedi_driver_unregister(comedi_driver);
+}
+EXPORT_SYMBOL(comedi_pci_driver_unregister);
+
static int comedi_old_usb_auto_config(struct usb_interface *intf,
struct comedi_driver *driver)
{
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


abbotti at mev

May 11, 2012, 10:32 AM

Post #2 of 13 (145 views)
Permalink
Re: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On 2012-05-11 18:07, H Hartley Sweeten wrote:
> +int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
> + struct pci_driver *pci_driver)
> +{
> + int ret;
> +
> + ret = comedi_driver_register(comedi_driver);
> + if (ret< 0)
> + return ret;
> +
> + pci_driver->name = comedi_driver->driver_name;

I think it would be better if the driver modules that call this function
already had pci_driver->name initialized statically.

--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti [at] mev> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hartleys at visionengravers

May 11, 2012, 10:34 AM

Post #3 of 13 (147 views)
Permalink
RE: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On Friday, May 11, 2012 10:32 AM, Ian Abbott wrote:
> On 2012-05-11 18:07, H Hartley Sweeten wrote:
>> +int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
>> + struct pci_driver *pci_driver)
>> +{
>> + int ret;
>> +
>> + ret = comedi_driver_register(comedi_driver);
>> + if (ret< 0)
>> + return ret;
>> +
>> + pci_driver->name = comedi_driver->driver_name;
>
> I think it would be better if the driver modules that call this function
> already had pci_driver->name initialized statically.

I agree, but this is how all the pci drivers do it now.

To change this will require auditing all the comedi pci drivers and adding
the static name.

Regards,
Hartley

{.n++%lzwmb맲rzXw{ayʇڙ,jfhzw j:+vwjmzZ+ݢj"!iOzv^m nƊY&


abbotti at mev

May 11, 2012, 10:42 AM

Post #4 of 13 (149 views)
Permalink
Re: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On 2012-05-11 18:34, H Hartley Sweeten wrote:
> On Friday, May 11, 2012 10:32 AM, Ian Abbott wrote:
>> On 2012-05-11 18:07, H Hartley Sweeten wrote:
>>> +int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
>>> + struct pci_driver *pci_driver)
>>> +{
>>> + int ret;
>>> +
>>> + ret = comedi_driver_register(comedi_driver);
>>> + if (ret< 0)
>>> + return ret;
>>> +
>>> + pci_driver->name = comedi_driver->driver_name;
>>
>> I think it would be better if the driver modules that call this function
>> already had pci_driver->name initialized statically.
>
> I agree, but this is how all the pci drivers do it now.
>
> To change this will require auditing all the comedi pci drivers and adding
> the static name.

But if you're going to be calling this function from those drivers
anyway, you might as well add the static names at the same time! Saves
another round of patches.

--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti [at] mev> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hartleys at visionengravers

May 11, 2012, 10:50 AM

Post #5 of 13 (146 views)
Permalink
RE: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On Friday, May 11, 2012 10:42 AM, Ian Abbott wrote:
> On 2012-05-11 18:34, H Hartley Sweeten wrote:
>> On Friday, May 11, 2012 10:32 AM, Ian Abbott wrote:
>>> On 2012-05-11 18:07, H Hartley Sweeten wrote:
>>>> +int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
>>>> + struct pci_driver *pci_driver)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + ret = comedi_driver_register(comedi_driver);
>>>> + if (ret< 0)
>>>> + return ret;
>>>> +
>>>> + pci_driver->name = comedi_driver->driver_name;
>>>
>>> I think it would be better if the driver modules that call this function
>>> already had pci_driver->name initialized statically.
>>
>> I agree, but this is how all the pci drivers do it now.
>>
>> To change this will require auditing all the comedi pci drivers and adding
>> the static name.
>
> But if you're going to be calling this function from those drivers
> anyway, you might as well add the static names at the same time! Saves
> another round of patches.

True, but many of the pci drivers still need quite a bit of cleanup.
After this patch was accepted I was going to just change the ones
that have already had the init/exit stuff cleaned up. Then as I hit
all the others I would change them.

I guess I could do it this way:

1) add the "name" to all the pci drivers and remove the
"pci_driver->name = comedi_driver->name" from all the
module_init functions.

2) add the module_comedi_pci_driver stuff, without the
"pci_driver->name = comedi_driver->name"

3) Update all the refactored comedi pci drivers to use
the module_comedi_pci_driver macro.

4) As the other comedi pci drivers are refactored, use
the module_comedi_pci_driver macro.

How does that sound?

Regards,
Hartley

칻&~&+-ݶw˛mbdzޖ)w*jgݢj/zޖ2ޙ&)ߡaGhj:+vw٥>WiaxPjm -+d_


gregkh at linuxfoundation

May 11, 2012, 10:59 AM

Post #6 of 13 (143 views)
Permalink
Re: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On Fri, May 11, 2012 at 10:07:43AM -0700, H Hartley Sweeten wrote:
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -950,6 +950,34 @@ void comedi_pci_auto_unconfig(struct pci_dev *pcidev)
> }
> EXPORT_SYMBOL_GPL(comedi_pci_auto_unconfig);
>
> +int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
> + struct pci_driver *pci_driver)
> +{
> + int ret;
> +
> + ret = comedi_driver_register(comedi_driver);
> + if (ret < 0)
> + return ret;
> +
> + pci_driver->name = comedi_driver->driver_name;
> + ret = pci_register_driver(pci_driver);
> + if (ret < 0) {
> + comedi_driver_unregister(comedi_driver);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(comedi_pci_driver_register);
> +
> +void comedi_pci_driver_unregister(struct comedi_driver *comedi_driver,
> + struct pci_driver *pci_driver)
> +{
> + pci_unregister_driver(pci_driver);
> + comedi_driver_unregister(comedi_driver);
> +}
> +EXPORT_SYMBOL(comedi_pci_driver_unregister);

EXPORT_SYMBOL_GPL() for these two functions?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hartleys at visionengravers

May 11, 2012, 11:04 AM

Post #7 of 13 (145 views)
Permalink
RE: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On Friday, May 11, 2012 11:00 AM, Greg KH wrote:
> On Fri, May 11, 2012 at 10:07:43AM -0700, H Hartley Sweeten wrote:
>> --- a/drivers/staging/comedi/drivers.c
>> +++ b/drivers/staging/comedi/drivers.c

<snip>

>> +EXPORT_SYMBOL(comedi_pci_driver_register);

<snip>

>> +EXPORT_SYMBOL(comedi_pci_driver_unregister);
>
> EXPORT_SYMBOL_GPL() for these two functions?

I wasn't sure about that...

Question.. when "should" EXPORT_SYMBOL be used and
when should it be EXPORT_SYMBOL_GPL?

Also, do you have any comments on Ian Abbott's comment
about the:

> + pci_driver->name = comedi_driver->driver_name;

In the register function?

Regards,
Hartley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


abbotti at mev

May 11, 2012, 11:14 AM

Post #8 of 13 (149 views)
Permalink
Re: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On 2012-05-11 18:50, H Hartley Sweeten wrote:
> On Friday, May 11, 2012 10:42 AM, Ian Abbott wrote:
>> On 2012-05-11 18:34, H Hartley Sweeten wrote:
>>> On Friday, May 11, 2012 10:32 AM, Ian Abbott wrote:
>>>> On 2012-05-11 18:07, H Hartley Sweeten wrote:
>>>>> +int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
>>>>> + struct pci_driver *pci_driver)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + ret = comedi_driver_register(comedi_driver);
>>>>> + if (ret< 0)
>>>>> + return ret;
>>>>> +
>>>>> + pci_driver->name = comedi_driver->driver_name;
>>>>
>>>> I think it would be better if the driver modules that call this function
>>>> already had pci_driver->name initialized statically.
>>>
>>> I agree, but this is how all the pci drivers do it now.
>>>
>>> To change this will require auditing all the comedi pci drivers and adding
>>> the static name.
>>
>> But if you're going to be calling this function from those drivers
>> anyway, you might as well add the static names at the same time! Saves
>> another round of patches.
>
> True, but many of the pci drivers still need quite a bit of cleanup.
> After this patch was accepted I was going to just change the ones
> that have already had the init/exit stuff cleaned up. Then as I hit
> all the others I would change them.
>
> I guess I could do it this way:
>
> 1) add the "name" to all the pci drivers and remove the
> "pci_driver->name = comedi_driver->name" from all the
> module_init functions.
>
> 2) add the module_comedi_pci_driver stuff, without the
> "pci_driver->name = comedi_driver->name"
>
> 3) Update all the refactored comedi pci drivers to use
> the module_comedi_pci_driver macro.
>
> 4) As the other comedi pci drivers are refactored, use
> the module_comedi_pci_driver macro.
>
> How does that sound?

Okay, but I don't think it's worth doing three patches for each driver
where one would do. Adding a one-line .name = "foo", to the single
patch for each driver you were planning to do anyway seems like a
trivial addition that could be slotted into the patch without too much
argument from the "one change per patch" folks, especially if you
mention the purpose of that line in the commit message. (I'd defer to
the better judgement of Greg though.)

--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti [at] mev> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


abbotti at mev

May 11, 2012, 11:14 AM

Post #9 of 13 (145 views)
Permalink
Re: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On 2012-05-11 19:04, H Hartley Sweeten wrote:
> On Friday, May 11, 2012 11:00 AM, Greg KH wrote:
>> On Fri, May 11, 2012 at 10:07:43AM -0700, H Hartley Sweeten wrote:
>>> +EXPORT_SYMBOL(comedi_pci_driver_unregister);
>>
>> EXPORT_SYMBOL_GPL() for these two functions?
>
> I wasn't sure about that...
>
> Question.. when "should" EXPORT_SYMBOL be used and
> when should it be EXPORT_SYMBOL_GPL?

Adding EXPORT_SYMBOL_GPL shouldn't remove previous functionality. It
doesn't in this case, so that's okay!

--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti [at] mev> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hartleys at visionengravers

May 11, 2012, 11:18 AM

Post #10 of 13 (144 views)
Permalink
RE: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On Friday, May 11, 2012 11:15 AM, Ian Abbott wrote:
> On 2012-05-11 19:04, H Hartley Sweeten wrote:
>> On Friday, May 11, 2012 11:00 AM, Greg KH wrote:
>>> On Fri, May 11, 2012 at 10:07:43AM -0700, H Hartley Sweeten wrote:
>>>> +EXPORT_SYMBOL(comedi_pci_driver_unregister);
>>>
>>> EXPORT_SYMBOL_GPL() for these two functions?
>>
>> I wasn't sure about that...
>>
>> Question.. when "should" EXPORT_SYMBOL be used and
>> when should it be EXPORT_SYMBOL_GPL?
>
> Adding EXPORT_SYMBOL_GPL shouldn't remove previous functionality. It
> doesn't in this case, so that's okay!

OK. I'll change it.

Regards,
Hartley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


hartleys at visionengravers

May 11, 2012, 11:26 AM

Post #11 of 13 (144 views)
Permalink
RE: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On Friday, May 11, 2012 11:14 AM, Ian Abbott wrote:
> Okay, but I don't think it's worth doing three patches for each driver
> where one would do. Adding a one-line .name = "foo", to the single
> patch for each driver you were planning to do anyway seems like a
> trivial addition that could be slotted into the patch without too much
> argument from the "one change per patch" folks, especially if you
> mention the purpose of that line in the commit message. (I'd defer to
> the better judgement of Greg though.)

With this patch my main goal was to not "break" any of the drivers.

As a follow-up to this patch I was going to use the macro in all the
pci drivers that have been refactored. Similar to what I did with the
module_comedi_driver macro. When I do that I can add the static
name field to those drivers.

How about doing this in the register function:

If (!pci_driver->name)
pci_driver->name = comedi_driver->driver_name;

This way all the drivers that still need to be refactored will still work.

After all the drivers have been updated, those two lines can be
removed.

So we have this patch and one big patch updating all the refactored
drivers. Some number of patches refactoring the remaining drivers
and using the module_comedi_pci_driver macro. Then one last
patch removing the check above.

How does that sound?

Regards,
Hartley

NrybXǧv^)޺{.n+{zXܨ}Ơz&j:+vzZ++zfh~izw?&)ߢf^jǫym@Aa 0hi


gregkh at linuxfoundation

May 11, 2012, 2:59 PM

Post #12 of 13 (148 views)
Permalink
Re: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On Fri, May 11, 2012 at 01:26:37PM -0500, H Hartley Sweeten wrote:
> On Friday, May 11, 2012 11:14 AM, Ian Abbott wrote:
> > Okay, but I don't think it's worth doing three patches for each driver
> > where one would do. Adding a one-line .name = "foo", to the single
> > patch for each driver you were planning to do anyway seems like a
> > trivial addition that could be slotted into the patch without too much
> > argument from the "one change per patch" folks, especially if you
> > mention the purpose of that line in the commit message. (I'd defer to
> > the better judgement of Greg though.)
>
> With this patch my main goal was to not "break" any of the drivers.
>
> As a follow-up to this patch I was going to use the macro in all the
> pci drivers that have been refactored. Similar to what I did with the
> module_comedi_driver macro. When I do that I can add the static
> name field to those drivers.
>
> How about doing this in the register function:
>
> If (!pci_driver->name)
> pci_driver->name = comedi_driver->driver_name;
>
> This way all the drivers that still need to be refactored will still work.
>
> After all the drivers have been updated, those two lines can be
> removed.
>
> So we have this patch and one big patch updating all the refactored
> drivers. Some number of patches refactoring the remaining drivers
> and using the module_comedi_pci_driver macro. Then one last
> patch removing the check above.
>
> How does that sound?

That sounds reasonable to me.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


abbotti at mev

May 12, 2012, 10:22 AM

Post #13 of 13 (130 views)
Permalink
Re: [PATCH] staging: comedi: Add helper macro for comedi pci driver boilerplate [In reply to]
On 11/05/2012 22:59, gregkh [at] linuxfoundation wrote:
> On Fri, May 11, 2012 at 01:26:37PM -0500, H Hartley Sweeten wrote:
>> How about doing this in the register function:
>>
>> If (!pci_driver->name)
>> pci_driver->name = comedi_driver->driver_name;
>>
>> This way all the drivers that still need to be refactored will still work.
>>
>> After all the drivers have been updated, those two lines can be
>> removed.
>>
>> So we have this patch and one big patch updating all the refactored
>> drivers. Some number of patches refactoring the remaining drivers
>> and using the module_comedi_pci_driver macro. Then one last
>> patch removing the check above.
>>
>> How does that sound?
>
> That sounds reasonable to me.

Sounds reasonable to me too.

--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti [at] mev> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.