Remove implicit devres from pci_intx()

+/**
+ * pci_intx - enables/disables PCI INTx for device dev, unmanaged version

+ * @pdev: the PCI device to operate on
+ * @enable: boolean: whether to enable or disable PCI INTx
+ *
+ * Enables/disables PCI INTx for device @pdev
+ *
+ * This function behavios identically to pci_intx(), but is never managed with
+ * devres.
+ */
+void pci_intx_unmanaged(struct pci_dev *pdev, int enable)

Quoted 3 lines · Philipp Stanner

+/**
+ * pci_intx - enables/disables PCI INTx for device dev, unmanaged version

mismatch vs. actual function name.

Quoted 11 lines

+ * @pdev: the PCI device to operate on
+ * @enable: boolean: whether to enable or disable PCI INTx
+ *
+ * Enables/disables PCI INTx for device @pdev
+ *
+ * This function behavios identically to pci_intx(), but is never managed with
+ * devres.
+ */
+void pci_intx_unmanaged(struct pci_dev *pdev, int enable)

This is a misnomer. The function controls the INTX_DISABLE bit of a PCI device. Something like this:

void __pci_intx_control()
{
}

static inline void pci_intx_enable(d)
{
        __pci_intx_control(d, true);
}

.....

makes it entirely clear what this is about.

Hmm?

Thanks,

        tglx

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

hda_intel enables its PCI-Device with pcim_enable_device(). Thus, it needs the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---
 sound/pci/hda/hda_intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_intel.c +0 -0

index b4540c5cd2a6..b44ca7b6e54f 100644

Quoted 31 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through
devres. To remove this hybrid nature from pci_intx(), it is necessary to
port users to either an always-managed or a never-managed version.

hda_intel enables its PCI-Device with pcim_enable_device(). Thus, it needs
the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---
 sound/pci/hda/hda_intel.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_intel.c +1 -1

index b4540c5cd2a6..b44ca7b6e54f 100644
@@ -786,7 +786,7 @@ static int azx_acquire_irq(struct azx *chip,
 int do_disconnect)
  	}
  	bus->irq = chip->pci->irq;
  	chip->card->sync_irq = bus->irq;
-	pci_intx(chip->pci, !chip->msi);
+	pcim_intx(chip->pci, !chip->msi);
  	return 0;
  }

Quoted 1 line

Hm, it's OK-ish to do this as it's practically same as what pci_intx()
currently does.  But, the current code can be a bit inconsistent about
the original intx value.  pcim_intx() always stores !enable to res->orig_intx unconditionally, and it means that the orig_intx value gets overridden at each time pcim_intx() gets called.

Meanwhile, HD-audio driver does release and re-acquire the interrupt after disabling MSI when something goes wrong, and pci_intx() call above is a part of that procedure.  So, it can rewrite the res->orig_intx to another value by retry without MSI.  And after the driver removal, it'll lead to another state.

In anyway, as it doesn't change the current behavior, feel free to take my ack for now:

Acked-by Takashi Iwai <[email protected]>

thanks,

Takashi

Quoted 35 lines · Takashi Iwai

Quoted 31 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through
devres. To remove this hybrid nature from pci_intx(), it is necessary to
port users to either an always-managed or a never-managed version.

hda_intel enables its PCI-Device with pcim_enable_device(). Thus, it needs
the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---
 sound/pci/hda/hda_intel.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_intel.c +1 -1

index b4540c5cd2a6..b44ca7b6e54f 100644
@@ -786,7 +786,7 @@ static int azx_acquire_irq(struct azx *chip,
 int do_disconnect)
  	}
  	bus->irq = chip->pci->irq;
  	chip->card->sync_irq = bus->irq;
-	pci_intx(chip->pci, !chip->msi);
+	pcim_intx(chip->pci, !chip->msi);
  	return 0;
  }

Quoted 1 line

Quoted 7 lines

Hm, it's OK-ish to do this as it's practically same as what pci_intx()
currently does.  But, the current code can be a bit inconsistent about
the original intx value.  pcim_intx() always stores !enable to res->orig_intx unconditionally, and it means that the orig_intx value gets overridden at each time pcim_intx() gets called.

Yes.

Quoted 1 line

Quoted 5 lines

Meanwhile, HD-audio driver does release and re-acquire the interrupt after disabling MSI when something goes wrong, and pci_intx() call above is a part of that procedure.  So, it can rewrite the res->orig_intx to another value by retry without MSI.  And after the driver removal, it'll lead to another state.

I'm not sure that I understand this paragraph completely. Still, could a solution for the driver on the long-term just be to use pci_intx()?

Quoted 62 lines · Philipp Stanner

Quoted 39 lines · Takashi Iwai

Quoted 34 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through
devres. To remove this hybrid nature from pci_intx(), it is necessary to
port users to either an always-managed or a never-managed version.

hda_intel enables its PCI-Device with pcim_enable_device(). Thus,
it needs
the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---
 sound/pci/hda/hda_intel.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_intel.c +1 -1

index b4540c5cd2a6..b44ca7b6e54f 100644
@@ -786,7 +786,7 @@ static int azx_acquire_irq(struct azx
 *chip,
 int do_disconnect)
  	}
  	bus->irq = chip->pci->irq;
  	chip->card->sync_irq = bus->irq;
-	pci_intx(chip->pci, !chip->msi);
+	pcim_intx(chip->pci, !chip->msi);
  	return 0;

Quoted 2 lines

 }

Quoted 8 lines

Hm, it's OK-ish to do this as it's practically same as what pci_intx()
currently does.  But, the current code can be a bit inconsistent about
the original intx value.  pcim_intx() always stores !enable to res->orig_intx unconditionally, and it means that the orig_intx value
gets overridden at each time pcim_intx() gets called.

Yes.

