[PATCH v1 1/1] device property: Document how to check for the property presence

[PATCH v1 1/1] device property: Document how to check for the property presence

[Andy Shevchenko]

Currently it's unclear if one may or may not rely on the error codes
returned from the property getters to check for the property presence.
Clarify this by mass updating kernel-doc for fwnode_property_*() and
device_property_*() where it's applicable.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Closes: 4b24f1f4-b395-467a-81b7-1334a2d48845@roeck-us.net
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8d9a34be57fb..bffa0070ab13 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -38,6 +38,8 @@ EXPORT_SYMBOL_GPL(__dev_fwnode_const);
  * @propname: Name of the property
  *
  * Check if property @propname is present in the device firmware description.
+ * This function is the correct way to check that given property is present
+ * in the device firmware description.
  *
  * Return: true if property @propname is present. Otherwise, returns false.
  */
@@ -52,6 +54,10 @@ EXPORT_SYMBOL_GPL(device_property_present);
  * @fwnode: Firmware node whose property to check
  * @propname: Name of the property
  *
+ * Check if property @propname is present in the firmware node description.
+ * This function is the correct way to check that given property is present
+ * in the firmware node description.
+ *
  * Return: true if property @propname is present. Otherwise, returns false.
  */
 bool fwnode_property_present(const struct fwnode_handle *fwnode,
@@ -75,9 +81,9 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
  * @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.
+ * Use device_property_present() to check for the property presence.
  *
- * Return: true if property @propname is present. Otherwise, returns false.
+ * Return: if property @propname is true or false in the device firmware description.
  */
 bool device_property_read_bool(const struct device *dev, const char *propname)
 {
@@ -90,7 +96,9 @@ EXPORT_SYMBOL_GPL(device_property_read_bool);
  * @fwnode: Firmware node whose property to check
  * @propname: Name of the property
  *
- * Return if property @propname is true or false in the firmware description.
+ * Use fwnode_property_present() to check for the property presence.
+ *
+ * Return: if property @propname is true or false in the firmware node description.
  */
 bool fwnode_property_read_bool(const struct fwnode_handle *fwnode,
 			     const char *propname)
@@ -121,6 +129,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_bool);
  * It's recommended to call device_property_count_u8() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use device_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -149,6 +159,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u8_array);
  * It's recommended to call device_property_count_u16() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use device_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -177,6 +189,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u16_array);
  * It's recommended to call device_property_count_u32() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use device_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -205,6 +219,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u32_array);
  * It's recommended to call device_property_count_u64() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use device_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -233,6 +249,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
  * It's recommended to call device_property_string_array_count() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use device_property_present().
+ *
  * Return: number of values read on success if @val is non-NULL,
  *	   number of values available on success if @val is NULL,
  *	   %-EINVAL if given arguments are not valid,
@@ -257,6 +275,8 @@ EXPORT_SYMBOL_GPL(device_property_read_string_array);
  * Function reads property @propname from the device firmware description and
  * stores the value into @val if found. The value is checked to be a string.
  *
+ * In order to check for the property presence, use device_property_present().
+ *
  * Return: %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
  *	   %-ENODATA if the property does not have a value,
@@ -324,6 +344,8 @@ static int fwnode_property_read_int_array(const struct fwnode_handle *fwnode,
  * It's recommended to call fwnode_property_count_u8() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use fwnode_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -353,6 +375,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
  * It's recommended to call fwnode_property_count_u16() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use fwnode_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -382,6 +406,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
  * It's recommended to call fwnode_property_count_u32() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use fwnode_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -411,6 +437,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
  * It's recommended to call fwnode_property_count_u64() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use fwnode_property_present().
+ *
  * Return: number of values if @val was %NULL,
  *         %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
@@ -440,6 +468,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
  * It's recommended to call fwnode_property_string_array_count() instead of calling
  * this function with @val equals %NULL and @nval equals 0.
  *
+ * In order to check for the property presence, use fwnode_property_present().
+ *
  * Return: number of values read on success if @val is non-NULL,
  *	   number of values available on success if @val is NULL,
  *	   %-EINVAL if given arguments are not valid,
@@ -476,6 +506,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
  * Read property @propname from the given firmware node and store the value into
  * @val if found.  The value is checked to be a string.
  *
+ * In order to check for the property presence, use fwnode_property_present().
+ *
  * Return: %0 if the property was found (success),
  *	   %-EINVAL if given arguments are not valid,
  *	   %-ENODATA if the property does not have a value,
-- 
2.50.1

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Sakari Ailus]

Hi Andy,

Thanks for the patch.

On Tue, Mar 17, 2026 at 10:08:28PM +0100, Andy Shevchenko wrote:
> Currently it's unclear if one may or may not rely on the error codes
> returned from the property getters to check for the property presence.
> Clarify this by mass updating kernel-doc for fwnode_property_*() and
> device_property_*() where it's applicable.
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: 4b24f1f4-b395-467a-81b7-1334a2d48845@roeck-us.net
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/base/property.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 8d9a34be57fb..bffa0070ab13 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -38,6 +38,8 @@ EXPORT_SYMBOL_GPL(__dev_fwnode_const);
>   * @propname: Name of the property
>   *
>   * Check if property @propname is present in the device firmware description.
> + * This function is the correct way to check that given property is present
> + * in the device firmware description.
>   *
>   * Return: true if property @propname is present. Otherwise, returns false.
>   */
> @@ -52,6 +54,10 @@ EXPORT_SYMBOL_GPL(device_property_present);
>   * @fwnode: Firmware node whose property to check
>   * @propname: Name of the property
>   *
> + * Check if property @propname is present in the firmware node description.
> + * This function is the correct way to check that given property is present
> + * in the firmware node description.
> + *
>   * Return: true if property @propname is present. Otherwise, returns false.
>   */
>  bool fwnode_property_present(const struct fwnode_handle *fwnode,
> @@ -75,9 +81,9 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
>   * @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.
> + * Use device_property_present() to check for the property presence.
>   *
> - * Return: true if property @propname is present. Otherwise, returns false.
> + * Return: if property @propname is true or false in the device firmware description.
>   */
>  bool device_property_read_bool(const struct device *dev, const char *propname)
>  {
> @@ -90,7 +96,9 @@ EXPORT_SYMBOL_GPL(device_property_read_bool);
>   * @fwnode: Firmware node whose property to check
>   * @propname: Name of the property
>   *
> - * Return if property @propname is true or false in the firmware description.
> + * Use fwnode_property_present() to check for the property presence.
> + *
> + * Return: if property @propname is true or false in the firmware node description.
>   */
>  bool fwnode_property_read_bool(const struct fwnode_handle *fwnode,
>  			     const char *propname)
> @@ -121,6 +129,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_bool);
>   * It's recommended to call device_property_count_u8() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().

Do you really think we should add this clause for each of these functions?
I don't think it belongs here.

The error code list doesn't document what is returned if a property doesn't
exist (-EINVAL) and it'd be helpful to add this. It would have been best to
have a separate error code for this albeit changing this now might not be
that troublesome either: very, very few callers depend on receiving such an
error code but there are still many callers.

> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -149,6 +159,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u8_array);
>   * It's recommended to call device_property_count_u16() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -177,6 +189,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u16_array);
>   * It's recommended to call device_property_count_u32() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -205,6 +219,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u32_array);
>   * It's recommended to call device_property_count_u64() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -233,6 +249,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
>   * It's recommended to call device_property_string_array_count() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values read on success if @val is non-NULL,
>   *	   number of values available on success if @val is NULL,
>   *	   %-EINVAL if given arguments are not valid,
> @@ -257,6 +275,8 @@ EXPORT_SYMBOL_GPL(device_property_read_string_array);
>   * Function reads property @propname from the device firmware description and
>   * stores the value into @val if found. The value is checked to be a string.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
>   *	   %-ENODATA if the property does not have a value,
> @@ -324,6 +344,8 @@ static int fwnode_property_read_int_array(const struct fwnode_handle *fwnode,
>   * It's recommended to call fwnode_property_count_u8() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -353,6 +375,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
>   * It's recommended to call fwnode_property_count_u16() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -382,6 +406,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
>   * It's recommended to call fwnode_property_count_u32() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -411,6 +437,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
>   * It's recommended to call fwnode_property_count_u64() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
> @@ -440,6 +468,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
>   * It's recommended to call fwnode_property_string_array_count() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values read on success if @val is non-NULL,
>   *	   number of values available on success if @val is NULL,
>   *	   %-EINVAL if given arguments are not valid,
> @@ -476,6 +506,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
>   * Read property @propname from the given firmware node and store the value into
>   * @val if found.  The value is checked to be a string.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: %0 if the property was found (success),
>   *	   %-EINVAL if given arguments are not valid,
>   *	   %-ENODATA if the property does not have a value,

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Guenter Roeck]

On Wed, Mar 18, 2026 at 12:27:24AM +0200, Sakari Ailus wrote:
> Hi Andy,
> 
> Thanks for the patch.
> 
> On Tue, Mar 17, 2026 at 10:08:28PM +0100, Andy Shevchenko wrote:
> > Currently it's unclear if one may or may not rely on the error codes
> > returned from the property getters to check for the property presence.
> > Clarify this by mass updating kernel-doc for fwnode_property_*() and
> > device_property_*() where it's applicable.
> > 
> > Reported-by: Guenter Roeck <linux@roeck-us.net>
> > Closes: 4b24f1f4-b395-467a-81b7-1334a2d48845@roeck-us.net
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/base/property.c | 38 +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 35 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index 8d9a34be57fb..bffa0070ab13 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -38,6 +38,8 @@ EXPORT_SYMBOL_GPL(__dev_fwnode_const);
> >   * @propname: Name of the property
> >   *
> >   * Check if property @propname is present in the device firmware description.
> > + * This function is the correct way to check that given property is present
> > + * in the device firmware description.
> >   *
> >   * Return: true if property @propname is present. Otherwise, returns false.
> >   */
> > @@ -52,6 +54,10 @@ EXPORT_SYMBOL_GPL(device_property_present);
> >   * @fwnode: Firmware node whose property to check
> >   * @propname: Name of the property
> >   *
> > + * Check if property @propname is present in the firmware node description.
> > + * This function is the correct way to check that given property is present
> > + * in the firmware node description.
> > + *
> >   * Return: true if property @propname is present. Otherwise, returns false.
> >   */
> >  bool fwnode_property_present(const struct fwnode_handle *fwnode,
> > @@ -75,9 +81,9 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
> >   * @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.
> > + * Use device_property_present() to check for the property presence.
> >   *
> > - * Return: true if property @propname is present. Otherwise, returns false.
> > + * Return: if property @propname is true or false in the device firmware description.
> >   */
> >  bool device_property_read_bool(const struct device *dev, const char *propname)
> >  {
> > @@ -90,7 +96,9 @@ EXPORT_SYMBOL_GPL(device_property_read_bool);
> >   * @fwnode: Firmware node whose property to check
> >   * @propname: Name of the property
> >   *
> > - * Return if property @propname is true or false in the firmware description.
> > + * Use fwnode_property_present() to check for the property presence.
> > + *
> > + * Return: if property @propname is true or false in the firmware node description.
> >   */
> >  bool fwnode_property_read_bool(const struct fwnode_handle *fwnode,
> >  			     const char *propname)
> > @@ -121,6 +129,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_bool);
> >   * It's recommended to call device_property_count_u8() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use device_property_present().
> 
> Do you really think we should add this clause for each of these functions?
> I don't think it belongs here.
> 
> The error code list doesn't document what is returned if a property doesn't
> exist (-EINVAL) and it'd be helpful to add this. It would have been best to
> have a separate error code for this albeit changing this now might not be
> that troublesome either: very, very few callers depend on receiving such an
> error code but there are still many callers.
> 

I also find it a bit confusing since the _bool functions are defined to return
false if the property is not present, and true if it is.

Guenter

> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -149,6 +159,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u8_array);
> >   * It's recommended to call device_property_count_u16() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use device_property_present().
> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -177,6 +189,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u16_array);
> >   * It's recommended to call device_property_count_u32() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use device_property_present().
> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -205,6 +219,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u32_array);
> >   * It's recommended to call device_property_count_u64() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use device_property_present().
> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -233,6 +249,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
> >   * It's recommended to call device_property_string_array_count() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use device_property_present().
> > + *
> >   * Return: number of values read on success if @val is non-NULL,
> >   *	   number of values available on success if @val is NULL,
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -257,6 +275,8 @@ EXPORT_SYMBOL_GPL(device_property_read_string_array);
> >   * Function reads property @propname from the device firmware description and
> >   * stores the value into @val if found. The value is checked to be a string.
> >   *
> > + * In order to check for the property presence, use device_property_present().
> > + *
> >   * Return: %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> >   *	   %-ENODATA if the property does not have a value,
> > @@ -324,6 +344,8 @@ static int fwnode_property_read_int_array(const struct fwnode_handle *fwnode,
> >   * It's recommended to call fwnode_property_count_u8() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use fwnode_property_present().
> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -353,6 +375,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
> >   * It's recommended to call fwnode_property_count_u16() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use fwnode_property_present().
> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -382,6 +406,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
> >   * It's recommended to call fwnode_property_count_u32() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use fwnode_property_present().
> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -411,6 +437,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
> >   * It's recommended to call fwnode_property_count_u64() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use fwnode_property_present().
> > + *
> >   * Return: number of values if @val was %NULL,
> >   *         %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -440,6 +468,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
> >   * It's recommended to call fwnode_property_string_array_count() instead of calling
> >   * this function with @val equals %NULL and @nval equals 0.
> >   *
> > + * In order to check for the property presence, use fwnode_property_present().
> > + *
> >   * Return: number of values read on success if @val is non-NULL,
> >   *	   number of values available on success if @val is NULL,
> >   *	   %-EINVAL if given arguments are not valid,
> > @@ -476,6 +506,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
> >   * Read property @propname from the given firmware node and store the value into
> >   * @val if found.  The value is checked to be a string.
> >   *
> > + * In order to check for the property presence, use fwnode_property_present().
> > + *
> >   * Return: %0 if the property was found (success),
> >   *	   %-EINVAL if given arguments are not valid,
> >   *	   %-ENODATA if the property does not have a value,
> 
> -- 
> Kind regards,
> 
> Sakari Ailus

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Andy Shevchenko]

