diff mbox

[U-Boot,v2,07/18] core: device: Add dev_get_by_ofname function

Message ID 20170111150102.7399-8-mario.six@gdsys.cc
State Superseded
Delegated to: Stefan Roese
Headers show

Commit Message

Mario Six Jan. 11, 2017, 3 p.m. UTC
On boards that use DM, it is sometimes convenient and quicker to get a device
via its device tree path, since the devices used in the board initialization
routines are usually known beforehand.

This patch adds such a convenience function.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
---
Changes in v2:

New in v2
---
 drivers/core/device.c | 7 +++++++
 include/dm/device.h   | 8 ++++++++
 2 files changed, 15 insertions(+)

--
2.9.0

Comments

Stefan Roese Jan. 18, 2017, 4:23 p.m. UTC | #1
On 11.01.2017 16:00, Mario Six wrote:
> On boards that use DM, it is sometimes convenient and quicker to get a device
> via its device tree path, since the devices used in the board initialization
> routines are usually known beforehand.
>
> This patch adds such a convenience function.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
> Changes in v2:
>
> New in v2
> ---
>  drivers/core/device.c | 7 +++++++
>  include/dm/device.h   | 8 ++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index ed553d7..39d30b3 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -627,6 +627,13 @@ const char *dev_get_uclass_name(struct udevice *dev)
>  	return dev->uclass->uc_drv->name;
>  }
>
> +void dev_get_by_ofname(char *ofname, struct udevice *dev)
> +{
> +	int offset = fdt_path_offset(gd->fdt_blob, ofname);
> +
> +	device_get_global_by_of_offset(offset, &dev);
> +}
> +
>  fdt_addr_t dev_get_addr_index(struct udevice *dev, int index)
>  {
>  #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 9948bd4..402482c 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -340,6 +340,14 @@ enum uclass_id device_get_uclass_id(struct udevice *dev);
>  const char *dev_get_uclass_name(struct udevice *dev);
>
>  /**
> + * dev_get_by_ofname() - find a device via its device tree path
> + *
> + * @ofname: Device tree path to use ("/path1/path2/...")
> + * @dev:    The found device or NULL
> + */
> +void dev_get_by_ofname(char *ofname, struct udevice *dev);

I'm not 100% sure if the function naming is perfect here. Usually
these dev_get_xxx function work on a "dev" provided by the caller
and return something else (addr, size). The functions returning a
"device" are called device_get_xxx. And perhaps its also better stick 
with using "struct udevice **devp" as parameter for
consistency as the other device_ functions do.

Simon, what do you think?

Thanks,
Stefan
Simon Glass Jan. 19, 2017, 1:57 p.m. UTC | #2
Hi,

On 18 January 2017 at 09:23, Stefan Roese <sr@denx.de> wrote:
> On 11.01.2017 16:00, Mario Six wrote:
>>
>> On boards that use DM, it is sometimes convenient and quicker to get a
>> device
>> via its device tree path, since the devices used in the board
>> initialization
>> routines are usually known beforehand.
>>
>> This patch adds such a convenience function.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>> ---
>> Changes in v2:
>>
>> New in v2
>> ---
>>  drivers/core/device.c | 7 +++++++
>>  include/dm/device.h   | 8 ++++++++
>>  2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index ed553d7..39d30b3 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -627,6 +627,13 @@ const char *dev_get_uclass_name(struct udevice *dev)
>>         return dev->uclass->uc_drv->name;
>>  }
>>
>> +void dev_get_by_ofname(char *ofname, struct udevice *dev)
>> +{
>> +       int offset = fdt_path_offset(gd->fdt_blob, ofname);
>> +
>> +       device_get_global_by_of_offset(offset, &dev);
>> +}
>> +
>>  fdt_addr_t dev_get_addr_index(struct udevice *dev, int index)
>>  {
>>  #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>> diff --git a/include/dm/device.h b/include/dm/device.h
>> index 9948bd4..402482c 100644
>> --- a/include/dm/device.h
>> +++ b/include/dm/device.h
>> @@ -340,6 +340,14 @@ enum uclass_id device_get_uclass_id(struct udevice
>> *dev);
>>  const char *dev_get_uclass_name(struct udevice *dev);
>>
>>  /**
>> + * dev_get_by_ofname() - find a device via its device tree path
>> + *
>> + * @ofname: Device tree path to use ("/path1/path2/...")
>> + * @dev:    The found device or NULL
>> + */
>> +void dev_get_by_ofname(char *ofname, struct udevice *dev);
>
>
> I'm not 100% sure if the function naming is perfect here. Usually
> these dev_get_xxx function work on a "dev" provided by the caller
> and return something else (addr, size). The functions returning a
> "device" are called device_get_xxx. And perhaps its also better stick with
> using "struct udevice **devp" as parameter for
> consistency as the other device_ functions do.
>
> Simon, what do you think?

Agreed.

Also I'm not sure we should be hunting around a path in the DT.
Shouldn't we use compatible strings?

Regards,
Simon
diff mbox

Patch

diff --git a/drivers/core/device.c b/drivers/core/device.c
index ed553d7..39d30b3 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -627,6 +627,13 @@  const char *dev_get_uclass_name(struct udevice *dev)
 	return dev->uclass->uc_drv->name;
 }

+void dev_get_by_ofname(char *ofname, struct udevice *dev)
+{
+	int offset = fdt_path_offset(gd->fdt_blob, ofname);
+
+	device_get_global_by_of_offset(offset, &dev);
+}
+
 fdt_addr_t dev_get_addr_index(struct udevice *dev, int index)
 {
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
diff --git a/include/dm/device.h b/include/dm/device.h
index 9948bd4..402482c 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -340,6 +340,14 @@  enum uclass_id device_get_uclass_id(struct udevice *dev);
 const char *dev_get_uclass_name(struct udevice *dev);

 /**
+ * dev_get_by_ofname() - find a device via its device tree path
+ *
+ * @ofname: Device tree path to use ("/path1/path2/...")
+ * @dev:    The found device or NULL
+ */
+void dev_get_by_ofname(char *ofname, struct udevice *dev);
+
+/**
  * device_get_child() - Get the child of a device by index
  *
  * Returns the numbered child, 0 being the first. This does not use