On 09/23/11 18:56, Wolfram Sang wrote:
> On Fri, Sep 23, 2011 at 11:10:17AM +0200, Markus Plessing wrote:
>> Hi Oliver,
>>
>> looks quite good for me - more comments inline.
>>
>> Am 14.09.2011 15:01, schrieb Oliver Hartkopp:
>>> On 09/13/11 09:56, Marc Kleine-Budde wrote:
>>>
>>>> On 09/12/2011 08:44 PM, Oliver Hartkopp wrote:
>>>
>>>>> +static irqreturn_t ems_pcmcia_interrupt(int irq, void *dev_id)
>>>>> +{
>>>>> + struct ems_pcmcia_card *card = dev_id;
>>>>> + struct net_device *dev;
>>>>> + irqreturn_t retval = IRQ_NONE;
>>>>> + int i, again;
>>>>> +
>>>>> + /* Card not present */
>>>>> + if (readw(card->base_addr) != 0xAA55)
>>>>> + return IRQ_HANDLED;
>>>>
>>>> this looks fishy. Is it possible, that there is no card here? Why return
>>>> IRQ_HANDLED in this case?
>>>>
>>> This has been introduced to handle PCMCIA card ejects under heavy load.
>>> Probably IRQ_NONE could be returned here also - but if we come to this point
>>> everything is removed anyway.
>>>
>>
>> This is right, the CPC-Card may be removed unsave and on heavy load
>> there may be unhandled interrupts. The IRQ_HANDLED is not mandatory
>> here, I think this refers to our experience in embedded development
>> where the interrupt flag has to be cleared (handled) even on error.
>>
>> It's up to you what you think what's the best return value here.
>
> I still say IRQ_HANDLED is plain wrong. You can't ack an IRQ if you _know_
> the card isn't there. I wonder if that isn't obvious?
>
> Return IRQ_NONE here and you will at least be notified that there was this
> unwanted interrupt. At least, pcmcia network drivers do this, too. Have you
> maybe checked other pcmcia drivers?
Yes i did. Where the IRQ_NONE is returned e.g. in the 3com cards the function
netif_device_present() is used to determine the link state. So this is a
normal operation where a spurious irq *is* an error.
In the case of an unfriendly release of the EMS PCMCIA card this error
*usually* happens due to the irq sharing for the two SJA1000s on the card.
As this is a not nice but usual behaviour at card release, why generating an
annoying error in the kernel log, people may stumble about? 50 ms later the
netdevices are entirely removed and also the irq statistics. So what would the
notification help in this special case?
> And you should really start sending this driver to the pcmcia- and
> netdev-lists
> so they can suggest handling the error-printouts there...
I just did :-)
Tnx,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