On Wed, Mar 18, 2026 at 12:27:24AM +0200, Sakari Ailus wrote:
> On Tue, Mar 17, 2026 at 10:08:28PM +0100, Andy Shevchenko wrote:

...

> > + * In order to check for the property presence, use device_property_present().
> 
> Do you really think we should add this clause for each of these functions?

Yes, as Guenter pointed out that this has to be documented clearly.

> I don't think it belongs here.

And? What should we do then (taking into account my below comments)?

> The error code list doesn't document what is returned if a property doesn't
> exist (-EINVAL) and it'd be helpful to add this.

No, this change is exactly against this. Because using an error code that may
cover not only that case is at bare minimum fragile and layering violation.
APIs that require to know the implementation details are not good APIs.

> It would have been best to have a separate error code for this albeit
> changing this now might not be that troublesome either: very, very few
> callers depend on receiving such an error code but there are still many
> callers.

I'm against this because we have already a dedicated API to check for property
presence, why do we need to have another (confusing!) way of doing the same?

Having a dedicated code may help to debug, but shouldn't be used as a main
feature in my opinion.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Sakari Ailus]

Hi Andy,

On Wed, Mar 18, 2026 at 10:03:27AM +0200, Andy Shevchenko wrote:
> On Wed, Mar 18, 2026 at 12:27:24AM +0200, Sakari Ailus wrote:
> > On Tue, Mar 17, 2026 at 10:08:28PM +0100, Andy Shevchenko wrote:
> 
> ...
> 
> > > + * In order to check for the property presence, use device_property_present().
> > 
> > Do you really think we should add this clause for each of these functions?
> 
> Yes, as Guenter pointed out that this has to be documented clearly.
> 
> > I don't think it belongs here.
> 
> And? What should we do then (taking into account my below comments)?
> 
> > The error code list doesn't document what is returned if a property doesn't
> > exist (-EINVAL) and it'd be helpful to add this.
> 
> No, this change is exactly against this. Because using an error code that may
> cover not only that case is at bare minimum fragile and layering violation.
> APIs that require to know the implementation details are not good APIs.

