Re: [PATCH] staging: iio: accel: remove impossible condition

[Luis de Bethencourt]

On 31/05/16 21:23, Jonathan Cameron wrote:
> 
> 
> On 31 May 2016 20:47:50 BST, Luis de Bethencourt <luisbg@xxxxxxxxxxxxxxx> wrote:
>> val is set to the value of ret right after ret is checked. If ret is
>> not
>> zero it goes to error_ret. So only value ret can have is zero, which
>> makes
>> the switch (val & 0x03) only match the case 0x00. Removing the switch
>> and
>> since val is only used for this, removing val as well.
> There is clearly an issue here. However it looks like it is that if(ret) which is wrong.
> 
> The code as it stands clearly doesn't work as intended. Fixing the bug would be
> 
>  more useful than removing code that 'should' be accessible.
> 
> I happen to fire the relevant hardware up yesterday for the first time in a while so
> 
> can easily verify the operation of a fix if you want to take another look.
> 
> Jonathan

Jonathan and Andrew are right.

sca3000_read_ctrl_reg() returns a negative number on failure.

So line 597 should be: if (ret < 0)

If everything goes well in sca3000_read_ctrl_reg() it returns st->rx[0], which explains
the switch case.

I am going to send a new patch with this fix.

Thanks so much for the review and sorry for the initial confusion,
Luis