Quoted 9 lines

Meanwhile, HD-audio driver does release and re-acquire the interrupt
after disabling MSI when something goes wrong, and pci_intx() call
above is a part of that procedure.  So, it can rewrite the res->orig_intx to another value by retry without MSI.  And after the
driver removal, it'll lead to another state.

Quoted 4 lines

I'm not sure that I understand this paragraph completely. Still, could
a solution for the driver on the long-term just be to use pci_intx()?

pci_intx() misses the restore of the original value, so it's no long-term solution, either.

What I meant is that pcim_intx() blindly assumes the negative of the passed argument as the original state, which isn't always true.  e.g. when the driver calls it twice with different values, a wrong value may be remembered.

That said, I thought of something like below.

thanks,

Takashi

-- 8< --

drivers/pci/devres.c +0 -0

@@ -438,8 +438,17 @@ static void pcim_intx_restore(struct device
 *dev, void *data)
  	__pcim_intx(pdev, res->orig_intx);
  }

-static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)
+static void save_orig_intx(struct pci_dev *pdev)
 {

drivers/pci/devres.c +7 -0

+	u16 pci_command;
+
+	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+	res->orig_intx = !(pci_command & PCI_COMMAND_INTX_DISABLE);
+}
+
+static struct pcim_intx_devres *get_or_create_intx_devres(struct

pci_dev *pdev)

drivers/pci/devres.c +2 -0

+{
+	struct device *dev = &pdev->dev;

  struct pcim_intx_devres *res;

  res = devres_find(dev, pcim_intx_restore, NULL, NULL);

drivers/pci/devres.c +0 -0

@@ -447,8 +456,10 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
  		return res;

  res = devres_alloc(pcim_intx_restore, sizeof(*res), GFP_KERNEL);

drivers/pci/devres.c +2 -1

-	if (res)
+	if (res) {
+		save_orig_intx(pdev);

  devres_add(dev, res);

drivers/pci/devres.c +1 -0

+	}

  return res;
 }

drivers/pci/devres.c +0 -0

@@ -467,11 +478,10 @@ int pcim_intx(struct pci_dev *pdev, int enable)
  {
  	struct pcim_intx_devres *res;

drivers/pci/devres.c +1 -1

-	res = get_or_create_intx_devres(&pdev->dev);
+	res = get_or_create_intx_devres(pdev);

  if (!res)
  return -ENOMEM;

drivers/pci/devres.c +0 -1

-	res->orig_intx = !enable;

  __pcim_intx(pdev, enable);

  return 0;

Quoted 64 lines · Takashi Iwai

Quoted 62 lines · Philipp Stanner

Quoted 39 lines · Takashi Iwai

Quoted 34 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through
devres. To remove this hybrid nature from pci_intx(), it is necessary to
port users to either an always-managed or a never-managed version.

hda_intel enables its PCI-Device with pcim_enable_device(). Thus,
it needs
the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---
 sound/pci/hda/hda_intel.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_intel.c +1 -1

index b4540c5cd2a6..b44ca7b6e54f 100644
@@ -786,7 +786,7 @@ static int azx_acquire_irq(struct azx
 *chip,
 int do_disconnect)
  	}
  	bus->irq = chip->pci->irq;
  	chip->card->sync_irq = bus->irq;
-	pci_intx(chip->pci, !chip->msi);
+	pcim_intx(chip->pci, !chip->msi);
  	return 0;

Quoted 2 lines

 }

Quoted 8 lines

Hm, it's OK-ish to do this as it's practically same as what pci_intx()
currently does.  But, the current code can be a bit inconsistent about
the original intx value.  pcim_intx() always stores !enable to res->orig_intx unconditionally, and it means that the orig_intx value
gets overridden at each time pcim_intx() gets called.

Yes.

Quoted 9 lines

Meanwhile, HD-audio driver does release and re-acquire the interrupt
after disabling MSI when something goes wrong, and pci_intx() call
above is a part of that procedure.  So, it can rewrite the res->orig_intx to another value by retry without MSI.  And after the
driver removal, it'll lead to another state.

Quoted 7 lines

Quoted 4 lines

I'm not sure that I understand this paragraph completely. Still, could
a solution for the driver on the long-term just be to use pci_intx()?

pci_intx() misses the restore of the original value, so it's no long-term solution, either.

Sure that is missing – I was basically asking whether the driver could live without that feature.

Consider that point obsolete, see below

Quoted 5 lines

What I meant is that pcim_intx() blindly assumes the negative of the passed argument as the original state, which isn't always true.  e.g. when the driver calls it twice with different values, a wrong value may be remembered.

Ah, I see – thoguh the issue is when it's called several times with the *same* value, isn't it?

E.g.

pcim_intx(pdev, 1); // 0 is remembered as the old value pcim_intx(pdev, 1); // 0 is falsely remembered as the old value

Also, it would seem that calling the function for the first time like that:

pcim_intx(pdev, 0); // old value: 1

is at least incorrect, because INTx should be 0 per default, shouldn't it? Could then even be a 1st class bug, because INTx would end up being enabled despite having been disabled all the time.

Quoted 1 line

That said, I thought of something like below.

At first glance that looks like a good idea to me, thanks for working this out!

IMO you can submit that as a patch so we can discuss it separately.

Quoted 108 lines · Philipp Stanner

Quoted 69 lines · Takashi Iwai

Quoted 67 lines · Philipp Stanner

Quoted 43 lines · Takashi Iwai

Quoted 35 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed
through
devres. To remove this hybrid nature from pci_intx(), it is necessary to
port users to either an always-managed or a never-managed version.

hda_intel enables its PCI-Device with pcim_enable_device(). Thus,
it needs
the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---
 sound/pci/hda/hda_intel.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_intel.c +1 -1

