Marc Kleine-Budde wrote:
> Moin,
>
> Oliver Hartkopp wrote:
>
>> diff --git a/drivers/net/can/sja1000/sja1000.c
>> b/drivers/net/can/sja1000/sja1000.c
>> index 16d2ecd..988fa45 100644
>> --- a/drivers/net/can/sja1000/sja1000.c
>> +++ b/drivers/net/can/sja1000/sja1000.c
>> @@ -303,7 +303,18 @@ static void sja1000_rx(struct net_device *dev)
>> skb->protocol = htons(ETH_P_CAN);
>
>> fi = priv->read_reg(priv, REG_FI);
>> +
>> dlc = fi & 0x0F;
>> + if (unlikely(dlc > 8)) {
>> +
>> + static int print_dlc_err;
>> +
>> + if (!print_dlc_err) {
>> + dev_err(dev->dev.parent, "illegal dlc %d\n", dlc);
>> + print_dlc_err = 1;
>> + }
>
> You should use something like "WARN_ONCE" instead of open-coding it.
WARN_ONCE is OK if it's acceptable that dlc > 8 happens.
>> + dlc = 8;
>> + }
>
>> if (fi & FI_FF) {
>> /* extended frame format (EFF) */
>
> The general question is should a driver print this message or not. The
> at91 silently limits the dlc to 8. I'm not sure what the other mainline
> drivers do.
It depends. If it's due to a malfunctioning of the hardware (or
software), we should inform the user. Unfortunately, the drivers do not
yet handle that case consistently. I have realized:
- ignore dlc > 8
- dlc &= 0xf
- print error message (or oops)
- min(dlc, 8)
This is just for incoming messages. The dlc of out-going messages could
be check by the upper layer. Is that already done? We need to fix that
issue consistently for all drivers.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
