| Message ID | 20250109-dt-type-warnings-v1-1-0150e32e716c@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | device property: Warn on (of|fwnode|device)_property_read_bool() for non-boolean properties | expand |
On Thu, Jan 09, 2025 at 01:42:05PM -0600, Rob Herring (Arm) wrote: > The fwnode/device property API currently implement > (fwnode|device)_property_read_bool() with (fwnode|device)_property_present(). > That does not allow having different behavior depending on the backend. > > Specifically, the usage of (fwnode|device)_property_read_bool() on > non-boolean properties is deprecated on DT. In order to add a warning > on this deprecated use, these 2 APIs need separate ops for the backend. > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Thu, Jan 09, 2025 at 01:42:05PM -0600, Rob Herring (Arm) wrote: > The fwnode/device property API currently implement > (fwnode|device)_property_read_bool() with (fwnode|device)_property_present(). > That does not allow having different behavior depending on the backend. > > Specifically, the usage of (fwnode|device)_property_read_bool() on > non-boolean properties is deprecated on DT. In order to add a warning > on this deprecated use, these 2 APIs need separate ops for the backend. > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> > --- > drivers/acpi/property.c | 1 + > drivers/base/property.c | 38 ++++++++++++++++++++++++++++++++++++++ > drivers/base/swnode.c | 1 + > drivers/of/property.c | 7 +++++++ > include/linux/fwnode.h | 3 +++ > include/linux/of.h | 4 +++- > include/linux/property.h | 15 +++------------ > 7 files changed, 56 insertions(+), 13 deletions(-) Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On Thu, Jan 9, 2025 at 8:42 PM Rob Herring (Arm) <robh@kernel.org> wrote: > > The fwnode/device property API currently implement > (fwnode|device)_property_read_bool() with (fwnode|device)_property_present(). > That does not allow having different behavior depending on the backend. > > Specifically, the usage of (fwnode|device)_property_read_bool() on > non-boolean properties is deprecated on DT. In order to add a warning > on this deprecated use, these 2 APIs need separate ops for the backend. > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> and please route this as needed. > --- > drivers/acpi/property.c | 1 + > drivers/base/property.c | 38 ++++++++++++++++++++++++++++++++++++++ > drivers/base/swnode.c | 1 + > drivers/of/property.c | 7 +++++++ > include/linux/fwnode.h | 3 +++ > include/linux/of.h | 4 +++- > include/linux/property.h | 15 +++------------ > 7 files changed, 56 insertions(+), 13 deletions(-) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index 80a52a4e66dd167e70c80a164a89beb8c834ae13..8c53959ee31e700004de098bdab21dec118cf111 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -1656,6 +1656,7 @@ static int acpi_fwnode_irq_get(const struct fwnode_handle *fwnode, > acpi_fwnode_device_dma_supported, \ > .device_get_dma_attr = acpi_fwnode_device_get_dma_attr, \ > .property_present = acpi_fwnode_property_present, \ > + .property_read_bool = acpi_fwnode_property_present, \ > .property_read_int_array = \ > acpi_fwnode_property_read_int_array, \ > .property_read_string_array = \ > diff --git a/drivers/base/property.c b/drivers/base/property.c > index 837d77e3af2bd5bf655e6fa0f64c10a05b361b90..c1392743df9c3f6c554f093f4efae3ea1ad1fdf2 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -70,6 +70,44 @@ bool fwnode_property_present(const struct fwnode_handle *fwnode, > } > EXPORT_SYMBOL_GPL(fwnode_property_present); > > +/** > + * device_property_read_bool - Return the value for a boolean property of a device > + * @dev: Device whose property is being checked > + * @propname: Name of the property > + * > + * Return if property @propname is true or false in the device firmware description. > + * > + * Return: true if property @propname is present. Otherwise, returns false. > + */ > +bool device_property_read_bool(const struct device *dev, const char *propname) > +{ > + return fwnode_property_read_bool(dev_fwnode(dev), propname); > +} > +EXPORT_SYMBOL_GPL(device_property_read_bool); > + > +/** > + * fwnode_property_read_bool - Return the value for a boolean property of a firmware node > + * @fwnode: Firmware node whose property to check > + * @propname: Name of the property > + * > + * Return if property @propname is true or false in the firmware description. > + */ > +bool fwnode_property_read_bool(const struct fwnode_handle *fwnode, > + const char *propname) > +{ > + bool ret; > + > + if (IS_ERR_OR_NULL(fwnode)) > + return false; > + > + ret = fwnode_call_bool_op(fwnode, property_read_bool, propname); > + if (ret) > + return ret; > + > + return fwnode_call_bool_op(fwnode->secondary, property_read_bool, propname); > +} > +EXPORT_SYMBOL_GPL(fwnode_property_read_bool); > + > /** > * device_property_read_u8_array - return a u8 array property of a device > * @dev: Device to get the property of > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c > index eb6eb25b343bafaa9c03f881b98a528305fa6093..b1726a3515f6fbe13c2186af1f74479263798e42 100644 > --- a/drivers/base/swnode.c > +++ b/drivers/base/swnode.c > @@ -677,6 +677,7 @@ static const struct fwnode_operations software_node_ops = { > .get = software_node_get, > .put = software_node_put, > .property_present = software_node_property_present, > + .property_read_bool = software_node_property_present, > .property_read_int_array = software_node_read_int_array, > .property_read_string_array = software_node_read_string_array, > .get_name = software_node_get_name, > diff --git a/drivers/of/property.c b/drivers/of/property.c > index 519bf9229e613906547b57d8c68e7b8558eff327..1b46be88cc0498fcbe74e7b988f22a86245c366e 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -965,6 +965,12 @@ 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_present(to_of_node(fwnode), propname); > +} > + > +static bool of_fwnode_property_read_bool(const struct fwnode_handle *fwnode, > + const char *propname) > { > return of_property_read_bool(to_of_node(fwnode), propname); > } > @@ -1562,6 +1568,7 @@ const struct fwnode_operations of_fwnode_ops = { > .device_dma_supported = of_fwnode_device_dma_supported, > .device_get_dma_attr = of_fwnode_device_get_dma_attr, > .property_present = of_fwnode_property_present, > + .property_read_bool = of_fwnode_property_read_bool, > .property_read_int_array = of_fwnode_property_read_int_array, > .property_read_string_array = of_fwnode_property_read_string_array, > .get_name = of_fwnode_get_name, > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h > index 0d79070c5a70f21e9d73751941f28cefb244950d..0731994b9d7c832cae8a30063f3a64194e4f19aa 100644 > --- a/include/linux/fwnode.h > +++ b/include/linux/fwnode.h > @@ -112,6 +112,7 @@ struct fwnode_reference_args { > * @device_is_available: Return true if the device is available. > * @device_get_match_data: Return the device driver match data. > * @property_present: Return true if a property is present. > + * @property_read_bool: Return a boolean property value. > * @property_read_int_array: Read an array of integer properties. Return zero on > * success, a negative error code otherwise. > * @property_read_string_array: Read an array of string properties. Return zero > @@ -141,6 +142,8 @@ struct fwnode_operations { > (*device_get_dma_attr)(const struct fwnode_handle *fwnode); > bool (*property_present)(const struct fwnode_handle *fwnode, > const char *propname); > + bool (*property_read_bool)(const struct fwnode_handle *fwnode, > + const char *propname); > int (*property_read_int_array)(const struct fwnode_handle *fwnode, > const char *propname, > unsigned int elem_size, void *val, > diff --git a/include/linux/of.h b/include/linux/of.h > index f921786cb8ac782286ed5ff4425a35668204d050..1cb4eb7fc2eded2246c697c3bcaf1b85d43108ab 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -1271,7 +1271,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; > } > > /** > diff --git a/include/linux/property.h b/include/linux/property.h > index 61fc20e5f81f8da992aa021996ddf68ef3eff0ed..e214ecd241eb4e6d9e0873d094f2c956bbf66c34 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -37,6 +37,7 @@ struct fwnode_handle *__dev_fwnode(struct device *dev); > struct device *: __dev_fwnode)(dev) > > bool device_property_present(const struct device *dev, const char *propname); > +bool device_property_read_bool(const struct device *dev, const char *propname); > int device_property_read_u8_array(const struct device *dev, const char *propname, > u8 *val, size_t nval); > int device_property_read_u16_array(const struct device *dev, const char *propname, > @@ -54,6 +55,8 @@ int device_property_match_string(const struct device *dev, > > bool fwnode_property_present(const struct fwnode_handle *fwnode, > const char *propname); > +bool fwnode_property_read_bool(const struct fwnode_handle *fwnode, > + const char *propname); > int fwnode_property_read_u8_array(const struct fwnode_handle *fwnode, > const char *propname, u8 *val, > size_t nval); > @@ -207,12 +210,6 @@ int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name); > > unsigned int device_get_child_node_count(const struct device *dev); > > -static inline bool device_property_read_bool(const struct device *dev, > - const char *propname) > -{ > - return device_property_present(dev, propname); > -} > - > static inline int device_property_read_u8(const struct device *dev, > const char *propname, u8 *val) > { > @@ -263,12 +260,6 @@ static inline int device_property_string_array_count(const struct device *dev, > return device_property_read_string_array(dev, propname, NULL, 0); > } > > -static inline bool fwnode_property_read_bool(const struct fwnode_handle *fwnode, > - const char *propname) > -{ > - return fwnode_property_present(fwnode, propname); > -} > - > static inline int fwnode_property_read_u8(const struct fwnode_handle *fwnode, > const char *propname, u8 *val) > { > > -- > 2.45.2 >
On Thu, Jan 09, 2025 at 01:42:05PM -0600, Rob Herring (Arm) wrote: > The fwnode/device property API currently implement > (fwnode|device)_property_read_bool() with (fwnode|device)_property_present(). > That does not allow having different behavior depending on the backend. > > Specifically, the usage of (fwnode|device)_property_read_bool() on > non-boolean properties is deprecated on DT. In order to add a warning > on this deprecated use, these 2 APIs need separate ops for the backend. > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> Thanks! Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index 80a52a4e66dd167e70c80a164a89beb8c834ae13..8c53959ee31e700004de098bdab21dec118cf111 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -1656,6 +1656,7 @@ static int acpi_fwnode_irq_get(const struct fwnode_handle *fwnode, acpi_fwnode_device_dma_supported, \ .device_get_dma_attr = acpi_fwnode_device_get_dma_attr, \ .property_present = acpi_fwnode_property_present, \ + .property_read_bool = acpi_fwnode_property_present, \ .property_read_int_array = \ acpi_fwnode_property_read_int_array, \ .property_read_string_array = \ diff --git a/drivers/base/property.c b/drivers/base/property.c index 837d77e3af2bd5bf655e6fa0f64c10a05b361b90..c1392743df9c3f6c554f093f4efae3ea1ad1fdf2 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -70,6 +70,44 @@ bool fwnode_property_present(const struct fwnode_handle *fwnode, } EXPORT_SYMBOL_GPL(fwnode_property_present); +/** + * device_property_read_bool - Return the value for a boolean property of a device + * @dev: Device whose property is being checked + * @propname: Name of the property + * + * Return if property @propname is true or false in the device firmware description. + * + * Return: true if property @propname is present. Otherwise, returns false. + */ +bool device_property_read_bool(const struct device *dev, const char *propname) +{ + return fwnode_property_read_bool(dev_fwnode(dev), propname); +} +EXPORT_SYMBOL_GPL(device_property_read_bool); + +/** + * fwnode_property_read_bool - Return the value for a boolean property of a firmware node + * @fwnode: Firmware node whose property to check + * @propname: Name of the property + * + * Return if property @propname is true or false in the firmware description. + */ +bool fwnode_property_read_bool(const struct fwnode_handle *fwnode, + const char *propname) +{ + bool ret; + + if (IS_ERR_OR_NULL(fwnode)) + return false; + + ret = fwnode_call_bool_op(fwnode, property_read_bool, propname); + if (ret) + return ret; + + return fwnode_call_bool_op(fwnode->secondary, property_read_bool, propname); +} +EXPORT_SYMBOL_GPL(fwnode_property_read_bool); + /** * device_property_read_u8_array - return a u8 array property of a device * @dev: Device to get the property of diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index eb6eb25b343bafaa9c03f881b98a528305fa6093..b1726a3515f6fbe13c2186af1f74479263798e42 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -677,6 +677,7 @@ static const struct fwnode_operations software_node_ops = { .get = software_node_get, .put = software_node_put, .property_present = software_node_property_present, + .property_read_bool = software_node_property_present, .property_read_int_array = software_node_read_int_array, .property_read_string_array = software_node_read_string_array, .get_name = software_node_get_name, diff --git a/drivers/of/property.c b/drivers/of/property.c index 519bf9229e613906547b57d8c68e7b8558eff327..1b46be88cc0498fcbe74e7b988f22a86245c366e 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -965,6 +965,12 @@ 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_present(to_of_node(fwnode), propname); +} + +static bool of_fwnode_property_read_bool(const struct fwnode_handle *fwnode, + const char *propname) { return of_property_read_bool(to_of_node(fwnode), propname); } @@ -1562,6 +1568,7 @@ const struct fwnode_operations of_fwnode_ops = { .device_dma_supported = of_fwnode_device_dma_supported, .device_get_dma_attr = of_fwnode_device_get_dma_attr, .property_present = of_fwnode_property_present, + .property_read_bool = of_fwnode_property_read_bool, .property_read_int_array = of_fwnode_property_read_int_array, .property_read_string_array = of_fwnode_property_read_string_array, .get_name = of_fwnode_get_name, diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index 0d79070c5a70f21e9d73751941f28cefb244950d..0731994b9d7c832cae8a30063f3a64194e4f19aa 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -112,6 +112,7 @@ struct fwnode_reference_args { * @device_is_available: Return true if the device is available. * @device_get_match_data: Return the device driver match data. * @property_present: Return true if a property is present. + * @property_read_bool: Return a boolean property value. * @property_read_int_array: Read an array of integer properties. Return zero on * success, a negative error code otherwise. * @property_read_string_array: Read an array of string properties. Return zero @@ -141,6 +142,8 @@ struct fwnode_operations { (*device_get_dma_attr)(const struct fwnode_handle *fwnode); bool (*property_present)(const struct fwnode_handle *fwnode, const char *propname); + bool (*property_read_bool)(const struct fwnode_handle *fwnode, + const char *propname); int (*property_read_int_array)(const struct fwnode_handle *fwnode, const char *propname, unsigned int elem_size, void *val, diff --git a/include/linux/of.h b/include/linux/of.h index f921786cb8ac782286ed5ff4425a35668204d050..1cb4eb7fc2eded2246c697c3bcaf1b85d43108ab 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1271,7 +1271,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; } /** diff --git a/include/linux/property.h b/include/linux/property.h index 61fc20e5f81f8da992aa021996ddf68ef3eff0ed..e214ecd241eb4e6d9e0873d094f2c956bbf66c34 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -37,6 +37,7 @@ struct fwnode_handle *__dev_fwnode(struct device *dev); struct device *: __dev_fwnode)(dev) bool device_property_present(const struct device *dev, const char *propname); +bool device_property_read_bool(const struct device *dev, const char *propname); int device_property_read_u8_array(const struct device *dev, const char *propname, u8 *val, size_t nval); int device_property_read_u16_array(const struct device *dev, const char *propname, @@ -54,6 +55,8 @@ int device_property_match_string(const struct device *dev, bool fwnode_property_present(const struct fwnode_handle *fwnode, const char *propname); +bool fwnode_property_read_bool(const struct fwnode_handle *fwnode, + const char *propname); int fwnode_property_read_u8_array(const struct fwnode_handle *fwnode, const char *propname, u8 *val, size_t nval); @@ -207,12 +210,6 @@ int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name); unsigned int device_get_child_node_count(const struct device *dev); -static inline bool device_property_read_bool(const struct device *dev, - const char *propname) -{ - return device_property_present(dev, propname); -} - static inline int device_property_read_u8(const struct device *dev, const char *propname, u8 *val) { @@ -263,12 +260,6 @@ static inline int device_property_string_array_count(const struct device *dev, return device_property_read_string_array(dev, propname, NULL, 0); } -static inline bool fwnode_property_read_bool(const struct fwnode_handle *fwnode, - const char *propname) -{ - return fwnode_property_present(fwnode, propname); -} - static inline int fwnode_property_read_u8(const struct fwnode_handle *fwnode, const char *propname, u8 *val) {
The fwnode/device property API currently implement (fwnode|device)_property_read_bool() with (fwnode|device)_property_present(). That does not allow having different behavior depending on the backend. Specifically, the usage of (fwnode|device)_property_read_bool() on non-boolean properties is deprecated on DT. In order to add a warning on this deprecated use, these 2 APIs need separate ops for the backend. Signed-off-by: Rob Herring (Arm) <robh@kernel.org> --- drivers/acpi/property.c | 1 + drivers/base/property.c | 38 ++++++++++++++++++++++++++++++++++++++ drivers/base/swnode.c | 1 + drivers/of/property.c | 7 +++++++ include/linux/fwnode.h | 3 +++ include/linux/of.h | 4 +++- include/linux/property.h | 15 +++------------ 7 files changed, 56 insertions(+), 13 deletions(-)