Re: pch_can probable bug

[Christian Bendele <Christian.Bendele@gmx.net>]

Hi,

Wolfgang Grandegger <wg <at> grandegger.com> writes:

> Yes, note that the PCH does use a C_CAN core (even the manual is identical).

Indeed.


> Please do not invest time fixing the pch_can driver. We want to get rid
> of it asap.

Okay, I have turned my efforts towards the c-can driver now. I've had some
issues with some of the design decisions of the pch driver anyway. Some of those
issues seem to be absent from the c-can driver after a first glance.


> > That means that the message(s) stored in this message object are never read
> > out, and the INTPND (interrupt pending, is set together with NEWDAT when a
> > message is stored into the object) bit in that message object's control
> > register is never cleared.
> 
> Looking to c_can.c, I see this problem as well. The above check should
> be done after reading out the message.

That's what I thought as well, but after a closer look at c_can_do_rx_poll()
method of the c_can driver I disagree. I think the way this driver works the
check for the EOB bit should not be necessary at all. Neither before nor after
reading out the message. The loop will already be exited after that message
object is read out due to "msg_obj <= C_CAN_MSG_OBJ_RX_LAST" in the loop header.


> Could you please use my "RFC V3" c_can_pci driver patches I sent out an
> hour ago. Anyway, avoiding out-of-order reception with a message FIFO is
> tricky and we need to fix that part as well. Unfortunately, I do not
> have a system for development and testing.

I tried your RFC V3 drivers now. As you sent them they work about as well as the
pch driver. The generally seem to have similar bugs as the latter, too. 

I was quite surprised, however, that the c_can driver actually caused (very
slightly) higher CPU load for a given CANBUS load than the pch driver.
I had looked at the code before testing it, and some issues I had with design
decisions of the pch driver (among other things it failed to make proper use of
the message handling registers the c-can controller provides) seemed to be much
better with the c_can driver. I had therefore expected it to cause _lower_ cpu
load than the pch driver and was somewhat disappointed. I'll have to give this a
closer look after the holidays, maybe I can find the reasons why this happens.

I then removed the two lines that check for the EOB bit. That seems to fix all
hangs that I observed on the RX end. It seems to be quite stable now, even under
high load. I'm not really sure if I consider the out-of-order reception problems
important right now. 

The TX side still seems to have hickups (hangs, as observed by others as well).
I'll try to look into that before the holidays, but I make no promises that I'll
find the time.

Christian