[PATCH 08/11] staging: comedi: rtd520: tidy up rtd_ao_winsn()

[Ian Abbott]

On 24/09/15 18:43, Ian Abbott wrote:
> On 24/09/15 18:20, Hartley Sweeten wrote:
>> I guess the sign bit would also need to be extended for the bipolar
>> values. So:
>>
>>        for (i = 0; i < insn->n; ++i) {
>>         unsigned int val = data[i];
>>
>>         /* bipolar ranges use 2's complement values */
>>         if (comedi_range_is_bipolar(s, range)) {
>>             val = comedi_offset_munge(s, val);
>>             /* extend the sign bit */
>>             if (val > 2048)
>>                 val |= 0x1000;
>>         }
>>
>>         /* shift 12-bit data (+sign) to match the register */
>>         val <<= 3;
>>
>> How does that look?
>
> It looks okay except that the test for extending the sign bit should be
> 'if (val >= 2048)'.  You could also avoid the conditional and extend the
> bit regardless:
>
>              val += (val & 0x800);

Although that assumes that val is in range to start with, otherwise the 
carry from the '+' might propagate too far.  There are various ways to 
fix that, e.g.:

		val |= (val & 0x800) << 1;

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-