[U-Boot,1/4] dm: core: Add of_alias_get_highest_id()

The same functionality was added to Linux for i2c bus registration with this
commit message:

"
of: base: add function to get highest id of an alias stem

I2C supports adding adapters using either a dynamic or fixed id. The
latter is provided by aliases in the DT case. To prevent id collisions
of those two types, install this function which gives us the highest
fixed id, so we can then let the dynamically created ones come after
this highest number.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
"

Add it also to U-Boot for DM I2C support.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/core/of_access.c | 18 ++++++++++++++++++
 include/dm/of_access.h   |  9 +++++++++
 2 files changed, 27 insertions(+)

Hi Simon,

On 18. 01. 19 16:13, Michal Simek wrote:
> The same functionality was added to Linux for i2c bus registration with this
> commit message:
> 
> "
> of: base: add function to get highest id of an alias stem
> 
> I2C supports adding adapters using either a dynamic or fixed id. The
> latter is provided by aliases in the DT case. To prevent id collisions
> of those two types, install this function which gives us the highest
> fixed id, so we can then let the dynamically created ones come after
> this highest number.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> "
> 
> Add it also to U-Boot for DM I2C support.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  drivers/core/of_access.c | 18 ++++++++++++++++++
>  include/dm/of_access.h   |  9 +++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
> index 14c020a687b7..7c2df2354109 100644
> --- a/drivers/core/of_access.c
> +++ b/drivers/core/of_access.c
> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
>  	return id;
>  }
>  
> +int of_alias_get_highest_id(const char *stem)
> +{
> +	struct alias_prop *app;
> +	int id = -ENODEV;
> +
> +	mutex_lock(&of_mutex);
> +	list_for_each_entry(app, &aliases_lookup, link) {
> +		if (strcmp(app->stem, stem) != 0)
> +			continue;
> +
> +		if (app->id > id)
> +			id = app->id;
> +	}
> +	mutex_unlock(&of_mutex);
> +
> +	return id;
> +}
> +
>  struct device_node *of_get_stdout(void)
>  {
>  	return of_stdout;
> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> index 5ed1a0cdb427..5cbfd220bfd4 100644
> --- a/include/dm/of_access.h
> +++ b/include/dm/of_access.h
> @@ -425,6 +425,15 @@ int of_alias_scan(void);
>  int of_alias_get_id(const struct device_node *np, const char *stem);
>  
>  /**
> + * of_alias_get_highest_id - Get highest alias id for the given stem
> + * @stem:	Alias stem to be examined
> + *
> + * The function travels the lookup table to get the highest alias id for the
> + * given alias stem. It returns the alias id if found.
> + */
> +int of_alias_get_highest_id(const char *stem);
> +
> +/**
>   * of_get_stdout() - Get node to use for stdout
>   *
>   * @return node referred to by stdout-path alias, or NULL if none
> 

Can you please review 1/4 and 2/4 patches?

Thanks,
Michal

Hi Michal,

On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>
> The same functionality was added to Linux for i2c bus registration with this
> commit message:
>
> "
> of: base: add function to get highest id of an alias stem
>
> I2C supports adding adapters using either a dynamic or fixed id. The
> latter is provided by aliases in the DT case. To prevent id collisions
> of those two types, install this function which gives us the highest
> fixed id, so we can then let the dynamically created ones come after
> this highest number.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> "
>
> Add it also to U-Boot for DM I2C support.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>  drivers/core/of_access.c | 18 ++++++++++++++++++
>  include/dm/of_access.h   |  9 +++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
> index 14c020a687b7..7c2df2354109 100644
> --- a/drivers/core/of_access.c
> +++ b/drivers/core/of_access.c
> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
>         return id;
>  }
>
> +int of_alias_get_highest_id(const char *stem)
> +{
> +       struct alias_prop *app;
> +       int id = -ENODEV;
> +
> +       mutex_lock(&of_mutex);
> +       list_for_each_entry(app, &aliases_lookup, link) {
> +               if (strcmp(app->stem, stem) != 0)
> +                       continue;
> +
> +               if (app->id > id)
> +                       id = app->id;
> +       }
> +       mutex_unlock(&of_mutex);
> +
> +       return id;
> +}
> +
>  struct device_node *of_get_stdout(void)
>  {
>         return of_stdout;
> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> index 5ed1a0cdb427..5cbfd220bfd4 100644
> --- a/include/dm/of_access.h
> +++ b/include/dm/of_access.h
> @@ -425,6 +425,15 @@ int of_alias_scan(void);
>  int of_alias_get_id(const struct device_node *np, const char *stem);
>
>  /**
> + * of_alias_get_highest_id - Get highest alias id for the given stem
> + * @stem:      Alias stem to be examined
> + *
> + * The function travels the lookup table to get the highest alias id for the
> + * given alias stem. It returns the alias id if found.

@return

> + */
> +int of_alias_get_highest_id(const char *stem);
> +
> +/**
>   * of_get_stdout() - Get node to use for stdout
>   *
>   * @return node referred to by stdout-path alias, or NULL if none
> --
> 1.9.1
>

Can we place have a test that calls this for a few values?

Also see uclass_find_next_free_req_seq() and uclass_resolve_seq()
which is what U-Boot does today. It is first in, first served. This is
supposed to mean that devices without an alias don't get a req_seq
value, meaning that the seq value is set to the next available value.

I suspect it is OK to change the behaviour though. It might affect
some boards which rely on not having aliases but still getting bus
numbers.

Regards,
Simon

On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
> 
> On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> The same functionality was added to Linux for i2c bus registration with this
>> commit message:
>>
>> "
>> of: base: add function to get highest id of an alias stem
>>
>> I2C supports adding adapters using either a dynamic or fixed id. The
>> latter is provided by aliases in the DT case. To prevent id collisions
>> of those two types, install this function which gives us the highest
>> fixed id, so we can then let the dynamically created ones come after
>> this highest number.
>>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
>> "
>>
>> Add it also to U-Boot for DM I2C support.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  drivers/core/of_access.c | 18 ++++++++++++++++++
>>  include/dm/of_access.h   |  9 +++++++++
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
>> index 14c020a687b7..7c2df2354109 100644
>> --- a/drivers/core/of_access.c
>> +++ b/drivers/core/of_access.c
>> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
>>         return id;
>>  }
>>
>> +int of_alias_get_highest_id(const char *stem)
>> +{
>> +       struct alias_prop *app;
>> +       int id = -ENODEV;
>> +
>> +       mutex_lock(&of_mutex);
>> +       list_for_each_entry(app, &aliases_lookup, link) {
>> +               if (strcmp(app->stem, stem) != 0)
>> +                       continue;
>> +
>> +               if (app->id > id)
>> +                       id = app->id;
>> +       }
>> +       mutex_unlock(&of_mutex);
>> +
>> +       return id;
>> +}
>> +
>>  struct device_node *of_get_stdout(void)
>>  {
>>         return of_stdout;
>> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
>> index 5ed1a0cdb427..5cbfd220bfd4 100644
>> --- a/include/dm/of_access.h
>> +++ b/include/dm/of_access.h
>> @@ -425,6 +425,15 @@ int of_alias_scan(void);
>>  int of_alias_get_id(const struct device_node *np, const char *stem);
>>
>>  /**
>> + * of_alias_get_highest_id - Get highest alias id for the given stem
>> + * @stem:      Alias stem to be examined
>> + *
>> + * The function travels the lookup table to get the highest alias id for the
>> + * given alias stem. It returns the alias id if found.
> 
> @return
> 
>> + */
>> +int of_alias_get_highest_id(const char *stem);
>> +
>> +/**
>>   * of_get_stdout() - Get node to use for stdout
>>   *
>>   * @return node referred to by stdout-path alias, or NULL if none
>> --
>> 1.9.1
>>
> 
> Can we place have a test that calls this for a few values?
> 
> Also see uclass_find_next_free_req_seq() and uclass_resolve_seq()
> which is what U-Boot does today. It is first in, first served. This is
> supposed to mean that devices without an alias don't get a req_seq
> value, meaning that the seq value is set to the next available value.
> 

Let me play with it and see that calling sequence.


> I suspect it is OK to change the behaviour though. It might affect
> some boards which rely on not having aliases but still getting bus
> numbers.

Do you mean with I2C? Or different buses?

I don't think there is any affected board with DM_I2C and OF_CONTROL
because unlisted aliases are using req_seq -1 now.

Thanks,
Michal

Hi Michal,

On Thu, 31 Jan 2019 at 03:19, Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 31. 01. 19 11:04, Simon Glass wrote:
> > Hi Michal,
> >
> > On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com>
wrote:
> >>
> >> The same functionality was added to Linux for i2c bus registration
with this
> >> commit message:
> >>
> >> "
> >> of: base: add function to get highest id of an alias stem
> >>
> >> I2C supports adding adapters using either a dynamic or fixed id. The
> >> latter is provided by aliases in the DT case. To prevent id collisions
> >> of those two types, install this function which gives us the highest
> >> fixed id, so we can then let the dynamically created ones come after
> >> this highest number.
> >>
> >> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> >> "
> >>
> >> Add it also to U-Boot for DM I2C support.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >>  drivers/core/of_access.c | 18 ++++++++++++++++++
> >>  include/dm/of_access.h   |  9 +++++++++
> >>  2 files changed, 27 insertions(+)
> >>
> >> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
> >> index 14c020a687b7..7c2df2354109 100644
> >> --- a/drivers/core/of_access.c
> >> +++ b/drivers/core/of_access.c
> >> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np,
const char *stem)
> >>         return id;
> >>  }
> >>
> >> +int of_alias_get_highest_id(const char *stem)
> >> +{
> >> +       struct alias_prop *app;
> >> +       int id = -ENODEV;
> >> +
> >> +       mutex_lock(&of_mutex);
> >> +       list_for_each_entry(app, &aliases_lookup, link) {
> >> +               if (strcmp(app->stem, stem) != 0)
> >> +                       continue;
> >> +
> >> +               if (app->id > id)
> >> +                       id = app->id;
> >> +       }
> >> +       mutex_unlock(&of_mutex);
> >> +
> >> +       return id;
> >> +}
> >> +
> >>  struct device_node *of_get_stdout(void)
> >>  {
> >>         return of_stdout;
> >> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> >> index 5ed1a0cdb427..5cbfd220bfd4 100644
> >> --- a/include/dm/of_access.h
> >> +++ b/include/dm/of_access.h
> >> @@ -425,6 +425,15 @@ int of_alias_scan(void);
> >>  int of_alias_get_id(const struct device_node *np, const char *stem);
> >>
> >>  /**
> >> + * of_alias_get_highest_id - Get highest alias id for the given stem
> >> + * @stem:      Alias stem to be examined
> >> + *
> >> + * The function travels the lookup table to get the highest alias id
for the
> >> + * given alias stem. It returns the alias id if found.
> >
> > @return
> >
> >> + */
> >> +int of_alias_get_highest_id(const char *stem);
> >> +
> >> +/**
> >>   * of_get_stdout() - Get node to use for stdout
> >>   *
> >>   * @return node referred to by stdout-path alias, or NULL if none
> >> --
> >> 1.9.1
> >>
> >
> > Can we place have a test that calls this for a few values?
> >
> > Also see uclass_find_next_free_req_seq() and uclass_resolve_seq()
> > which is what U-Boot does today. It is first in, first served. This is
> > supposed to mean that devices without an alias don't get a req_seq
> > value, meaning that the seq value is set to the next available value.
> >
>
> Let me play with it and see that calling sequence.
>
>
> > I suspect it is OK to change the behaviour though. It might affect
> > some boards which rely on not having aliases but still getting bus
> > numbers.
>
> Do you mean with I2C? Or different buses?
>
> I don't think there is any affected board with DM_I2C and OF_CONTROL
> because unlisted aliases are using req_seq -1 now.

Yes I think that is right, actually.

Regards,
SImon