I have to say I disagree with that, there's nothing wrong with checking
error codes if you need to.

Either way, I checked the original patch. If you really think you need to
check for property presence and use default in the case the property isn't
found and error out on other errors, add helper functions for the purpose
instead of open-coding it all.

> 
> > It would have been best to have a separate error code for this albeit
> > changing this now might not be that troublesome either: very, very few
> > callers depend on receiving such an error code but there are still many
> > callers.
> 
> I'm against this because we have already a dedicated API to check for property
> presence, why do we need to have another (confusing!) way of doing the same?
> 
> Having a dedicated code may help to debug, but shouldn't be used as a main
> feature in my opinion.

-- 
Regards,

Sakari Ailus

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Andy Shevchenko]

On Wed, Mar 18, 2026 at 11:10:49AM +0200, Sakari Ailus wrote:
> On Wed, Mar 18, 2026 at 10:03:27AM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 18, 2026 at 12:27:24AM +0200, Sakari Ailus wrote:
> > > On Tue, Mar 17, 2026 at 10:08:28PM +0100, Andy Shevchenko wrote:

...

> > > > + * In order to check for the property presence, use device_property_present().
> > > 
> > > Do you really think we should add this clause for each of these functions?
> > 
> > Yes, as Guenter pointed out that this has to be documented clearly.
> > 
> > > I don't think it belongs here.
> > 
> > And? What should we do then (taking into account my below comments)?
> > 
> > > The error code list doesn't document what is returned if a property doesn't
> > > exist (-EINVAL) and it'd be helpful to add this.
> > 
> > No, this change is exactly against this. Because using an error code that may
> > cover not only that case is at bare minimum fragile and layering violation.
> > APIs that require to know the implementation details are not good APIs.
> 
> I have to say I disagree with that,

