Hi Oliver,

On 02/15/2011 05:52 PM, Oliver Hartkopp wrote:
> On 15.02.2011 10:34, Wolfgang Grandegger wrote:
> 
>> I now want to fix the CAN_ERR_ACK issue. As you might have realized, the
>> so called "no-ack-on-tx" or "ack slot" bus error is not handled
>> consistently. That's the error we get when no cable is connected.
>> The inconstancy is that some drivers just set:
>>
>>      cf->can_id |= CAN_ERR_ACK;
>>
>> while the SJA1000 sets:
>>
>>      cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>      cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
>>
>> Unfortunately, CAN_ERR_ACK defines it's own error class like
>> CAN_ERR_BUSERROR even if it's just another bus error. Therefore
>> it belongs to the protocol violation types in data[2]:
>>
>>   /* error in CAN protocol (type) / data[2] */
>>   #define CAN_ERR_PROT_UNSPEC      0x00 /* unspecified */
>>   #define CAN_ERR_PROT_BIT         0x01 /* single bit error */
>>   #define CAN_ERR_PROT_FORM        0x02 /* frame format error */
>>   #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_TX          0x80 /* error occured on transmission */
>>
>> All bits are already used, but CAN_ERR_PROT_ACTIVE is misplaced
>> here as well :-(. It belongs to the controller problems in data[1].
>> So, for a correct solution I would add here:
>>
>>   #define CAN_ERR_PROT_ACK         0x40 /* received no ACK on transmission */
> 
> ack.
> 
> This is definitely a protocol problem that can happen on TX.
> 
>>
>> and move CAN_ERR_PROT_ACTIVE to
>>  
>>   /* error status of CAN-controller / data[1] */
>>   #define CAN_ERR_CRTL_ACTIVE      0x40 /* back to error active */
>>
> 
> Hm. So far only 'problems' have been encoded into this byte. We could define
> 'zero' as having 'no problems' ... but adding a new 'good situation flag'
> doesn't fit to me.

Well, data[1] holds all our state changes. When we start to handle
decreasing state changes as well.

> IMHO i would leave this data[1] as-is and better add a new flag into the 
> CAN-ID:
> 
> #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 */
> 
> Analogue to the bus-off flags you can filter for a self recovering controller
> then.

Yes, to wait for "back-to-error-active" events when bus-off recovery has
completed might be useful.

>> Then the error message for the "no-ack-on-tx" error would be
>> composed the following way:
>>
>>      cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> 
> Better
>       cf->can_id |= CAN_ERR_PROT | CAN_ERR_ACK;
> ???

Why, it's a bus error just like a crc or frame error. CAN_ERR_ACK might
just be useful for backward compatibility but I don't think anybody uses
it. I would like to clean that up.

> To me CAN_ERR_PROT just point's to *additional* information, if wanted.
> CAN_ERR_ACK itself would be enough here also.
> 
>>      cf->data[2] = CAN_ERR_PROT_ACK;
>>
>> And CAN controllers providing more information like the SJA1000
>> would additionally set:
>>
>>      cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
>>
> 
> ack. This looks good.
> 
>> Unfortunately, the *correct* solution could rise some portability
>> issues as some users might already use CAN_ERR_PROT_ACK and especially
>> CAN_ERR_PROT_ACTIVE. What that be acceptible? What do you think?
> 
> IMO the cleanup is important. The earlier we do this, the better.
> 
> I assume the people going into these details are reading this mailing-list.
> 
> And as the support from the CAN controller drivers is currently very
> inconsistent, i won't think anyone is really relying on these bits in a
> non-development environment. But this is just a guess ...

OK. Note that the

> Thanks for your RFC!
> 
> Oliver
> _______________________________________________
> Socketcan-core mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/socketcan-core
> 
> 

_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to