linux-kernel - Re: [PATCH] platform/x86: barco-p50-gpio: use software nodes for gpio-leds/keys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c60ccef1-7213-4dd7-8c10-e8ef03675bd8@kernel.org>
Date: Mon, 11 Aug 2025 19:40:27 +0200
From: Hans de Goede <hansg@...nel.org>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Santosh Kumar Yadav <santoshkumar.yadav@...co.com>,
 Peter Korsgaard <peter.korsgaard@...co.com>,
 Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Arnd Bergmann <arnd@...db.de>, platform-driver-x86@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] platform/x86: barco-p50-gpio: use software nodes for
 gpio-leds/keys

Hi,

On 11-Aug-25 5:49 PM, Dmitry Torokhov wrote:
> On Mon, Aug 11, 2025 at 06:45:23PM +0300, Andy Shevchenko wrote:
>> On Mon, Aug 11, 2025 at 04:20:33PM +0200, Hans de Goede wrote:
>>> On 11-Aug-25 2:44 PM, Andy Shevchenko wrote:
>>>> On Sun, Aug 10, 2025 at 09:31:37PM -0700, Dmitry Torokhov wrote:
>>
>> ...
>>
>>>> Otherwise LGTM as here it looks like we establish platform device ourselves and
>>>> hence no need some additional magic Hans mentioned in the other series.
>>>
>>> Not entirely like with the x86-android-tablets patches this
>>> declares a software-node for the gpiochip:
>>>
>>> static const struct software_node gpiochip_node = {
>>> 	.name = DRIVER_NAME,
>>> };
>>>
>>> and registers that node, but nowhere does it actually
>>> get assigned to the gpiochip.
>>>
>>> This is going to need a line like this added to probe():
>>>
>>> 	p50->gc.fwnode = software_node_fwnode(&gpiochip_node);
>>>
>>> note the software_node_fwnode() call MUST be made after
>>> registering the software-nodes (group).
>>>
>>> Other then needing this single line things are indeed
>>> much easier when the code containing the software
>>> properties / nodes is the same code as which is
>>> registering the gpiochip.
>>
>> Ah, good point!
> 
> This is wrong though, the software node need not be attached to the
> gpiochip (and I wonder if it is even safe to do so). It simply provides
> a name by which gpiochip is looked up in swnode_get_gpio_device().

Ah interesting. This is very different from how fwnodes generally
work though. Generally speaking when a PROPERTY_ENTRY_REF() is used
like PROPERTY_ENTRY_GPIO() does then the lookup is done by matching
the reference to the fwnode of the type of device to which the
reference points.

IOW the standard way how this works for most other subsystems
is that gpiolib-swnode.c: swnode_get_gpio_device() would call
gpio_device_find() with a compare function which uses
device_match_fwnode().

I see that instead it uses the swnode name and passes that to
gpio_device_find_by_label().

I must say that AFAIK this is not how swnodes are supposed to
be used the swnode name field is supposed to only be there
for debugging use and may normally be left empty all together.

I guess using the swnode-name + gpio_device_find_by_label()
works but it goes against the design of how fw-nodes
and especially fwnode-references are supposed to be used...

Having a fwnode reference pointing to what is in essence
a dangling (not attached to any device) fwnode is weird.

Are there already any users of PROPERTY_ENTRY_GPIO() in
the kernel? If not then I think that we should fix things
up to actually do a reference match and not a name based
lookup.

Andy IIRC you've done quite a bit of work on software-nodes,
what is your take on this ?

Note this is likely my last email in this thread for
a while since I will be traveling without email access.

Regards,

Hans