Re: [PATCH v3] can/sja1000: add support for PEAK-System PCMCIA card

[Stephane Grosjean <s.grosjean@peak-system.com>]

Le 14/02/2012 10:59, Oliver Hartkopp a écrit :

> @@ -492,6 +492,9 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
>   	while ((isrc = priv->read_reg(priv, REG_IR))&&  (n<  SJA1000_MAX_IRQ)) {
>   		n++;
>   		status = priv->read_reg(priv, REG_SR);
> +		/* check for absent controller due to hw unplug */
> +		if (status == 0xFF&&  !sja1000_probe_chip(dev))
> +			break;
>
>   		if (isrc&  IRQ_WUI)
>   			netdev_warn(dev, "wakeup interrupt\n");
> @@ -508,6 +511,9 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
>   			while (status&  SR_RBS) {
>   				sja1000_rx(dev);
>   				status = priv->read_reg(priv, REG_SR);
> +				/* check for absent controller */
> +				if (status == 0xFF&&  !sja1000_probe_chip(dev))
> +					break;
>   			}
>   		}
>   		if (isrc&  (IRQ_DOI | IRQ_EI | IRQ_BEI | IRQ_EPI | IRQ_ALI)) {
>
> The big advantage is, that we do not need another expensive register read nor
> another loop counter variable in the case everything is working fine.
>
> Only when the _already_ read status variable becomes 0xFF we check in the
> mode register if the device is still present.
>
> And finally there's a very little chance left to create rubbish data.


Quite perfect, except that we will push a "rubbish" error frame before 
exiting, and we will return IRQ_HANDLED while I would prefer IRQ_NONE...
But does it matter?

Moreover, regarding to this patch, should the peak_pcmcia custom isr 
always test for the card presence? I mean, this costs at least 2xtwo 
ioread8 for each hw irq!


Stéphane

--
PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt 
Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt 
HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 
Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com