Hi Tom,
On 12/3/25 5:11 PM, Tom Rini wrote:
On Wed, Dec 03, 2025 at 04:36:49PM +0100, Quentin Schulz wrote:
Hi François,
On 12/2/25 7:39 PM, Francois Berder wrote:
If func->pins could not be allocated, one must also free
func variable that was allocated previously.
Well.... devm_* functions should take care of this when the device is
removed (or probe failed), but if and only if CONFIG_DEVRES is enabled.
However, in that case, this code may be executed outside of a probe scenario
I guess (it is called in set_state() callback from the pinctrl device). This
thus makes sense to me here.
Reviewed-by: Quentin Schulz <[email protected]>
I'm also wondering if we shouldn't check the return value of
single_configure_pins/bits in set_state instead of always returning 0?
We probably need to devm_kfree a bunch of other devm_ allocations as well?
I see a loop in single_add_gpio_func() we should probably unwind if there's
a devm_kzalloc which fails?
Maybe we need a .remove callback where we devm_kfree all of functions and
gpiofuncs lists from single_priv, in case DEVRES isn't actually set?
I've wondered, but not fired off a build to check, what the growth is
going to be on default y for DEVRES. There's only a few platforms /
chips doing it today and I think most drivers are written like the
kernel where freeing is automatic, so there's lots of cases like this
I see devm I assume it's automatically freed (or whatever counterpart of
the devm function is) if the device is removed or fails to probe but
that's rarely the actual case.
The issue here is that DEVRES is not enough, we would need SPL_DEVRES,
TPL_DEVRES, VPL_DEVRES, WHATEVER_DEVRES (added and) enabled as well as
we do compile devres only if $(CONFIG_$(PHASE_)DEVRES) and then surround
code with CONFIG_IS_ENABLED(DEVRES).
one here.
I think this one is an exception as the device may still exist if the
error path is taken, so I think it makes sense to explicitly free it
here, regardless of DEVRES compilation state.
Cheers,
Quentin
