of: introduce helper to manage boolean
PATCH 1/1 of: introduce helper to manage boolean
of_property_read_bool
Search for a property in a device node and read a 32-bit value from it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none.
this will allow to disable a boolean
is-ok; => true is-ok = <1>; => true is-ok = <0>; => false
---
Hi,
if this is ok I'll rebase my mtd and i2c patch to use it
Best Regards,
J.
drivers/of/base.c | 30 ++++++++++++++++++++++++++++++ include/linux/of.h | 8 ++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)
drivers/of/base.c +30 -0
@@ -686,6 +686,36 @@ int of_property_read_u64(const struct device_node *np, const char *propname,EXPORT_SYMBOL_GPL (of_property_read_u64);/** +* of_property_read_bool - Find and read a boolean from a property +* @np: device node from which the property value is to be read. +* @propname: name of the property to be searched. +* +* Search for a property in a device node and read a 32-bit value from +* it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none. +* +* is-ok; => true +* is-ok = <1>; => true +* is-ok = <0>; => false +*/ +int of_property_read_bool (const struct device_node *np,const char *propname) +{ +u32 reg; +int ret =of_property_read_u32 (np, propname, ®); + +switch (ret) { +case -EINVAL : +return false; +case -ENODATA : +return true; +case 0 : +return reg ==1 ; +default : +return ret; + } +} +EXPORT_SYMBOL_GPL (of_property_read_bool); + +/** * of_property_read_string - Find and read a string from a property * @np: device node from which the property value is to be read. * @propname: name of the property to be searched.
include/linux/of.h +8 -0
@@ -210,6 +210,8 @@ extern int of_property_read_u32_array(const struct device_node *np, size_t sz);extern int of_property_read_u64 (const struct device_node *np,const char *propname,u64 *out_value); +extern int of_property_read_bool (const struct device_node *np, +const char *propname); extern int of_property_read_string(struct device_node *np, const char *propname,
@@ -288,6 +290,12 @@ static inline int of_property_read_u32_array(const struct device_node *np,return -ENOSYS ; } +static inline int of_property_read_bool (const struct device_node *np, +const char *propname) +{ +return -ENOSYS ; +} +static inline int of_property_read_string (struct device_node *np,const char *propname,const char **out_string)
Signature · 1 line
1.7.7
PATCH 1/1 of: introduce helper to manage boolean
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
of_property_read_boolSearch for a property in a device node and read a 32-bit value from it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none.
Quoted 8 lines
this will allow to disable a boolean
is-ok; => true is-ok = <1>; => true is-ok = <0>; => falseSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc devicetree-discuss at lists.ozlabs.org
Quoted 5 lines
---
Hi,if this is ok I'll rebase my mtd and i2c patch to use it
I'm assuming you'll want to merge this patch with your mtd and i2c patches. If you want me to apply, let me know.
Rob
Quoted 73 lines
Best Regards,
J.drivers/of/base.c | 30 ++++++++++++++++++++++++++++++ include/linux/of.h | 8 ++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)drivers/of/base.c +30
-0@@ -686,6 +686,36 @@ int of_property_read_u64(const struct device_node *np, const char *propname,EXPORT_SYMBOL_GPL (of_property_read_u64);/** +* of_property_read_bool - Find and read a boolean from a property +* @np: device node from which the property value is to be read. +* @propname: name of the property to be searched. +* +* Search for a property in a device node and read a 32-bit value from +* it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none. +* +* is-ok; => true +* is-ok = <1>; => true +* is-ok = <0>; => false +*/ +int of_property_read_bool (const struct device_node *np,const char *propname) +{ +u32 reg; +int ret =of_property_read_u32 (np, propname, ®); + +switch (ret) { +case -EINVAL : +return false; +case -ENODATA : +return true; +case 0 : +return reg ==1 ; +default : +return ret; + } +} +EXPORT_SYMBOL_GPL (of_property_read_bool); + +/** * of_property_read_string - Find and read a string from a property * @np: device node from which the property value is to be read. * @propname: name of the property to be searched. include/linux/of.h +8
-0@@ -210,6 +210,8 @@ extern int of_property_read_u32_array(const struct device_node *np, size_t sz);extern int of_property_read_u64 (const struct device_node *np,const char *propname,u64 *out_value); +extern int of_property_read_bool (const struct device_node *np, +const char *propname); extern int of_property_read_string(struct device_node *np, const char *propname,@@ -288,6 +290,12 @@ static inline int of_property_read_u32_array(const struct device_node *np,return -ENOSYS ; } +static inline int of_property_read_bool (const struct device_node *np, +const char *propname) +{ +return -ENOSYS ; +} +static inline int of_property_read_string (struct device_node *np,const char *propname,const char **out_string)
PATCH 1/1 of: introduce helper to manage boolean
Quoted 17 lines · Rob Herring
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
of_property_read_boolSearch for a property in a device node and read a 32-bit value from it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none.
Quoted 8 lines
this will allow to disable a boolean
is-ok; => true is-ok = <1>; => true is-ok = <0>; => falseSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc devicetree-discuss at lists.ozlabs.orgAcked-by Rob Herring <[email protected]>
Quoted 8 lines
Quoted 5 lines
---
Hi,if this is ok I'll rebase my mtd and i2c patch to use it
I'm assuming you'll want to merge this patch with your mtd and i2c patches. If you want me to apply, let me know.
yes please
I rebase the of_mtd over it
if you can apply it too
Acked-by Grant
Best Regards,
J.
Quoted 76 lines
Rob
Quoted 73 lines
Best Regards,
J.drivers/of/base.c | 30 ++++++++++++++++++++++++++++++ include/linux/of.h | 8 ++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)drivers/of/base.c +30
-0@@ -686,6 +686,36 @@ int of_property_read_u64(const struct device_node *np, const char *propname,EXPORT_SYMBOL_GPL (of_property_read_u64);/** +* of_property_read_bool - Find and read a boolean from a property +* @np: device node from which the property value is to be read. +* @propname: name of the property to be searched. +* +* Search for a property in a device node and read a 32-bit value from +* it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none. +* +* is-ok; => true +* is-ok = <1>; => true +* is-ok = <0>; => false +*/ +int of_property_read_bool (const struct device_node *np,const char *propname) +{ +u32 reg; +int ret =of_property_read_u32 (np, propname, ®); + +switch (ret) { +case -EINVAL : +return false; +case -ENODATA : +return true; +case 0 : +return reg ==1 ; +default : +return ret; + } +} +EXPORT_SYMBOL_GPL (of_property_read_bool); + +/** * of_property_read_string - Find and read a string from a property * @np: device node from which the property value is to be read. * @propname: name of the property to be searched. include/linux/of.h +8
-0@@ -210,6 +210,8 @@ extern int of_property_read_u32_array(const struct device_node *np, size_t sz);extern int of_property_read_u64 (const struct device_node *np,const char *propname,u64 *out_value); +extern int of_property_read_bool (const struct device_node *np, +const char *propname); extern int of_property_read_string(struct device_node *np, const char *propname,@@ -288,6 +290,12 @@ static inline int of_property_read_u32_array(const struct device_node *np,return -ENOSYS ; } +static inline int of_property_read_bool (const struct device_node *np, +const char *propname) +{ +return -ENOSYS ; +} +static inline int of_property_read_string (struct device_node *np,const char *propname,const char **out_string)
PATCH 1/1 of: introduce helper to manage boolean
In reply to Jean-Christophe PLAGNIOL-VILLARDQuoted 45 lines · Jean-Christophe PLAGNIOL-VILLARD
of_property_read_boolSearch for a property in a device node and read a 32-bit value from it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none.
this will allow to disable a boolean
is-ok; => true is-ok = <1>; => true is-ok = <0>; => falseSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc devicetree-discuss at lists.ozlabs.org---
Hi,if this is ok I'll rebase my mtd and i2c patch to use it
Best Regards,
J.drivers/of/base.c | 30 ++++++++++++++++++++++++++++++ include/linux/of.h | 8 ++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)drivers/of/base.c +13
-0@@ -686,6 +686,36 @@ int of_property_read_u64(const struct device_node *np, const char *propname,EXPORT_SYMBOL_GPL (of_property_read_u64);/** +* of_property_read_bool - Find and read a boolean from a property +* @np: device node from which the property value is to be read. +* @propname: name of the property to be searched. +* +* Search for a property in a device node and read a 32-bit value from +* it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none. +* +* is-ok; => true +* is-ok = <1>; => true +* is-ok = <0>; => false +*/ +int of_property_read_bool (const struct device_node *np,const char *propname) +{
drivers/of/base.c int of_property_read_u64(const struct device_node *np, const char *propname, · line 711
Quoted 10 lines
drivers/of/base.c +10
-0+u32 reg; +int ret =of_property_read_u32 (np, propname, ®); + +switch (ret) { +case -EINVAL : +return false; +case -ENODATA : +return true; +case 0 : +return reg ==1 ;
Ugh. so any value other than 1 returns false? I think that will surprise most people.
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
g.
PATCH 1/1 of: introduce helper to manage boolean
Quoted 60 lines · Grant Likely
Quoted 45 lines · Jean-Christophe PLAGNIOL-VILLARD
of_property_read_boolSearch for a property in a device node and read a 32-bit value from it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none.
this will allow to disable a boolean
is-ok; => true is-ok = <1>; => true is-ok = <0>; => falseSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc devicetree-discuss at lists.ozlabs.org---
Hi,if this is ok I'll rebase my mtd and i2c patch to use it
Best Regards,
J.drivers/of/base.c | 30 ++++++++++++++++++++++++++++++ include/linux/of.h | 8 ++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)drivers/of/base.c +13
-0@@ -686,6 +686,36 @@ int of_property_read_u64(const struct device_node *np, const char *propname,EXPORT_SYMBOL_GPL (of_property_read_u64);/** +* of_property_read_bool - Find and read a boolean from a property +* @np: device node from which the property value is to be read. +* @propname: name of the property to be searched. +* +* Search for a property in a device node and read a 32-bit value from +* it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none. +* +* is-ok; => true +* is-ok = <1>; => true +* is-ok = <0>; => false +*/ +int of_property_read_bool (const struct device_node *np,const char *propname) +{drivers/of/base.c int of_property_read_u64(const struct device_node *np, const char *propname, · line 711
Quoted 10 lines
drivers/of/base.c +10
-0+u32 reg; +int ret =of_property_read_u32 (np, propname, ®); + +switch (ret) { +case -EINVAL : +return false; +case -ENODATA : +return true; +case 0 : +return reg ==1 ;Ugh. so any value other than 1 returns false? I think that will surprise most people.
Quoted 2 lines
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
if you we can do the the invert
if !0 => true
is-ok; => true
is-ok = <val != 0>; => true
is-ok = <0>; => false
Best Regards,
J.
PATCH 1/1 of: introduce helper to manage boolean
Quoted 67 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 60 lines · Grant Likely
Quoted 45 lines · Jean-Christophe PLAGNIOL-VILLARD
of_property_read_boolSearch for a property in a device node and read a 32-bit value from it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none.
this will allow to disable a boolean
is-ok; => true is-ok = <1>; => true is-ok = <0>; => falseSigned-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc devicetree-discuss at lists.ozlabs.org---
Hi,if this is ok I'll rebase my mtd and i2c patch to use it
Best Regards,
J.drivers/of/base.c | 30 ++++++++++++++++++++++++++++++ include/linux/of.h | 8 ++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)drivers/of/base.c +13
-0@@ -686,6 +686,36 @@ int of_property_read_u64(const struct device_node *np, const char *propname,EXPORT_SYMBOL_GPL (of_property_read_u64);/** +* of_property_read_bool - Find and read a boolean from a property +* @np: device node from which the property value is to be read. +* @propname: name of the property to be searched. +* +* Search for a property in a device node and read a 32-bit value from +* it. Returns 0 if <0> or if the property does not exist, 1 if <1> or none. +* +* is-ok; => true +* is-ok = <1>; => true +* is-ok = <0>; => false +*/ +int of_property_read_bool (const struct device_node *np,const char *propname) +{drivers/of/base.c int of_property_read_u64(const struct device_node *np, const char *propname, · line 711
Quoted 10 lines
drivers/of/base.c +10
-0+u32 reg; +int ret =of_property_read_u32 (np, propname, ®); + +switch (ret) { +case -EINVAL : +return false; +case -ENODATA : +return true; +case 0 : +return reg ==1 ;Ugh. so any value other than 1 returns false? I think that will surprise most people.
Quoted 2 lines
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
Quoted 7 lines
if you we can do the the invert
if !0 => true
is-ok; => true
is-ok = <val != 0>; => true
is-ok = <0>; => false
This is a failure of the dtc tool, not the binding. Accepting this binding means we have to live with it for a very long time. It needs to be fixed in dtc instead so that properties can be deleted instead of only modified.
g.
PATCH 1/1 of: introduce helper to manage boolean
Quoted 26 lines
Quoted 26 lines
Quoted 16 lines
drivers/of/base.c int of_property_read_u64(const struct device_node *np, const char *propname, · line 711
Quoted 12 lines
drivers/of/base.c +12
-0+int of_property_read_bool (const struct device_node *np,const char *propname) +{ +u32 reg; +int ret =of_property_read_u32 (np, propname, ®); + +switch (ret) { +case -EINVAL : +return false; +case -ENODATA : +return true; +case 0 : +return reg ==1 ;Ugh. so any value other than 1 returns false? I think that will surprise most people.
Quoted 2 lines
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
if you we can do the the invert
if !0 => true
Quoted 7 lines
Quoted 3 lines
is-ok; => true
is-ok = <val != 0>; => true
is-ok = <0>; => falseThis is a failure of the dtc tool, not the binding. Accepting this binding means we have to live with it for a very long time. It needs to be fixed in dtc instead so that properties can be deleted instead of only modified.
I understand your idea but today if you put and value in the property it's true.
So is-ok = <0>; is true also which is illogical as in any language a boolean is true (1) or false (0). When I read the property I will understand false not true
And I see no good way to add the "delete" in dtc.
Best Regards,
J.
PATCH 1/1 of: introduce helper to manage boolean
Quoted 17 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 17 lines
Quoted 16 lines
Quoted 5 lines
Ugh. so any value other than 1 returns false? I think that will surprise most people.
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
if you we can do the the invert
if !0 => true
is-ok; => true
is-ok = <val != 0>; => true
is-ok = <0>; => false
Quoted 7 lines
Quoted 3 lines
This is a failure of the dtc tool, not the binding. Accepting this binding means we have to live with it for a very long time. It needs to be fixed in dtc instead so that properties can be deleted instead of only modified.
I understand your idea but today if you put and value in the property it's true.
So is-ok = <0>; is true also which is illogical as in any language a boolean is true (1) or false (0). When I read the property I will understand false not true
You could say similar things about is-ok = "no" or is-ok = "" or is-ok = "I'd rather you didn't"... it's expected that violating the binding may produce illogical results.
Quoted 1 line
And I see no good way to add the "delete" in dtc.
It shouldn't be that hard, just needs a new keyword and/or bit of syntax to express what you want it to do.
-Scott
PATCH 1/1 of: introduce helper to manage boolean
Quoted 21 lines · Scott Wood
Quoted 20 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 20 lines
Quoted 9 lines
Quoted 5 lines
Ugh. so any value other than 1 returns false? I think that will surprise most people.
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
Quoted 7 lines
if you we can do the the invert
if !0 => true
is-ok; => true
is-ok = <val != 0>; => true
is-ok = <0>; => falseThis is a failure of the dtc tool, not the binding. Accepting this binding means we have to live with it for a very long time. It needs to be fixed in dtc instead so that properties can be deleted instead of only modified.
Quoted 8 lines
Quoted 4 lines
I understand your idea but today if you put and value in the property it's true.
So is-ok = <0>; is true also which is illogical as in any language a boolean is true (1) or false (0). When I read the property I will understand false not true
You could say similar things about is-ok = "no" or is-ok = "" or is-ok = "I'd rather you didn't"... it's expected that violating the binding may produce illogical results.
today is most of the binding people use a number whe the want to be able to delete it and it's the same in most of the promgramming language
Best Regards,
J.
PATCH 1/1 of: introduce helper to manage boolean
Quoted 24 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 23 lines · Scott Wood
Quoted 22 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 13 lines
Quoted 13 lines
Quoted 5 lines
Ugh. so any value other than 1 returns false? I think that will surprise most people.
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
if you we can do the the invert
if !0 => true
Quoted 7 lines
Quoted 3 lines
is-ok; => true
is-ok = <val != 0>; => true
is-ok = <0>; => falseThis is a failure of the dtc tool, not the binding. Accepting this binding means we have to live with it for a very long time. It needs to be fixed in dtc instead so that properties can be deleted instead of only modified.
I understand your idea but today if you put and value in the property it's true.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
So is-ok = <0>; is true also which is illogical as in any language a boolean is true (1) or false (0). When I read the property I will understand false not true
You could say similar things about is-ok = "no" or is-ok = "" or is-ok = "I'd rather you didn't"... it's expected that violating the binding may produce illogical results.
today is most of the binding people use a number whe the want to be able to delete it and it's the same in most of the promgramming language
It isn't yet a big pain point, so there isn't time pressure here. Fixing the tool is the better solution, and since the kernel carries a copy of the tool we don't need to worry about adding a feature that isn't available by the dtc packaged by a distribution.
Fixing the tool is the correct thing to do.
g.
PATCH 1/1 of: introduce helper to manage boolean
Quoted 34 lines · Grant Likely
Quoted 24 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 23 lines · Scott Wood
Quoted 22 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 13 lines
Quoted 13 lines
Quoted 5 lines
Ugh. so any value other than 1 returns false? I think that will surprise most people.
I don't like this api or binding. If it is a bool property, then why isn't simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
if you we can do the the invert
if !0 => true
Quoted 7 lines
Quoted 3 lines
is-ok; => true
is-ok = <val != 0>; => true
is-ok = <0>; => falseThis is a failure of the dtc tool, not the binding. Accepting this binding means we have to live with it for a very long time. It needs to be fixed in dtc instead so that properties can be deleted instead of only modified.
I understand your idea but today if you put and value in the property it's true.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
So is-ok = <0>; is true also which is illogical as in any language a boolean is true (1) or false (0). When I read the property I will understand false not true
You could say similar things about is-ok = "no" or is-ok = "" or is-ok = "I'd rather you didn't"... it's expected that violating the binding may produce illogical results.
today is most of the binding people use a number whe the want to be able to delete it and it's the same in most of the promgramming language
Quoted 6 lines
It isn't yet a big pain point, so there isn't time pressure here. Fixing the tool is the better solution, and since the kernel carries a copy of the tool we don't need to worry about adding a feature that isn't available by the dtc packaged by a distribution.
Fixing the tool is the correct thing to do.
I don't like it but ok
Can I get the ack on it and apply via my tree I've more tan 10 patches pending because of this
Best Regards,
J.
PATCH 1/1 of: introduce helper to manage boolean
In reply to Grant LikelyHi,
Quoted 53 lines · Grant Likely
Quoted 3 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 2 lines · Scott Wood
Quoted 1 line · Jean-Christophe PLAGNIOL-VILLARD
Quoted 1 line
Quoted 1 line
Quoted 1 line
Ugh. so any value other than 1 returns false? I think that will
surprise
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
most people.
I don't like this api or binding. If it is a bool property, then
why isn't
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
simply testing for the property existance sufficient?
no if you want to disable it
if a bool is define in the dtsi and want to disable it int the dts
if you we can do the the invert
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
if !0 => true
is-ok; => true is-ok = <val != 0>; => true is-ok = <0>; => falseThis is a failure of the dtc tool, not the binding. Accepting this
binding
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
means we have to live with it for a very long time. It needs to be
fixed
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
in dtc instead so that properties can be deleted instead of only
modified.
Quoted 1 line
Quoted 1 line
Quoted 1 line
I understand your idea but today if you put and value in the
property it's true.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
So is-ok = <0>; is true also which is illogical as in any language a
boolean is
Quoted 1 line
Quoted 1 line
Quoted 1 line
true (1) or false (0). When I read the property I will understand
false not true
Quoted 2 lines
Quoted 2 lines
You could say similar things about is-ok = "no" or is-ok = "" or is-ok
=
Quoted 3 lines
Quoted 2 lines
"I'd rather you didn't"... it's expected that violating the binding may produce illogical results.
today is most of the binding people use a number whe the want to be able
to
Quoted 1 line
delete it and it's the same in most of the promgramming language
It isn't yet a big pain point, so there isn't time pressure here. Fixing the tool
is the better solution, and since the kernel carries a copy of the tool we don't
need to worry about adding a feature that isn't available by the dtc packaged by
a distribution.
Quoted 2 lines
Fixing the tool is the correct thing to do.
I just wanted to pick up on this thread. Now in the kernel, absence of a property indicates false, and presence indicates true. This is a kernel convention, nothing to do with the dtb format. I think it is useful to support a boolean with a non-null value which can be 0, meaning true as Jean-Christophe says.
My reasons are:
1. dtc does not have a way to delete a property and it isn't clear what syntax could be used there. It also seems a little brittle to delete a property in one place and define it in another (ordering? confusion when people can't work out why it has gone?). So not only can dtc not do this, but I'm not sure that it should.
2. fdtput allows updating a property. It is pretty easy to do something like 'fdtput file.dtb /node propname $value' where value is 0 or 1. To use the current binding we need to either do:
if [[ $value == 0 ]]; then
fdtput -d file.dtb /node propname # new function to delete a property?
else
fdtput file.dtb /node propname 1
fi
or perhaps something like:
fdtput -B file.dtb /node propname ${value}where -B is a new option meaning either create an empty property, or delete any property it finds, based on the value being 0 or non-zero.
3. Discoverability: it is nice to be able to see the possible options, even if disabled. In particular, if a boolean value is true, and you later decide with fdtput to turn it off, you effectively remove all trace of it. This could be confusing for those who are looking for available options.
Arguably these issues could be fixed if we introduced some sort of type hinting into the dtb file format, but that seems a long shot. I am not sure that modifying the tools to support this arguable odd 'present/absent' boolean binding is the right approach.
So I think Jean-Christophe's idea has significant merit.
Thoughts?
Regards,
Simon
Quoted 8 lines
g.
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120710/80a0c4e9/attachment-0001.html>
PATCH 1/1 of: introduce helper to manage boolean
Quoted 60 lines · Simon Glass
Hi,
On Tue, Mar 13, 2012 at 5:16 AM, Grant Likely <[email protected] <mailto:[email protected]>> wrote:
On Tue, 13 Mar 2012 04:17:39 +0100, Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com <mailto:[email protected]>> wrote:
> On 14:39 Mon 12 Mar , Scott Wood wrote:
> > On 03/09/2012 10:36 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > >>>> Ugh. so any value other than 1 returns false? I think that will surprise
> > >>>> most people.
> > >>>>
> > >>>> I don't like this api or binding. If it is a bool property, then why isn't
> > >>>> simply testing for the property existance sufficient? > > >>> no if you want to disable it
> > >>>
> > >>> if a bool is define in the dtsi and want to disable it int the dts
> > >>>
> > >>> if you we can do the the invert
> > >>>
> > >>> if !0 => true
> > >>>
> > >>> is-ok; => true
> > >>> is-ok = <val != 0>; => true
> > >>> is-ok = <0>; => false
> > >>
> > >> This is a failure of the dtc tool, not the binding.
Accepting this binding
> > >> means we have to live with it for a very long time. It needs to be fixed
> > >> in dtc instead so that properties can be deleted instead of only modified.
> > > I understand your idea but today if you put and value in the property it's true.
> > >
> > > So is-ok = <0>; is true also which is illogical as in any language a boolean is
> > > true (1) or false (0). When I read the property I will understand false not true
> >
> > You could say similar things about is-ok = "no" or is-ok = "" or is-ok =
> > "I'd rather you didn't"... it's expected that violating the binding may
> > produce illogical results.
> today is most of the binding people use a number whe the want to be able to
> delete it and it's the same in most of the promgramming languageIt isn't yet a big pain point, so there isn't time pressure here.
Fixing the tool
is the better solution, and since the kernel carries a copy of the tool we don't
need to worry about adding a feature that isn't available by the dtc packaged by
a distribution.
Quoted 6 lines
Fixing the tool is the correct thing to do.
I just wanted to pick up on this thread. Now in the kernel, absence of a property indicates false, and presence indicates true. This is a kernel convention, nothing to do with the dtb format.
It's an aspect of the device binding, and was used by Open Firmware bindings before Linux came up with flat device trees.
Also note that even non-boolean properties can mean something different when absent. Sometimes this is a default value; sometimes, like with "ranges", it's something that can't be expressed with any value (empty ranges means all translations go straight through; no ranges means no translation is possible).
Quoted 8 lines
I think it is useful to
support a boolean with a non-null value which can be 0, meaning true as Jean-Christophe says.My reasons are:
1. dtc does not have a way to delete a property and it isn't clear what syntax could be used there.
Surely some syntax can be created for this. E.g. /delete-prop/ foo;
Quoted 4 lines
It also seems a little brittle to delete a
property in one place and define it in another (ordering? confusion when people can't work out why it has gone?). So not only can dtc not do this, but I'm not sure that it should.
I don't see how it's worse than defining it one place and then redefining it elsewhere.
Quoted 2 lines
3. Discoverability: it is nice to be able to see the possible options, even if disabled.
This assumes the possible options were known in advance, or that you don't maintain compatibility with old device trees when a new option is added.
Quoted 4 lines
In particular, if a boolean value is true, and you
later decide with fdtput to turn it off, you effectively remove all trace of it. This could be confusing for those who are looking for available options.
There should still be a "trace of it" in the binding document.
-Scott
PATCH 1/1 of: introduce helper to manage boolean
Hi Scott,
Quoted 64 lines · Scott Wood
Quoted 63 lines · Simon Glass
Hi,
On Tue, Mar 13, 2012 at 5:16 AM, Grant Likely <[email protected] <mailto:[email protected]>> wrote:
On Tue, 13 Mar 2012 04:17:39 +0100, Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com <mailto:[email protected]>> wrote:
> On 14:39 Mon 12 Mar , Scott Wood wrote:
> > On 03/09/2012 10:36 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > >>>> Ugh. so any value other than 1 returns false? I think that will surprise
> > >>>> most people.
> > >>>>
> > >>>> I don't like this api or binding. If it is a bool property, then why isn't
> > >>>> simply testing for the property existance sufficient? > > >>> no if you want to disable it
> > >>>
> > >>> if a bool is define in the dtsi and want to disable it int the dts
> > >>>
> > >>> if you we can do the the invert
> > >>>
> > >>> if !0 => true
> > >>>
> > >>> is-ok; => true
> > >>> is-ok = <val != 0>; => true
> > >>> is-ok = <0>; => false
> > >>
> > >> This is a failure of the dtc tool, not the binding.
Accepting this binding
> > >> means we have to live with it for a very long time. It needs to be fixed
> > >> in dtc instead so that properties can be deleted instead of only modified.
> > > I understand your idea but today if you put and value in the property it's true.
> > >
> > > So is-ok = <0>; is true also which is illogical as in any language a boolean is
> > > true (1) or false (0). When I read the property I will understand false not true
> >
> > You could say similar things about is-ok = "no" or is-ok = "" or is-ok =
> > "I'd rather you didn't"... it's expected that violating the binding may
> > produce illogical results.
> today is most of the binding people use a number whe the want to be able to
> delete it and it's the same in most of the promgramming languageIt isn't yet a big pain point, so there isn't time pressure here.
Fixing the tool
is the better solution, and since the kernel carries a copy of the tool we don't
need to worry about adding a feature that isn't available by the dtc packaged by
a distribution.Fixing the tool is the correct thing to do.
Quoted 7 lines
Quoted 3 lines
I just wanted to pick up on this thread. Now in the kernel, absence of a property indicates false, and presence indicates true. This is a kernel convention, nothing to do with the dtb format.
It's an aspect of the device binding, and was used by Open Firmware bindings before Linux came up with flat device trees.
OK. When I asked about the boolean property on the devicetree-discuss I understood that no one really cared, and it was a matter for Linux.
Quoted 7 lines
Also note that even non-boolean properties can mean something different when absent. Sometimes this is a default value; sometimes, like with "ranges", it's something that can't be expressed with any value (empty ranges means all translations go straight through; no ranges means no translation is possible).
That's fine, but I'm not sure I understand why that relates to boolean properties, which currently mean always the same thing (false) when absent. I don't think there is any intent to change that.
Quoted 11 lines
Quoted 8 lines
I think it is useful to
support a boolean with a non-null value which can be 0, meaning true as Jean-Christophe says.My reasons are:
1. dtc does not have a way to delete a property and it isn't clear what syntax could be used there.
Surely some syntax can be created for this. E.g. /delete-prop/ foo;
Yes, in fact I saw a patch after sending this email. So for normal values to change the value we do
prop = <23>;
but for booleans we must do EITHER:
bool;
or
/delete-prop/ bool;
depending on whether we want the make it true or false? Ick.
Quoted 9 lines
Quoted 5 lines
It also seems a little brittle to delete a
property in one place and define it in another (ordering? confusion when people can't work out why it has gone?). So not only can dtc not do this, but I'm not sure that it should.I don't see how it's worse than defining it one place and then redefining it elsewhere.
It seems worse to me, see above. Also if we end up with symbols it will be impossible to do something like:
bool-property = <WIBBLE_VALUE>;
You will have to do:
if WIBBLE_VALUE == 0
/delete-prop/ bool-property;
#else
bool-property;
#endif
or whatever, or maybe I got that around the wrong way.... Not nice IMO.
(any comments on point 2?)
Quoted 8 lines
Quoted 3 lines
3. Discoverability: it is nice to be able to see the possible options, even if disabled.
This assumes the possible options were known in advance, or that you don't maintain compatibility with old device trees when a new option is added.
You can still add the option with a zero value - or maybe I misunderstand what you mean.
Quoted 8 lines
Quoted 5 lines
In particular, if a boolean value is true, and you
later decide with fdtput to turn it off, you effectively remove all trace of it. This could be confusing for those who are looking for available options.There should still be a "trace of it" in the binding document.
Yes true, that's the saving grace.
I'm not even saying that we ever use
bool-prop = <1>;
Clearly that is redundant. I'm just agreeing with Jean-Chrstophe that it would be nice and clean to do something like:
bool-prop = <0>;
and it will make it false.
Regards,
Simon
Quoted 4 lines
-Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120711/ae4b5d65/attachment-0001.html>
PATCH 1/1 of: introduce helper to manage boolean
Quoted 12 lines · Simon Glass
Hi Scott,
On Tue, Jul 10, 2012 at 11:23 PM, Scott Wood <[email protected] <mailto:[email protected]>> wrote:
Also note that even non-boolean properties can mean something different when absent. Sometimes this is a default value; sometimes, like with "ranges", it's something that can't be expressed with any value (empty ranges means all translations go straight through; no ranges means no translation is possible).
Quoted 3 lines
That's fine, but I'm not sure I understand why that relates to boolean properties, which currently mean always the same thing (false) when absent. I don't think there is any intent to change that.
The point was this isn't the only scenario where the absence of a property means something, and where you might want to delete the property.
Quoted 21 lines
Quoted 2 lines
I think it is useful to
support a boolean with a non-null value which can be 0, meaningtrue as
> Jean-Christophe says.
>
> My reasons are:
>
> 1. dtc does not have a way to delete a property and it isn't clear what
> syntax could be used there.Surely some syntax can be created for this. E.g. /delete-prop/ foo;
Yes, in fact I saw a patch after sending this email. So for normal values to change the value we do
prop = <23>;but for booleans we must do EITHER:
Quoted 7 lines
bool;or
/delete-prop/ bool;depending on whether we want the make it true or false? Ick.
Doesn't seem that bad to me except in the case you show below where you're trying to do it programmatically.
Quoted 1 line
It seems worse to me, see above. Also if we end up with symbols it will
*> be impossible to do something like:
Quoted 5 lines
bool-property = <WIBBLE_VALUE>;You will have to do:
Quoted 7 lines
if
WIBBLE_VALUE== 0
/delete-prop/ bool-property;
#else
bool-property;
#endifor whatever, or maybe I got that around the wrong way.... Not nice IMO.
Yeah, that's unpleasant.
I don't hugely object to a new boolean type for use in new bindings, where <0> or absence can both be used to indicate false, as long as it's clearly documented. I'm hesitant to start doing this on existing bindings, though, and in any case would like to see support for property/node deletion in dtc.
Quoted 1 line
(any comments on point 2?)
It's basically the same as the above, right?
Quoted 10 lines
Quoted 2 lines
3. Discoverability: it is nice to be able to see the possible options, even if disabled.
This assumes the possible options were known in advance, or that you don't maintain compatibility with old device trees when a new option is added.
You can still add the option with a zero value - or maybe I misunderstand what you mean.
We normally want to work with existing device trees (which could be partially produced by firmware, so changing can be unpleasant), so the absence of the property is still going to be a possibility. Plus enumerating every possibility, no matter how rarely used, in every node of a certain type seems excessively verbose. It's not like these are user configuration knobs.
-Scott
PATCH 1/1 of: introduce helper to manage boolean
Hi Scott,
Quoted 13 lines · Scott Wood
Quoted 12 lines · Simon Glass
Hi Scott,
On Tue, Jul 10, 2012 at 11:23 PM, Scott Wood <[email protected] <mailto:[email protected]>> wrote:
Also note that even non-boolean properties can mean something different when absent. Sometimes this is a default value; sometimes, like with "ranges", it's something that can't be expressed with any value (empty ranges means all translations go straight through; no ranges means no translation is possible).
Quoted 6 lines
Quoted 3 lines
That's fine, but I'm not sure I understand why that relates to boolean properties, which currently mean always the same thing (false) when absent. I don't think there is any intent to change that.
The point was this isn't the only scenario where the absence of a property means something, and where you might want to delete the property.
OK I see. We don't have that option for booleans, since absence will always mean false.
Quoted 48 lines
Quoted 22 lines
Quoted 3 lines
I think it is useful to
support a boolean with a non-null value which can be 0, meaningtrue as
> Jean-Christophe says.
>
> My reasons are:
>
> 1. dtc does not have a way to delete a property and it isn't clear what
> syntax could be used there.Surely some syntax can be created for this. E.g. /delete-prop/ foo;
Yes, in fact I saw a patch after sending this email. So for normal values to change the value we do
prop = <23>;but for booleans we must do EITHER:
Quoted 7 lines
bool;or
/delete-prop/ bool;depending on whether we want the make it true or false? Ick.
Doesn't seem that bad to me except in the case you show below where you're trying to do it programmatically.
Quoted 1 line
It seems worse to me, see above. Also if we end up with symbols it will
*> be impossible to do something like:
Quoted 5 lines
bool-property = <WIBBLE_VALUE>;You will have to do:
Quoted 7 lines
if
WIBBLE_VALUE== 0
/delete-prop/ bool-property;
#else
bool-property;
#endifor whatever, or maybe I got that around the wrong way.... Not nice IMO.
Quoted 7 lines
Yeah, that's unpleasant.
I don't hugely object to a new boolean type for use in new bindings, where <0> or absence can both be used to indicate false, as long as it's clearly documented. I'm hesitant to start doing this on existing bindings, though, and in any case would like to see support for property/node deletion in dtc.
Fair enough. Jean-Christophe's patch certainly changes existing behaviour, and without that it might be confusing - we don't want two functions for reading boolean properties. The only plus is that (perhaps) no one expects "bool-prop = <0>" to mean true, so it might be safe.
Quoted 4 lines
Quoted 2 lines
(any comments on point 2?)
It's basically the same as the above, right?
Yes, and you commented above.
Quoted 12 lines
Quoted 11 lines
Quoted 3 lines
3. Discoverability: it is nice to be able to see the possible options, even if disabled.
This assumes the possible options were known in advance, or that you don't maintain compatibility with old device trees when a new option is added.
You can still add the option with a zero value - or maybe I misunderstand what you mean.
Quoted 6 lines
We normally want to work with existing device trees (which could be partially produced by firmware, so changing can be unpleasant), so the absence of the property is still going to be a possibility. Plus enumerating every possibility, no matter how rarely used, in every node of a certain type seems excessively verbose. It's not like these are user configuration knobs.
Yes agreed. The point of this is just to make it easier (I think) to make a boolean value false either in the source, or later using fdtput.
So what do we need to do? Take a vote? Revisit Jean-Christophe's patch?
Quoted 3 lines
-Scott
Regards,
Simon