On 16.02.2011 09:10, Wolfgang Grandegger wrote: > On 02/15/2011 09:56 PM, Oliver Hartkopp wrote:
>> So when CAN_ERR_BUSERROR is set, i may check CAN_ERR_PROT and then into >> data [2..3], right? >> >> So CAN_ERR_ACK would be obsolete ... > > Yes. > >> I think, the reason why CAN_ERR_ACK has made it's way into the error class >> bits in the CAN-ID was because of missing ACKs usually happen, when there's >> only one CAN node on the bus. The idea was to detect missing CAN nodes or >> wrong termination directly through CAN_ERR_ACK. > > Yes, you seem to remember correctly. See: > > https://lists.berlios.de/pipermail/socketcan-core/2006-May/000089.html > Oh, the good old days :-) > But wrong termination may result in other errors as well and there I > think it could not be used reliably for the purpose you describe above. Ok. But i think we could leave it as-is. I don't think it's worth to break the definition here. >> Logically this is redundant to the bitvalue in CAN_ERR_PROT_ACK >> Yes. From the current point of view they are. I thought a CAN_ERR_BUSERROR >> could be triggered by anything else then protocol violations ... >> Then it might have become interesting when using a special filter mask for >> error frames. > > You mean a bit in the CAN id for the five bus error types? That's probably too much. 8-) I think when CAN_ERR_PROT is set, looking into data[2] is pretty ok. But in general the idea was to have error and status information in the CAN-ID for easy access & easy filtering - and give additional problem information in the data[] fields. For that reason i would prefer to have the CAN_ERR_ACTIVE bit *only* in the CAN-ID status bits and not in the detailed data[1] (CAN_ERR_CTRL_ACTIVE) as you and Kurt suggested. > 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? Better we do a final cleanup and then it's done. IMO this patch would be enough: diff --git a/include/linux/can/error.h b/include/linux/can/error.h index d4127fd..6eddd01 100644 --- a/include/linux/can/error.h +++ b/include/linux/can/error.h @@ -26,6 +26,7 @@ #define CAN_ERR_BUSOFF 0x00000040U /* bus off */ #define CAN_ERR_BUSERROR 0x00000080U /* bus error (may flood!) */ #define CAN_ERR_RESTARTED 0x00000100U /* controller restarted */ +#define CAN_ERR_ACTIVE 0x00000200U /* controller (self) recovered */ /* arbitration lost in bit ... / data[0] */ #define CAN_ERR_LOSTARB_UNSPEC 0x00 /* unspecified */ @@ -50,7 +51,7 @@ #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_ACK 0x40 /* received no ACK on transmission */ #define CAN_ERR_PROT_TX 0x80 /* error occured on transmission */ /* error in CAN protocol (location) / data[3] */ Ok, the bits CAN_ERR_ACK(can_id) and CAN_ERR_PROT_ACK(data[2]) are set together. But who cares? CAN_ERR_BUSERROR(can_id) could be defined as the or-ed value from (CAN_ERR_LOSTARB | CAN_ERR_PROT | CAN_ERR_CRTL) That looks consistent to me after all the discussions. Regards, Oliver _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
