Skip to contentSkip to conversation

net/am2150: fix nmclan_cs.c shared interrupt handling

PATCH In review Arnd Bergmann 2 participants 4 messages Nov 20, 2014 – Nov 24, 2014

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 net/am2150: fix nmclan_cs.c shared interrupt handling

A recent patch tried to work around a valid warning for the use of a deprecated interface by blindly changing from the old pcmcia_request_exclusive_irq() interface to pcmcia_request_irq().

This driver has an interrupt handler that is not currently aware of shared interrupts, but can be easily converted to be. At the moment, the driver reads the interrupt status register repeatedly until it contains only zeroes in the interesting bits, and handles each bit individually.

This patch adds the missing part of returning IRQ_NONE in case none of the bits are set to start with, so we can move on to the next interrupt source.

Fixes 5f5316fcd08ef7 ("am2150: Update nmclan_cs.c to use update PCMCIA API")

---
I had this patch in my queue of things to submit and noticed that the warning had gone away upstream but my patch was still there.

For all I can tell, the driver is broken without this, although it would rarely be a problem.

drivers/net/ethernet/amd/nmclan_cs.c +2 -0
index 5b22764ba88d..27245efe9f50 100644
@@ -952,6 +952,8 @@ static irqreturn_t mace_interrupt(int irq, void *dev_id)
   do {
     /* WARNING: MACE_IR is a READ/CLEAR port! */
     status = inb(ioaddr + AM2150_MACE_BASE + MACE_IR);
+    if (!(status & ~MACE_IMR_DEFAULT) && IntrCnt == MACE_MAX_IR_ITERATIONS)
+      return IRQ_NONE;
 
     pr_debug("mace_interrupt: irq 0x%X status 0x%X.\n", irq, status);
 
#1 lore raw

PATCH 2/2 pcmcia: remove pcmcia_request_exclusive_irq

The last user of the deprecated pcmcia_request_exclusive_irq function, was removed in 5f5316fcd08e ("am2150: Update nmclan_cs.c to use update PCMCIA API"), so we can clean up the core code and remove the interface and its documentation.

---
 Documentation/pcmcia/driver-changes.txt |  3 ---
 drivers/pcmcia/pcmcia_resource.c        | 39 ---------------------------------------
 include/pcmcia/ds.h                     | 10 ----------
 3 files changed, 0 insertions(+), 52 deletions(-)

---
The patch that removed the last user was merged in 3.18-rc1, so this should be safe to apply independently now.

Documentation/pcmcia/driver-changes.txt +0 -3
index dd04361dd361..78355c4c268a 100644
@@ -46,9 +46,6 @@ This file details changes in 2.6 which affect PCMCIA card driver authors:
    - use pcmcia_request_irq(p_dev, handler_t); the PCMCIA core will
      clean up automatically on calls to pcmcia_disable_device() or
      device ejection.
-   - drivers still not capable of IRQF_SHARED (or not telling us so) may
-     use the deprecated pcmcia_request_exclusive_irq() for the time
-     being; they might receive a shared IRQ nonetheless.
 
 * no cs_error / CS_CHECK / CONFIG_PCMCIA_DEBUG (as of 2.6.33)
    Instead of the cs_error() callback or the CS_CHECK() macro, please use
drivers/pcmcia/pcmcia_resource.c +0 -39
index e8c19def1b0f..23a123da3dd7 100644
@@ -712,45 +712,6 @@ int __must_check pcmcia_request_irq(struct pcmcia_device *p_dev,
 }
 EXPORT_SYMBOL(pcmcia_request_irq);
 
-
-/**
- * pcmcia_request_exclusive_irq() - attempt to request an exclusive IRQ first
- * @p_dev: the associated PCMCIA device
- * @handler: IRQ handler to register
- *
- * pcmcia_request_exclusive_irq() is a wrapper around request_irq() which
- * attempts first to request an exclusive IRQ. If it fails, it also accepts
- * a shared IRQ, but prints out a warning. PCMCIA drivers should allow for
- * IRQ sharing and either use request_irq directly (then they need to call
- * free_irq() themselves, too), or the pcmcia_request_irq() function.
- */
-int __must_check
-__pcmcia_request_exclusive_irq(struct pcmcia_device *p_dev,
-			irq_handler_t handler)
-{
-	int ret;
-
-	if (!p_dev->irq)
-		return -EINVAL;
-
-	ret = request_irq(p_dev->irq, handler, 0, p_dev->devname, p_dev->priv);
-	if (ret) {
-		ret = pcmcia_request_irq(p_dev, handler);
-		dev_printk(KERN_WARNING, &p_dev->dev, "pcmcia: "
-			"request for exclusive IRQ could not be fulfilled.\n");
-		dev_printk(KERN_WARNING, &p_dev->dev, "pcmcia: the driver "
-			"needs updating to supported shared IRQ lines.\n");
-	}
-	if (ret)
-		dev_printk(KERN_INFO, &p_dev->dev, "request_irq() failed\n");
-	else
-		p_dev->_irq = 1;
-
-	return ret;
-} /* pcmcia_request_exclusive_irq */
-EXPORT_SYMBOL(__pcmcia_request_exclusive_irq);
-
-
 #ifdef CONFIG_PCMCIA_PROBE
 
 /* mask of IRQs already reserved by other cards, we should avoid using them */
include/pcmcia/ds.h +0 -10
index 2d56e428506c..3037157855f0 100644
@@ -206,16 +206,6 @@ int pcmcia_write_config_byte(struct pcmcia_device *p_dev, off_t where, u8 val);
 /* device configuration */
 int pcmcia_request_io(struct pcmcia_device *p_dev);
 
-int __must_check
-__pcmcia_request_exclusive_irq(struct pcmcia_device *p_dev,
-				irq_handler_t handler);
-static inline __must_check __deprecated int
-pcmcia_request_exclusive_irq(struct pcmcia_device *p_dev,
-				irq_handler_t handler)
-{
-	return __pcmcia_request_exclusive_irq(p_dev, handler);
-}
-
 int __must_check pcmcia_request_irq(struct pcmcia_device *p_dev,
 				irq_handler_t handler);
 
maintainer Acked-by on v1 1/2
#2 lore raw
In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>
Date: Thu, 20 Nov 2014 16:11:14 +0100

Quoted 14 lines

A recent patch tried to work around a valid warning for the use of a deprecated interface by blindly changing from the old pcmcia_request_exclusive_irq() interface to pcmcia_request_irq().

This driver has an interrupt handler that is not currently aware of shared interrupts, but can be easily converted to be. At the moment, the driver reads the interrupt status register repeatedly until it contains only zeroes in the interesting bits, and handles each bit individually.

This patch adds the missing part of returning IRQ_NONE in case none of the bits are set to start with, so we can move on to the next interrupt source.

Quoted 8 lines
Fixes 5f5316fcd08ef7 ("am2150: Update nmclan_cs.c to use update PCMCIA API")

---
I had this patch in my queue of things to submit and noticed that the warning had gone away upstream but my patch was still there.

For all I can tell, the driver is broken without this, although it would rarely be a problem.

I'm happy for this to go alongside patch #2 via another tree:

#3 lore raw
Quoted 23 lines · On Friday 21 November 2014 15:17:35 David Miller wrote:

From: Arnd Bergmann <[email protected]>
Date: Thu, 20 Nov 2014 16:11:14 +0100

Quoted 20 lines

A recent patch tried to work around a valid warning for the use of a deprecated interface by blindly changing from the old pcmcia_request_exclusive_irq() interface to pcmcia_request_irq().

This driver has an interrupt handler that is not currently aware of shared interrupts, but can be easily converted to be. At the moment, the driver reads the interrupt status register repeatedly until it contains only zeroes in the interesting bits, and handles each bit individually.

This patch adds the missing part of returning IRQ_NONE in case none of the bits are set to start with, so we can move on to the next interrupt source.

Fixes 5f5316fcd08ef7 ("am2150: Update nmclan_cs.c to use update PCMCIA API")

---
I had this patch in my queue of things to submit and noticed that the warning had gone away upstream but my patch was still there.

Quoted 6 lines
Quoted 2 lines

For all I can tell, the driver is broken without this, although it would rarely be a problem.

I'm happy for this to go alongside patch #2 via another tree:

I was hoping you could take at least this one, possibly both. Since you already had Jeff's patch in it, the first patch now fixes a (mostly harmless) 3.18 regression, and the second patch is just a cosmetic cleanup, but there is no longer any dependency between the two.

It looks like Greg is now handling most of the PCMCIA patches in the absence of a subsystem maintainer, so I'll send him the second one for 3.19, let me know if you still want me to send him the first one too.

Arnd