Re: [PATCH RFC 1/4] regulator: add devm_fwnode family of functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The fallback stubs for `devm_fwnode_regulator_get` and its variants incorrectly return `NULL` instead of `ERR_PTR(-ENODEV)` when `CONFIG_OF` is disabled but `CONFIG_REGULATOR` is enabled, leading to a guaranteed kernel panic on non-OF systems.
--

commit f8899ceec7e07e935c3ae44032450cd2b8f68352
Author: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>

regulator: add devm_fwnode family of functions

Add devm_fwnode_regulator_get and variants.

These function wrappers allow regulators to be accessed from the fwnode
struct without any casts.

> diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
> --- a/include/linux/regulator/consumer.h
> +++ b/include/linux/regulator/consumer.h
[ ... ]
> +static inline struct regulator *__must_check
> +devm_fwnode_regulator_get(struct device *dev, struct fwnode_handle *fwnode,
> +	const char *id)
> +{
> +	return NULL;
> +}

Will returning NULL here instead of ERR_PTR(-ENODEV) cause a panic on non-OF
systems?

If CONFIG_REGULATOR is enabled but CONFIG_OF is disabled, a driver calling
this will get NULL. Since drivers typically check the return value with
IS_ERR(), IS_ERR(NULL) evaluates to false. The driver would assume it
successfully acquired a valid regulator.

Later, when the driver calls regulator_enable(), it will pass this NULL
pointer. Since CONFIG_REGULATOR is enabled, regulator_enable() is not stubbed
out and will unconditionally dereference the pointer, leading to a NULL
pointer dereference.

Should these stubs return ERR_PTR(-ENODEV) instead to safely signal the
absence of a regulator and match the expected regulator API semantics?

> +
> +static inline struct regulator *__must_check
> +devm_fwnode_regulator_get_exclusive(struct device *dev, struct fwnode_handle *fwnode,
> +	const char *id)
> +{
> +	return NULL;
> +}
> +
> +static inline struct regulator *__must_check
> +devm_fwnode_regulator_get_optional(struct device *dev, struct fwnode_handle *fwnode,
> +	const char *id)
> +{
> +	return NULL;
> +}

Does returning NULL for the optional and exclusive variants violate the
regulator API semantics which require returning ERR_PTR(-ENODEV) to safely
signal the absence of a regulator?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-gpiokeys-vdd-supply-v1-0-0bb32e8e6428@fairphone.com?part=1