All of lore.kernel.org
 help / color / mirror / Atom feed
From: andy.shevchenko@gmail.com
To: Philipp Stanner <pstanner@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>,
	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, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH 00/10] Make PCI's devres API more consistent
Date: Tue, 16 Jan 2024 23:17:23 +0200	[thread overview]
Message-ID: <ZabyY3csP0y-p7lb@surfacebook.localdomain> (raw)
In-Reply-To: <20240115144655.32046-2-pstanner@redhat.com>

Mon, Jan 15, 2024 at 03:46:11PM +0100, Philipp Stanner kirjoitti:
> ¡Hola!

i? Vim user? :-)

> 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.

Instead of adding pcim_intx(), please provide proper one for
pci_alloc_irq_vectors(). Ideally it would be nice to deprecate
old IRQ management functions in PCI core and delete them in the
future.

-- 
With Best Regards,
Andy Shevchenko



WARNING: multiple messages have this Message-ID (diff)
From: andy.shevchenko@gmail.com
To: Philipp Stanner <pstanner@redhat.com>
Cc: Daniel Vetter <daniel@ffwll.ch>, Jonathan Corbet <corbet@lwn.net>,
	Sam Ravnborg <sam@ravnborg.org>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Maxime Ripard <mripard@kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	dakr@redhat.com, dri-devel@lists.freedesktop.org,
	Thomas Zimmermann <tzimmermann@suse.de>,
	linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
	David Airlie <airlied@gmail.com>
Subject: Re: [PATCH 00/10] Make PCI's devres API more consistent
Date: Tue, 16 Jan 2024 23:17:23 +0200	[thread overview]
Message-ID: <ZabyY3csP0y-p7lb@surfacebook.localdomain> (raw)
In-Reply-To: <20240115144655.32046-2-pstanner@redhat.com>

Mon, Jan 15, 2024 at 03:46:11PM +0100, Philipp Stanner kirjoitti:
> ¡Hola!

i? Vim user? :-)

> 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.

Instead of adding pcim_intx(), please provide proper one for
pci_alloc_irq_vectors(). Ideally it would be nice to deprecate
old IRQ management functions in PCI core and delete them in the
future.

-- 
With Best Regards,
Andy Shevchenko



  parent reply	other threads:[~2024-01-16 21:18 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-15 14:46 [PATCH 00/10] Make PCI's devres API more consistent Philipp Stanner
2024-01-15 14:46 ` Philipp Stanner
2024-01-15 14:46 ` [PATCH 01/10] pci: add new set of devres functions Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 18:44   ` Bjorn Helgaas
2024-01-16 18:44     ` Bjorn Helgaas
2024-01-17  8:54     ` Philipp Stanner
2024-01-17  8:54       ` Philipp Stanner
2024-01-19 22:52       ` Bjorn Helgaas
2024-01-19 22:52         ` Bjorn Helgaas
2024-01-16 21:15   ` andy.shevchenko
2024-01-16 21:15     ` andy.shevchenko
2024-01-17  9:21     ` Philipp Stanner
2024-01-17  9:21       ` Philipp Stanner
2024-01-15 14:46 ` [PATCH 02/10] pci: deprecate iomap-table functions Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 21:27   ` andy.shevchenko
2024-01-16 21:27     ` andy.shevchenko
2024-01-17  9:40     ` Philipp Stanner
2024-01-17  9:40       ` Philipp Stanner
2024-01-15 14:46 ` [PATCH 03/10] pci: warn users about complicated devres nature Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-15 14:46 ` [PATCH 04/10] pci: devres: make devres region requests consistent Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 21:29   ` andy.shevchenko
2024-01-16 21:29     ` andy.shevchenko
2024-01-15 14:46 ` [PATCH 05/10] pci: move enabled status bit to pci_dev struct Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-15 14:46 ` [PATCH 06/10] pci: move pinned " Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 21:34   ` andy.shevchenko
2024-01-16 21:34     ` andy.shevchenko
2024-01-17  9:02     ` Philipp Stanner
2024-01-17  9:02       ` Philipp Stanner
2024-01-15 14:46 ` [PATCH 07/10] pci: devres: give mwi its own callback Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 18:35   ` Bjorn Helgaas
2024-01-16 18:35     ` Bjorn Helgaas
2024-01-15 14:46 ` [PATCH 08/10] pci: devres: give pci(m)_intx " Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 21:37   ` andy.shevchenko
2024-01-16 21:37     ` andy.shevchenko
2024-01-15 14:46 ` [PATCH 09/10] pci: devres: remove legacy pcim_release() Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 21:40   ` andy.shevchenko
2024-01-16 21:40     ` andy.shevchenko
2024-01-17 13:49     ` Philipp Stanner
2024-01-17 13:49       ` Philipp Stanner
2024-01-15 14:46 ` [PATCH 10/10] drm/vboxvideo: fix mapping leaks Philipp Stanner
2024-01-15 14:46   ` Philipp Stanner
2024-01-16 18:29 ` [PATCH 00/10] Make PCI's devres API more consistent Bjorn Helgaas
2024-01-16 18:29   ` Bjorn Helgaas
2024-01-16 21:17 ` andy.shevchenko [this message]
2024-01-16 21:17   ` andy.shevchenko
2024-01-17  9:59   ` Philipp Stanner
2024-01-17  9:59     ` Philipp Stanner
2024-01-18  8:48     ` Philipp Stanner
2024-01-18  8:48       ` 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=ZabyY3csP0y-p7lb@surfacebook.localdomain \
    --to=andy.shevchenko@gmail.com \
    --cc=airlied@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=corbet@lwn.net \
    --cc=dakr@redhat.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --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.