Re: [PATCH] platform/x86: barco-p50-gpio: attach software node to its target GPIO device

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Mar 31, 2026 at 01:28:19PM +0200, Bartosz Golaszewski wrote:
> The software node representing the GPIO controller to consumers is
> "dangling": it's not really attached to the device. The GPIO lookup
> relies on matching the name of the node to the chip's label. Set it as
> the secondary firmware node of the platform device to enable proper
> fwnode-based GPIO lookup.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  drivers/platform/x86/barco-p50-gpio.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/platform/x86/barco-p50-gpio.c b/drivers/platform/x86/barco-p50-gpio.c
> index 2a6d8607c402..5f4ffa584295 100644
> --- a/drivers/platform/x86/barco-p50-gpio.c
> +++ b/drivers/platform/x86/barco-p50-gpio.c
> @@ -365,6 +365,8 @@ static int p50_gpio_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(&pdev->dev, ret, "failed to register software nodes");
>  
> +	set_secondary_fwnode(&pdev->dev, software_node_fwnode(&gpiochip_node));
> +
>  	led_info.fwnode = software_node_fwnode(&gpio_leds_node);
>  	p50->leds_pdev = platform_device_register_full(&led_info);
>  	if (IS_ERR(p50->leds_pdev)) {

I looks like http://sashiko.dev patch critique is on point:

"
Is the software node attached too late to take effect?

In the probe function, devm_gpiochip_add_data() is called before this
set_secondary_fwnode() call. During GPIO chip registration, the gpiolib
core snapshots the parent device's fwnode.

Because the secondary fwnode is not yet set on pdev->dev when this snapshot
happens, the GPIO device is registered with a NULL fwnode, which seems to
defeat the purpose of enabling fwnode-based lookups.

If the order is reversed, would we need to tie the software node
registration to devres (e.g., via devm_add_action_or_reset)? Otherwise, a
manual software_node_unregister_node_group() in the probe error path might
free the software node while the devm-managed gpiochip still holds a pointer
to it.

Additionally, could this leave a dangling pointer on probe failure or driver
unbind?

If a subsequent step fails (like registering keys_pdev), the probe error
path calls software_node_unregister_node_group(p50_swnodes), which frees
the underlying memory.

Because set_secondary_fwnode(&pdev->dev, NULL) is never called to clear it,
pdev->dev.fwnode would point to freed memory. Any subsequent access to the
device's firmware node could trigger a use-after-free.
"

Thanks.

-- 
Dmitry