index b4540c5cd2a6..b44ca7b6e54f 100644
@@ -786,7 +786,7 @@ static int azx_acquire_irq(struct azx
 *chip,
 int do_disconnect)
  	}
  	bus->irq = chip->pci->irq;
  	chip->card->sync_irq = bus->irq;
-	pci_intx(chip->pci, !chip->msi);
+	pcim_intx(chip->pci, !chip->msi);
  	return 0;

Quoted 2 lines

 }

Hm, it's OK-ish to do this as it's practically same as what pci_intx()
currently does.  But, the current code can be a bit

Quoted 8 lines

inconsistent
about
the original intx value.  pcim_intx() always stores !enable to
res->orig_intx unconditionally, and it means that the orig_intx
value
gets overridden at each time pcim_intx() gets called.

Yes.

Quoted 10 lines

Meanwhile, HD-audio driver does release and re-acquire the interrupt
after disabling MSI when something goes wrong, and pci_intx() call
above is a part of that procedure.  So, it can rewrite the res->orig_intx to another value by retry without MSI.  And after
the
driver removal, it'll lead to another state.

Quoted 8 lines

Quoted 5 lines

I'm not sure that I understand this paragraph completely. Still,
could
a solution for the driver on the long-term just be to use pci_intx()?

pci_intx() misses the restore of the original value, so it's no long-term solution, either.

Sure that is missing – I was basically asking whether the driver could
live without that feature.

Consider that point obsolete, see below

Quoted 8 lines

What I meant is that pcim_intx() blindly assumes the negative of the
passed argument as the original state, which isn't always true. e.g.
when the driver calls it twice with different values, a wrong value
may be remembered.

Ah, I see – thoguh the issue is when it's called several times with the
*same* value, isn't it?

E.g.

pcim_intx(pdev, 1); // 0 is remembered as the old value pcim_intx(pdev, 1); // 0 is falsely remembered as the old value

Also, it would seem that calling the function for the first time like
that:

Quoted 7 lines

pcim_intx(pdev, 0); // old value: 1

is at least incorrect, because INTx should be 0 per default, shouldn't
it? Could then even be a 1st class bug, because INTx would end up being
enabled despite having been disabled all the time.

Yeah, and the unexpected restore can happen even with a single call of
pcim_intx(), if the driver calls it unnecessarily.

Quoted 2 lines

Quoted 1 line

That said, I thought of something like below.

Quoted 5 lines

At first glance that looks like a good idea to me, thanks for working
this out!

IMO you can submit that as a patch so we can discuss it separately.

Sure, I'm going to submit later.

thanks,

Takashi

Quoted 124 lines · Takashi Iwai

Quoted 108 lines · Philipp Stanner

Quoted 69 lines · Takashi Iwai

Quoted 67 lines · Philipp Stanner

Quoted 43 lines · Takashi Iwai

Quoted 35 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed
through
devres. To remove this hybrid nature from pci_intx(), it is necessary to
port users to either an always-managed or a never-managed version.

hda_intel enables its PCI-Device with pcim_enable_device(). Thus,
it needs
the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---
 sound/pci/hda/hda_intel.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

sound/pci/hda/hda_intel.c +1 -1

index b4540c5cd2a6..b44ca7b6e54f 100644
@@ -786,7 +786,7 @@ static int azx_acquire_irq(struct azx
 *chip,
 int do_disconnect)
  	}
  	bus->irq = chip->pci->irq;
  	chip->card->sync_irq = bus->irq;
-	pci_intx(chip->pci, !chip->msi);
+	pcim_intx(chip->pci, !chip->msi);
  	return 0;

Quoted 2 lines

 }

Hm, it's OK-ish to do this as it's practically same as what pci_intx()
currently does.  But, the current code can be a bit

Quoted 8 lines

inconsistent
about
the original intx value.  pcim_intx() always stores !enable to
res->orig_intx unconditionally, and it means that the orig_intx
value
gets overridden at each time pcim_intx() gets called.

Yes.

Quoted 10 lines

Meanwhile, HD-audio driver does release and re-acquire the interrupt
after disabling MSI when something goes wrong, and pci_intx() call
above is a part of that procedure.  So, it can rewrite the res->orig_intx to another value by retry without MSI.  And after
the
driver removal, it'll lead to another state.

Quoted 8 lines

Quoted 5 lines

I'm not sure that I understand this paragraph completely. Still,
could
a solution for the driver on the long-term just be to use pci_intx()?

pci_intx() misses the restore of the original value, so it's no long-term solution, either.

Sure that is missing – I was basically asking whether the driver could
live without that feature.

Consider that point obsolete, see below

Quoted 8 lines

What I meant is that pcim_intx() blindly assumes the negative of the
passed argument as the original state, which isn't always true. e.g.
when the driver calls it twice with different values, a wrong value
may be remembered.

Ah, I see – thoguh the issue is when it's called several times with the
*same* value, isn't it?

E.g.

pcim_intx(pdev, 1); // 0 is remembered as the old value pcim_intx(pdev, 1); // 0 is falsely remembered as the old value

Also, it would seem that calling the function for the first time like
that:

Quoted 7 lines

pcim_intx(pdev, 0); // old value: 1

is at least incorrect, because INTx should be 0 per default, shouldn't
it? Could then even be a 1st class bug, because INTx would end up being
enabled despite having been disabled all the time.

Yeah, and the unexpected restore can happen even with a single call of
pcim_intx(), if the driver calls it unnecessarily.

Quoted 2 lines

Quoted 1 line

That said, I thought of something like below.

Quoted 7 lines

Quoted 5 lines

At first glance that looks like a good idea to me, thanks for working
this out!

IMO you can submit that as a patch so we can discuss it separately.

Sure, I'm going to submit later.

