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@xxxxxxxxxxxxxxxxxxx>
>> Cc: Ian Abbott <abbotti@xxxxxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> ---
>> 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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
