From patchwork Thu Dec 10 13:51:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 7819041 X-Patchwork-Delegate: geert@linux-m68k.org Return-Path: X-Original-To: patchwork-linux-sh@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 5B92DBEEE1 for ; Thu, 10 Dec 2015 13:52:25 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5469420555 for ; Thu, 10 Dec 2015 13:52:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A7D02201EC for ; Thu, 10 Dec 2015 13:52:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752326AbbLJNvf (ORCPT ); Thu, 10 Dec 2015 08:51:35 -0500 Received: from mout.kundenserver.de ([212.227.17.13]:59934 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751051AbbLJNvc (ORCPT ); Thu, 10 Dec 2015 08:51:32 -0500 Received: from wuerfel.localnet ([134.3.118.24]) by mrelayeu.kundenserver.de (mreue104) with ESMTPSA (Nemesis) id 0MhU26-1ZuSYO0SzS-00McJ9; Thu, 10 Dec 2015 14:51:02 +0100 From: Arnd Bergmann To: Wolfram Sang Cc: 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: Thu, 10 Dec 2015 14:51 +0100 Message-ID: <2319149.G7spjXxTsa@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20151210133446.GC1573@katana> References: <2144222.LfzPx9mJ0p@wuerfel> <20151210133446.GC1573@katana> MIME-Version: 1.0 X-Provags-ID: V03:K0:+J2u+qaFasqT1IpO5r2b9UXmaFSdmOG1h6ovlFS/BrCRddkOTHP MMcDEzsdfIDIAExw2JSN0K8ooniSL3XMd7/eZMrYGqJ/c9mERUtiSsdijVItTFnObDGrvqf GbtoA3hPkXykrmQVGsGrGYSlZevOpAV3Cx4oWggGH7o21xaK8SK/5vxFKNRBlYbierNfIIQ gjuAd1qIXgx8dOY62BM6A== X-UI-Out-Filterresults: notjunk:1; V01:K0:/n/7IAjkS4A=:OECjd6qAoLrQ6ftakX10pT Ux1OLdhQBW+atZ0Ch3jWFrYYtiS2Y9fhbDDTts4/mjREy307PatuTJPlVmXaJKovB9GFCsfeO UgnlO4xoPNUiac7zL2aHWyECgjl7AOA/+IeuX4vQVevy5PECOdJ/RnGryDzDee9yjln1D12rP XXYp/s/+8LP43yB/N2Ug7bwDaJNQav/E5al1AoT5Tppqk4955gFsYvqqjvPEmx6L8W3lvgpxd 6lAOzrowKjE54qeDPeJ/xMsHxu9EsyvAgfkNLiV4iAFIUl/WlZvrQbuaEY/MrvdatvopdEAn8 qqONjcPfvkI7ji73ARjCUYAXXzsgP/M3O14ESRbNCuOdIbR/H+FOmaLe/BT2aNjHAOA/enpHa e+3HNHs5640J2D3BiMbf+Lsi42SjoyQyjg07RP9/le4LGsQIuuzY0Y/v8tUmBunPJORBLsrUQ lCC72uAtvJncP2rTDz6FrxlrfRurGvOYXj590AlH2FX/7YWwI+RHgpVSBaymQ33cqI54VPzzc BM4yvScEc6127pPNEDczlZI/WrIOwTX6ZMtnJlUilOploj/CpyaSw6LlfN7UChQ3ZdxJ92qTu 7XehZBo6Wh5f4nmSjUghUZGjbFHCVkxziy/klu5SpQkIqAgOHBW+4G3ka+kKPmFeuhC0UDcB8 YiL6zqr/q8v4UPHuYWkXP+JIq2KXCOFkGj6ZMG3YEMXFbrUghrr3MRYlsdddaGz730X00foPe FI9yNStko9A0gLZ4 Sender: linux-sh-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sh@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thursday 10 December 2015 14:34:46 Wolfram Sang wrote: > On Thu, Dec 10, 2015 at 02:14:49PM +0100, Arnd Bergmann wrote: > > The emev2 driver stopped compiling in today's linux-next kernel: > > > > drivers/i2c/busses/i2c-emev2.c: In function 'em_i2c_slave_irq': > > drivers/i2c/busses/i2c-emev2.c:233:23: error: storage size of 'event' isn't known > > drivers/i2c/busses/i2c-emev2.c:250:3: error: implicit declaration of function 'i2c_slave_event' [-Werror=implicit-function-declaration] > > drivers/i2c/busses/i2c-emev2.c:250:32: error: 'I2C_SLAVE_STOP' undeclared (first use in this function) > > > > It works again if we enable CONFIG_I2C_SLAVE, but it seems wrong > > to add a dependency on that symbol: > > > > * The symbol is user-selectable, but only one or two (including this > > one) bus drivers actually implement it, and it makes no sense > > if you don't have one of them. > > > > * The other driver (R-Car) uses 'select I2C_SLAVE', which seems > > reasonable in principle, but we should not do that on user > > visible symbols. > > > > * I2C slave mode could be implemented in a lot of other drivers > > as an optional feature, but we shouldn't require enabling it > > if we don't use it. > > > > This changes the two drivers that provide I2C slave mode so they > > can again build if the slave mode being disabled. To do this, I > > move the definition of i2c_slave_event() and enum i2c_slave_event > > out of the #ifdef and instead make the assignment of the reg_slave > > and unreg_slave pointers optional in the bus drivers. The functions > > implementing the feature are unused in that case, so they get > > marked as __maybe_unused in order to still give compile-time > > coverage. > > Thanks a lot! Making this clear and consistent was on my todo-list, > unfortunately below some other items. > > Both drivers have quite orthogonal slave_irq routines. What do you think > about grouping this and the reg/unreg-calls together and compile them > conditionally on I2C_SLAVE? I think the code savings are worth it. Ah, I had missed that part, good point. The change below should take care of leaving out the extra code here. Do you want to just fold that in? Arnd --- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c index 75d6095c5fe1..b01c9f30c545 100644 --- a/drivers/i2c/busses/i2c-emev2.c +++ b/drivers/i2c/busses/i2c-emev2.c @@ -303,7 +303,7 @@ static irqreturn_t em_i2c_irq_handler(int this_irq, void *dev_id) { struct em_i2c_device *priv = dev_id; - if (em_i2c_slave_irq(priv)) + if (IS_ENABLED(CONFIG_I2C_SLAVE) && em_i2c_slave_irq(priv)) return IRQ_HANDLED; complete(&priv->msg_done); diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index e67824adeba0..a1ea66d6267e 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -428,7 +428,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr) /* Only handle interrupts that are currently enabled */ msr &= rcar_i2c_read(priv, ICMIER); if (!msr) { - if (rcar_i2c_slave_irq(priv)) + if (IS_ENABLED(CONFIG_I2C_SLAVE) && rcar_i2c_slave_irq(priv)) return IRQ_HANDLED; return IRQ_NONE;