And I definitely disagree with tribal knowledge based (implementation details
as it's not properly documented and can't, because -EINVAL is overloaded) and
confusing approach that is in use and may be amended.

> there's nothing wrong with checking
> error codes if you need to.

If that error code defines the case. Here we have not a such.

The wrong parameter to the function will lead to the same error code
which is simply wrong.

> Either way, I checked the original patch. If you really think you need to
> check for property presence and use default in the case the property isn't
> found and error out on other errors, add helper functions for the purpose
> instead of open-coding it all.

You mean adding 20+ helpers (at least 5 for arrays, 5 for single element,
and doubled for device_/fwnode_) ?! This sounds like way over verbose
approach.

> > > It would have been best to have a separate error code for this albeit
> > > changing this now might not be that troublesome either: very, very few
> > > callers depend on receiving such an error code but there are still many
> > > callers.
> > 
> > I'm against this because we have already a dedicated API to check for property
> > presence, why do we need to have another (confusing!) way of doing the same?
> > 
> > Having a dedicated code may help to debug, but shouldn't be used as a main
> > feature in my opinion.

I will send a next version of this patch touching only the present/bool functions.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Sakari Ailus]

On Wed, Mar 18, 2026 at 11:41:57AM +0200, Andy Shevchenko wrote:
> On Wed, Mar 18, 2026 at 11:10:49AM +0200, Sakari Ailus wrote:
> > On Wed, Mar 18, 2026 at 10:03:27AM +0200, Andy Shevchenko wrote:
> > > On Wed, Mar 18, 2026 at 12:27:24AM +0200, Sakari Ailus wrote:
> > > > On Tue, Mar 17, 2026 at 10:08:28PM +0100, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > + * In order to check for the property presence, use device_property_present().
> > > > 
> > > > Do you really think we should add this clause for each of these functions?
> > > 
> > > Yes, as Guenter pointed out that this has to be documented clearly.
> > > 
> > > > I don't think it belongs here.
> > > 
> > > And? What should we do then (taking into account my below comments)?
> > > 
> > > > The error code list doesn't document what is returned if a property doesn't
> > > > exist (-EINVAL) and it'd be helpful to add this.
> > > 
> > > No, this change is exactly against this. Because using an error code that may
> > > cover not only that case is at bare minimum fragile and layering violation.
> > > APIs that require to know the implementation details are not good APIs.
> > 
> > I have to say I disagree with that,
> 
> And I definitely disagree with tribal knowledge based (implementation details
> as it's not properly documented and can't, because -EINVAL is overloaded) and
> confusing approach that is in use and may be amended.

