[PATCH] [RFC] cmd: i2c: fix default address len for DM_I2C

[Simon Glass]

Hi Tim,

On Thu, 11 Aug 2022 at 11:57, Tim Harvey <tharvey at gateworks.com> wrote:
>
> According to the comment block "The default {addr} parameter is one byte
> (.1) which works well for memories and registers with 8 bits of address
> space."
>
> While this is true for legacy I2C a default length of -1 is being passed
> for DM_I2C which results in a usage error.
>
> Restore the documented behavior by always using a default alen of 1.
>
> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>
> This is an RFC as I'm unclear if we want to restore the legacy usage or
> enforce a new usage (in which case the comment block should be updated)
> and I'm not clear if this is documented in other places. If the decision
> is to enforce a new usage then it is unclear to me how to specifiy the
> default alen as there is no command for that (i2c alen [len]?).
> ---
>  cmd/i2c.c | 10 ----------
>  1 file changed, 10 deletions(-)
>

Can you dig into this a little more on your board? DEFAULT_ADDR_LEN is
set to -1 so that by default it does not get set by the command, and
the existing alen is used.

This is necessary for driver model, since the alen can be set by the
peripheral device and we don't want to overwrite it:

if (!ret && alen != -1)
    ret = i2c_set_chip_offset_len(dev, alen);


> diff --git a/cmd/i2c.c b/cmd/i2c.c
> index bd04b14024be..c57271479e81 100644
> --- a/cmd/i2c.c
> +++ b/cmd/i2c.c
> @@ -118,17 +118,7 @@ static uchar i2c_no_probes[] = CONFIG_SYS_I2C_NOPROBES;
>  #endif
>
>  #define DISP_LINE_LEN  16
> -
> -/*
> - * Default for driver model is to use the chip's existing address length.
> - * For legacy code, this is not stored, so we need to use a suitable
> - * default.
> - */
> -#if CONFIG_IS_ENABLED(DM_I2C)
> -#define DEFAULT_ADDR_LEN       (-1)
> -#else
>  #define DEFAULT_ADDR_LEN       1
> -#endif
>
>  #if CONFIG_IS_ENABLED(DM_I2C)
>  static struct udevice *i2c_cur_bus;
> --
> 2.25.1
>

Regards,
Simon