Re: [RFC PATCH v2 5/6] i2c: of: mark a whole array of regs as reserved

From: Luca Ceresoli
Date: Mon Apr 13 2020 - 05:55:21 EST


Hi,

On 18/03/20 16:00, Wolfram Sang wrote:
> Back then, 'reg' properties in I2C DT bindings only contained one
> address and this address was assigned a device and, thus, blocked.
> Meanwhile, chips using multiple addresses are common and the 'reg'
> property can be an array described by 'reg-names'. This code enhances
> I2C DT parsing, so it will reserve all addresses described in an array.
> They will be bound to the 'dummy' driver as 'reserved' iff the first
> address can be assigned successfully. If that is not the case, the array
> is not further considered. If one later address of the array can not be
> assigned, it will be reported but we don't bail out. The driver has to
> decide if that address is critical or not.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/i2c/i2c-core-of.c | 70 +++++++++++++++++++++++++--------------
> 1 file changed, 46 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
> index f2d09ea0d336..67eb2cd305cf 100644
> --- a/drivers/i2c/i2c-core-of.c
> +++ b/drivers/i2c/i2c-core-of.c
> @@ -16,25 +16,18 @@
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/of_device.h>
> #include <linux/sysfs.h>
>
> #include "i2c-core.h"
>
> -int of_i2c_get_board_info(struct device_node *node, struct i2c_board_info *info)
> +static void of_i2c_decode_board_info(struct device_node *node, u32 addr,
> + bool first_addr, struct i2c_board_info *info)

While I confirm the patch looks generally OK, let me add the name of
this function is not quite self-explaining. The difference between "get"
and "decode" has nothing to do with the different actions these
functions do, i.e. the new function gets (or: decodes) info about a
single address that is passed, the old "get" function gets the info for
the first address.

I'd suggest the new function be named of_i2c_get_board_info_one_addr or
similar. Not super nice, a bit long, but self-explanatory.

--
Luca