In general it's a good idea to document API functions' return values. It
certainly won't add to confusion, will it?

> 
> > there's nothing wrong with checking
> > error codes if you need to.
> 
> If that error code defines the case. Here we have not a such.
> 
> The wrong parameter to the function will lead to the same error code
> which is simply wrong.
> 
> > Either way, I checked the original patch. If you really think you need to
> > check for property presence and use default in the case the property isn't
> > found and error out on other errors, add helper functions for the purpose
> > instead of open-coding it all.
> 
> You mean adding 20+ helpers (at least 5 for arrays, 5 for single element,
> and doubled for device_/fwnode_) ?! This sounds like way over verbose
> approach.

It's still better than open coding this where differentiating behaviour is
desired. You'd probably need a few macros to generate the functions for
each type needed.

> 
> > > > It would have been best to have a separate error code for this albeit
> > > > changing this now might not be that troublesome either: very, very few
> > > > callers depend on receiving such an error code but there are still many
> > > > callers.
> > > 
> > > I'm against this because we have already a dedicated API to check for property
> > > presence, why do we need to have another (confusing!) way of doing the same?
> > > 
> > > Having a dedicated code may help to debug, but shouldn't be used as a main
> > > feature in my opinion.
> 
> I will send a next version of this patch touching only the present/bool functions.

