Re: [PATCH] can: fix sja1000 pre_irq/post_irq handling

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 18.11.2013 08:48, Wolfgang Grandegger wrote:

> 
> 
> Another thing I don't like is that the device present check is done for
> 
> all devices including devices which will never hotplug. I think this
> 
> code has been introduced for the PCMCIA/PCcard CAN controllers. I would
> 
> prefer somethink like:
> 
> 
> 
>          if (priv->hotpluggable && status == 0xFF &&
> 
>              sja1000_is_absent(priv)) {
> 
>                  ...
> 
>          }
> 
> 
> 
> What do you think?

The status variable is retrieved from the sja1000 register anyway and sits in
the CPU register.

Checking the status against 0xFF already hints to several problems (bus-off
together with overrun, etc. ) which are unusual in normal operation.
Only in this unusual cased the sja1000_is_absent() is invoked.

This is pretty optimal.

Accessing priv->hotpluggable needs additional pointer dereferencing which is
more costly than accessing/checking the already retrieved status variable.

Best regards,
Oliver