diff mbox series

[1/2] of: add support for value "false" to of_property_read_bool

Message ID 20241112-am64-overlay-bool-v1-1-b9d1faff444e@solid-run.com
State Changes Requested
Headers show
Series of: add support for value "false" to of_property_read_bool | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied fail build log

Commit Message

Josua Mayer Nov. 12, 2024, 6:41 a.m. UTC
Boolean type properties are usually considered true if present and false
when they do not exist. This works well for many in-tree board dts and
existing drivers.

When users need to overrride boolean values from included dts includes,
/delete-property/ is recommend. This however does not work in overlays
(addons).

Several places use string "true" ([1], [2], [3]) and one uses string
"false" ([1]). This suggests that at some point the value of a type
string property was to be taken into account during evaluation.

Change of_property_read_bool to only return true if a property is both
present, and not equal "false".

Existing usage in drivers/of and include/linux/of.h are updated
accordingly.
Other places should be reviewed as needed wrt. changed semantics.

[1] Documentation/devicetree/bindings/sound/rt5651.txt
[2] Documentation/devicetree/bindings/sound/pcm3060.txt
[3] arch/arm/boot/dts/ti/omap/am335x-baltos.dtsi

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/of/property.c |  2 +-
 include/linux/of.h    | 13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

Comments

Josua Mayer Nov. 12, 2024, 6:52 a.m. UTC | #1
Am 12.11.24 um 08:41 schrieb Josua Mayer:
> Boolean type properties are usually considered true if present and false
> when they do not exist. This works well for many in-tree board dts and
> existing drivers.
>
> When users need to overrride boolean values from included dts includes,
> /delete-property/ is recommend. This however does not work in overlays
> (addons).
>
> Several places use string "true" ([1], [2], [3]) and one uses string
> "false" ([1]). This suggests that at some point the value of a type
> string property was to be taken into account during evaluation.
>
> Change of_property_read_bool to only return true if a property is both
> present, and not equal "false".
>
> Existing usage in drivers/of and include/linux/of.h are updated
> accordingly.
> Other places should be reviewed as needed wrt. changed semantics.
>
> [1] Documentation/devicetree/bindings/sound/rt5651.txt
> [2] Documentation/devicetree/bindings/sound/pcm3060.txt
> [3] arch/arm/boot/dts/ti/omap/am335x-baltos.dtsi
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
Please note this patch-set has only been tested to compile,
I do not consider it ready for current merge window.

I would however appreciate comments.
diff mbox series

Patch

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 11b922fde7af167cc69f6dd89826219653cd1ee8..a51132b883fb3fa0b1a120bacb298abf72f67c5b 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -893,7 +893,7 @@  of_fwnode_device_get_dma_attr(const struct fwnode_handle *fwnode)
 static bool of_fwnode_property_present(const struct fwnode_handle *fwnode,
 				       const char *propname)
 {
-	return of_property_read_bool(to_of_node(fwnode), propname);
+	return of_property_present(to_of_node(fwnode), propname);
 }
 
 static int of_fwnode_property_read_int_array(const struct fwnode_handle *fwnode,
diff --git a/include/linux/of.h b/include/linux/of.h
index 85b60ac9eec50bb202aa774cab273e1d947e394d..ea65c7d1176194d6ce79432782361f2cdab84f8b 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1247,14 +1247,15 @@  static inline int of_property_read_string_index(const struct device_node *np,
  * Search for a boolean property in a device node. Usage on non-boolean
  * property types is deprecated.
  *
- * Return: true if the property exists false otherwise.
+ * Return: true if the property exists and value is not "false",
+ * false otherwise.
  */
 static inline bool of_property_read_bool(const struct device_node *np,
 					 const char *propname)
 {
-	struct property *prop = of_find_property(np, propname, NULL);
+	int ret = of_property_match_string(np, propname, "false");
 
-	return prop ? true : false;
+	return ret == -ENODATA ? true : false;
 }
 
 /**
@@ -1268,7 +1269,9 @@  static inline bool of_property_read_bool(const struct device_node *np,
  */
 static inline bool of_property_present(const struct device_node *np, const char *propname)
 {
-	return of_property_read_bool(np, propname);
+	struct property *prop = of_find_property(np, propname, NULL);
+
+	return prop ? true : false;
 }
 
 /**
@@ -1679,7 +1682,7 @@  static inline int of_reconfig_get_state_change(unsigned long action,
  * of_device_is_system_power_controller - Tells if system-power-controller is found for device_node
  * @np: Pointer to the given device_node
  *
- * Return: true if present false otherwise
+ * Return: true if enabled, false otherwise
  */
 static inline bool of_device_is_system_power_controller(const struct device_node *np)
 {