From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752407AbbLLVGK (ORCPT ); Sat, 12 Dec 2015 16:06:10 -0500 Received: from mout.kundenserver.de ([217.72.192.74]:53127 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751640AbbLLVGI (ORCPT ); Sat, 12 Dec 2015 16:06:08 -0500 From: Arnd Bergmann To: Wolfram Sang Cc: kbuild test robot , kbuild-all@01.org, linux-i2c@vger.kernel.org, Niklas =?ISO-8859-1?Q?S=F6derlund?= , linux-sh@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] i2c: allow building emev2 without slave mode again Date: Sat, 12 Dec 2015 22:05:29 +0100 Message-ID: <2157023.1Ftpc9ZciN@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20151212162057.GA1530@katana> References: <201512102224.cVm7Hcp0%fengguang.wu@intel.com> <2592093.cGKEcKVqLU@wuerfel> <20151212162057.GA1530@katana> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:dgxMzAlSfen1oSkAxQE+2zBDw+x1UflECX51G5LRcF0yVoXo+/+ uEhhdvA5Glw9pdCPbuRaocOtRng0mlFpe5QXPOcT8b8H7PDK15iT29vwG6QwTn6EmIBprU5 7pKCWQcT7+rrsK/cvWpRTgoM9Aq22Dk7um+4taT1FSe0G/yQ1zqZAO0WFq3BuPH9e8EGd1f v2cqmC3sMiw161W5PzHag== X-UI-Out-Filterresults: notjunk:1;V01:K0:gcVLj8zoCco=:g/LuGl1BCXye9SNDVyRUVC 6xmWtkEsUGiHf488uSP82buisZM3e+gxDFkIsWmpymvN8NQN9S5E3irCGdW0Aq7b8jVFrhIVS gzcmXbPtdZTHRAdouB/e8c16HoA4gwYha18jKctu7QSrb+yH5cBA/e+EA79qGJBdgh0NsV2+U d6+k16ynu2iCIbuEbWwe3tohdoN+mz7Ji3As9sdZxOEYcJsEvblnooxIshsozP/fPPYAZFjfw 2mCyPt/9sWML3yoOCQgnmRV8imRuJi4a4I6U3WPIYXqpPfW2K79PFeRcVa3+vLugQ+o1XI9wt oemNfqezLJi7dXeDdgfIq+M0HSsbhEMwkbHrPOVKpct4eq0Kt/1f0CFaNLmEoDAOmaoGTvOPf oJOljjdOJo9ETzPQ56o5hpD0Cv21zi4ygXgAnWohKl/Zpl1ZvvnOvVL1bS2xLZvKVTjwMom7c QVbztn2D3drL4yyKl3puIfTNpBKMGYRoeY68Ddhjaujta9IIrD1eJxXhsHWtkEbhiJRz4QORz JO1/+x348HGqDjrIeMYvJETorn6wZ9WadOLR6s1UC28Y6UZZ48On/bjmlSkfP2Q0VjPtKYsIS FVswM9yJT8JhrvJdlNyMXnfKCRz7Lax+N8wF8gLYjflbA6K36Kyw5KKsScAf5kPuY/f3jy2ki a2Li8dNQBHjITr4cyMJYQFA/oxfpnsJvxFOqJufoCaO0m5HRoeeKYDdzUty1Jc7YtftrHplWJ E15qNx3gWVeo5dda Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Saturday 12 December 2015 17:20:57 Wolfram Sang wrote: > Hi Arnd, > > thanks for looking into this, but I don't get your point yet. > > > The slave_cb callback function is supposed to set the 'value' > > here, > > Only if a master wants to READ from us. Right, and can this never fail? > > but it might return an error not assign the pointer, > > An error is only returned if a WRITE from a master was not accepted by > the slave backend. > > > It might be best to change the callback to return 'void' and not > > allow it to fail. > > We need that because in case of an errno, the slave should send NACK to > the master instead of ACK. > > > At least the eeprom slave cannot fail anyway, and it is the only > > implementation we have at the moment. > > True. But giving a slave the possibility to NACK a write should be > present IMO. Ok, fair enough. > > Alternatively, the inline could return an error, and both bus > > drivers check for the error before using 'value'. > > Hum, it does return an error? > > return client->slave_cb(client, event, val); > > You probably mean something else? I mean specifically this code in em_i2c_slave_irq(): /* Send data */ event = status & I2C_BIT_STD0 ? I2C_SLAVE_READ_REQUESTED : I2C_SLAVE_READ_PROCESSED; i2c_slave_event(priv->slave, event, &value); writeb(value, priv->base + I2C_OFS_IIC0); With my current code to turn i2c_slave_event() into an empty inline function in case of !CONFIG_I2C_SLAVE, the compiler knows that "value" is uninitialized at the point where we write it to the register, and warns about it. The code will of course never run if slave mode is not allowed, but we should still shut up the warning, either by making the inline i2c_slave_event set 'value' to zero or 0xff, or by adding an error check: ret = i2c_slave_event(priv->slave, event, &value); if (!ret) writeb(value, priv->base + I2C_OFS_IIC0); and making the empty i2c_slave_event() function return -ENOSYS or -ENOTSUPP. Arnd