linux-kernel - Re: [PATCH 1/6] software node: Provide replacement for device_add

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: <20210203094535.GC1687065@kuha.fi.intel.com>
Date:   Wed, 3 Feb 2021 11:45:35 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Felipe Balbi <balbi@...nel.org>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" 
        <linux-usb@...r.kernel.org>
Subject: Re: [PATCH 1/6] software node: Provide replacement for
 device_add_properties()

On Tue, Feb 02, 2021 at 05:08:40PM +0100, Rafael J. Wysocki wrote:
> It looks like there is a use case that cannot be addressed by using
> device_add_properties() and that's why you need this new function.
> 
> Can you describe that use case, please, and explain what the problem
> with using device_add_properties() in it is?

The problem with device_add_properties() is that it gives false
impression that the device properties are somehow directly attached to
the devices, which is not true. Now, that should not be a major issue,
but it seems that it is. I think Lee Jones basically used that as an
argument to refuse changes (and pretty minor changes) that would have
allowed us to use software nodes with the MFD drivers.

Nevertheless, I was not planning to provide a replacement for it
originally. We do in any case have the real issue caused by that
device_remove_properties() call in device_del() which has to be fixed.
I started to fix that by moving device_add_properties() under
drivers/base/swnode.c so I can implement it like I've implemented now
that new function, but after that when I started to tackle the second
issue by removing the subsystem wrappers like
platform_device_add_device_properties() and replacing them with things
like platform_device_add_software_node() in order to give the drivers
a chance to actually use software nodes, I realised that there isn't
much use for device_add_properties() after that.

Also, even though I'm not super happy about adding more API to this
thing, this function - device_create_managed_software_node() - I do
like. Not only is it implemented so that we don't have to rely on
anything else in drivers core in order to achieve the lifetime link
with the device, it is also much more descriptive. The function name
alone does not leave any questions about what is going to happen if
that function is called. You'll end up with a software node that just
happens to be attached to the device.

On top of those two things, by adding the new function it also gives
me a nice stepping stone to do these changes in the normal stages:

        1. Add feature/modification.
        2. Convert users.
        3. Cleanup.

And finally, even though we may not have any users left for
device_add_properties() after I have updated all the subsystems and
drivers, this new function will still be useful. That is because, even
though it can be used as a drop-in replacement for
device_add_properties(), it does add that one small but important
change. It allows the nodes created with it to be part of node
hierarchy, and that alone is useful to me, and I'm planning on using
that feature later.

I'm sorry about the long explanation.

Br,

-- 
heikki