On Tuesday, April 23, 2013 5:24 AM, Ian Abbott wrote:
> On 2013-04-22 20:33, H Hartley Sweeten wrote:
>> Currently the ni_labpc driver is used by the legacy (ISA), PCI, and
>> PCMCIA versions of the LabPC board. The driver is enabled under the
>> COMEDI_PCI_DRIVERS section of the Kconfig. This means that PCI support
>> must be enabled in order to use the ni_labpc driver for the PCI or
>> PCMCIA drivers.
>>
>> Split the PCI support code out of the ni_labpc driver into a separate
>> driver, ni_labpc_pci. The PCMCIA support is already slip out as
>> ni_labpc_cs.
>>
>> Modify the Kconfig so that the common code in ni_labpc is enabled by the
>> Kconfig option COMEDI_NI_LABPC. The ISA support code is currently still
>> in the ni_labpc driver but is only compiled in if COMEDI_NI_LABPC_ISA is
>> also enabled.
>>
>> This allows the PCI and PCMCIA drivers to be enabled without the need
>> for the ISA stuff.
>>
>> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
>> Cc: Ian Abbott <abbotti@xxxxxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> Just minor niggles, can be fixed up later.
>
> [snip]
>> diff --git a/drivers/staging/comedi/drivers/ni_labpc.c b/drivers/staging/comedi/drivers/ni_labpc.c
>> index 96a6837..e8fc6a1 100644
>> --- a/drivers/staging/comedi/drivers/ni_labpc.c
>> +++ b/drivers/staging/comedi/drivers/ni_labpc.c
> [snip]
>> @@ -241,6 +234,7 @@ static inline void labpc_writeb(unsigned int byte, unsigned long address)
>> writeb(byte, (void __iomem *)address);
>> }
>>
>> +#ifdef CONFIG_COMEDI_NI_LABPC_ISA
>
> That #ifdef and the matching #endif isn't required as labpc_boards gets
> optimized out.
True. I didn't consider that. But, I think this #ifdef makes in clearer.
I really would like to split the legacy support out of the common code but
with all the DMA stuff scattered in the driver if might be really messy.
Gathering all the DMA stuff into a single #ifdef block would at least make
it cleaner.
>> static const struct labpc_boardinfo labpc_boards[] = {
>> {
>> .name = "lab-pc-1200",
>> @@ -268,21 +262,8 @@ static const struct labpc_boardinfo labpc_boards[] = {
>> .ai_range_table = &range_labpc_plus_ai,
>> .ai_range_code = labpc_plus_ai_gain_bits,
>> },
>> -#ifdef CONFIG_COMEDI_PCI_DRIVERS
>> - {
>> - .name = "pci-1200",
>> - .device_id = 0x161,
>> - .ai_speed = 10000,
>> - .bustype = pci_bustype,
>> - .register_layout = labpc_1200_layout,
>> - .has_ao = 1,
>> - .ai_range_table = &range_labpc_1200_ai,
>> - .ai_range_code = labpc_1200_ai_gain_bits,
>> - .ai_scan_up = 1,
>> - .has_mmio = 1,
>> - },
>> -#endif
>> };
>> +#endif
>
> [snip]
>> +#ifdef CONFIG_COMEDI_NI_LABPC_ISA
>> static int labpc_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>> {
>> const struct labpc_boardinfo *board = comedi_board(dev);
>
>'board' is now an unused variable here and can be removed.
Ah, overlooked that one. Thanks.
> [snip]
>> -void labpc_common_detach(struct comedi_device *dev)
>> +void labpc_detach(struct comedi_device *dev)
>
> 'labpc_detach' should be declared 'static'.
Ugh.. My bad. Thanks.
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
