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

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

Linus Walleij wrote:

quoted

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

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?

-- 
Dmitry