[PATCH] pinctrl: single: Add missing free in single_allocate_function

[Quentin Schulz]

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 <quentin.schulz at cherry.de>
>>
>> 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