staging: comedi: refactor ni_pcimio driver and use module_comedi_pci_driver
PATCH 1/1 staging: comedi: refactor ni_pcimio driver and use module_comedi_pci_driver
Move the module_init/module_exit routines and the associated struct comedi_drive and struct pci_driver to the end of the source. This is more typical of how other drivers are written and removes the need for the forward declarations.
Convert the driver to use the module_comedi_pci_driver() macro which makes the code smaller and a bit simpler.
---
This is only a partial refactor. The attach/detach routines still need to be moved to remove the remaining forward declarations.
drivers/staging/comedi/drivers/ni_pcimio.c | 192 ++++++++++++---------------- 1 files changed, 84 insertions(+), 108 deletions(-)
drivers/staging/comedi/drivers/ni_pcimio.c +84 -108
@@ -129,66 +129,6 @@ Bugs:#define DRV_NAME "ni_pcimio" -/* The following two tables must be in the same order */ -static DEFINE_PCI_DEVICE_TABLE (ni_pci_table) = { - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x0162 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1170 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1180 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1190 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x11b0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x11c0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x11d0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1270 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1330 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1340 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1350 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x14e0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x14f0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1580 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x15b0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1880 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1870 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x18b0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x18c0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2410 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2420 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2430 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2890 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x28c0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2a60 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2a70 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2a80 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2ab0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2b80 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2b90 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2c80 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2ca0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70aa )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70ab )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70ac )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70af )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b4 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b6 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b7 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b8 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70bc )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70bd )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70bf )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70c0 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70f2 )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x710d )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x716c )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x716d )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x717f )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x71bc )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x717d )}, - {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x72e8 )}, - {0 } -}; - -MODULE_DEVICE_TABLE (pci, ni_pci_table); -/* These are not all the possible ao ranges for 628x boards. They can do OFFSET +- REFERENCE where OFFSET can be 0V, 5V, APFI<0,1>, or AO<0...3> and RANGE can
@@ -1250,54 +1190,6 @@ static const struct ni_board_struct ni_boards[] = {#define n_pcimio_boards ARRAY_SIZE(ni_boards) -static int pcimio_attach (struct comedi_device *dev, -struct comedi_devconfig *it); -static int pcimio_detach (struct comedi_device *dev); -static struct comedi_driver driver_pcimio = { - .driver_name =DRV_NAME , - .module =THIS_MODULE , - .attach = pcimio_attach, - .detach = pcimio_detach, -}; - -static int __devinit driver_pcimio_pci_probe (struct pci_dev *dev, -const struct pci_device_id *ent) -{ -return comedi_pci_auto_config (dev, &driver_pcimio); -} - -static void __devexit driver_pcimio_pci_remove(struct pci_dev *dev) -{ -comedi_pci_auto_unconfig (dev); -} - -static struct pci_driver driver_pcimio_pci_driver = { - .id_table = ni_pci_table, - .probe = &driver_pcimio_pci_probe, - .remove =__devexit_p (&driver_pcimio_pci_remove) -}; - -static int __init driver_pcimio_init_module (void ) -{ -int retval; - - retval =comedi_driver_register (&driver_pcimio); -if (retval <0 ) -return retval; - - driver_pcimio_pci_driver.name = (char *)driver_pcimio.driver_name; -return pci_register_driver (&driver_pcimio_pci_driver); -} - -static void __exit driver_pcimio_cleanup_module(void) -{ -pci_unregister_driver (&driver_pcimio_pci_driver); -comedi_driver_unregister (&driver_pcimio); -} - -module_init (driver_pcimio_init_module); -module_exit (driver_pcimio_cleanup_module); -struct ni_private { NI_PRIVATE_COMMON};#define devpriv ((struct ni_private *)dev->private)
@@ -1874,6 +1766,90 @@ static int pcimio_dio_change(struct comedi_device *dev,return 0 ; } +static struct comedi_driver ni_pcimio_driver = { + .driver_name ="ni_pcimio" , + .module =THIS_MODULE , + .attach = pcimio_attach, + .detach = pcimio_detach, +}; + +static int __devinit ni_pcimio_pci_probe (struct pci_dev *dev, +const struct pci_device_id *ent) +{ +return comedi_pci_auto_config (dev, &ni_pcimio_driver); +} + +static void __devexit ni_pcimio_pci_remove(struct pci_dev *dev) +{ +comedi_pci_auto_unconfig (dev); +} + +static DEFINE_PCI_DEVICE_TABLE (ni_pcimio_pci_table) = { + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x0162 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1170 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1180 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1190 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x11b0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x11c0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x11d0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1270 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1330 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1340 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1350 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x14e0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x14f0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1580 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x15b0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1880 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x1870 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x18b0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x18c0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2410 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2420 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2430 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2890 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x28c0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2a60 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2a70 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2a80 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2ab0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2b80 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2b90 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2c80 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x2ca0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70aa ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70ab ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70ac ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70af ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b4 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b6 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b7 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70b8 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70bc ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70bd ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70bf ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70c0 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x70f2 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x710d ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x716c ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x716d ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x717f ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x71bc ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x717d ) }, + {PCI_DEVICE (PCI_VENDOR_ID_NI ,0x72e8 ) }, + {0 } +}; +MODULE_DEVICE_TABLE (pci, ni_pcimio_pci_table); + +static struct pci_driver ni_pcimio_pci_driver = { + .name ="ni_pcimio" , + .id_table = ni_pcimio_pci_table, + .probe = ni_pcimio_pci_probe, + .remove =__devexit_p (ni_pcimio_pci_remove) +}; +module_comedi_pci_driver(ni_pcimio_driver, ni_pcimio_pci_driver) +MODULE_AUTHOR ("Comedi http://www.comedi.org" );MODULE_DESCRIPTION ("Comedi low-level driver" );MODULE_LICENSE ("GPL" );
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1770
Quoted 2 lines · H Hartley Sweeten
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct comedi_driver ni_pcimio_driver = { + .driver_name ="ni_pcimio" ,
The original used the DRV_NAME macro expanding to "ni_pcimio" here...
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1846
Quoted 2 lines
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct pci_driver ni_pcimio_pci_driver = { + .name ="ni_pcimio" ,
... which could also be used here.
(I realize this is a bit late as the patch is already committed!)
Signature · 2 lines
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
Quoted 6 lines · Ian Abbott
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1770
Quoted 2 lines · H Hartley Sweeten
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct comedi_driver ni_pcimio_driver = { + .driver_name ="ni_pcimio" ,The original used the
DRV_NAMEmacro expanding to "ni_pcimio" here...
Quoted 6 lines
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1846
Quoted 2 lines
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct pci_driver ni_pcimio_pci_driver = { + .name ="ni_pcimio" ,... which could also be used here.
(I realize this is a bit late as the patch is already committed!)
I noticed that. A couple of the comedi drivers do this but it's not consistent.
Personally I don't care for the use of the DRV_NAME (or DRIVER_NAME or whatever someone wants to call it) usage. It doesn't same any code space and always makes me have to do a second search to see what the "name" actually is.
Regardsless, if you prefer this in the drivers please let me know. It would be nice to have them somewhat consistent before the comedi support leaves staging.
Regards,
Hartley
���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�
Quoted 16 lines · H Hartley Sweeten
Quoted 6 lines · Ian Abbott
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1770
Quoted 2 lines · H Hartley Sweeten
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct comedi_driver ni_pcimio_driver = { + .driver_name ="ni_pcimio" ,The original used the
DRV_NAMEmacro expanding to "ni_pcimio" here...Quoted 6 lines
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1846
Quoted 2 lines
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct pci_driver ni_pcimio_pci_driver = { + .name ="ni_pcimio" ,... which could also be used here.
(I realize this is a bit late as the patch is already committed!)
I noticed that. A couple of the comedi drivers do this but it's not consistent.
Quoted 8 lines
Personally I don't care for the use of the
DRV_NAME(orDRIVER_NAMEor whatever someone wants to call it) usage. It doesn't same any code space and always makes me have to do a second search to see what the "name" actually is.Regardsless, if you prefer this in the drivers please let me know. It would be nice to have them somewhat consistent before the comedi support leaves staging.
It doesn't really matter as long as it's consistent within the driver. I tend to use macros for literals used several times as it often allows mistakes to be caught at compile-time.
I think DRV_NAME is still used in the request_irq() call though.
Signature · 2 lines
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
Quoted 26 lines · Ian Abbott
Quoted 16 lines · H Hartley Sweeten
Quoted 6 lines · Ian Abbott
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1770
Quoted 2 lines · H Hartley Sweeten
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct comedi_driver ni_pcimio_driver = { + .driver_name ="ni_pcimio" ,The original used the
DRV_NAMEmacro expanding to "ni_pcimio" here...Quoted 6 lines
drivers/staging/comedi/drivers/ni_pcimio.c static int pcimio_dio_change(struct comedi_device *dev, · line 1846
Quoted 2 lines
drivers/staging/comedi/drivers/ni_pcimio.c +2
-0+static struct pci_driver ni_pcimio_pci_driver = { + .name ="ni_pcimio" ,... which could also be used here.
(I realize this is a bit late as the patch is already committed!)
I noticed that. A couple of the comedi drivers do this but it's not consistent.
Quoted 8 lines
Personally I don't care for the use of the
DRV_NAME(orDRIVER_NAMEor whatever someone wants to call it) usage. It doesn't same any code space and always makes me have to do a second search to see what the "name" actually is.Regardsless, if you prefer this in the drivers please let me know. It would be nice to have them somewhat consistent before the comedi support leaves staging.
Quoted 5 lines
It doesn't really matter as long as it's consistent within the driver. I tend to use macros for literals used several times as it often allows mistakes to be caught at compile-time.
I think
DRV_NAMEis still used in therequest_irq()call though.
Yah, I missed that one.
I was going to clean up all the request_irq()'s so that dev->driver->driver_name is used for all "name" parameters. This fixes a number of drivers that directly reference the static comedi_driver variable. And it removes all the static "names".
When I do this I will also drop all the DRV_NAME macros that are no longer used.
Regards,
Hartley
���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