LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Zhangfei Gao <zhangfei.gao@linaro.org>,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v3 1/3] PCI: Convert to device_create_managed_software_node()
Date: Thu, 7 Oct 2021 14:03:50 +0300	[thread overview]
Message-ID: <YV7UFoAXb5MrkaFg@kuha.fi.intel.com> (raw)
In-Reply-To: <20211006184754.GA1171384@bhelgaas>

On Wed, Oct 06, 2021 at 01:47:54PM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 06, 2021 at 02:26:41PM +0300, Heikki Krogerus wrote:
> > In quirk_huawei_pcie_sva(), use device_create_managed_software_node()
> > instead of device_add_properties() to set the "dma-can-stall"
> > property.
> > 
> > This is the last user of device_add_properties() that relied on
> > device_del() to take care of also calling device_remove_properties().
> > After this change we can finally get rid of that
> > device_remove_properties() call in device_del().
> > 
> > After that device_remove_properties() call has been removed from
> > device_del(), the software nodes that hold the additional device
> > properties become reusable and shareable as there is no longer a
> > default assumption that those nodes are lifetime bound the first
> > device they are associated with.
> 
> This does not help me determine whether this patch is safe.
> device_create_managed_software_node() sets swnode->managed = true,
> but device_add_properties() did not.  I still don't know what the
> effect of that is.

OK. So how about this:

        PCI: Convert to device_create_managed_software_node()

        In quirk_huawei_pcie_sva(), device_add_properties() is used to
        inject additional device properties, but there is no
        device_remove_properties() call anywhere to remove those
        properties. The assumption is most likely that the device is
        never removed, and the properties therefore do not also never
        need to be removed.

        Even though it is unlikely that the device is ever removed in
        this case, it is safer to make sure that the properties are
        also removed if the device ever does get unregistered.

        To achieve this, instead of adding a separate quirk for the
        case of device removal where device_remove_properties() is
        called, using device_create_managed_software_node() instead of
        device_add_properties(). Both functions create a software node
        (a type of fwnode) that holds the device properties, which is
        then assigned to the device very much the same way.

        The difference between the two functions is, that
        device_create_managed_software_node() guarantees that the
        software node (together with the properties) is removed when
        the device is removed. The function device_add_property() does
        _not_ guarantee that, so the properties added with it should
        always be removed with device_remove_properties().

        SoB...

thanks,

-- 
heikki

  reply	other threads:[~2021-10-07 11:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-06 11:26 [PATCH v3 0/3] device property: Remove device_add_properties() Heikki Krogerus
2021-10-06 11:26 ` [PATCH v3 1/3] PCI: Convert to device_create_managed_software_node() Heikki Krogerus
2021-10-06 18:47   ` Bjorn Helgaas
2021-10-07 11:03     ` Heikki Krogerus [this message]
2021-10-07 19:35       ` Bjorn Helgaas
2021-10-06 11:26 ` [PATCH v3 2/3] driver core: Don't call device_remove_properties() from device_del() Heikki Krogerus
2021-10-06 11:26 ` [PATCH v3 3/3] device property: Remove device_add_properties() API Heikki Krogerus

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=YV7UFoAXb5MrkaFg@kuha.fi.intel.com \
    --to=heikki.krogerus@linux.intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=zhangfei.gao@linaro.org \
    --subject='Re: [PATCH v3 1/3] PCI: Convert to device_create_managed_software_node()' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).