[Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode

[bugzilla-daemon@bugs.ecos.sourceware.org]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001453

--- Comment #15 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-01-25 20:07:05 GMT ---
(In reply to comment #9, comment #10, comment #11, comment #14)
> (In reply to comment #8)

References: Attachment 1527, Attachment 1528, Attachment 1531

> > So far only one comment.
> > 
> > Still try to be very careful when planning to change generic CAN I/O
> > API. There is not only a dependence (LPC2XXX).

[snip]

> I've seen this. The only change that impacts other packages is the
> CYGNUM_CAN_EVENT_OVERRUN_RX event.
> 
> Today this event has two meaning: 1) because the eCos RX queue is
> overwritten by a new message 2) a hardware related overrun. So when
> the event occurs, one can't know if it's because the receive queue is
> undersized (or the application is to slow to empty the queue), or if
> the driver isn't fast enough to process CAN bus activity, which is
> very different. So I've made a CYGNUM_CAN_EVENT_OVERRUN_RX_HW event
> for the second case.

It seems it makes sense. Agree with you.

> I've modified the AT91SAM7 and MCF52XX driver accordingly (one line is
> patched just to change the name of the event since this event is
> generated only in one place. Since all CAN drivers have been written
> by Uwe Kindler and follow the same logic it's easy to understand the
> code from a driver to the other).

Ok. Attachment 1531 in this part looks safe. As I can only test builds I
hope I missed nothing and we will break nothing.

Minor: Attachment 1527: ChangeLog's record missed that you updated and
``can_driver_doc.html'' (No need to repost the patch).

> LPC2XXX driver I'll patch without my other changes, so every driver
> will be kept coherent with the CAN IO package.

I see. Sorry that you did not find volunteers that have the eCos CAN
hardware. However, with my own eyes, I have not found anything
suspicious. Tests for ea2468 target with applied patches were built
without any errors. (Next, I will continue to test builds for MCF52XX).

By the way, we may on occasion remove all the warnings in builds of CAN
tests. I found such annoying warnings (of course, they did exist before
the patching):

  cc1: warning: command line option "-Woverloaded-virtual" is valid for
C++/ObjC++ but not for C

Medicine: s/\$\(CFLAGS\)/$(ACTUAL_CFLAGS)/g for make rules in CAN config
files

  io/can/current/cdl/io_can.cdl
  devs/can/m68k/mcf52xx/current/cdl/can_mcf52xx.cdl
  devs/can/arm/lpc2xxx/current/cdl/can_lpc2xxx.cdl

and may be this can be fixed in another patch-set. Your decision.

> Everything else I added is supported by older driver since it's just a
> matter of API convention defaulting to the set of features supported
> by older drivers.

Bernard, your work looks great for me. In particular, thank you for
updated documentation.  I understand (agree with) your arguments. I
plan to accept all changes after further testing, if nobody objects.

Sergei

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.