public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Felipe Balbi <balbi@kernel.org>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" 
	<linux-usb@vger.kernel.org>
Subject: Re: [PATCH 1/6] software node: Provide replacement for device_add_properties()
Date: Wed, 3 Feb 2021 11:45:35 +0200	[thread overview]
Message-ID: <20210203094535.GC1687065@kuha.fi.intel.com> (raw)
In-Reply-To: <CAJZ5v0hVZBhqzLPGPHDZYPcYyJPfwgYwjzKGYaUMZOBw7Eh7CQ@mail.gmail.com>

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

  reply	other threads:[~2021-02-03  9:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-02 12:50 [PATCH 0/6] usb: Handle device properties with software node API Heikki Krogerus
2021-02-02 12:50 ` [PATCH 1/6] software node: Provide replacement for device_add_properties() Heikki Krogerus
2021-02-02 14:44   ` Rafael J. Wysocki
2021-02-02 15:01     ` Heikki Krogerus
2021-02-02 16:08       ` Rafael J. Wysocki
2021-02-03  9:45         ` Heikki Krogerus [this message]
2021-02-03 13:50           ` Rafael J. Wysocki
2021-02-03 14:26             ` Heikki Krogerus
2021-02-03 14:39               ` Rafael J. Wysocki
2021-02-03 14:51                 ` Heikki Krogerus
2021-02-02 12:50 ` [PATCH 2/6] usb: dwc2: pci: Drop the empty quirk function Heikki Krogerus
2021-02-02 12:50 ` [PATCH 3/6] usb: dwc3: haps: Constify the software node Heikki Krogerus
2021-02-02 16:45   ` kernel test robot
2021-02-02 19:27   ` kernel test robot
2021-02-02 12:50 ` [PATCH 4/6] usb: dwc3: qcom: " Heikki Krogerus
2021-02-02 16:40   ` kernel test robot
2021-02-02 16:54   ` kernel test robot
2021-02-02 12:50 ` [PATCH 5/6] usb: dwc3: host: Use software node API with the properties Heikki Krogerus
2021-02-02 12:50 ` [PATCH 6/6] xhci: ext-caps: " Heikki Krogerus
2021-02-02 13:53   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210203094535.GC1687065@kuha.fi.intel.com \
    --to=heikki.krogerus@linux.intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox