Re: [PATCH] platform/x86: x86-android-tablets: fix gpio_secondary_fwnode_init() not working

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

On Mon, Aug 31, 2026 at 10:28:33PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 31-Aug-26 22:22, Dmitry Torokhov wrote:
> > On Mon, Aug 31, 2026 at 10:11:57PM +0200, Hans de Goede wrote:
> >> acpi_bus_find_device_by_name() call returns a pointer to the device object
> >> on the ACPI bus, aka the ACPI companion device.
> >>
> >> gpio_secondary_fwnode_init() then continues with setting the secondary
> >> fwnode on this device. But this is not the actual physical device for
> >> the GPIO controller (e.g. the GPIO controller platform bus device).
> >>
> >> This mismatch is causing GPIO lookups by secondary fwnode to not work.
> >>
> >> Modify gpio_secondary_fwnode_init() to instead set the secondary fwnode
> >> of the first physical device associated with the ACPI companion device.
> >>
> >> This fixes the GPIO lookups not working.
> > 
> > Nice find, thank you!
> > 
> >>
> >> Fixes: 1448c2d2ca5c ("platform/x86: x86-android-tablets: enable fwnode matching of GPIO chips")
> >> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> >> ---
> >>  drivers/platform/x86/x86-android-tablets/core.c | 10 ++++++++--
> >>  1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> >> index b028af1c9942..1706287fd136 100644
> >> --- a/drivers/platform/x86/x86-android-tablets/core.c
> >> +++ b/drivers/platform/x86/x86-android-tablets/core.c
> >> @@ -417,9 +417,15 @@ static int gpio_secondary_fwnode_init(struct device *parent,
> >>  		if (WARN_ON(!fwnode))
> >>  			return -ENOENT;
> >>  
> >> -		set_secondary_fwnode(dev, fwnode);
> >> +		struct device *phys_dev = acpi_get_first_physical_node(to_acpi_device(dev));
> >> +		if (!phys_dev)
> >> +			return dev_err_probe(parent,
> >> +					     -ENODEV, "No physical device for ACPI GPIO dev: %s\n",
> >> +					     (*swnode)->name);
> >>  
> >> -		ret = devm_add_action_or_reset(parent, gpio_secondary_unset, get_device(dev));
> >> +		set_secondary_fwnode(phys_dev, fwnode);
> >> +
> >> +		ret = devm_add_action_or_reset(parent, gpio_secondary_unset, get_device(phys_dev));
> > 
> > I am surprised that acpi_get_first_physical_node() does not bump up
> > refcount of the device...
> > >>  		if (ret)
> >>  			return ret;
> > 
> > This leaks refcount on error though...
> 
> The or_reset part of devm_add_action_or_reset() will call gpio_secondary_unset
> on failure which puts the ref we take here on phys_dev.

Ah, yes, you are right. Sorry for the noise.

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry