On Friday, April 12, 2013 9:02 AM, Ian Abbott wrote:
> The "amplc_pc263" module is a hybrid driver for Amplicon PC263 (ISA) and
> PCI263 (PCI) and uses conditional compilation (and compiler
> optimization) to compile in the support for the different bus types.
>
> Split out support for the PCI263 into a new module "amplc_pci263",
> retaining support for the PC263 in the existing module "amplc_pc263".
>
> Don't bother supporting the legacy attach mechanism for PCI board in the
> new module as that is no longer in vogue for the comedi drivers and the
> PCI263 board has no special configuration requirements.
>
> Although the code to handle the single subdevice of each board is the
> same for both drivers, this is only a small amount of code and I don't
> think it's worth creating a common module to handle it.
>
> Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> ---
> drivers/staging/comedi/Kconfig | 13 +-
> drivers/staging/comedi/drivers/Makefile | 3 +-
> drivers/staging/comedi/drivers/amplc_pc263.c | 278 ++------------------------
> drivers/staging/comedi/drivers/amplc_pci263.c | 127 ++++++++++++
> 4 files changed, 152 insertions(+), 269 deletions(-)
> create mode 100644 drivers/staging/comedi/drivers/amplc_pci263.c
<snip>
> diff --git a/drivers/staging/comedi/drivers/amplc_pc263.c b/drivers/staging/comedi/drivers/amplc_pc263.c
> index 1b1c1aa..1ffe379 100644
> --- a/drivers/staging/comedi/drivers/amplc_pc263.c
> +++ b/drivers/staging/comedi/drivers/amplc_pc263.c
<snip>
> struct pc263_board {
> const char *name;
> - unsigned short devid;
> - enum pc263_bustype bustype;
> - enum pc263_model model;
> };
The boardinfo now only contains the 'name'. How about just removing it.
If the legacy matching is typically done against this name instead of the
driver_name, just change the driver_name.
<snip>
> static struct comedi_driver amplc_pc263_driver = {
> .driver_name = PC263_DRIVER_NAME,
> .module = THIS_MODULE,
> .attach = pc263_attach,
> - .auto_attach = pc263_auto_attach,
> .detach = pc263_detach,
> .board_name = &pc263_boards[0].name,
> .offset = sizeof(struct pc263_board),
> .num_names = ARRAY_SIZE(pc263_boards),
> };
Then the boardinfo stuff can also be removed from the comedi_driver.
Also, I think PC263_DRIVER_NAME is only being used in the comedi_driver.
How about removing it and just open-coding the string here?
<snip>
> diff --git a/drivers/staging/comedi/drivers/amplc_pci263.c b/drivers/staging/comedi/drivers/amplc_pci263.c
> new file mode 100644
> index 0000000..8b57533
> --- /dev/null
> +++ b/drivers/staging/comedi/drivers/amplc_pci263.c
<snip>
+#define PCI263_DRIVER_NAME "amplc_pci263"
Again, is this define really necessary?
> +
> +/* PCI263 PCI configuration register information */
> +#define PCI_DEVICE_ID_AMPLICON_PCI263 0x000c
This one also, it's only used in the id table. Just open code the value there.
Other than these comments:
Reviewed-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
