Re: [PATCH] i2c: core: helper function to detect slave mode
From: Luis Oliveira
Date: Fri Jan 06 2017 - 12:15:21 EST
On 06-Jan-17 16:29, Andy Shevchenko wrote:
> On Thu, Jan 5, 2017 at 7:24 PM, Luis Oliveira
> <Luis.Oliveira@xxxxxxxxxxxx> 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@xxxxxxxxx>
>
> 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);
>