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

On Thu, Oct 11, 2012 at 5:41 AM, Christopher Heiny [off-list ref] wrote:

Linus Walleij wrote:

quoted

But please use arithmetic operators (I think I said this on the last
review):

dest[0] = src & 0xFF;
dest[1] = src >> 8;

Doing it the above way makes artithmetic look like maths, and it isn't.
Besides it's done this way in most parts of the kernel and we're
familiar with it.

Yes, you mentioned it previously.  I'm somewhat paranoid, though, and
don't trust the shift/mask method to work correctly on big-endian
machines.  If the shifts can be relied on to behave (I'm guessing the
answer is "yes", since you say this idiom is used widely in the
kernel), then I'll change it.

If the behaviour was not consistent across different endianness
it would not be part of the C language specification...

<< means shift left in the accumulator or whatever you have.
It will work the same no matter how bits are laid out in
memory.

quoted

quoted

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

See Dmitry's comment ...

Basically my stance is that you should not lower yourself to the
level of others not getting the point of your technical solution
by making unelegant compromises, what
you should do is to bring them up to your level so they
understand that your solution is elegant.

Maybe a bit utopist I know...

Yours,
Linus Walleij