From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Cameron Subject: Re: [PATCH V1 1/1] iio: Added Capella cm3232 ambient light sensor driver. Date: Sat, 10 Jan 2015 14:06:15 +0000 Message-ID: <54B131D7.4040705@kernel.org> References: <1420071030-888-1-git-send-email-ktsai@capellamicro.com> <1420463396.2652.8.camel@perches.com> <1420476166.2652.14.camel@perches.com> <1420480611.2652.19.camel@perches.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1420480611.2652.19.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> Sender: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Joe Perches , Daniel Baluta Cc: Kevin Tsai , Wolfram Sang , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Grant Likely , Andrew Morton , "David S. Miller" , Greg Kroah-Hartman , Mauro Carvalho Chehab , Antti Palosaari , Archana Patni , linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Linux Kernel Mailing List List-Id: devicetree@vger.kernel.org On 05/01/15 17:56, Joe Perches wrote: > On Mon, 2015-01-05 at 19:50 +0200, Daniel Baluta wrote: >> On Mon, Jan 5, 2015 at 6:42 PM, Joe Perches wrote: >>> On Mon, 2015-01-05 at 16:20 +0200, Daniel Baluta wrote: >>>> On Mon, Jan 5, 2015 at 3:09 PM, Joe Perches wrote: >>>>> On Mon, 2015-01-05 at 12:51 +0200, Daniel Baluta wrote: >>>>>> On Thu, Jan 1, 2015 at 2:10 AM, Kevin Tsai wrote: >>>>>>> CM3232 is an advanced ambient light sensor with I2C protocol interface. >>>>>>> The I2C slave address is internally hardwired as 0x10 (7-bit). Writing >>>>>>> to configure register is byte mode, but reading ALS register requests to >>>>>>> use word mode for 16-bit resolution. >>> [] >>>>>> You could directly return i2c_smbus_write_byte_data(..). >>>>> >>>>> Sometimes it's better to return a specific value >>>>> for the error instead of depending on correctness >>>>> of all the indirect functions in the call chain. >>>>> >>>>> In this case, all the smbus_xfer functions must >>>>> return 0 on success. Do they? >>>> >>>> Yes. >>>> >>>> http://lxr.free-electrons.com/source/drivers/i2c/i2c-core.c#L2845 >>> >>> This doesn't show that adapter->algo->smbus_xfer() >>> returns 0, you have to look at the code for that >>> indirectly called function. >> >> I based my answer on the comment at the top of the function: >> >> 2845 * This executes an SMBus protocol operation, and returns a negative >> 2846 * errno code else zero on success. > > Sure, but comments and code often differ and the > implementation of any of those smbus_xfer functions > could return a positive value like the byte value or > the number of bytes written instead of 0. > > For correctness, you'd have to inspect them all. > > If some new future smbus_xfer function was written > incorrectly, the return value from this function could > now be positive. > That would however, clearly be a bug and likely to cause all sorts of issues elsewhere given the documentation requires that it does return 0 on success and people will have been relying on it. Jonathan > cheers, Joe >