From: Philipp Stanner <pstanner@redhat.com>
To: 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
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, Philipp Stanner <pstanner@redhat.com>
Subject: [PATCH v9 00/13] Make PCI's devres API more consistent
Date: Thu, 13 Jun 2024 13:50:13 +0200 [thread overview]
Message-ID: <20240613115032.29098-1-pstanner@redhat.com> (raw)
Changes in v9:
- Remove forgotten dead code ('enabled' bit in struct pci_dev) in
patch No.8 ("Move pinned status bit...")
- Rework patch No.3:
- Change title from "Reimplement plural devres functions"
to "Add partial-BAR devres support".
- Drop excessive details about the general cleanup from the commit
message. Only motivate why this patch's new infrastructure is
necessary.
- Fix some minor spelling issues (s/pci/PCI ...)
Changes in v8:
- Rebase the series on the already merged patches which were slightly
modified by Bjorn Helgaas.
- Reword the pci_intx() commit message so it clearly states it's about
reworking pci_intx().
- Move the removal of find_pci_dr() from patch "Remove legacy
pcim_release()" to patch "Give pci_intx() its own devres callback"
since this later patch already removed all calls to that function.
- In patch "Give pci_intx() its own devres callback": use
pci_is_enabled() (and, thus, the enabled_cnt in struct pci_dev)
instead of a separate enabled field. (Bjorn)
Changes in v7:
- Split the entire series in smaller, more atomic chunks / patches
(Bjorn)
- Remove functions (such as pcim_iomap_region_range()) that do not yet
have a user (Bjorn)
- Don't export interfaces publicly anymore, except for
pcim_iomap_range(), needed by vboxvideo (Bjorn)
- Mention the actual (vboxvideo) bug in "PCI: Warn users..." commit
(Bjorn)
- Drop docstring warnings on PCI-internal functions (Bjorn)
- Rework docstring warnings
- Fix spelling in a few places. Rewrapp paragraphs (Bjorn)
Changes in v6:
- Restructure the cleanup in pcim_iomap_regions_request_all() so that
it doesn't trigger a (false positive) test robot warning. No
behavior change intended. (Dan Carpenter)
Changes in v5:
- Add Hans's Reviewed-by to vboxvideo patch (Hans de Goede)
- Remove stable-kernel from CC in vboxvideo patch (Hans de Goede)
Changes in v4:
- Rebase against linux-next
Changes in v3:
- Use the term "PCI devres API" at some forgotten places.
- Fix more grammar errors in patch #3.
- Remove the comment advising to call (the outdated) pcim_intx() in pci.c
- Rename __pcim_request_region_range() flags-field "exclusive" to
"req_flags", since this is what the int actually represents.
- Remove the call to pcim_region_request() from patch #10. (Hans)
Changes in v2:
- Make commit head lines congruent with PCI's style (Bjorn)
- Add missing error checks for devm_add_action(). (Andy)
- Repair the "Returns: " marks for docu generation (Andy)
- Initialize the addr_devres struct with memset(). (Andy)
- Make pcim_intx() a PCI-internal function so that new drivers won't
be encouraged to use the outdated pci_intx() mechanism.
(Andy / Philipp)
- Fix grammar and spelling (Bjorn)
- Be more precise on why pcim_iomap_table() is problematic (Bjorn)
- Provide the actual structs' and functions' names in the commit
messages (Bjorn)
- Remove redundant variable initializers (Andy)
- Regroup PM bitfield members in struct pci_dev (Andy)
- Make pcim_intx() visible only for the PCI subsystem so that new
drivers won't use this outdated API (Andy, Myself)
- Add a NOTE to pcim_iomap() to warn about this function being the one
exception that does just return NULL.
- Consistently use the term "PCI devres API"; also in Patch #10 (Bjorn)
¡Hola!
PCI's devres API suffers several weaknesses:
1. There are functions prefixed with pcim_. Those are always managed
counterparts to never-managed functions prefixed with pci_ – or so one
would like to think. There are some apparently unmanaged functions
(all region-request / release functions, and pci_intx()) which
suddenly become managed once the user has initialized the device with
pcim_enable_device() instead of pci_enable_device(). This "sometimes
yes, sometimes no" nature of those functions is confusing and
therefore bug-provoking. In fact, it has already caused a bug in DRM.
The last patch in this series fixes that bug.
2. iomappings: Instead of giving each mapping its own callback, the
existing API uses a statically allocated struct tracking one mapping
per bar. This is not extensible. Especially, you can't create
_ranged_ managed mappings that way, which many drivers want.
3. Managed request functions only exist as "plural versions" with a
bit-mask as a parameter. That's quite over-engineered considering
that each user only ever mapps one, maybe two bars.
This series:
- add a set of new "singular" devres functions that use devres the way
its intended, with one callback per resource.
- deprecates the existing iomap-table mechanism.
- deprecates the hybrid nature of pci_ functions.
- preserves backwards compatibility so that drivers using the existing
API won't notice any changes.
- adds documentation, especially some warning users about the
complicated nature of PCI's devres.
Note that this series is based on my "unify pci_iounmap"-series from a
few weeks ago. [1]
I tested this on a x86 VM with a simple pci test-device with two
regions. Operates and reserves resources as intended on my system.
Kasan and kmemleak didn't find any problems.
I believe this series cleans the API up as much as possible without
having to port all existing drivers to the new API. Especially, I think
that this implementation is easy to extend if the need for new managed
functions arises :)
Greetings,
P.
Philipp Stanner (13):
PCI: Add and use devres helper for bit masks
PCI: Add devres helpers for iomap table
PCI: Add partial-BAR devres support
PCI: Deprecate two surplus devres functions
PCI: Make devres region requests consistent
PCI: Warn users about complicated devres nature
PCI: Remove enabled status bit from pci_devres
PCI: Move pinned status bit to struct pci_dev
PCI: Give pcim_set_mwi() its own devres callback
PCI: Give pci_intx() its own devres callback
PCI: Remove legacy pcim_release()
PCI: Add pcim_iomap_range()
drm/vboxvideo: fix mapping leaks
drivers/gpu/drm/vboxvideo/vbox_main.c | 20 +-
drivers/pci/devres.c | 903 +++++++++++++++++++++-----
drivers/pci/iomap.c | 16 +
drivers/pci/pci.c | 94 ++-
drivers/pci/pci.h | 23 +-
include/linux/pci.h | 5 +-
6 files changed, 858 insertions(+), 203 deletions(-)
--
2.45.0
next reply other threads:[~2024-06-13 11:50 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-13 11:50 Philipp Stanner [this message]
2024-06-13 11:50 ` [PATCH v9 01/13] PCI: Add and use devres helper for bit masks Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 02/13] PCI: Add devres helpers for iomap table Philipp Stanner
2025-11-23 16:42 ` [PATCH v9 02/13] PCI: Add devres helpers for iomap table [resulting in backtraces on HPPA] Guenter Roeck
2025-11-25 15:48 ` Philipp Stanner
2025-11-25 16:12 ` Guenter Roeck
2025-11-25 16:28 ` Philipp Stanner
2025-11-25 18:49 ` Guenter Roeck
2024-06-13 11:50 ` [PATCH v9 03/13] PCI: Add partial-BAR devres support Philipp Stanner
2024-06-13 21:28 ` Bjorn Helgaas
2024-06-14 8:01 ` Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 04/13] PCI: Deprecate two surplus devres functions Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 05/13] PCI: Make devres region requests consistent Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 06/13] PCI: Warn users about complicated devres nature Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 07/13] PCI: Remove enabled status bit from pci_devres Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 08/13] PCI: Move pinned status bit to struct pci_dev Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 09/13] PCI: Give pcim_set_mwi() its own devres callback Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 10/13] PCI: Give pci_intx() " Philipp Stanner
2024-06-13 21:06 ` Bjorn Helgaas
2024-06-14 8:09 ` Philipp Stanner
2024-06-14 16:14 ` Bjorn Helgaas
2024-06-17 8:21 ` Philipp Stanner
2024-06-17 16:46 ` Bjorn Helgaas
2024-06-18 7:56 ` Philipp Stanner
2024-07-08 21:46 ` Ashish Kalra
2024-07-09 7:21 ` Philipp Stanner
2024-07-09 8:12 ` Kalra, Ashish
2024-07-09 8:56 ` Philipp Stanner
2024-07-09 18:46 ` Kalra, Ashish
2024-07-10 4:08 ` Krzysztof Wilczyński
2024-07-10 4:43 ` Krzysztof Wilczyński
2024-06-13 11:50 ` [PATCH v9 11/13] PCI: Remove legacy pcim_release() Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 12/13] PCI: Add pcim_iomap_range() Philipp Stanner
2024-06-13 11:50 ` [PATCH v9 13/13] drm/vboxvideo: fix mapping leaks Philipp Stanner
2024-06-13 21:57 ` [PATCH v9 00/13] Make PCI's devres API more consistent Bjorn Helgaas
2024-06-14 11:38 ` Philipp Stanner
2024-06-14 16:16 ` Bjorn Helgaas
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=20240613115032.29098-1-pstanner@redhat.com \
--to=pstanner@redhat.com \
--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=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.