All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Philipp Stanner <pstanner@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	dakr@redhat.com, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v6 03/10] PCI: Warn users about complicated devres nature
Date: Wed, 24 Apr 2024 15:12:53 -0500	[thread overview]
Message-ID: <20240424201253.GA499493@bhelgaas> (raw)
In-Reply-To: <20240408084423.6697-4-pstanner@redhat.com>

On Mon, Apr 08, 2024 at 10:44:15AM +0200, Philipp Stanner wrote:
> The PCI region-request functions become managed functions when
> pcim_enable_device() has been called previously instead of
> pci_enable_device().
> 
> This has already caused bugs by confusing users, who came to believe
> that all pci functions, such as pci_iomap_range(), suddenly are managed
> that way.

Since you mention a bug, it'd be nice to include a URL or commit if
you have one.

> This is not the case.
> 
> Add comments to the relevant functions' docstrings that warn users about
> this behavior.

> @@ -3914,6 +3916,13 @@ EXPORT_SYMBOL(pci_release_region);
>   *
>   * Returns 0 on success, or %EBUSY on error.  A warning
>   * message is also printed on failure.
> + *
> + * NOTE:
> + * This is a "hybrid" function: Its normally unmanaged, but becomes managed
> + * when pcim_enable_device() has been called in advance.
> + * This hybrid feature is DEPRECATED! If you need to implement a new pci
> + * function that does automatic cleanup, write a new pcim_* function that uses
> + * devres directly.

This note makes sense on exported functions, but I think it's overkill
on static ones like this.

s/Its normally/It's normally/
s/new pci/new PCI/

Rewrap into one paragraph (or separate by blank line).

Applies to all the hunks of this patch.

>  static int __pci_request_region(struct pci_dev *pdev, int bar,
>  				const char *res_name, int exclusive)

  reply	other threads:[~2024-04-24 20:12 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-08  8:44 [PATCH v6 00/10] Make PCI's devres API more consistent Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 01/10] PCI: Add new set of devres functions Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 02/10] PCI: Deprecate iomap-table functions Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 03/10] PCI: Warn users about complicated devres nature Philipp Stanner
2024-04-24 20:12   ` Bjorn Helgaas [this message]
2024-04-08  8:44 ` [PATCH v6 04/10] PCI: Make devres region requests consistent Philipp Stanner
2024-04-24 20:12   ` Bjorn Helgaas
2024-04-26  7:45     ` Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 05/10] PCI: Move dev-enabled status bit to struct pci_dev Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 06/10] PCI: Move pinned " Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 07/10] PCI: Give pcim_set_mwi() its own devres callback Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 08/10] PCI: Give pci(m)_intx " Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 09/10] PCI: Remove legacy pcim_release() Philipp Stanner
2024-04-08  8:44 ` [PATCH v6 10/10] drm/vboxvideo: fix mapping leaks Philipp Stanner
2024-04-22  7:23 ` [PATCH v6 00/10] Make PCI's devres API more consistent Philipp Stanner
2024-04-24 20:12 ` Bjorn Helgaas
2024-04-26  8:07   ` Philipp Stanner
2024-04-26 22:01     ` Bjorn Helgaas
2024-05-07  8:11       ` Philipp Stanner

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=20240424201253.GA499493@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=airlied@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=dakr@redhat.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=pstanner@redhat.com \
    --cc=sam@ravnborg.org \
    --cc=tzimmermann@suse.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.