On 10/11/2012 01:24 AM, Dmitry Torokhov wrote:

On Thu, Oct 11, 2012 at 03:41:41AM +0000, Christopher Heiny wrote:

quoted

quoted

Linus Walleij wrote:

quoted

quoted

On Sat, Oct 6, 2012 at 6:09 AM, Christopher Heiny[off-list ref]  wrote:

quoted

quoted

+#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.