I just took a look into the old implementation of pci_intx() (there was no pcim_intx() back then), before I started cleaning up PCI's devres. This what it looked like before 25216afc9db53d85dc648aba8fb7f6d31f2c8731:

void pci_intx(struct pci_dev *pdev, int enable)

{
	u16 pci_command, new;

	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);

	if (enable)
		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
	else
		new = pci_command | PCI_COMMAND_INTX_DISABLE;

	if (new != pci_command) {
		struct pci_devres *dr;

		pci_write_config_word(pdev, PCI_COMMAND, new);

		dr = find_pci_dr(pdev);
		if (dr && !dr->restore_intx) {
			dr->restore_intx = 1;
			dr->orig_intx = !enable;
		}
	}
}

EXPORT_SYMBOL_GPL(pci_intx);

If I'm not mistaken the old version did not have the problem because the value to be restored only changed if new != pci_command.

That should always be correct, what do you think?

If so, only my commit 25216afc9db53d85dc648aba8fb7f6d31f2c8731 needs to be fixed.

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

All users in ata enable their PCI-Device with pcim_enable_device(). Thus, they need the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

All users in ata enable their PCI-Device with pcim_enable_device(). Thus, they need the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

---

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

qtnfmac enables its PCI-Device with pcim_enable_device(). Thus, it needs the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>

pci_intx() and its managed counterpart pcim_intx() only exist for older drivers which have not been ported yet for various reasons. Future drivers should preferably use pci_alloc_irq_vectors().

Mark pci_intx() and pcim_intx() as deprecated and encourage usage of pci_alloc_irq_vectors() in its place.

Signed-off-by Philipp Stanner <[email protected]>

---
 drivers/pci/devres.c | 5 ++++-
 drivers/pci/pci.c    | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

drivers/pci/devres.c +4 -1

