lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 3 Apr 2014 15:05:10 -0700 (PDT)
From:	Chase Southwood <>
To:	Dan Carpenter <>
Cc:	"" <>,
	"" <>,
	"" <>,
	"" <>
Subject: Re: [PATCH v2] staging: comedi: s626: use comedi_timeout() on remaining loops

>On Thursday, April 3, 2014 3:38 AM, Dan Carpenter <> wrote:

>>On Tue, Mar 25, 2014 at 10:43:58PM -0500, Chase Southwood wrote:
>>There were just a handful of more while loops in this file that needed
>>timeouts, and this patch takes care of them.  One new callback is
>>introduced, and all of the proper comedi_timeout() calls are then used.
>>Signed-off-by: Chase Southwood <>
>>2: s626_i2c_handshake_eoc() can be used in s626_initialize() as noted by
>>Ian.  So, s626_initialize_eoc() has been removed, and its uses swapped
>>for s626_i2c_handshake_eoc().
>> drivers/staging/comedi/drivers/s626.c | 34 ++++++++++++++++++++++++++--------
>> 1 file changed, 26 insertions(+), 8 deletions(-)
>>diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c
>>index 95fadf3..865cf93 100644
>>--- a/drivers/staging/comedi/drivers/s626.c
>>+++ b/drivers/staging/comedi/drivers/s626.c
>>@@ -295,10 +295,24 @@ static void s626_debi_replace(struct comedi_device *dev, unsigned int addr,
>> /* **************  EEPROM ACCESS FUNCTIONS  ************** */
>>+static int s626_i2c_handshake_eoc(struct comedi_device *dev,
>>+                 struct comedi_subdevice *s,
>>+                 struct comedi_insn *insn,
>>+                 unsigned long context)
>>+    unsigned int status;
>This should probably be bool.

Oh, whoops...yeah s626_mc_test() definitely returns bool...I'll fix this up.

>>+    status = s626_mc_test(dev, S626_MC2_UPLD_IIC, S626_P_MC2);
>>+    if (status)
>>+        return 0;
>>+    return -EBUSY;
>> static uint32_t s626_i2c_handshake(struct comedi_device *dev, uint32_t val)
>> {
>>     struct s626_private *devpriv = dev->private;
>>     unsigned int ctrl;
>>+    uint32_t ret;
>This should be int.  I get really suspicious when people start using
>uint32_t types.  Why does it have to be 32 bits?  Unsigned is wrong as

Yeah...I originally did that to conform to the current return type of the function,
not sure how I didn't manage to see that trying to return a negative error code as
an unsigned int is clearly a bug.  Sorry, I'll fix this up as well.

Thanks for the review, Dan.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

Powered by blists - more mailing lists