[U-Boot] `uclass_find_device_by_name` fails to find the device
Hi everybody, I've noticed that commit 4213609cc7fb78f84b2ea63f4a5691b60d01c248 changes how `uclass.c:uclass_find_device_by_name` function compares the names for finding devices.
Because of this I have an issue when having a DTS and trying to find a device by name.
The `lists.c:lists_bind_fdt` function will bind a device to a device node tree and use the node name as device name.
``` lists.c: int lists_bind_fdt(..) ... name = ofnode_get_name(node); ...
``` Because of this devices will end up having names like: i2c@021a0000, pfuze100@8...
When using `uclass.c:uclass_find_device_by_name` to get a device, before the change, this was not an issue since `strncmp` was used with the length of the name making a call like `uclass_find_device_by_name(pfuze100)` successfully even if the device name was `pfuze100@8`. ``` if (!strncmp(dev->name, name, strlen(name))) { ```
However after the change (which I think is correct), the check fails, resulting in not finding the expected device, since the device name has also the suffix `@...` included.
What's the proper fix here:
1. Changing the `lists.c:lists_bind_fdt` to drop the `@` suffix? Note: Could this be a problem when having the same node names but different addressees in DTS {e.g.foo@1, foo@2}
2. When using `uclass.c:uclass_find_device_by_name` the user should take in consideration that `@..` suffix needs to be added. Note: This could be a problem when same code is used with different DTSs.
3. Changing the comparation in `uclass.c:uclass_find_device_by_name` to ignore the part after `@` for device names.
4. What else?
Regads, Nandor
HI Nandor,
On Wed, 6 Nov 2019 at 06:09, Nandor Han nandor.han@vaisala.com wrote:
Hi everybody, I've noticed that commit 4213609cc7fb78f84b2ea63f4a5691b60d01c248 changes how `uclass.c:uclass_find_device_by_name` function compares the names for finding devices.
Because of this I have an issue when having a DTS and trying to find a device by name.
The `lists.c:lists_bind_fdt` function will bind a device to a device node tree and use the node name as device name.
lists.c: int lists_bind_fdt(..) ... name = ofnode_get_name(node); ...Because of this devices will end up having names like: i2c@021a0000, pfuze100@8...
When using `uclass.c:uclass_find_device_by_name` to get a device, before the change, this was not an issue since `strncmp` was used with the length of the name making a call like `uclass_find_device_by_name(pfuze100)` successfully even if the device name was `pfuze100@8`.
if (!strncmp(dev->name, name, strlen(name))) {However after the change (which I think is correct), the check fails, resulting in not finding the expected device, since the device name has also the suffix `@...` included.
What's the proper fix here:
- Changing the `lists.c:lists_bind_fdt` to drop the `@` suffix? Note: Could this be a problem when having the same node names but
different addressees in DTS {e.g.foo@1, foo@2}
- When using `uclass.c:uclass_find_device_by_name` the user should take
in consideration that `@..` suffix needs to be added. Note: This could be a problem when same code is used with different DTSs.
- Changing the comparation in `uclass.c:uclass_find_device_by_name` to
ignore the part after `@` for device names.
- What else?
Can you use a phandle to refer to the device from another node, perhaps a UCLASS_BOARD node if needed? It is not a good idea to use this function.
Alternatively we could add a new function which finds by partial name.
Regards, Simon
Hi Simon and thanks,
On 2019-12-24 17:58, Simon Glass wrote:
<snip>
Can you use a phandle to refer to the device from another node, perhaps a UCLASS_BOARD node if needed? It is not a good idea to use this function.
I'm not sure, I'll have to check that. Anyway `uclass_find_device_by_name` is used by `uclass_get_device_by_name`, which already seems to be used in plenty of places.
Alternatively we could add a new function which finds by partial name.
My fix was to ignore the suffix starting from "@", if that exist, since that's more like metadata...and compare entire string when "@" is missing.
Something like:
uclass_foreach_dev(dev, uc) { - if (!strcmp(dev->name, name)) { + char *at_char = strchr(dev->name, '@'); + int name_length; + + if (at_char) + name_length = at_char - dev->name; + else + name_length = max(strlen(dev->name), strlen(name)); + + if (!strncmp(dev->name, name, name_length)) { *devp = dev; return 0; }
I could probably to prepare an RFC with this change.
Regards, Nandor
Hi Nandor,
On Tue, 7 Jan 2020 at 02:16, Nandor Han nandor.han@vaisala.com wrote:
Hi Simon and thanks,
On 2019-12-24 17:58, Simon Glass wrote:
<snip>
Can you use a phandle to refer to the device from another node, perhaps a UCLASS_BOARD node if needed? It is not a good idea to use this function.
I'm not sure, I'll have to check that.
OK, that is the preferred method.
Anyway `uclass_find_device_by_name` is used by `uclass_get_device_by_name`, which already seems to be used in plenty of places.
Alternatively we could add a new function which finds by partial name.
My fix was to ignore the suffix starting from "@", if that exist, since that's more like metadata...and compare entire string when "@" is missing.
Something like:
uclass_foreach_dev(dev, uc) {
if (!strcmp(dev->name, name)) {
char *at_char = strchr(dev->name, '@');int name_length;if (at_char)name_length = at_char - dev->name;elsename_length = max(strlen(dev->name), strlen(name));if (!strncmp(dev->name, name, name_length)) { *devp = dev; return 0; }I could probably to prepare an RFC with this change.
I think this is OK as a new function, but do add a comment in the header that it should not be used and phandles should be used instead. Also please remember to add a test.
Regards, Simon
participants (2)
-
Nandor Han -
Simon Glass