Sounds good.

I'd also drop this from the patch

+ * This function is the correct way to check that given property is present    
+ * in the firmware node description.                                           

as redundant.

-- 
Regards,

Sakari Ailus

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Andy Shevchenko]

On Wed, Mar 18, 2026 at 01:26:28PM +0200, Sakari Ailus wrote:
> On Wed, Mar 18, 2026 at 11:41:57AM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 18, 2026 at 11:10:49AM +0200, Sakari Ailus wrote:
> > > On Wed, Mar 18, 2026 at 10:03:27AM +0200, Andy Shevchenko wrote:
> > > > On Wed, Mar 18, 2026 at 12:27:24AM +0200, Sakari Ailus wrote:
> > > > > On Tue, Mar 17, 2026 at 10:08:28PM +0100, Andy Shevchenko wrote:

...

> > > > > > + * In order to check for the property presence, use device_property_present().
> > > > > 
> > > > > Do you really think we should add this clause for each of these functions?
> > > > 
> > > > Yes, as Guenter pointed out that this has to be documented clearly.
> > > > 
> > > > > I don't think it belongs here.
> > > > 
> > > > And? What should we do then (taking into account my below comments)?
> > > > 
> > > > > The error code list doesn't document what is returned if a property doesn't
> > > > > exist (-EINVAL) and it'd be helpful to add this.
> > > > 
> > > > No, this change is exactly against this. Because using an error code that may
> > > > cover not only that case is at bare minimum fragile and layering violation.
> > > > APIs that require to know the implementation details are not good APIs.
> > > 
> > > I have to say I disagree with that,
> > 
> > And I definitely disagree with tribal knowledge based (implementation details
> > as it's not properly documented and can't, because -EINVAL is overloaded) and
> > confusing approach that is in use and may be amended.
> 
> In general it's a good idea to document API functions' return values. It
> certainly won't add to confusion, will it?

No, but it's already documented. -EINVAL is for "invalid parameters". Of course
one may describe that it's not only that, but also property not present and
possibly other cases hidden deeper in the stack of the backend functions. This
will just show my point, that -EINVAL is not guaranteed to be unique for the
purpose.

> > > there's nothing wrong with checking
> > > error codes if you need to.
> > 
> > If that error code defines the case. Here we have not a such.
> > 
> > The wrong parameter to the function will lead to the same error code
> > which is simply wrong.
> > 
> > > Either way, I checked the original patch. If you really think you need to
> > > check for property presence and use default in the case the property isn't
> > > found and error out on other errors, add helper functions for the purpose
> > > instead of open-coding it all.
> > 
> > You mean adding 20+ helpers (at least 5 for arrays, 5 for single element,
> > and doubled for device_/fwnode_) ?! This sounds like way over verbose
> > approach.
> 
> It's still better than open coding this where differentiating behaviour is
> desired. You'd probably need a few macros to generate the functions for
> each type needed.

There are not so many cases, and actually in some of them open coded variant
suits very well. I do not see enough justification in adding those helpers.

> > > > > It would have been best to have a separate error code for this albeit
> > > > > changing this now might not be that troublesome either: very, very few
> > > > > callers depend on receiving such an error code but there are still many
> > > > > callers.
> > > > 
> > > > I'm against this because we have already a dedicated API to check for property
> > > > presence, why do we need to have another (confusing!) way of doing the same?
> > > > 
> > > > Having a dedicated code may help to debug, but shouldn't be used as a main
> > > > feature in my opinion.
> > 
> > I will send a next version of this patch touching only the present/bool functions.
> 
> Sounds good.
> 
> I'd also drop this from the patch
> 
> + * This function is the correct way to check that given property is present
> + * in the firmware node description.
> 
> as redundant.

Then it will be no sense in the patch, will it be? The whole point is to clarify
how one should use API when the desire is to detect if property is present or not.

So, I send my v2 as I see it useful. Then you can NAK it at your wish.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Document how to check for the property presence

[Geert Uytterhoeven]

CC devicetree

On Tue, 17 Mar 2026 at 22:08, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Currently it's unclear if one may or may not rely on the error codes
> returned from the property getters to check for the property presence.
> Clarify this by mass updating kernel-doc for fwnode_property_*() and
> device_property_*() where it's applicable.
>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: 4b24f1f4-b395-467a-81b7-1334a2d48845@roeck-us.net

Missing "https://lore.kernel.org/" prefix.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/base/property.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 8d9a34be57fb..bffa0070ab13 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -38,6 +38,8 @@ EXPORT_SYMBOL_GPL(__dev_fwnode_const);
>   * @propname: Name of the property
>   *
>   * Check if property @propname is present in the device firmware description.
> + * This function is the correct way to check that given property is present
> + * in the device firmware description.
>   *
>   * Return: true if property @propname is present. Otherwise, returns false.
>   */
> @@ -52,6 +54,10 @@ EXPORT_SYMBOL_GPL(device_property_present);
>   * @fwnode: Firmware node whose property to check
>   * @propname: Name of the property
>   *
> + * Check if property @propname is present in the firmware node description.
> + * This function is the correct way to check that given property is present
> + * in the firmware node description.
> + *
>   * Return: true if property @propname is present. Otherwise, returns false.
>   */
>  bool fwnode_property_present(const struct fwnode_handle *fwnode,
> @@ -75,9 +81,9 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
>   * @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.
> + * Use device_property_present() to check for the property presence.
>   *
> - * Return: true if property @propname is present. Otherwise, returns false.
> + * Return: if property @propname is true or false in the device firmware description.
>   */
>  bool device_property_read_bool(const struct device *dev, const char *propname)
>  {
> @@ -90,7 +96,9 @@ EXPORT_SYMBOL_GPL(device_property_read_bool);
>   * @fwnode: Firmware node whose property to check
>   * @propname: Name of the property
>   *
> - * Return if property @propname is true or false in the firmware description.
> + * Use fwnode_property_present() to check for the property presence.
> + *
> + * Return: if property @propname is true or false in the firmware node description.
>   */
>  bool fwnode_property_read_bool(const struct fwnode_handle *fwnode,
>                              const char *propname)
> @@ -121,6 +129,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_bool);
>   * It's recommended to call device_property_count_u8() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -149,6 +159,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u8_array);
>   * It's recommended to call device_property_count_u16() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -177,6 +189,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u16_array);
>   * It's recommended to call device_property_count_u32() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -205,6 +219,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u32_array);
>   * It's recommended to call device_property_count_u64() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -233,6 +249,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
>   * It's recommended to call device_property_string_array_count() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: number of values read on success if @val is non-NULL,
>   *        number of values available on success if @val is NULL,
>   *        %-EINVAL if given arguments are not valid,
> @@ -257,6 +275,8 @@ EXPORT_SYMBOL_GPL(device_property_read_string_array);
>   * Function reads property @propname from the device firmware description and
>   * stores the value into @val if found. The value is checked to be a string.
>   *
> + * In order to check for the property presence, use device_property_present().
> + *
>   * Return: %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
>   *        %-ENODATA if the property does not have a value,
> @@ -324,6 +344,8 @@ static int fwnode_property_read_int_array(const struct fwnode_handle *fwnode,
>   * It's recommended to call fwnode_property_count_u8() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -353,6 +375,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
>   * It's recommended to call fwnode_property_count_u16() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -382,6 +406,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
>   * It's recommended to call fwnode_property_count_u32() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -411,6 +437,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
>   * It's recommended to call fwnode_property_count_u64() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values if @val was %NULL,
>   *         %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
> @@ -440,6 +468,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
>   * It's recommended to call fwnode_property_string_array_count() instead of calling
>   * this function with @val equals %NULL and @nval equals 0.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: number of values read on success if @val is non-NULL,
>   *        number of values available on success if @val is NULL,
>   *        %-EINVAL if given arguments are not valid,
> @@ -476,6 +506,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
>   * Read property @propname from the given firmware node and store the value into
>   * @val if found.  The value is checked to be a string.
>   *
> + * In order to check for the property presence, use fwnode_property_present().
> + *
>   * Return: %0 if the property was found (success),
>   *        %-EINVAL if given arguments are not valid,
>   *        %-ENODATA if the property does not have a value,
> --
> 2.50.1

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds