Re: [RFC PATCH 01/06] input/rmi4: Public header and documentation

From: Christopher Heiny
Date: Tue Oct 23 2012 - 18:55:03 EST


On 10/11/2012 01:24 AM, Dmitry Torokhov wrote:
On Thu, Oct 11, 2012 at 03:41:41AM +0000, Christopher Heiny wrote:
>Linus Walleij wrote:
> >On Sat, Oct 6, 2012 at 6:09 AM, Christopher Heiny<cheiny@xxxxxxxxxxxxx> wrote:
> >
> > >+#ifdef CONFIG_RMI4_DEBUG
> > >+/**
> > >+ * Utility routine to handle writes to read-only attributes. Hopefully
> > >+ * this will never happen, but if the user does something stupid, we
> > >don't
> > >+ * want to accept it quietly (which is what can happen if you just put
> > >NULL + * for the attribute's store function).
> > >+ */
> > >+static inline ssize_t rmi_store_error(struct device *dev,
> > >+ struct device_attribute *attr,
> > >+ const char *buf, size_t count)
> > >+{
> > >+ dev_warn(dev,
> > >+ "WARNING: Attempt to write %d characters to read-only
> > >attribute %s.", + count, attr->attr.name);
> > >+ return -EPERM;
> > >+}
> >
> >Here it looks like you're hiding a lot of stuff that should be dev_warn()?
> >Consider my earlier point about dynamic debug.
>
>In previous patch submissions, we always used these warning functions.
>But in the feedback on those patches, we were asked to just make sysfs
>show/store NULL if the attribute is write/read only. However, during
>their development process, our customers want to see the warnings if
>the attributes are accessed incorrectly. So we made these warnings a
>debug option.
>
I think it is the case when customer is not always right. Given that
the attributes are created with S_IRUGO mask how will we even get these
methods to fire?

We were able to get those to fire in earlier kernels under some UIs (such as Android). However, we no longer support those earlier version. I have checked the behavior on up-to-date kernels and UI versions, and everyone seems to handle this correctly. That means we can drop these definitions entirely, so we'll do that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/