index 6f8f712fe34e..4c76fc063104 100644
@@ -435,7 +435,7 @@ static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)
 }
 
 /**
- * pcim_intx - managed pci_intx()
+ * pcim_intx - managed pci_intx() (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx
  *

@@ -443,6 +443,9 @@ static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)
  *
  * Enable/disable PCI INTx for device @pdev.
  * Restore the original state on driver detach.
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead (there is no managed version, currently).
  */
 int pcim_intx(struct pci_dev *pdev, int enable)
 {

drivers/pci/pci.c +4 -1

index 7ce1d0e3a1d5..dc69e23b8982 100644
@@ -4477,11 +4477,14 @@ void pci_disable_parity(struct pci_dev *dev)
 }
 
 /**
- * pci_intx - enables/disables PCI INTx for device dev
+ * pci_intx - enables/disables PCI INTx for device dev (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx
  *
  * Enables/disables PCI INTx for device @pdev
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead.
  */
 void pci_intx(struct pci_dev *pdev, int enable)
 {

Quoted 8 lines · On Tue, 15 Oct 2024 20:51:23 +0200 Philipp Stanner <[email protected]> wrote:

pci_intx() and its managed counterpart pcim_intx() only exist for older
drivers which have not been ported yet for various reasons. Future drivers should preferably use pci_alloc_irq_vectors().

Mark pci_intx() and pcim_intx() as deprecated and encourage usage of
pci_alloc_irq_vectors() in its place.

I don't really understand this.  As we've discussed previously pci_alloc_irq_vectors() is, unsurprisingly, for allocating PCI IRQ vectors while pci_intx() is for manipulating the INTx disable bit on PCI devices.  The latter is a generic mechanism for preventing PCI devices from generating INTx, regardless of whether there's a vector allocated for it.  How does the former replace the latter and why do we
feel the need to deprecate the latter?

It feels like this fits some narrow narrative and makes all users of these now deprecated functions second class citizens.  Why?  At it's root these are simply providing mask and set or mask and clear register
bit operations.  Thanks,

Alex

Quoted 54 lines

Signed-off-by Philipp Stanner <[email protected]>

---
 drivers/pci/devres.c | 5 ++++-
 drivers/pci/pci.c    | 5 ++++-

 2 files changed, 8 insertions(+), 2 deletions(-)

drivers/pci/devres.c +0 -0

index 6f8f712fe34e..4c76fc063104 100644
@@ -435,7 +435,7 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
  }

 /**
- * pcim_intx - managed pci_intx()
+ * pcim_intx - managed pci_intx() (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *

drivers/pci/devres.c +3 -0

@@ -443,6 +443,9 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
   *
   * Enable/disable PCI INTx for device @pdev.
   * Restore the original state on driver detach.
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead (there is no managed

version, currently).
  */
 int pcim_intx(struct pci_dev *pdev, int enable)
 {

drivers/pci/pci.c +0 -0

index 7ce1d0e3a1d5..dc69e23b8982 100644
@@ -4477,11 +4477,14 @@ void pci_disable_parity(struct pci_dev
 *dev)
  }

 /**
- * pci_intx - enables/disables PCI INTx for device dev
+ * pci_intx - enables/disables PCI INTx for device dev (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *
  * Enables/disables PCI INTx for device @pdev
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead.
  */
 void pci_intx(struct pci_dev *pdev, int enable)
 {

Quoted 21 lines · Alex Williamson

Quoted 8 lines · On Tue, 15 Oct 2024 20:51:23 +0200 Philipp Stanner <[email protected]> wrote:

pci_intx() and its managed counterpart pcim_intx() only exist for older
drivers which have not been ported yet for various reasons. Future drivers should preferably use pci_alloc_irq_vectors().

Mark pci_intx() and pcim_intx() as deprecated and encourage usage of
pci_alloc_irq_vectors() in its place.

I don't really understand this.  As we've discussed previously pci_alloc_irq_vectors() is, unsurprisingly, for allocating PCI IRQ vectors while pci_intx() is for manipulating the INTx disable bit on PCI devices.  The latter is a generic mechanism for preventing PCI devices from generating INTx, regardless of whether there's a vector allocated for it.  How does the former replace the latter and why do we
feel the need to deprecate the latter?

Quoted 5 lines

It feels like this fits some narrow narrative and makes all users of these now deprecated functions second class citizens.  Why?  At it's root these are simply providing mask and set or mask and clear register
bit operations.  Thanks,

I got the feeling from the RFC discussion that that was basically the
consensus: people should use pci_alloc_irq_vectors(). Or did I misunderstand Andy and Heiner?

I'm perfectly happy with dropping this patch and continue offering pci{m}_intx() to users, since after removing that hybrid hazzard I don't see any harm in them anymore.

P.

Quoted 58 lines

Alex

Quoted 54 lines

Signed-off-by Philipp Stanner <[email protected]>

---
 drivers/pci/devres.c | 5 ++++-
 drivers/pci/pci.c    | 5 ++++-

 2 files changed, 8 insertions(+), 2 deletions(-)

drivers/pci/devres.c +0 -0

index 6f8f712fe34e..4c76fc063104 100644
@@ -435,7 +435,7 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
  }

 /**
- * pcim_intx - managed pci_intx()
+ * pcim_intx - managed pci_intx() (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *

drivers/pci/devres.c +3 -0

@@ -443,6 +443,9 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
   *
   * Enable/disable PCI INTx for device @pdev.
   * Restore the original state on driver detach.
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead (there is no managed

version, currently).
  */
 int pcim_intx(struct pci_dev *pdev, int enable)
 {

drivers/pci/pci.c +0 -0

index 7ce1d0e3a1d5..dc69e23b8982 100644
@@ -4477,11 +4477,14 @@ void pci_disable_parity(struct pci_dev
 *dev)
  }

 /**
- * pci_intx - enables/disables PCI INTx for device dev
+ * pci_intx - enables/disables PCI INTx for device dev (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *
  * Enables/disables PCI INTx for device @pdev
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead.
  */
 void pci_intx(struct pci_dev *pdev, int enable)
 {

Quoted 38 lines · Philipp Stanner

Quoted 29 lines · Alex Williamson

Quoted 11 lines · On Tue, 15 Oct 2024 20:51:23 +0200 Philipp Stanner <[email protected]> wrote:

pci_intx() and its managed counterpart pcim_intx() only exist for
older
drivers which have not been ported yet for various reasons. Future
drivers should preferably use pci_alloc_irq_vectors().

Mark pci_intx() and pcim_intx() as deprecated and encourage usage
of
pci_alloc_irq_vectors() in its place.

I don't really understand this.  As we've discussed previously pci_alloc_irq_vectors() is, unsurprisingly, for allocating PCI IRQ
vectors while pci_intx() is for manipulating the INTx disable bit on
PCI devices.  The latter is a generic mechanism for preventing PCI
devices from generating INTx, regardless of whether there's a vector
allocated for it.  How does the former replace the latter and why do
we
feel the need to deprecate the latter?

Quoted 7 lines

It feels like this fits some narrow narrative and makes all users of
these now deprecated functions second class citizens.  Why?  At it's
root these are simply providing mask and set or mask and clear register
bit operations.  Thanks,

Quoted 5 lines

I got the feeling from the RFC discussion that that was basically the
consensus: people should use pci_alloc_irq_vectors(). Or did I misunderstand Andy and Heiner?

I think there are two different use cases for pci_intx(). At first there are several drivers where the direct usage of pci_intx()
can be eliminated by switching to the pci_alloc_irq_vectors() API.

And then there's usage of pci_intx() in drivers/vfio/pci/vfio_pci_intrs.c drivers/xen/xen-pciback/conf_space_header.c
There we have to keep the (AFAICS unmanaged) pci_intx() calls.

Quoted 68 lines

I'm perfectly happy with dropping this patch and continue offering pci{m}_intx() to users, since after removing that hybrid hazzard I don't see any harm in them anymore.

P.

Quoted 58 lines

Alex

Quoted 54 lines

Signed-off-by Philipp Stanner <[email protected]>

---
 drivers/pci/devres.c | 5 ++++-
 drivers/pci/pci.c    | 5 ++++-

 2 files changed, 8 insertions(+), 2 deletions(-)

drivers/pci/devres.c +0 -0

index 6f8f712fe34e..4c76fc063104 100644
@@ -435,7 +435,7 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
  }

 /**
- * pcim_intx - managed pci_intx()
+ * pcim_intx - managed pci_intx() (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *

drivers/pci/devres.c +3 -0

@@ -443,6 +443,9 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
   *
   * Enable/disable PCI INTx for device @pdev.
   * Restore the original state on driver detach.
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead (there is no managed

version, currently).
  */
 int pcim_intx(struct pci_dev *pdev, int enable)
 {

drivers/pci/pci.c +0 -0

index 7ce1d0e3a1d5..dc69e23b8982 100644
@@ -4477,11 +4477,14 @@ void pci_disable_parity(struct pci_dev
 *dev)
  }

 /**
- * pci_intx - enables/disables PCI INTx for device dev
+ * pci_intx - enables/disables PCI INTx for device dev (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *
  * Enables/disables PCI INTx for device @pdev
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead.
  */
 void pci_intx(struct pci_dev *pdev, int enable)
 {

Quoted 49 lines · Heiner Kallweit

Quoted 38 lines · Philipp Stanner

Quoted 29 lines · Alex Williamson

Quoted 11 lines · On Tue, 15 Oct 2024 20:51:23 +0200 Philipp Stanner <[email protected]> wrote:

pci_intx() and its managed counterpart pcim_intx() only exist for
older
drivers which have not been ported yet for various reasons. Future
drivers should preferably use pci_alloc_irq_vectors().

Mark pci_intx() and pcim_intx() as deprecated and encourage usage
of
pci_alloc_irq_vectors() in its place.

I don't really understand this.  As we've discussed previously pci_alloc_irq_vectors() is, unsurprisingly, for allocating PCI IRQ
vectors while pci_intx() is for manipulating the INTx disable bit on
PCI devices.  The latter is a generic mechanism for preventing PCI
devices from generating INTx, regardless of whether there's a vector
allocated for it.  How does the former replace the latter and why do
we
feel the need to deprecate the latter?

Quoted 7 lines

It feels like this fits some narrow narrative and makes all users of
these now deprecated functions second class citizens.  Why?  At it's
root these are simply providing mask and set or mask and clear register
bit operations.  Thanks,

Quoted 5 lines

I got the feeling from the RFC discussion that that was basically the
consensus: people should use pci_alloc_irq_vectors(). Or did I misunderstand Andy and Heiner?

I think there are two different use cases for pci_intx(). At first there are several drivers where the direct usage of pci_intx()
can be eliminated by switching to the pci_alloc_irq_vectors() API.

Quoted 4 lines

And then there's usage of pci_intx() in drivers/vfio/pci/vfio_pci_intrs.c drivers/xen/xen-pciback/conf_space_header.c
There we have to keep the (AFAICS unmanaged) pci_intx() calls.

There is also the usage within PCI itself, in MSI. Patch №8 touches that.

It's why I think this series should land before anyone should port direct pci_intx() users to the irq vectors function, because the latter also uses pci_intx() and its own devres, which sounds explosive to me.

Quoted 3 lines

Quoted 3 lines

I'm perfectly happy with dropping this patch and continue offering pci{m}_intx() to users, since after removing that hybrid hazzard I don't see any harm in them anymore.

Quoted 58 lines

Quoted 57 lines

Quoted 55 lines

Quoted 54 lines

Signed-off-by Philipp Stanner <[email protected]>

---
 drivers/pci/devres.c | 5 ++++-
 drivers/pci/pci.c    | 5 ++++-

 2 files changed, 8 insertions(+), 2 deletions(-)

drivers/pci/devres.c +0 -0

index 6f8f712fe34e..4c76fc063104 100644
@@ -435,7 +435,7 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
  }

 /**
- * pcim_intx - managed pci_intx()
+ * pcim_intx - managed pci_intx() (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *

drivers/pci/devres.c +3 -0

@@ -443,6 +443,9 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
   *
   * Enable/disable PCI INTx for device @pdev.
   * Restore the original state on driver detach.
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead (there is no managed

version, currently).
  */
 int pcim_intx(struct pci_dev *pdev, int enable)
 {

drivers/pci/pci.c +0 -0

index 7ce1d0e3a1d5..dc69e23b8982 100644
@@ -4477,11 +4477,14 @@ void pci_disable_parity(struct pci_dev
 *dev)
  }

 /**
- * pci_intx - enables/disables PCI INTx for device dev
+ * pci_intx - enables/disables PCI INTx for device dev (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *
  * Enables/disables PCI INTx for device @pdev
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead.
  */
 void pci_intx(struct pci_dev *pdev, int enable)
 {

Quoted 75 lines · Philipp Stanner

Quoted 60 lines · Heiner Kallweit

Quoted 47 lines · Philipp Stanner

Quoted 35 lines · Alex Williamson

Quoted 8 lines · On Tue, 15 Oct 2024 20:51:23 +0200 Philipp Stanner <[email protected]> wrote:

pci_intx() and its managed counterpart pcim_intx() only exist
for
older
drivers which have not been ported yet for various reasons. Future
drivers should preferably use pci_alloc_irq_vectors().

Quoted 4 lines

Mark pci_intx() and pcim_intx() as deprecated and encourage usage
of
pci_alloc_irq_vectors() in its place.

I don't really understand this.  As we've discussed previously
pci_alloc_irq_vectors() is, unsurprisingly, for allocating PCI
IRQ
vectors while pci_intx() is for manipulating the INTx disable bit
on
PCI devices.  The latter is a generic mechanism for preventing
PCI
devices from generating INTx, regardless of whether there's a vector
allocated for it.  How does the former replace the latter and why
do
we
feel the need to deprecate the latter?

Quoted 10 lines

It feels like this fits some narrow narrative and makes all users
of
these now deprecated functions second class citizens.  Why? At
it's
root these are simply providing mask and set or mask and clear
register
bit operations.  Thanks,

Quoted 6 lines

I got the feeling from the RFC discussion that that was basically
the
consensus: people should use pci_alloc_irq_vectors(). Or did I misunderstand Andy and Heiner?

I think there are two different use cases for pci_intx(). At first there are several drivers where the direct usage of pci_intx()
can be eliminated by switching to the pci_alloc_irq_vectors() API.

Quoted 4 lines

And then there's usage of pci_intx() in drivers/vfio/pci/vfio_pci_intrs.c drivers/xen/xen-pciback/conf_space_header.c
There we have to keep the (AFAICS unmanaged) pci_intx() calls.

There is also the usage within PCI itself, in MSI. Patch №8 touches that.

It's why I think this series should land before anyone should port direct pci_intx() users to the irq vectors function, because the latter
also uses pci_intx() and its own devres, which sounds explosive to me.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

I'm perfectly happy with dropping this patch and continue offering
pci{m}_intx() to users, since after removing that hybrid hazzard I
don't see any harm in them anymore.

So is the bottom line that we should drop *this* patch and apply the rest of the series?

Quoted 62 lines

Quoted 61 lines

Quoted 60 lines

Quoted 58 lines

Quoted 57 lines

Signed-off-by Philipp Stanner <[email protected]>

---
 drivers/pci/devres.c | 5 ++++-
 drivers/pci/pci.c    | 5 ++++-

 2 files changed, 8 insertions(+), 2 deletions(-)

drivers/pci/devres.c +0 -0

index 6f8f712fe34e..4c76fc063104 100644
@@ -435,7 +435,7 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
  }

 /**
- * pcim_intx - managed pci_intx()
+ * pcim_intx - managed pci_intx() (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *

drivers/pci/devres.c +3 -0

@@ -443,6 +443,9 @@ static struct pcim_intx_devres
 *get_or_create_intx_devres(struct device *dev)
   *
   * Enable/disable PCI INTx for device @pdev.
   * Restore the original state on driver detach.
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead (there is no

managed
version, currently).
  */
 int pcim_intx(struct pci_dev *pdev, int enable)
 {

drivers/pci/pci.c +0 -0

index 7ce1d0e3a1d5..dc69e23b8982 100644
@@ -4477,11 +4477,14 @@ void pci_disable_parity(struct

pci_dev
*dev)
 }

 /**
- * pci_intx - enables/disables PCI INTx for device dev
+ * pci_intx - enables/disables PCI INTx for device dev (DEPRECATED)
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx *
  * Enables/disables PCI INTx for device @pdev
+ *
+ * This function is DEPRECATED. Do not use it in new code.
+ * Use pci_alloc_irq_vectors() instead.
  */
 void pci_intx(struct pci_dev *pdev, int enable)
 {

pci_intx() and its managed counterpart pcim_intx() only exist for older drivers which have not been ported yet for various reasons. Future drivers should preferably use pci_alloc_irq_vectors().

Mark pci_intx() and pcim_intx() as deprecated and encourage usage of pci_alloc_irq_vectors() in its place.

@Driver-Maintainers: Your driver might be touched by patch "Remove devres from pci_intx()". You might want to take a look.

Changes since the RFC [1]:
- Add a patch deprecating pci{m}_intx(). (Heiner, Andy, Me)
- Add Acked-by's already given.
- Export pcim_intx() as a GPL function. (Alex)
- Drop patch for rts5280, since this driver will be removed quite soon. (Philipp Hortmann, Greg)
- Use early-return in pci_intx_unmanaged() and pci_intx(). (Andy)

Hi all,

this series removes a problematic feature from pci_intx(). That function sometimes implicitly uses devres for automatic cleanup. We should get rid of this implicit behavior.

To do so, a pci_intx() version that is always-managed, and one that is never-managed are provided. Then, all pci_intx() users are ported to the version they need. Afterwards, pci_intx() can be cleaned up and the users of the never-managed version be ported back to pci_intx().

This way we'd get this PCI API consistent again.

Patch "Remove devres from pci_intx()" obviously reverts the previous patches that made drivers use pci_intx_unmanaged(). But this way it's easier to review and approve. It also makes sure that each checked out commit should provide correct behavior, not just the entire series as a whole.

Merge plan for this is to enter through the PCI tree.

[1] https://lore.kernel.org/all/[email protected]/

Philipp Stanner (13):
PCI: Prepare removing devres from pci_intx()
ALSA: hda_intel: Use always-managed version of pcim_intx() drivers/xen: Use never-managed version of pci_intx() net/ethernet: Use never-managed version of pci_intx() net/ntb: Use never-managed version of pci_intx()
misc: Use never-managed version of pci_intx() vfio/pci: Use never-managed version of pci_intx()
PCI: MSI: Use never-managed version of pci_intx()
ata: Use always-managed version of pci_intx()
wifi: qtnfmac: use always-managed version of pcim_intx()
HID: amd_sfh: Use always-managed version of pcim_intx() Remove devres from pci_intx()
PCI: Deprecate pci_intx(), pcim_intx()

 drivers/ata/ahci.c                            |  2 +-
 drivers/ata/ata_piix.c                        |  2 +-
 drivers/ata/pata_rdc.c                        |  2 +-
 drivers/ata/sata_sil24.c                      |  2 +-
 drivers/ata/sata_sis.c                        |  2 +-
 drivers/ata/sata_uli.c                        |  2 +-
 drivers/ata/sata_vsc.c                        |  2 +-
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        |  4 +--
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  2 +-
 .../wireless/quantenna/qtnfmac/pcie/pcie.c    |  2 +-
 drivers/pci/devres.c                          | 29 +++++--------------
 drivers/pci/pci.c                             | 19 ++++--------
 include/linux/pci.h                           |  1 +
 sound/pci/hda/hda_intel.c                     |  2 +-
 14 files changed, 26 insertions(+), 47 deletions(-)

Signature · 1 line

2.47.0

Quoted 30 lines · Philipp Stanner

@Driver-Maintainers: Your driver might be touched by patch "Remove devres from pci_intx()". You might want to take a look.

Changes since the RFC [1]:
- Add a patch deprecating pci{m}_intx(). (Heiner, Andy, Me)
- Add Acked-by's already given.
- Export pcim_intx() as a GPL function. (Alex)
- Drop patch for rts5280, since this driver will be removed quite soon. (Philipp Hortmann, Greg)
- Use early-return in pci_intx_unmanaged() and pci_intx(). (Andy)

Hi all,

this series removes a problematic feature from pci_intx(). That function sometimes implicitly uses devres for automatic cleanup. We should get rid of this implicit behavior.

To do so, a pci_intx() version that is always-managed, and one that is never-managed are provided. Then, all pci_intx() users are ported to the version they need. Afterwards, pci_intx() can be cleaned up and the users of the never-managed version be ported back to pci_intx().

This way we'd get this PCI API consistent again.

Patch "Remove devres from pci_intx()" obviously reverts the previous patches that made drivers use pci_intx_unmanaged(). But this way it's easier to review and approve. It also makes sure that each checked out commit should provide correct behavior, not just the entire series as a whole.

Quoted 3 lines

Merge plan for this is to enter through the PCI tree.

[1] https://lore.kernel.org/all/[email protected]/

I *think* this series depends on resolution of Takashi's "Restore the original INTX_DISABLE bit by pcim_intx()" patch [2], right?

For now I'm postponing this series, but let me know if that's not the right thing.

[2] https://lore.kernel.org/r/[email protected]

Quoted 34 lines

Philipp Stanner (13):
PCI: Prepare removing devres from pci_intx()
ALSA: hda_intel: Use always-managed version of pcim_intx() drivers/xen: Use never-managed version of pci_intx() net/ethernet: Use never-managed version of pci_intx() net/ntb: Use never-managed version of pci_intx()
misc: Use never-managed version of pci_intx() vfio/pci: Use never-managed version of pci_intx()
PCI: MSI: Use never-managed version of pci_intx()
ata: Use always-managed version of pci_intx()
wifi: qtnfmac: use always-managed version of pcim_intx()
HID: amd_sfh: Use always-managed version of pcim_intx() Remove devres from pci_intx()
PCI: Deprecate pci_intx(), pcim_intx()

 drivers/ata/ahci.c                            |  2 +-
 drivers/ata/ata_piix.c                        |  2 +-
 drivers/ata/pata_rdc.c                        |  2 +-
 drivers/ata/sata_sil24.c                      |  2 +-
 drivers/ata/sata_sis.c                        |  2 +-
 drivers/ata/sata_uli.c                        |  2 +-
 drivers/ata/sata_vsc.c                        |  2 +-
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        |  4 +--
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  2 +-
 .../wireless/quantenna/qtnfmac/pcie/pcie.c    |  2 +-
 drivers/pci/devres.c                          | 29 +++++--------------
 drivers/pci/pci.c                             | 19 ++++--------
 include/linux/pci.h                           |  1 +
 sound/pci/hda/hda_intel.c                     |  2 +-
 14 files changed, 26 insertions(+), 47 deletions(-)

Signature · 1 line

2.47.0

Quoted 49 lines · Bjorn Helgaas

Quoted 46 lines · Philipp Stanner

@Driver-Maintainers: Your driver might be touched by patch "Remove
devres from pci_intx()". You might want to take a look.

Changes since the RFC [1]:
  - Add a patch deprecating pci{m}_intx(). (Heiner, Andy, Me)
  - Add Acked-by's already given.
  - Export pcim_intx() as a GPL function. (Alex)
  - Drop patch for rts5280, since this driver will be removed quite
    soon. (Philipp Hortmann, Greg)
  - Use early-return in pci_intx_unmanaged() and pci_intx(). (Andy)

Hi all,

this series removes a problematic feature from pci_intx(). That function
sometimes implicitly uses devres for automatic cleanup. We should get
rid of this implicit behavior.

To do so, a pci_intx() version that is always-managed, and one that is
never-managed are provided. Then, all pci_intx() users are ported to the
version they need. Afterwards, pci_intx() can be cleaned up and the
users of the never-managed version be ported back to pci_intx().

This way we'd get this PCI API consistent again.

Patch "Remove devres from pci_intx()" obviously reverts the previous
patches that made drivers use pci_intx_unmanaged(). But this way it's
easier to review and approve. It also makes sure that each checked out
commit should provide correct behavior, not just the entire series as a
whole.

Merge plan for this is to enter through the PCI tree.

[1]
https://lore.kernel.org/all/[email protected]/

Quoted 3 lines

I *think* this series depends on resolution of Takashi's "Restore the
original INTX_DISABLE bit by pcim_intx()" patch [2], right?

IIUC, it's not really dependent, as pcim_intx() has been used in pci_intx() internally when the PCI device is already managed. My patch is to correct the already existing behavior.

  So I guess you
can take this series, and I'll post the revised patch later (sorry, I was too busy for other tasks).

thanks,

Takashi

Quoted 4 lines

For now I'm postponing this series, but let me know if that's not the
right thing.

Quoted 38 lines

[2] https://lore.kernel.org/r/[email protected]

Quoted 35 lines

Philipp Stanner (13):
  PCI: Prepare removing devres from pci_intx()
  ALSA: hda_intel: Use always-managed version of pcim_intx() drivers/xen: Use never-managed version of pci_intx() net/ethernet: Use never-managed version of pci_intx() net/ntb: Use never-managed version of pci_intx()
  misc: Use never-managed version of pci_intx() vfio/pci: Use never-managed version of pci_intx()
  PCI: MSI: Use never-managed version of pci_intx()
  ata: Use always-managed version of pci_intx()
  wifi: qtnfmac: use always-managed version of pcim_intx()
  HID: amd_sfh: Use always-managed version of pcim_intx() Remove devres from pci_intx()
  PCI: Deprecate pci_intx(), pcim_intx()

 drivers/ata/ahci.c                            |  2 +- drivers/ata/ata_piix.c                        |  2 +- drivers/ata/pata_rdc.c                        |  2 +- drivers/ata/sata_sil24.c                      |  2 +-

 drivers/ata/sata_sis.c                        |  2 +-
 drivers/ata/sata_uli.c                        |  2 +-
 drivers/ata/sata_vsc.c                        |  2 +-

 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        |  4 +-- drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  2 +- .../wireless/quantenna/qtnfmac/pcie/pcie.c    |  2 +- drivers/pci/devres.c                          | 29 +++++--------
------
 drivers/pci/pci.c                             | 19 ++++-------- include/linux/pci.h                           |  1 + sound/pci/hda/hda_intel.c                     |  2 +-

 14 files changed, 26 insertions(+), 47 deletions(-)

Signature · 1 line

2.47.0