From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933626AbdAFRPS (ORCPT ); Fri, 6 Jan 2017 12:15:18 -0500 Received: from smtprelay.synopsys.com ([198.182.60.111]:46207 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752570AbdAFRPI (ORCPT ); Fri, 6 Jan 2017 12:15:08 -0500 Subject: Re: [PATCH] i2c: core: helper function to detect slave mode To: Andy Shevchenko , Luis Oliveira References: CC: Wolfram Sang , Rob Herring , "Mark Rutland" , Jarkko Nikula , Andy Shevchenko , Mika Westerberg , , devicetree , "linux-kernel@vger.kernel.org" , , Joao Pinto , From: Luis Oliveira Message-ID: <475d9c2a-da11-97d0-d0b6-37ccd4990f18@synopsys.com> Date: Fri, 6 Jan 2017 17:15:03 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.107.25.129] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06-Jan-17 16:29, Andy Shevchenko wrote: > On Thu, Jan 5, 2017 at 7: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). > > The code looks good, one important comment below, after addressing it: > > Reviewed-by: Andy Shevchenko > > P.S. Btw, we have Suggested-by tag for giving credit for suggestion. > Please use it. > I will do it in the V2, thank you. >> --- a/drivers/i2c/i2c-core.c >> +++ b/drivers/i2c/i2c-core.c >> @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client) > > Please, add kernel doc description here, important thing is to explain > return codes in Return: section of it. > >> +int i2c_slave_mode_detect(struct device *dev) >> +{ > >> + struct device_node *child; >> + u32 reg; > > I would consider them as private to the OF branch. But it's really > minor and up to you (I don't remember if we have such style examples > in i2c core code). I can change that in the V2 too. > >> + >> + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) { >> + for_each_child_of_node(dev->of_node, child) { >> + of_property_read_u32(child, "reg", ®); >> + if (reg & I2C_OWN_SLAVE_ADDRESS) >> + return 1; >> + } >> + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) { >> + dev_dbg(dev, "ACPI slave is not supported yet\n"); >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect); > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis Oliveira Subject: Re: [PATCH] i2c: core: helper function to detect slave mode Date: Fri, 6 Jan 2017 17:15:03 +0000 Message-ID: <475d9c2a-da11-97d0-d0b6-37ccd4990f18@synopsys.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-i2c-owner@vger.kernel.org To: Andy Shevchenko , Luis Oliveira Cc: Wolfram Sang , Rob Herring , Mark Rutland , Jarkko Nikula , Andy Shevchenko , Mika Westerberg , linux-i2c@vger.kernel.org, devicetree , "linux-kernel@vger.kernel.org" , Ramiro.Oliveira@synopsys.com, Joao Pinto , CARLOS.PALMINHA@synopsys.com List-Id: devicetree@vger.kernel.org On 06-Jan-17 16:29, Andy Shevchenko wrote: > On Thu, Jan 5, 2017 at 7: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). > > The code looks good, one important comment below, after addressing it: > > Reviewed-by: Andy Shevchenko > > P.S. Btw, we have Suggested-by tag for giving credit for suggestion. > Please use it. > I will do it in the V2, thank you. >> --- a/drivers/i2c/i2c-core.c >> +++ b/drivers/i2c/i2c-core.c >> @@ -3691,6 +3691,25 @@ int i2c_slave_unregister(struct i2c_client *client) > > Please, add kernel doc description here, important thing is to explain > return codes in Return: section of it. > >> +int i2c_slave_mode_detect(struct device *dev) >> +{ > >> + struct device_node *child; >> + u32 reg; > > I would consider them as private to the OF branch. But it's really > minor and up to you (I don't remember if we have such style examples > in i2c core code). I can change that in the V2 too. > >> + >> + if (IS_BUILTIN(CONFIG_OF) && dev->of_node) { >> + for_each_child_of_node(dev->of_node, child) { >> + of_property_read_u32(child, "reg", ®); >> + if (reg & I2C_OWN_SLAVE_ADDRESS) >> + return 1; >> + } >> + } else if (IS_BUILTIN(CONFIG_ACPI) && ACPI_HANDLE(dev)) { >> + dev_dbg(dev, "ACPI slave is not supported yet\n"); >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(i2c_slave_mode_detect); >