diff mbox

[RESEND,RFC,4/6] device_tree: introduce qemu_fdt_getprop_optional

Message ID 1447946528-1533-5-git-send-email-eric.auger@linaro.org
State New
Headers show

Commit Message

Auger Eric Nov. 19, 2015, 3:22 p.m. UTC 
Current qemu_fdt_getprop exits if the property is not found. It is
sometimes needed to read an optional property, in which case we do
not wish to exit but simply returns a null value.

This is what this new qemu_fdt_getprop_optional function does.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
 device_tree.c                | 17 +++++++++++++++++
 include/sysemu/device_tree.h |  2 ++
 2 files changed, 19 insertions(+)

-- 
1.8.3.2

Comments

Alex Bennée Nov. 26, 2015, 1:13 p.m. UTC | #1 
Eric Auger <eric.auger@linaro.org> writes:

> Current qemu_fdt_getprop exits if the property is not found. It is

> sometimes needed to read an optional property, in which case we do

> not wish to exit but simply returns a null value.

>

> This is what this new qemu_fdt_getprop_optional function does.

>

> Signed-off-by: Eric Auger <eric.auger@linaro.org>


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> ---

>  device_tree.c                | 17 +++++++++++++++++

>  include/sysemu/device_tree.h |  2 ++

>  2 files changed, 19 insertions(+)

>

> diff --git a/device_tree.c b/device_tree.c

> index f184e3c..a318683 100644

> --- a/device_tree.c

> +++ b/device_tree.c

> @@ -280,6 +280,23 @@ const void *qemu_fdt_getprop(void *fdt, const char *node_path,

>      return r;

>  }

>

> +const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,

> +                             const char *property, bool optional, int *lenp)

> +{

> +    int len;

> +    const void *r;

> +    if (!lenp) {

> +        lenp = &len;

> +    }

> +    r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);

> +    if (!r && !optional) {

> +        error_report("%s: Couldn't get %s/%s: %s", __func__,

> +                     node_path, property, fdt_strerror(*lenp));

> +        exit(1);

> +    }

> +    return r;

> +}

> +

>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,

>                                 const char *property)

>  {

> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h

> index f9e6e6e..10cbe8e 100644

> --- a/include/sysemu/device_tree.h

> +++ b/include/sysemu/device_tree.h

> @@ -34,6 +34,8 @@ int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,

>                               const char *target_node_path);

>  const void *qemu_fdt_getprop(void *fdt, const char *node_path,

>                               const char *property, int *lenp);

> +const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,

> +                             const char *property, bool optional, int *lenp);

>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,

>                                 const char *property);

>  uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);



--
Alex Bennée
Auger Eric Dec. 3, 2015, 3:48 p.m. UTC | #2 
Hi Peter,
On 11/27/2015 08:38 PM, Peter Crosthwaite wrote:
> On Thu, Nov 19, 2015 at 7:22 AM, Eric Auger <eric.auger@linaro.org> wrote:

>> Current qemu_fdt_getprop exits if the property is not found. It is

>> sometimes needed to read an optional property, in which case we do

>> not wish to exit but simply returns a null value.

>>

>> This is what this new qemu_fdt_getprop_optional function does.

>>

>> Signed-off-by: Eric Auger <eric.auger@linaro.org>

>> ---

>>  device_tree.c                | 17 +++++++++++++++++

>>  include/sysemu/device_tree.h |  2 ++

>>  2 files changed, 19 insertions(+)

>>

>> diff --git a/device_tree.c b/device_tree.c

>> index f184e3c..a318683 100644

>> --- a/device_tree.c

>> +++ b/device_tree.c

>> @@ -280,6 +280,23 @@ const void *qemu_fdt_getprop(void *fdt, const char *node_path,

>>      return r;

>>  }

>>

>> +const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,

>> +                             const char *property, bool optional, int *lenp)

>> +{

>> +    int len;

>> +    const void *r;

>> +    if (!lenp) {

>> +        lenp = &len;

>> +    }

>> +    r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);

>> +    if (!r && !optional) {

>> +        error_report("%s: Couldn't get %s/%s: %s", __func__,

>> +                     node_path, property, fdt_strerror(*lenp));

>> +        exit(1);

>> +    }

>> +    return r;

>> +}

>> +

> 

> The real problem here is that the device-tree API is self-asserting.

> This looks the old _nofail system that we removed but just named in

> reverse. The correct solution here is to use the Error API properly.

> Convert qemu_fdt_getprop to accept an Error **, and all existing users

> are converted to pass &error_fatal. This will preserve existing

> behaviour. Then to use the API with your optional semantic your pass

> NULL for the Error **.

OK I will implement your proposal. Thanks for your time.

Best Regards

Eric
> 

> Regards,

> Peter

> 

>>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,

>>                                 const char *property)

>>  {

>> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h

>> index f9e6e6e..10cbe8e 100644

>> --- a/include/sysemu/device_tree.h

>> +++ b/include/sysemu/device_tree.h

>> @@ -34,6 +34,8 @@ int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,

>>                               const char *target_node_path);

>>  const void *qemu_fdt_getprop(void *fdt, const char *node_path,

>>                               const char *property, int *lenp);

>> +const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,

>> +                             const char *property, bool optional, int *lenp);

>>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,

>>                                 const char *property);

>>  uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);

>> --

>> 1.8.3.2

>>

>>
diff mbox

Patch

diff --git a/device_tree.c b/device_tree.c
index f184e3c..a318683 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -280,6 +280,23 @@  const void *qemu_fdt_getprop(void *fdt, const char *node_path,
     return r;
 }
 
+const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,
+                             const char *property, bool optional, int *lenp)
+{
+    int len;
+    const void *r;
+    if (!lenp) {
+        lenp = &len;
+    }
+    r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);
+    if (!r && !optional) {
+        error_report("%s: Couldn't get %s/%s: %s", __func__,
+                     node_path, property, fdt_strerror(*lenp));
+        exit(1);
+    }
+    return r;
+}
+
 uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
                                const char *property)
 {
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index f9e6e6e..10cbe8e 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -34,6 +34,8 @@  int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
                              const char *target_node_path);
 const void *qemu_fdt_getprop(void *fdt, const char *node_path,
                              const char *property, int *lenp);
+const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,
+                             const char *property, bool optional, int *lenp);
 uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
                                const char *property);
 uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);