linux-kernel - Re: [PATCH] i2c: core: helper function to detect slave mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <475d9c2a-da11-97d0-d0b6-37ccd4990f18@synopsys.com>
Date:   Fri, 6 Jan 2017 17:15:03 +0000
From:   Luis Oliveira <Luis.Oliveira@...opsys.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Luis Oliveira <Luis.Oliveira@...opsys.com>
CC:     Wolfram Sang <wsa@...-dreams.de>, Rob Herring <robh+dt@...nel.org>,
        "Mark Rutland" <mark.rutland@....com>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        <linux-i2c@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        <Ramiro.Oliveira@...opsys.com>,
        Joao Pinto <Joao.Pinto@...opsys.com>,
        <CARLOS.PALMINHA@...opsys.com>
Subject: Re: [PATCH] i2c: core: helper function to detect slave mode

On 06-Jan-17 16:29, Andy Shevchenko wrote:
> On Thu, Jan 5, 2017 at 7:24 PM, Luis Oliveira
> <Luis.Oliveira@...opsys.com> 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 <andy.shevchenko@...il.com>
> 
> 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", &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);
>