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

2019-02-01 Thread Simon Glass
Hi Michal,

On Thu, 31 Jan 2019 at 03:19, Michal Simek  wrote:
>
> On 31. 01. 19 11:04, Simon Glass wrote:
> > Hi Michal,
> >
> > On Fri, 18 Jan 2019 at 08: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 
> >> Acked-by: Rob Herring 
> >> Signed-off-by: Wolfram Sang 
> >> "
> >>
> >> Add it also to U-Boot for DM I2C support.
> >>
> >> Signed-off-by: Michal Simek 
> >> ---
> >>
> >>  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(_mutex);
> >> +   list_for_each_entry(app, _lookup, link) {
> >> +   if (strcmp(app->stem, stem) != 0)
> >> +   continue;
> >> +
> >> +   if (app->id > id)
> >> +   id = app->id;
> >> +   }
> >> +   mutex_unlock(_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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-01-31 Thread Michal Simek
On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
> 
> On Fri, 18 Jan 2019 at 08: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 
>> Acked-by: Rob Herring 
>> Signed-off-by: Wolfram Sang 
>> "
>>
>> Add it also to U-Boot for DM I2C support.
>>
>> Signed-off-by: Michal Simek 
>> ---
>>
>>  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(_mutex);
>> +   list_for_each_entry(app, _lookup, link) {
>> +   if (strcmp(app->stem, stem) != 0)
>> +   continue;
>> +
>> +   if (app->id > id)
>> +   id = app->id;
>> +   }
>> +   mutex_unlock(_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

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-01-31 Thread Simon Glass
Hi Michal,

On Fri, 18 Jan 2019 at 08: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 
> Acked-by: Rob Herring 
> Signed-off-by: Wolfram Sang 
> "
>
> Add it also to U-Boot for DM I2C support.
>
> Signed-off-by: Michal Simek 
> ---
>
>  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(_mutex);
> +   list_for_each_entry(app, _lookup, link) {
> +   if (strcmp(app->stem, stem) != 0)
> +   continue;
> +
> +   if (app->id > id)
> +   id = app->id;
> +   }
> +   mutex_unlock(_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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-01-28 Thread Michal Simek
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 
> Acked-by: Rob Herring 
> Signed-off-by: Wolfram Sang 
> "
> 
> Add it also to U-Boot for DM I2C support.
> 
> Signed-off-by: Michal Simek 
> ---
> 
>  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(_mutex);
> + list_for_each_entry(app, _lookup, link) {
> + if (strcmp(app->stem, stem) != 0)
> + continue;
> +
> + if (app->id > id)
> + id = app->id;
> + }
> + mutex_unlock(_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

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs




signature.asc
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot