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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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