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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
