On 02/16/2011 09:31 PM, Oliver Hartkopp wrote:
> On 16.02.2011 21:04, Wolfgang Grandegger wrote:
...
>>>> We then could or even should add a version information into the error
>>>> message. I think data[5] could be used for that purpose.
>>>
>>> It's like having the CAN core version information where Dave was complaining
>>> when it gone Mainline. Who should check version numbers and maintain all
>>> this?
>>
>> This would allow users already using the error flags to easily migrate
>> to the new and changed error types. My idea here is:
>>
>> cf->data[5] = CAN_ERR_VERSION;
>>
>> Also candump could use it to avoid displaying rubbish with old kernels.
>
> Hm - maybe then it's better to retrieve the Kernel version with uname(2) ...
The does not clearly identify the version of the error message, e..g
when drivers from BerliOS are used.
>> Furthermore I would like to use:
>>
>> s/CAN_ERR_PROT_OVERLOAD/CAN_ERR_PROT_CRC/
>>
>> for the CRC errors.
>
> ok.
>
>>>
>>> CAN_ERR_ACK(can_id) and CAN_ERR_PROT_ACK(data[2])
>>>
>>> are set together. But who cares?
>>
>> Me, see above.
>
> ok.
>
>>
>>> CAN_ERR_BUSERROR(can_id) could be defined as the or-ed value from
>>>
>>> (CAN_ERR_LOSTARB | CAN_ERR_PROT | CAN_ERR_CRTL)
>>
>> What do you mean here. Currently CAN_ERR_BUSERROR behave like
>> CAN_ERR_PROT but bus errors are *not* state changes or lostarb errors.
>> In principle we could define:
>>
>> #define CAN_ERR_BUSERROR CAN_ERR_PROT
>>
>> But maybe we leave it as is to allowing for other protocol violations
>> which are not classical bus errors.
>
> ok.
>
> So the final error.h patch would look like this then:
>
> diff --git a/include/linux/can/error.h b/include/linux/can/error.h
> index d4127fd..f9245e5 100644
> --- a/include/linux/can/error.h
> +++ b/include/linux/can/error.h
> @@ -22,7 +22,7 @@
> #define CAN_ERR_CRTL 0x00000004U /* controller problems / data[1]
> */
> #define CAN_ERR_PROT 0x00000008U /* protocol violations / data[2..3]
> */
> #define CAN_ERR_TRX 0x00000010U /* transceiver status / data[4]
> */
> -#define CAN_ERR_ACK 0x00000020U /* received no ACK on transmission
> */
> +#define CAN_ERR_ACTIVE 0x00000020U /* controller (self) recovered */
> #define CAN_ERR_BUSOFF 0x00000040U /* bus off */
> #define CAN_ERR_BUSERROR 0x00000080U /* bus error (may flood!) */
> #define CAN_ERR_RESTARTED 0x00000100U /* controller restarted */
> @@ -49,8 +49,8 @@
> #define CAN_ERR_PROT_STUFF 0x04 /* bit stuffing error */
> #define CAN_ERR_PROT_BIT0 0x08 /* unable to send dominant bit */
> #define CAN_ERR_PROT_BIT1 0x10 /* unable to send recessive bit */
> -#define CAN_ERR_PROT_OVERLOAD 0x20 /* bus overload */
> -#define CAN_ERR_PROT_ACTIVE 0x40 /* active error announcement */
> +#define CAN_ERR_PROT_CRC 0x20 /* CRC error */
> +#define CAN_ERR_PROT_ACK 0x40 /* received no ACK on transmission */
> #define CAN_ERR_PROT_TX 0x80 /* error occured on transmission */
>
> /* error in CAN protocol (location) / data[3] */
>
> Right?
Yeü.
>
>> I started writing the patch and realized that we need some more
>> clarification and cleanup. Next point is how we increment the
>> stats->[r|t]x_error counters? It's not done consistently and it's not
>> even possible to assign the direction of bus errors properly. Have a
>> look to the drivers to realizes the mess. Should we do that at all? We
>> have our own stats of CAN errors:
>>
>> struct can_device_stats {
>> __u32 bus_error; /* Bus errors */
>> __u32 error_warning; /* Changes to error warning state */
>> __u32 error_passive; /* Changes to error passive state */
>> __u32 bus_off; /* Changes to bus off state */
>> __u32 arbitration_lost; /* Arbitration lost errors */
>> __u32 restarts; /* CAN controller re-starts */
>> };
>>
>> I tend to drop touching the network stats completely. What do you and
>> others think.
>
> Using the common network stats would have the advantage, that you can easily
> see problems with standard tools like 'ifconfig' or 'cat /proc/net/dev'
>
> But maybe a cleanup is needed anyway ;-)
See my similar comment to Kurt mail.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
