On Mon, Jun 3, 2013 at 12:04 PM, Jingoo Han <[email protected]> wrote: > The usage of strict_strtoul() is not preferred, because > strict_strtoul() is obsolete. Thus, kstrtoul() should be > used.
Few comments below. After addressing them take my Reviewed-by: Andy Shevchenko <[email protected]> > --- a/drivers/mfd/ab3100-core.c > +++ b/drivers/mfd/ab3100-core.c > @@ -514,22 +514,20 @@ static ssize_t ab3100_get_set_reg(struct file *file, > /* > * Advance pointer to end of string then terminate > * the register string. This is needed to satisfy > - * the strict_strtoul() function. > + * the kstrtou8() function. > */ > while ((i < buf_size) && (buf[i] != ' ')) > i++; > buf[i] = '\0'; > > - err = strict_strtoul(&buf[regp], 16, &user_reg); > + err = kstrtou8(&buf[regp], 16, &user_reg); > if (err) > return err; > - if (user_reg > 0xff) > - return -EINVAL; > > /* Either we read or we write a register here */ > if (!priv->mode) { > /* Reading */ > - u8 reg = (u8) user_reg; > + u8 reg = user_reg; Since the reg and user_reg have same type I think no harm to drop away this local reg variable. > u8 regvalue; > > ab3100_get_register_interruptible(ab3100, reg, ®value); > @@ -540,7 +538,7 @@ static ssize_t ab3100_get_set_reg(struct file *file, > } else { > int valp; > unsigned long user_value; > - u8 reg = (u8) user_reg; > + u8 reg = user_reg; Same idea. > u8 value; > u8 regvalue; > > @@ -557,11 +555,9 @@ static ssize_t ab3100_get_set_reg(struct file *file, > i++; > buf[i] = '\0'; > > - err = strict_strtoul(&buf[valp], 16, &user_value); > + err = kstrtoul(&buf[valp], 16, &user_value); kstrtou8() as well (see below) > if (err) > return err; > - if (user_reg > 0xff) > - return -EINVAL; It seems a bug, and further the u8 is used for value as well. > value = (u8) user_value; Thus, you can easily use u8 type for user_value and drop off val variable. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
