Re: [PATCH v1 1/1] hwmon: (isl28022) Don't check for specific errors when parsing properties

[Andy Shevchenko]

On Fri, Feb 20, 2026 at 12:49:25PM -0800, Guenter Roeck wrote:
> On 2/19/26 06:30, Andy Shevchenko wrote:
> > On Thu, Feb 19, 2026 at 03:21:29PM +0100, Geert Uytterhoeven wrote:
> > > On Thu, 19 Feb 2026 at 15:06, Andy Shevchenko
> > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > > Instead of checking for the specific error codes (that can be considered
> > > > a layering violation to some extent) check for the property existence first
> > > > and then either parse it, or apply a default value.
> > 
> > > IIRC, we have removed superfluous presence checks all over the tree
> > > during the past few years? E.g. of_property_read_*() is documented to
> > > return -EINVAL if a property does not exist.
> > 
> > Even though, it's still fragile. When we have a check for explicit device
> > presence, we wouldn't care of the error code we get in case of unsuccessful
> > parsing.
> > 
> > > So this patch looks like a step back to me...
> > 
> > Obviously I have a disagreement here, this is step forward to weaken
> > the dependency on the certain error code in the cases when we can avoid
> > that. Motivation is mentioned in the commit message.
> > 
> > Also note, -EINVAL can sneak in tons of mysterious ways as it's one of
> > the most overloaded error code in the kernel, its semantic is basically
> > equals to "an error happened".
> > 
> > Having the code above, we make it robust against some subtle nuances which
> > may not be discovered in time.
> > 
> 
> Is that documented somewhere ? I have been asking people to use the current
> approach to reduce code size. device_property_present() isn't even mentioned
> as API in Documentation/. If "do not rely on error codes from device property
> API functions to determine if a default should be applied" or similar is a new
> rule or guidance, it should be clearly documented somewhere.

Fair enough.

Currently if one reads kernel-doc for fwnode_property_*() / device_property_*()
APIs, there is no clear specification that -EINVAL is equal to "property not
present". It's documented as

 *         %-EINVAL if given arguments are not valid,

which may be the case, if the parameter itself is wrong, for example fwnode is
invalid to begin with. Checking against -EINVAL for 'property not present' is
layering violation and relying on deep implementation detail.

I will craft a change to put something like this to the
fwnode_property_present() and device_property_present() kernel-docs.

-- 
With Best Regards,
Andy Shevchenko