Re: sja1000 interrupt problem

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 10.12.2013 15:41, Wolfgang Grandegger wrote:
> On Tue, 10 Dec 2013 14:47:24 +0100, Oliver Hartkopp

>> +       writew(0x00C3, chan->cfg_base + PITA_ICR);
> 
> This may kill unhandled (shared) IRQs. You can do that only if all
> handlers have been called in advance (with a global ISR).
> 

So here's my patch for PITA access protection in peak_pci_post_irq().

Note that we need a single lock for all (e.g. 4) channels of the PITA.
So my idea was to create the lock in the per-channel structure of the first
channel and then let the channels 1,2,3 refer to this lock.

Maybe there's a better solution for that. E.g. the driver cleanup at removal
time may get into problems with this approach?!?

So far the system is running. I'll take a look tomorrow morning if its still
alive :-) Maybe Austin can do some tests with this patch too.

Regards,
Oliver

--- linux-source-3.10/drivers/net/can/sja1000/peak_pci.c	2013-09-08 07:10:14.000000000 +0200
+++ linux-source-3.10-rt/drivers/net/can/sja1000/peak_pci.c	2013-12-10 16:58:16.381938240 +0100
@@ -41,6 +41,8 @@
 struct peak_pciec_card;
 struct peak_pci_chan {
 	void __iomem *cfg_base;		/* Common for all channels */
+	spinlock_t pita_lock;
+	spinlock_t *pita_lock_ptr;
 	struct net_device *prev_dev;	/* Chain of network devices */
 	u16 icr_mask;			/* Interrupt mask for fast ack */
 	struct peak_pciec_card *pciec_card;	/* only for PCIeC LEDs */
@@ -539,12 +541,17 @@
 static void peak_pci_post_irq(const struct sja1000_priv *priv)
 {
 	struct peak_pci_chan *chan = priv->priv;
+	unsigned long flags;
 	u16 icr;
 
 	/* Select and clear in PITA stored interrupt */
+	spin_lock_irqsave(chan->pita_lock_ptr, flags);
+
 	icr = readw(chan->cfg_base + PITA_ICR);
 	if (icr & chan->icr_mask)
 		writew(chan->icr_mask, chan->cfg_base + PITA_ICR);
+
+	spin_unlock_irqrestore(chan->pita_lock_ptr, flags);
 }
 
 static int peak_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -553,6 +560,7 @@
 	struct peak_pci_chan *chan;
 	struct net_device *dev;
 	void __iomem *cfg_base, *reg_base;
+	spinlock_t *pita_lock_ptr0;
 	u16 sub_sys_id, icr;
 	int i, err, channels;
 
@@ -611,6 +619,7 @@
 	icr = readw(cfg_base + PITA_ICR + 2);
 
 	for (i = 0; i < channels; i++) {
+
 		dev = alloc_sja1000dev(sizeof(struct peak_pci_chan));
 		if (!dev) {
 			err = -ENOMEM;
@@ -623,6 +632,13 @@
 		chan->cfg_base = cfg_base;
 		priv->reg_base = reg_base + i * PEAK_PCI_CHAN_SIZE;
 
+		if (i == 0) {
+			spin_lock_init(&chan->pita_lock);
+			pita_lock_ptr0 = &chan->pita_lock;
+		}
+
+		chan->pita_lock_ptr = pita_lock_ptr0;
+
 		priv->read_reg = peak_pci_read_reg;
 		priv->write_reg = peak_pci_write_reg;
 		priv->post_irq = peak_pci_post_irq;