Messages in this thread | | | From | "AnilKumar, Chimata" <> | Subject | RE: [PATCH] lis3lv02d: Add STMicroelectronics lis33ldlh digital | Date | Wed, 18 Jan 2012 07:51:36 +0000 |
| |
Hi Andrew,
Thanks for reviewing the patch
On Wed, Jan 18, 2012 at 05:17:02, Andrew Morton wrote: > On Tue, 17 Jan 2012 07:32:47 +0000 > "AnilKumar, Chimata" <anilkumar@ti.com> wrote: > > > Hi All, > > > > Recalling the patch, provide the comments if there are any if not please include > > this patch to v3.3 kernel. > > The patch is all mangled by someone's email client. Wordwrapping, mime > crap which my MUA can't decrypt, etc.
My bad, in a hurry I have sent a mail by doing "reply to all" that makes all this non-sense will not repeated again. Thanks for your patience here.
...snip...
> > A pet peeve which I haven't told anyone about. If you've cc'ed someone > on a patch then I want to cc them on the patch too. That means I have > to add their Cc: lines to the changelog. But such Cc: lines include > their real names. By omitting their real names in the above fashion, > you cause extra hassle for me. Ideally, you should add their Cc: to > the changelog as well as to the mail headers, to give thsoe people the > best chance of seeing what is happening with the patch. >
Above comment applies here as well.
> > > > .. > > > > +static ssize_t lis3lv02d_range_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + lis3lv02d_sysfs_poweron(&lis3_dev); > > + return sprintf(buf, "%d\n", lis3_dev.g_range); > > +} > > Are these interfaces documented anywhere? If so, please update that > documentation. If not, why not ;)
Yes, these interfaces are documented at "Documentation/misc-devices/lis3lv02d" but range is not included there, which is added in this patch. Will be updated and send it as a separate patch.
> > > @@ -809,15 +881,33 @@ static ssize_t lis3lv02d_rate_set(struct device *dev, > > return count; > > } > > > > +static ssize_t lis3lv02d_range_set(struct device *dev, > > + struct device_attribute *attr, const char *buf, > > + size_t count) > > +{ > > + unsigned long range; > > + > > + if (strict_strtoul(buf, 0, &range)) > > checkpatch would have told you that strict_strtoul() is deprecated. > Please always use checkpatch. >
Somehow I missed this, I will update with "kstrtoul". I have to update the same API in other set function.
>
Thanks AnilKumar
| |