From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965051AbdAFVqi (ORCPT ); Fri, 6 Jan 2017 16:46:38 -0500 Received: from mleia.com ([178.79.152.223]:49646 "EHLO mail.mleia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753506AbdAFVqe (ORCPT ); Fri, 6 Jan 2017 16:46:34 -0500 Subject: Re: [PATCH] i2c: core: helper function to detect slave mode To: Luis Oliveira , wsa@the-dreams.de, robh+dt@kernel.org, mark.rutland@arm.com, jarkko.nikula@linux.intel.com, andriy.shevchenko@linux.intel.com, mika.westerberg@linux.intel.com, linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: Cc: Ramiro.Oliveira@synopsys.com, Joao.Pinto@synopsys.com, CARLOS.PALMINHA@synopsys.com From: Vladimir Zapolskiy Message-ID: <73246c4a-504c-52d7-dde4-970a45dca0bd@mleia.com> Date: Fri, 6 Jan 2017 23:46:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-49551924 X-CRM114-CacheID: sfid-20170106_214632_153548_61993529 X-CRM114-Status: GOOD ( 23.83 ) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Luis, On 01/05/2017 07:24 PM, Luis Oliveira wrote: > This function has the purpose of mode detection by checking the > device nodes for a reg matching with the I2C_OWN_SLAVE_ADDREESS flag. > Currently only checks using OF functions (ACPI slave not supported yet). > > Signed-off-by: Luis Oliveira > --- > Due to the need of checking if the I2C slave address is our own (in > other words: if we are the I2C slave) I created a helper function > (proposed to me by @Andy) to enable that check. > Currently (because I am not able to test it using ACPI) it only > supports devicetree declarations. > > drivers/i2c/i2c-core.c | 19 +++++++++++++++++++ > include/linux/i2c.h | 1 + > 2 files changed, 20 insertions(+) > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c > index 3de95a29024c..48e705b23c59 100644 > --- a/drivers/i2c/i2c-core.c > +++ b/drivers/i2c/i2c-core.c > @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client) > return ret; > } > EXPORT_SYMBOL_GPL(i2c_slave_unregister); > + > +int i2c_slave_mode_detect(struct device *dev) > +{ > + struct device_node *child; > + u32 reg; > + > + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) { IS_BUILTIN(CONFIG_OF) looks excessive, check for non-NULL dev->of_node should be sufficient. But then you may do for_each_child_of_node() loop totally unconditionally, because of_get_next_child() correctly handle NULL as the first argument. > + for_each_child_of_node(dev->of_node, child) { > + of_property_read_u32(child, "reg", ®); > + if (reg & I2C_OWN_SLAVE_ADDRESS) > + return 1; Leaked incremented reference to a child device node, please add of_node_put(child) before return in the loop body. Also reg variable may be uninitialized here, if child device node does not have "reg" property. > + } > + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) { > + dev_dbg(dev, "ACPI slave is not supported yet\n"); > + } If so, then it might be better to drop else-if stub for now. > + return 0; > +} > +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect); > + > #endif > > MODULE_AUTHOR("Simon G. Vogl "); > diff --git a/include/linux/i2c.h b/include/linux/i2c.h > index b2109c522dec..53cf99569af5 100644 > --- a/include/linux/i2c.h > +++ b/include/linux/i2c.h > @@ -282,6 +282,7 @@ enum i2c_slave_event { > > extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb); > extern int i2c_slave_unregister(struct i2c_client *client); > +extern int i2c_slave_mode_detect(struct device *dev); > > static inline int i2c_slave_event(struct i2c_client *client, > enum i2c_slave_event event, u8 *val) > -- With best wishes, Vladimir