Re: [PATCH Resend V2] dt: add helper function to read u8 & u16variables & arrays

[viresh kumar]

On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring <robherring2@xxxxxxxxx> wrote:
>> +#define of_property_read_array(_np, _pname, _out, _sz)                       \

>> +     while (_sz--)                                                   \
>> +             *_out++ = (typeof(*_out))be32_to_cpup(_val++);          \

> This will not work. You are incrementing _out by 1, 2, or 4 bytes, but
> _val is always incremented by 4 bytes.
>
> According to the dtc commit adding this feature, the values are packed:
>
> With this patch the following property assignment:
>
>     property = /bits/ 16 <0x1234 0x5678 0x0 0xffff>;
>
> is equivalent to:
>
>     property = <0x12345678 0x0000ffff>;

I thought of it a bit more and wasn't actually aligned with your explanation :(

If that is the case, how will current implementation of u32 array will work
if we pass something like: 0x88 0x84000000 0x5890 from DT?

So, i did a dummy test of my current implementation, with following changes
in one of my drivers:

dts changes:

                cluster0: cluster@0 {
+                       data1 = <0x50 0x60 0x70>;
+                       data2 = <0x5000 0x6000 0x7000>;
+                       data3 = <0x50000000 0x60000000 0x70000000>;
               }

driver changes:

+void test(struct device_node *cluster)
+{
+       u8 data1[3];
+       u16 data2[3];
+       u32 data3[3], i;
+
+       of_property_read_u8_array(cluster, "data1", data1, 3);
+       of_property_read_u16_array(cluster, "data2", data2, 3);
+       of_property_read_u32_array(cluster, "data3", data3, 3);
+
+       for (i = 0; i < 3; i++) {
+               printk(KERN_INFO "u8 %d: %x\n", i, data1[i]);
+               printk(KERN_INFO "u16 %d: %x\n", i, data2[i]);
+               printk(KERN_INFO "u32 %d: %x\n", i, data3[i]);
+       }
+}

And following is the output

[    4.087205] u8 0: 50
[    4.093746] u16 0: 5000
[    4.101067] u32 0: 50000000
[    4.109512] u8 1: 60
[    4.116036] u16 1: 6000
[    4.123357] u32 1: 60000000
[    4.131718] u8 2: 70
[    4.138241] u16 2: 7000
[    4.145573] u32 2: 70000000

which looks to be what we were looking for, isn't it?

Following is fixup for the doc comment missing:

commit 00803aed0781de451048df0d15a3e8c814a343c8
Author: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Date:   Wed Nov 7 09:48:46 2012 +0530

    fixup! dt: add helper function to read u8 & u16 variables & arrays
---
 drivers/of/base.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index fbb634b..4a6632e 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -669,6 +669,7 @@ EXPORT_SYMBOL(of_find_node_by_phandle);
  * @np:                device node from which the property value is to be read.
  * @propname:  name of the property to be searched.
  * @out_value: pointer to return value, modified only if return value is 0.
+ * @sz:                number of array elements to read
  *
  * Search for a property in a device node and read 8-bit value(s) from
  * it. Returns 0 on success, -EINVAL if the property does not exist,
@@ -690,6 +691,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u8_array);
  * @np:                device node from which the property value is to be read.
  * @propname:  name of the property to be searched.
  * @out_value: pointer to return value, modified only if return value is 0.
+ * @sz:                number of array elements to read
  *
  * Search for a property in a device node and read 16-bit value(s) from
  * it. Returns 0 on success, -EINVAL if the property does not exist,
@@ -712,6 +714,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u16_array);
  * @np:                device node from which the property value is to be read.
  * @propname:  name of the property to be searched.
  * @out_value: pointer to return value, modified only if return value is 0.
+ * @sz:                number of array elements to read
  *
  * Search for a property in a device node and read 32-bit value(s) from
  * it. Returns 0 on success, -EINVAL if the property does not exist,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/