Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 21:58, Stephen Warren wrote: > Support for byte- and word- properties is relatively recent I believe > (or at least, the /bits/ syntax is). Which dtc version are you using? Ok, i was on a older version. I just saw this patch now: commit cd296721a9645f9f28800a072490fa15458d1fb7 Author: Stephen Warren Date: Fri Sep 28 21:25:59 2012 + dtc: import latest upstream dtc This updates scripts/dtc to commit 317a5d9 "dtc: zero out new label objects" from git://git.jdl.com/software/dtc.git. This adds features such as: * /bits/ syntax for cell data. * Math expressions within cell data. * The ability to delete properties or nodes. * Support for #line directives in the input file, which allows the use of cpp on *.dts. * -i command-line option (/include/ path) * -W/-E command-line options for error/warning control. * Removal of spew to STDOUT containing the filename being compiled. * Many additions to the libfdt API. Signed-off-by: Stephen Warren Acked-by: Jon Loeliger Signed-off-by: Rob Herring Will try it tomorrow -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 11/18/2012 11:41 PM, Viresh Kumar wrote:
> On 19 November 2012 12:05, Rajanikanth HV wrote:
>> On 19 November 2012 12:00, Viresh Kumar wrote:
>>> Firstly you tried square braces [ ], I am not sure if that is allowed.
>>> Can you point me to the specification?
>> http://www.devicetree.org/Device_Tree_Usage
>> "
>> a-byte-data-property = [0x01 0x23 0x34 0x56];
>> "
>
> Ok, but what about 16 bit then {} :)
Support for byte- and word- properties is relatively recent I believe
(or at least, the /bits/ syntax is). Which dtc version are you using?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 11/18/2012 11:41 PM, Viresh Kumar wrote:
On 19 November 2012 12:05, Rajanikanth HV rajanikanth...@linaro.org wrote:
On 19 November 2012 12:00, Viresh Kumar viresh.ku...@linaro.org wrote:
Firstly you tried square braces [ ], I am not sure if that is allowed.
Can you point me to the specification?
http://www.devicetree.org/Device_Tree_Usage
a-byte-data-property = [0x01 0x23 0x34 0x56];
Ok, but what about 16 bit then {} :)
Support for byte- and word- properties is relatively recent I believe
(or at least, the /bits/ syntax is). Which dtc version are you using?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 19 November 2012 21:58, Stephen Warren swar...@wwwdotorg.org wrote: Support for byte- and word- properties is relatively recent I believe (or at least, the /bits/ syntax is). Which dtc version are you using? Ok, i was on a older version. I just saw this patch now: commit cd296721a9645f9f28800a072490fa15458d1fb7 Author: Stephen Warren swar...@nvidia.com Date: Fri Sep 28 21:25:59 2012 + dtc: import latest upstream dtc This updates scripts/dtc to commit 317a5d9 dtc: zero out new label objects from git://git.jdl.com/software/dtc.git. This adds features such as: * /bits/ syntax for cell data. * Math expressions within cell data. * The ability to delete properties or nodes. * Support for #line directives in the input file, which allows the use of cpp on *.dts. * -i command-line option (/include/ path) * -W/-E command-line options for error/warning control. * Removal of spew to STDOUT containing the filename being compiled. * Many additions to the libfdt API. Signed-off-by: Stephen Warren swar...@nvidia.com Acked-by: Jon Loeliger j...@jdl.com Signed-off-by: Rob Herring rob.herr...@calxeda.com Will try it tomorrow -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 12:05, Rajanikanth HV wrote:
> On 19 November 2012 12:00, Viresh Kumar wrote:
>> Firstly you tried square braces [ ], I am not sure if that is allowed.
>> Can you point me to the specification?
> http://www.devicetree.org/Device_Tree_Usage
> "
> a-byte-data-property = [0x01 0x23 0x34 0x56];
> "
Ok, but what about 16 bit then {} :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 12:00, Viresh Kumar wrote: > Firstly you tried square braces [ ], I am not sure if that is allowed. > Can you point me to the specification? http://www.devicetree.org/Device_Tree_Usage " a-byte-data-property = [0x01 0x23 0x34 0x56]; " > > And simply passing 0x50, 0x60 etc.. will make them u32 instead of > u8. i.e. they will be stored as 0x0050, etc. > > -- > viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 11:54, Rajanikanth HV wrote: >> data1 = /bits/ 8 <0x50 0x60 0x70>; > as per spec, format for data byte defines will be: > data1 = [ 0x50 0x60 0x70 ]; > however, i see a parse error from device tree compiler when i tried. Firstly you tried square braces [ ], I am not sure if that is allowed. Can you point me to the specification? And simply passing 0x50, 0x60 etc.. will make them u32 instead of u8. i.e. they will be stored as 0x0050, etc. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 19 November 2012 09:24, Viresh Kumar wrote: > On 12 November 2012 09:03, Viresh Kumar wrote: >> On 12 November 2012 01:12, Rob Herring wrote: >>> I don't think the size is stored in the dtb. It is only in the dts. You >>> need to define the size in the binding definitions and use '/bits/' >>> annotation. With this the data is packed. Then the array function used >>> should match what the binding defines. > > Hi Rob, > > Can you please help me in getting the correct format to do this from dts. > I tried: > > data1 = /bits/ 8 <0x50 0x60 0x70>; as per spec, format for data byte defines will be: data1 = [ 0x50 0x60 0x70 ]; however, i see a parse error from device tree compiler when i tried. > > as written in your earlier mail... but it didn't worked. Tried to > search too, but > couldn't find it. > > ___ > linaro-dev mailing list > linaro-...@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 12 November 2012 09:03, Viresh Kumar wrote: > On 12 November 2012 01:12, Rob Herring wrote: >> I don't think the size is stored in the dtb. It is only in the dts. You >> need to define the size in the binding definitions and use '/bits/' >> annotation. With this the data is packed. Then the array function used >> should match what the binding defines. Hi Rob, Can you please help me in getting the correct format to do this from dts. I tried: data1 = /bits/ 8 <0x50 0x60 0x70>; as written in your earlier mail... but it didn't worked. Tried to search too, but couldn't find it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 12 November 2012 09:03, Viresh Kumar viresh.ku...@linaro.org wrote: On 12 November 2012 01:12, Rob Herring robherri...@gmail.com wrote: I don't think the size is stored in the dtb. It is only in the dts. You need to define the size in the binding definitions and use '/bits/' annotation. With this the data is packed. Then the array function used should match what the binding defines. Hi Rob, Can you please help me in getting the correct format to do this from dts. I tried: data1 = /bits/ 8 0x50 0x60 0x70; as written in your earlier mail... but it didn't worked. Tried to search too, but couldn't find it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 19 November 2012 09:24, Viresh Kumar viresh.ku...@linaro.org wrote: On 12 November 2012 09:03, Viresh Kumar viresh.ku...@linaro.org wrote: On 12 November 2012 01:12, Rob Herring robherri...@gmail.com wrote: I don't think the size is stored in the dtb. It is only in the dts. You need to define the size in the binding definitions and use '/bits/' annotation. With this the data is packed. Then the array function used should match what the binding defines. Hi Rob, Can you please help me in getting the correct format to do this from dts. I tried: data1 = /bits/ 8 0x50 0x60 0x70; as per spec, format for data byte defines will be: data1 = [ 0x50 0x60 0x70 ]; however, i see a parse error from device tree compiler when i tried. as written in your earlier mail... but it didn't worked. Tried to search too, but couldn't find it. ___ linaro-dev mailing list linaro-...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 19 November 2012 11:54, Rajanikanth HV rajanikanth...@linaro.org wrote: data1 = /bits/ 8 0x50 0x60 0x70; as per spec, format for data byte defines will be: data1 = [ 0x50 0x60 0x70 ]; however, i see a parse error from device tree compiler when i tried. Firstly you tried square braces [ ], I am not sure if that is allowed. Can you point me to the specification? And simply passing 0x50, 0x60 etc.. will make them u32 instead of u8. i.e. they will be stored as 0x0050, etc. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 19 November 2012 12:00, Viresh Kumar viresh.ku...@linaro.org wrote: Firstly you tried square braces [ ], I am not sure if that is allowed. Can you point me to the specification? http://www.devicetree.org/Device_Tree_Usage a-byte-data-property = [0x01 0x23 0x34 0x56]; And simply passing 0x50, 0x60 etc.. will make them u32 instead of u8. i.e. they will be stored as 0x0050, etc. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 19 November 2012 12:05, Rajanikanth HV rajanikanth...@linaro.org wrote:
On 19 November 2012 12:00, Viresh Kumar viresh.ku...@linaro.org wrote:
Firstly you tried square braces [ ], I am not sure if that is allowed.
Can you point me to the specification?
http://www.devicetree.org/Device_Tree_Usage
a-byte-data-property = [0x01 0x23 0x34 0x56];
Ok, but what about 16 bit then {} :)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 12 November 2012 01:12, Rob Herring wrote:
> On 11/11/2012 11:27 AM, Viresh Kumar wrote:
>> On 11 November 2012 19:42, Rob Herring wrote:
>>> On 11/06/2012 10:22 PM, viresh kumar wrote:
>>
cluster0: cluster@0 {
+ data1 = <0x50 0x60 0x70>;
+ data2 = <0x5000 0x6000 0x7000>;
+ data3 = <0x5000 0x6000 0x7000>;
>>>
>>> So there is a mismatch in our assumptions. You are just truncating
>>> 32-bit values. I assumed you were using the 8 and 16 bit sizes that are
>>> now supported in dts. I don't think we should just truncate values
>>> blindly. We have support for specifying 8 and 16 values now so you
>>> should use that and define that as part of a binding.
>>
>> Sorry couldn't get your point at all :(
>> What did you mean by "truncating 32 bit values" and how should we
>> tell via DT, that the value passed is 8 bit, 16 bit or 32 bit?
>>
>
> You are trying to retrieve an array of 8 or 16-bit values which are
> stored as 32-bit values in dtb. Why not define them in the binding as 8
> or 16 bit to begin with. Then there is never any ambiguity about their size.
>
> I don't think the size is stored in the dtb. It is only in the dts. You
> need to define the size in the binding definitions and use '/bits/'
> annotation. With this the data is packed. Then the array function used
> should match what the binding defines.
Aha, and in that case incrementing address by 4 in my patch will fail. Right?
Will fix it. Thanks for increasing my knowledge on this :)
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 11/11/2012 11:27 AM, Viresh Kumar wrote:
> On 11 November 2012 19:42, Rob Herring wrote:
>> On 11/06/2012 10:22 PM, viresh kumar wrote:
>
>>> cluster0: cluster@0 {
>>> + data1 = <0x50 0x60 0x70>;
>>> + data2 = <0x5000 0x6000 0x7000>;
>>> + data3 = <0x5000 0x6000 0x7000>;
>>
>> So there is a mismatch in our assumptions. You are just truncating
>> 32-bit values. I assumed you were using the 8 and 16 bit sizes that are
>> now supported in dts. I don't think we should just truncate values
>> blindly. We have support for specifying 8 and 16 values now so you
>> should use that and define that as part of a binding.
>
> Sorry couldn't get your point at all :(
> What did you mean by "truncating 32 bit values" and how should we
> tell via DT, that the value passed is 8 bit, 16 bit or 32 bit?
>
You are trying to retrieve an array of 8 or 16-bit values which are
stored as 32-bit values in dtb. Why not define them in the binding as 8
or 16 bit to begin with. Then there is never any ambiguity about their size.
I don't think the size is stored in the dtb. It is only in the dts. You
need to define the size in the binding definitions and use '/bits/'
annotation. With this the data is packed. Then the array function used
should match what the binding defines.
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 11 November 2012 19:42, Rob Herring wrote:
> On 11/06/2012 10:22 PM, viresh kumar wrote:
>> cluster0: cluster@0 {
>> + data1 = <0x50 0x60 0x70>;
>> + data2 = <0x5000 0x6000 0x7000>;
>> + data3 = <0x5000 0x6000 0x7000>;
>
> So there is a mismatch in our assumptions. You are just truncating
> 32-bit values. I assumed you were using the 8 and 16 bit sizes that are
> now supported in dts. I don't think we should just truncate values
> blindly. We have support for specifying 8 and 16 values now so you
> should use that and define that as part of a binding.
Sorry couldn't get your point at all :(
What did you mean by "truncating 32 bit values" and how should we
tell via DT, that the value passed is 8 bit, 16 bit or 32 bit?
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 11/06/2012 10:22 PM, viresh kumar wrote:
> On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring 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 0x>;
>>
>> is equivalent to:
>>
>> property = <0x12345678 0x>;
>
> 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 0x8400 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 = <0x5000 0x6000 0x7000>;
So there is a mismatch in our assumptions. You are just truncating
32-bit values. I assumed you were using the 8 and 16 bit sizes that are
now supported in dts. I don't think we should just truncate values
blindly. We have support for specifying 8 and 16 values now so you
should use that and define that as part of a binding.
Rob
>}
>
> 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: 5000
> [4.109512] u8 1: 60
> [4.116036] u16 1: 6000
> [4.123357] u32 1: 6000
> [4.131718] u8 2: 70
> [4.138241] u16 2: 7000
> [4.145573] u32 2: 7000
>
> 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
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 11/06/2012 10:22 PM, viresh kumar wrote:
On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring robherri...@gmail.com 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 0x;
is equivalent to:
property = 0x12345678 0x;
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 0x8400 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 = 0x5000 0x6000 0x7000;
So there is a mismatch in our assumptions. You are just truncating
32-bit values. I assumed you were using the 8 and 16 bit sizes that are
now supported in dts. I don't think we should just truncate values
blindly. We have support for specifying 8 and 16 values now so you
should use that and define that as part of a binding.
Rob
}
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: 5000
[4.109512] u8 1: 60
[4.116036] u16 1: 6000
[4.123357] u32 1: 6000
[4.131718] u8 2: 70
[4.138241] u16 2: 7000
[4.145573] u32 2: 7000
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.ku...@linaro.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 11 November 2012 19:42, Rob Herring robherri...@gmail.com wrote:
On 11/06/2012 10:22 PM, viresh kumar wrote:
cluster0: cluster@0 {
+ data1 = 0x50 0x60 0x70;
+ data2 = 0x5000 0x6000 0x7000;
+ data3 = 0x5000 0x6000 0x7000;
So there is a mismatch in our assumptions. You are just truncating
32-bit values. I assumed you were using the 8 and 16 bit sizes that are
now supported in dts. I don't think we should just truncate values
blindly. We have support for specifying 8 and 16 values now so you
should use that and define that as part of a binding.
Sorry couldn't get your point at all :(
What did you mean by truncating 32 bit values and how should we
tell via DT, that the value passed is 8 bit, 16 bit or 32 bit?
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 11/11/2012 11:27 AM, Viresh Kumar wrote:
On 11 November 2012 19:42, Rob Herring robherri...@gmail.com wrote:
On 11/06/2012 10:22 PM, viresh kumar wrote:
cluster0: cluster@0 {
+ data1 = 0x50 0x60 0x70;
+ data2 = 0x5000 0x6000 0x7000;
+ data3 = 0x5000 0x6000 0x7000;
So there is a mismatch in our assumptions. You are just truncating
32-bit values. I assumed you were using the 8 and 16 bit sizes that are
now supported in dts. I don't think we should just truncate values
blindly. We have support for specifying 8 and 16 values now so you
should use that and define that as part of a binding.
Sorry couldn't get your point at all :(
What did you mean by truncating 32 bit values and how should we
tell via DT, that the value passed is 8 bit, 16 bit or 32 bit?
You are trying to retrieve an array of 8 or 16-bit values which are
stored as 32-bit values in dtb. Why not define them in the binding as 8
or 16 bit to begin with. Then there is never any ambiguity about their size.
I don't think the size is stored in the dtb. It is only in the dts. You
need to define the size in the binding definitions and use '/bits/'
annotation. With this the data is packed. Then the array function used
should match what the binding defines.
Rob
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 12 November 2012 01:12, Rob Herring robherri...@gmail.com wrote:
On 11/11/2012 11:27 AM, Viresh Kumar wrote:
On 11 November 2012 19:42, Rob Herring robherri...@gmail.com wrote:
On 11/06/2012 10:22 PM, viresh kumar wrote:
cluster0: cluster@0 {
+ data1 = 0x50 0x60 0x70;
+ data2 = 0x5000 0x6000 0x7000;
+ data3 = 0x5000 0x6000 0x7000;
So there is a mismatch in our assumptions. You are just truncating
32-bit values. I assumed you were using the 8 and 16 bit sizes that are
now supported in dts. I don't think we should just truncate values
blindly. We have support for specifying 8 and 16 values now so you
should use that and define that as part of a binding.
Sorry couldn't get your point at all :(
What did you mean by truncating 32 bit values and how should we
tell via DT, that the value passed is 8 bit, 16 bit or 32 bit?
You are trying to retrieve an array of 8 or 16-bit values which are
stored as 32-bit values in dtb. Why not define them in the binding as 8
or 16 bit to begin with. Then there is never any ambiguity about their size.
I don't think the size is stored in the dtb. It is only in the dts. You
need to define the size in the binding definitions and use '/bits/'
annotation. With this the data is packed. Then the array function used
should match what the binding defines.
Aha, and in that case incrementing address by 4 in my patch will fail. Right?
Will fix it. Thanks for increasing my knowledge on this :)
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
Ping!!
On 7 November 2012 09:52, viresh kumar wrote:
> On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring 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 0x>;
>>
>> is equivalent to:
>>
>> property = <0x12345678 0x>;
>
> 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 0x8400 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 = <0x5000 0x6000 0x7000>;
>}
>
> 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: 5000
> [4.109512] u8 1: 60
> [4.116036] u16 1: 6000
> [4.123357] u32 1: 6000
> [4.131718] u8 2: 70
> [4.138241] u16 2: 7000
> [4.145573] u32 2: 7000
>
> 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
> 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,
>
> ___
> linaro-dev mailing list
> linaro-...@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
Ping!!
On 7 November 2012 09:52, viresh kumar viresh.ku...@linaro.org wrote:
On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring robherri...@gmail.com 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 0x;
is equivalent to:
property = 0x12345678 0x;
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 0x8400 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 = 0x5000 0x6000 0x7000;
}
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: 5000
[4.109512] u8 1: 60
[4.116036] u16 1: 6000
[4.123357] u32 1: 6000
[4.131718] u8 2: 70
[4.138241] u16 2: 7000
[4.145573] u32 2: 7000
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.ku...@linaro.org
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,
___
linaro-dev mailing list
linaro-...@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring 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 0x>;
>
> is equivalent to:
>
> property = <0x12345678 0x>;
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 0x8400 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 = <0x5000 0x6000 0x7000>;
}
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: 5000
[ 4.109512] u8 1: 60
[ 4.116036] u16 1: 6000
[ 4.123357] u32 1: 6000
[ 4.131718] u8 2: 70
[ 4.138241] u16 2: 7000
[ 4.145573] u32 2: 7000
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
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH Resend V2] dt: add helper function to read u8 & u16 variables & arrays
On 10/25/2012 11:20 PM, Viresh Kumar wrote:
> This adds following helper routines:
> - of_property_read_u8_array()
> - of_property_read_u16_array()
> - of_property_read_u8()
> - of_property_read_u16()
>
> First two actually share most of the code with of_property_read_u32_array(),
> so
> the common part is taken out into a macro, which can be used by all three
> *_array() routines.
>
> Signed-off-by: Viresh Kumar
> ---
> V1->V2:
> -
> - Use typeof() in of_property_read_array() macro instead of passing type to it
>
> drivers/of/base.c | 73
> +++---
> include/linux/of.h | 30 ++
> 2 files changed, 89 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index af3b22a..039e178 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -670,6 +670,64 @@ struct device_node *of_find_node_by_phandle(phandle
> handle)
> }
> EXPORT_SYMBOL(of_find_node_by_phandle);
>
> +#define of_property_read_array(_np, _pname, _out, _sz)
> \
> + struct property *_prop = of_find_property(_np, _pname, NULL); \
> + const __be32 *_val; \
> + \
> + if (!_prop) \
> + return -EINVAL; \
> + if (!_prop->value) \
> + return -ENODATA;\
> + if ((_sz * sizeof(*_out)) > _prop->length) \
> + return -EOVERFLOW; \
> + \
> + _val = _prop->value;\
> + 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 0x>;
is equivalent to:
property = <0x12345678 0x>;
> + return 0;
> +
> +/**
> + * of_property_read_u8_array - Find and read an array of u8 from a property.
> + *
> + * @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.
> + *
Missing sz
> + * 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,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_value is modified only if a valid u8 value can be decoded.
> + */
> +int of_property_read_u8_array(const struct device_node *np,
> + const char *propname, u8 *out_values, size_t sz)
> +{
> + of_property_read_array(np, propname, out_values, sz);
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u8_array);
> +
> +/**
> + * of_property_read_u16_array - Find and read an array of u16 from a
> property.
> + *
> + * @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.
> + *
Ditto.
> + * 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,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_value is modified only if a valid u16 value can be decoded.
> + */
> +int of_property_read_u16_array(const struct device_node *np,
> + const char *propname, u16 *out_values, size_t sz)
> +{
> + of_property_read_array(np, propname, out_values, sz);
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u16_array);
> +
> /**
> * of_property_read_u32_array - Find and read an array of 32 bit integers
> * from a property.
> @@ -689,20 +747,7 @@ int of_property_read_u32_array(const struct device_node
> *np,
> const char *propname, u32 *out_values,
> size_t sz)
> {
> - struct property *prop = of_find_property(np, propname, NULL);
> - const __be32 *val;
> -
> - if (!prop)
> - return -EINVAL;
> - if (!prop->value)
> - return -ENODATA;
> - if ((sz * sizeof(*out_values)) > prop->length)
> - return -EOVERFLOW;
> -
> - val = prop->value;
> - while (sz--)
> -
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On 10/25/2012 11:20 PM, Viresh Kumar wrote:
This adds following helper routines:
- of_property_read_u8_array()
- of_property_read_u16_array()
- of_property_read_u8()
- of_property_read_u16()
First two actually share most of the code with of_property_read_u32_array(),
so
the common part is taken out into a macro, which can be used by all three
*_array() routines.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
V1-V2:
-
- Use typeof() in of_property_read_array() macro instead of passing type to it
drivers/of/base.c | 73
+++---
include/linux/of.h | 30 ++
2 files changed, 89 insertions(+), 14 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index af3b22a..039e178 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -670,6 +670,64 @@ struct device_node *of_find_node_by_phandle(phandle
handle)
}
EXPORT_SYMBOL(of_find_node_by_phandle);
+#define of_property_read_array(_np, _pname, _out, _sz)
\
+ struct property *_prop = of_find_property(_np, _pname, NULL); \
+ const __be32 *_val; \
+ \
+ if (!_prop) \
+ return -EINVAL; \
+ if (!_prop-value) \
+ return -ENODATA;\
+ if ((_sz * sizeof(*_out)) _prop-length) \
+ return -EOVERFLOW; \
+ \
+ _val = _prop-value;\
+ 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 0x;
is equivalent to:
property = 0x12345678 0x;
+ return 0;
+
+/**
+ * of_property_read_u8_array - Find and read an array of u8 from a property.
+ *
+ * @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.
+ *
Missing sz
+ * 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,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_value is modified only if a valid u8 value can be decoded.
+ */
+int of_property_read_u8_array(const struct device_node *np,
+ const char *propname, u8 *out_values, size_t sz)
+{
+ of_property_read_array(np, propname, out_values, sz);
+}
+EXPORT_SYMBOL_GPL(of_property_read_u8_array);
+
+/**
+ * of_property_read_u16_array - Find and read an array of u16 from a
property.
+ *
+ * @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.
+ *
Ditto.
+ * 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,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_value is modified only if a valid u16 value can be decoded.
+ */
+int of_property_read_u16_array(const struct device_node *np,
+ const char *propname, u16 *out_values, size_t sz)
+{
+ of_property_read_array(np, propname, out_values, sz);
+}
+EXPORT_SYMBOL_GPL(of_property_read_u16_array);
+
/**
* of_property_read_u32_array - Find and read an array of 32 bit integers
* from a property.
@@ -689,20 +747,7 @@ int of_property_read_u32_array(const struct device_node
*np,
const char *propname, u32 *out_values,
size_t sz)
{
- struct property *prop = of_find_property(np, propname, NULL);
- const __be32 *val;
-
- if (!prop)
- return -EINVAL;
- if (!prop-value)
- return -ENODATA;
- if ((sz * sizeof(*out_values)) prop-length)
- return -EOVERFLOW;
-
- val = prop-value;
- while (sz--)
- *out_values++ = be32_to_cpup(val++);
- return 0;
+ of_property_read_array(np, propname,
Re: [PATCH Resend V2] dt: add helper function to read u8 u16 variables arrays
On Tue, Nov 6, 2012 at 7:48 PM, Rob Herring robherri...@gmail.com 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 0x;
is equivalent to:
property = 0x12345678 0x;
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 0x8400 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 = 0x5000 0x6000 0x7000;
}
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: 5000
[ 4.109512] u8 1: 60
[ 4.116036] u16 1: 6000
[ 4.123357] u32 1: 6000
[ 4.131718] u8 2: 70
[ 4.138241] u16 2: 7000
[ 4.145573] u32 2: 7000
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.ku...@linaro.org
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
