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

[Wolfgang Grandegger <wg@grandegger.com>]

On 02/13/2012 11:41 AM, Oliver Hartkopp wrote:
> On 13.02.2012 11:14, Wolfgang Grandegger wrote:
> 
> 
>>>> diff --git a/drivers/net/can/sja1000/sja1000.c
>>>> b/drivers/net/can/sja1000/sja1000.c
>>>> index ebbcfca..f7526a7 100644
>>>> --- a/drivers/net/can/sja1000/sja1000.c
>>>> +++ b/drivers/net/can/sja1000/sja1000.c
>>>> @@ -493,6 +493,10 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
>>>>                  n++;
>>>>                  status = priv->read_reg(priv, REG_SR);
>>>>
>>>> +               /* check for absent controller due to hw unplug */
>>>> +               if (status == 0xFF)
>>>> +                       break;
>>>> +
>>>>                  if (isrc&  IRQ_WUI)
>>>>                          netdev_warn(dev, "wakeup interrupt\n");
>>>>
>>>> @@ -504,8 +508,8 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
>>>>                          netif_wake_queue(dev);
>>>>                  }
>>>>                  if (isrc&  IRQ_RI) {
>>>> -                       /* receive interrupt */
>>>> -                       while (status&  SR_RBS) {
>>>> +                       /* receive interrupt / check for absent controller */
>>>> +                       while (status&  SR_RBS&&  status != 0xFF) {
>>>>                                  sja1000_rx(dev);
>>>>                                  status = priv->read_reg(priv, REG_SR);
>>>>                          }
>>>>
>>>> @Stephane: Can you check that patch? I'm out of hw right now.
>>>
>>> I confirm that this patch works too...
>>> So I think I should be able to post a new version of the peak_pcmcia
>>> during that day (the previous should work but some calls to
>>> pcmcia_dev_present() are no more useful...)
>>
>> But that fix should not go to the common interrupt handler, if possible.
> 
> 
> Do you have an idea how to handle the while() statement without copying the
> entire interrupt handler code for the devices that might be unplugged?
> 
> IMHO this patch is pretty cheap.

I think it's enough to add a read in the custom pcmcia isr before
calling sja1000_interrupt(). Maybe the race window is a bit lower but we
are not able to react when the unplug happens anyway.

Wolfgang.