Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965358Ab2JWXS3 (ORCPT ); Tue, 23 Oct 2012 19:18:29 -0400 Received: from us-mx3.synaptics.com ([12.239.217.85]:36788 "EHLO us-mx3.synaptics.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933995Ab2JWXS1 (ORCPT ); Tue, 23 Oct 2012 19:18:27 -0400 Message-ID: <508725C1.5090401@synaptics.com> Date: Tue, 23 Oct 2012 16:18:25 -0700 From: Christopher Heiny Organization: Synaptics, Inc User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0 MIME-Version: 1.0 To: Linus Walleij , Mark Brown CC: Anton Vorontsov , Dmitry Torokhov , Jean Delvare , Linux Kernel , Linux Input , Allie Xiong , Vivian Ly , Daniel Rosenberg , Joerie de Gram , Wolfram Sang , Mathieu Poirier , Linus Walleij , Naveen Kumar Gaddipati , Alexandra Chin Subject: Re: [RFC PATCH 01/06] input/rmi4: Public header and documentation References: <1349496603-20775-1-git-send-email-cheiny@synaptics.com> <1349496603-20775-2-git-send-email-cheiny@synaptics.com> In-Reply-To: Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: AAAAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2806 Lines: 69 On 10/11/2012 08:32 AM, Linus Walleij wrote: > On Thu, Oct 11, 2012 at 5:41 AM, Christopher Heiny wrote: >> Linus Walleij wrote: > >>> 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. OK, after reviewing the spec I'll accept that. We'll make the change. >>>> +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... What's the old saying? "I want to live in Theory. Everything is always so nice there..." :-) Anyway, see my reply to Dmitry a bit ago. These are no longer needed, so we'll drop them. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/