[PATCH 05/11] staging: comedi: core: initialize subdevice s->io_bits in postconfig

Hartley Sweeten HartleyS at visionengravers.com
Thu Aug 29 16:43:49 UTC 2013


Om Thursday, August 29, 2013 5:40 AM, Ian Abbott wrote:
> On 2013-08-28 21:29, H Hartley Sweeten wrote:
>> The subdevice 'io_bits' is a bit mask of the i/o configuration for
>> digital subdevices. '0' values indicate that a channel is configured
>> as an input and '1' values that the channel is an output. Since the
>> subdevice data is kzalloc()'d, all channels default as inputs.
>>
>> Modify __comedi_device_postconfig() so that the 'io_bits' are correctly
>> initialized for Digital Output subdevices.
>>
>> Remove all the unnecessary initializations of 's->io_bits' from the
>> drivers. Also, remove the unnecessary initialization of the 's->state'.
>>
>> Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
>> Cc: Ian Abbott <abbotti at mev.co.uk>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> ---
>>   drivers/staging/comedi/drivers.c                       | 7 +++++++
>>   drivers/staging/comedi/drivers/8255.c                  | 3 ---
>>   drivers/staging/comedi/drivers/addi-data/addi_common.c | 2 --
>>   drivers/staging/comedi/drivers/addi_apci_3120.c        | 2 --
>>   drivers/staging/comedi/drivers/adl_pci6208.c           | 1 -
>>   drivers/staging/comedi/drivers/adl_pci9118.c           | 2 --
>>   drivers/staging/comedi/drivers/adv_pci1710.c           | 4 ----
>>   drivers/staging/comedi/drivers/amplc_dio200_common.c   | 2 --
>>   drivers/staging/comedi/drivers/icp_multi.c             | 3 ---
>>   drivers/staging/comedi/drivers/me_daq.c                | 1 -
>>   drivers/staging/comedi/drivers/ni_660x.c               | 1 -
>>   drivers/staging/comedi/drivers/ni_daq_700.c            | 1 -
>>   12 files changed, 7 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
>> index 2bbd9d0..7477514 100644
>> --- a/drivers/staging/comedi/drivers.c
>> +++ b/drivers/staging/comedi/drivers.c
>> @@ -310,6 +310,13 @@ static int __comedi_device_postconfig(struct comedi_device *dev)
>>   		if (s->type == COMEDI_SUBD_UNUSED)
>>   			continue;
>>
>> +		if (s->type == COMEDI_SUBD_DO) {
>> +			if (s->n_chan < 32)
>> +				s->io_bits = (1 << s->n_chan) - 1;
>> +			else
>> +				s->io_bits = 0xffffffff;
>> +		}
>> +
>>   		if (s->len_chanlist == 0)
>>   			s->len_chanlist = 1;
>>
>
> You don't really need to test s->n_chan < 32.  For s->n_chan >= 32, 
> s->io_bits will end up set to 0xffffffff anyway (and for s->n_chan > 32, 
> the low-level drivers shouldn't really be using s->state and s->io_bits 
> anyway).

I wasn't sure about that. 1 << 32 overflows the unsigned int to 0x100000000.
But I guess the 32-bit portion (0x00000000) - 1 is still 0xffffffff.

And, yes, drivers with > 32 channels need to handle the state and io_bits
internally and cannot use the subdevice variables. A comment in the struct
definition about this might a a good idea.

Regards,
Hartley



More information about the devel mailing list