public inbox for ecos-patches@sourceware.org
 help / color / mirror / Atom feed
* [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode
@ 2012-01-13 16:16 bugzilla-daemon
  2012-01-13 16:17 ` [Bug 1001453] " bugzilla-daemon
                   ` (22 more replies)
  0 siblings, 23 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-13 16:16 UTC (permalink / raw)
  To: ecos-patches

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

           Summary: CAN IO package: wider flags field, flag to report
                    return to 'error active' mode
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: Other
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: low
         Component: Patches and contributions
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: bernard.fouche@kuantic.com
                CC: ecos-patches@ecos.sourceware.org
             Class: Advice Request


The CAN IO package reports events when the controller is downgraded to 'error
passive' or 'bus off'. It does not report an event when it gets back to 'error
active' mode: user code must poll the controller state to know about that
status change instead of being able to keep the event callback logic for
everything that concerns the CAN bus activity.

The patch changes the 'flags' field of 'can_event_t' to 32 bits and add a new
event in bit 16. The fields in 'can_event_t' are reordered to continue to
provide a 24 bytes CAN event on an ARM MCU even if 'flags' is wider, so the
operational cost is nil for such targets.

Patch also includes doc fix and an option to add/suppress compiler flags for
the package.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
@ 2012-01-13 16:17 ` bugzilla-daemon
  2012-01-14 18:51 ` bugzilla-daemon
                   ` (21 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-13 16:17 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #1 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-13 16:16:51 GMT ---
Created an attachment (id=1517)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1517)
new event flag, wider 'flags' field for an event, doc, compiler flag option

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
  2012-01-13 16:17 ` [Bug 1001453] " bugzilla-daemon
@ 2012-01-14 18:51 ` bugzilla-daemon
  2012-01-14 22:40 ` bugzilla-daemon
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-14 18:51 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #2 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-01-14 18:51:18 GMT ---
Hi Bernard,

In a fact, your patch (by the subject) is small enough and you did
submit a huge delta. Well, I would filter it, but, I won't be able test
it. So, please, regenerate your patch against eCos CVS sources set
'--ignore-all-space' option for diff (the reason that your working eCos
tree has sources with stripped all trailing whitespaces). Also, please,
expand ChangeLog record, as you fixed not only one header, but and eCos
documentation and config files.

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
  2012-01-13 16:17 ` [Bug 1001453] " bugzilla-daemon
  2012-01-14 18:51 ` bugzilla-daemon
@ 2012-01-14 22:40 ` bugzilla-daemon
  2012-01-15 13:56 ` bugzilla-daemon
                   ` (19 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-14 22:40 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #3 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-14 22:39:39 GMT ---
I also forgot to document the meaning of the added flag ;-)

While thinking at other things I may have forgotten, I now see an issue with
the bitfield 'support_flags' in cyg_can_hdi.

Here is cyg_can_hdi:

typedef struct cyg_can_hdi_st
{
    cyg_uint8 support_flags;
    cyg_uint8 controller_type;
} cyg_can_hdi;

The issue is a lack of description of the low level driver filtering
capabilities.

The 'SW-Filt' flag has been replaced by 'autobaud' in the source code (my patch
fixes the doc about this).

Hence there is no more description of a hw driver filtering capabilities while
these capabilities are essential in a real world CAN network. The 'software
filtering' information was not very helpful to user code anyway, I suppose
that's why it has been removed and the corresponding bit recycled.

I suggest to use two reserved bits in 'support_flags':

- a bit to describe identifier range filtering capability (0=no range
filtering, this keep compatibility with current code)

- a bit to describe bitmask filtering capability (0=no bitmask filtering). I
think bitmask filtering is the most common and efficient way to filter CAN
frames. (While LPC17XX has range filtering capabilities, the upcoming LPC18XX
has bitmask filtering instead)

The side effect is a need for more config keys, to declare filtering
information.

The LPC2XXX driver provides identifier range filtering config keys (as a cdl
option), but since the CAN IO package does not support range filtering (in
terms of API convention), these supplementary config keys can be obtained by
user code only by including explicitly the LPC2XXX specific header file.

If these two new data bits in 'support_flags' are added, then the config keys
provided by the LPC2XXX driver can become the 'official' config keys for
identifier range filtering.

And of course there is also a need for config keys related to bitmask
filtering. AFAIK, bitmask filtering is made by declaring an identifier value
and a bitmask, so the config keys related to bitmask filtering would need 2 x
32 bits value for config data (like the LPC2XXX range filtering key)

Since the CAN IO package relay to the hardware layer the config keys it does
not handle itself, there would be no functional change in the package, like the
patch I proposed.

If this is ok I'll provide an updated patch (using the diff option you
mention), and combine these changes. 

Or I can provide two patches, one to fix the patch I proposed, and then I open
a new bugzilla entry with a new patch related to 'support_flags'.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (2 preceding siblings ...)
  2012-01-14 22:40 ` bugzilla-daemon
@ 2012-01-15 13:56 ` bugzilla-daemon
  2012-01-15 22:30 ` bugzilla-daemon
                   ` (18 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-15 13:56 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #4 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-01-15 13:56:08 GMT ---
(In reply to comment #3)
> I also forgot to document the meaning of the added flag ;-)

Of course, we need to keep in sync documentation and sources.

> While thinking at other things I may have forgotten, I now see an
> issue with the bitfield 'support_flags' in cyg_can_hdi.
> 
> Here is cyg_can_hdi:
> 
> typedef struct cyg_can_hdi_st
> {
>     cyg_uint8 support_flags;
>     cyg_uint8 controller_type;
> } cyg_can_hdi;
> 
> The issue is a lack of description of the low level driver filtering
> capabilities.
> 
> The 'SW-Filt' flag has been replaced by 'autobaud' in the source code
> (my patch fixes the doc about this).

I've seen. Thank you for the catch.

> Hence there is no more description of a hw driver filtering
> capabilities while these capabilities are essential in a real world
> CAN network. The 'software filtering' information was not very helpful
> to user code anyway, I suppose that's why it has been removed and the
> corresponding bit recycled.

It seems so.

> I suggest to use two reserved bits in 'support_flags':
> 
> - a bit to describe identifier range filtering capability (0=no range
> filtering, this keep compatibility with current code)
> 
> - a bit to describe bitmask filtering capability (0=no bitmask
> filtering). I think bitmask filtering is the most common and efficient
> way to filter CAN frames. (While LPC17XX has range filtering
> capabilities, the upcoming LPC18XX has bitmask filtering instead)

Agreed.

> The side effect is a need for more config keys, to declare filtering
> information.

IMO, it is not issue for eCos, more that default values would not break
old flag's value.

> The LPC2XXX driver provides identifier range filtering config keys (as
> a cdl option), but since the CAN IO package does not support range
> filtering (in terms of API convention), these supplementary config
> keys can be obtained by user code only by including explicitly the
> LPC2XXX specific header file.
> 
> If these two new data bits in 'support_flags' are added, then the
> config keys provided by the LPC2XXX driver can become the 'official'
> config keys for identifier range filtering.

Excellent coincidence.

> And of course there is also a need for config keys related to bitmask
> filtering.  AFAIK, bitmask filtering is made by declaring an
> identifier value and a bitmask, so the config keys related to bitmask
> filtering would need 2 x 32 bits value for config data (like the
> LPC2XXX range filtering key)
> 
> Since the CAN IO package relay to the hardware layer the config keys
> it does not handle itself, there would be no functional change in the
> package, like the patch I proposed.

Great.

> If this is ok I'll provide an updated patch (using the diff option you
> mention), and combine these changes.
> 
> Or I can provide two patches, one to fix the patch I proposed, and
> then I open a new bugzilla entry with a new patch related to
> 'support_flags'.

Bernard, thank you for your investigation. I think the patches can be
submitted here to save full history of issue, but, if you prefer a
separate Bugzilla report, please, create new one. One thing then. Now,
all your enhancements need to get a copyright assignment from you as I
see your "delta" won't be a-few-lines trivial patch.

Could you, please, initiate a copyright assignments process? You can
find more info here: http://ecos.sourceware.org/assign.html

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (3 preceding siblings ...)
  2012-01-15 13:56 ` bugzilla-daemon
@ 2012-01-15 22:30 ` bugzilla-daemon
  2012-01-16  8:16 ` bugzilla-daemon
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-15 22:30 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #5 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-15 22:30:39 GMT ---
(In reply to comment #4)
> Could you, please, initiate a copyright assignments process? You can
> find more info here: http://ecos.sourceware.org/assign.html

The papers have been already signed by a colleague and I, and sent to the FSF
last week ;-)

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (4 preceding siblings ...)
  2012-01-15 22:30 ` bugzilla-daemon
@ 2012-01-16  8:16 ` bugzilla-daemon
  2012-01-18 21:48 ` bugzilla-daemon
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-16  8:16 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #6 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-01-16 08:15:58 GMT ---
(In reply to comment #5)
> (In reply to comment #4)
> > Could you, please, initiate a copyright assignments process? You can
> > find more info here: http://ecos.sourceware.org/assign.html
> 
> The papers have been already signed by a colleague and I, and sent to
> the FSF last week ;-)

Great! To be clear, my request does not block patch review (feel free to
send your patches), it may delay a check-in only.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (5 preceding siblings ...)
  2012-01-16  8:16 ` bugzilla-daemon
@ 2012-01-18 21:48 ` bugzilla-daemon
  2012-01-19 20:46 ` bugzilla-daemon
                   ` (15 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-18 21:48 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #7 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-18 21:48:30 GMT ---
Yet more needed change:

- since flags will be 32 bits wide, then the flag_mask field in
cyg_can_callback_cfg_st must be expanded too. Must do a typedef (or a #define?)
for flags instead of using cyg_uint32, I missed the flag_mask expansion because
of this.

- in can_callbacks_t, describing the function pointers accessible to the low
level drivers, xmt_msg is today:

void (*xmt_msg)(can_channel *chan, void *pdata)

I'd like to have instead:

cyg_bool (*xmt_msg)(can_channel *chan, void *pdata)

This is because when the low level driver knows that it can send more than one
message, it has no way to know if high level Tx queue is empty or not. For
instance the driver I test now uses the 3 TX buffers of the LPC17XX. When X
buffers are free, the driver can just call X times xmt_msg, even if no message
is queued. Proposed changed is to have xmt_msg to report is a message was sent
when it was called. If yes, then call again xmt_msg if there are still free Tx
buffers. If no, stop calling since it's useless. On hardware supporting many
Message Objects this need will arise anyway. The only other possibility is to
use a global variable in the low level driver: reset the variable, call
xmt_msg() that calls driver _putmsg(), set variable there, go back to DSR, test
variable. Seems ugly an inefficient.

There are other issues:

- autobaud is said to be potentially supported by the CAN IO package, but there
is no infrastructure to manage it. I hardly see how it could without some
parameters defining the set of speeds to try, how long to wait for a
timeout,and how to warn the upper layer that autobaud found some speed (no
event like CYGNUM_CAN_EVENT_AUTOBAUD_END exists). No start/stop autobaud config
keys exist.   

- LPC2XXX driver supports listen-only mode also by providing unofficial config
keys. IMHO they should be made mainstream since this feature is more and more
available and is very handy, for instance to make a non-intrusive network
monitor (and eventually autobaud...). 

- the application can't know the current values of TX/RX Rec while these are
important to diagnose a bus. A _get_config() key seems appropriate.

BTW I start tomorrow a few days of field tests, I'll try to make an updated
patch next week.

  Bernard

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (6 preceding siblings ...)
  2012-01-18 21:48 ` bugzilla-daemon
@ 2012-01-19 20:46 ` bugzilla-daemon
  2012-01-24 19:25 ` bugzilla-daemon
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-19 20:46 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #8 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-01-19 20:45:54 GMT ---
(In reply to comment #7)
> Yet more needed change:
> 
> - since flags will be 32 bits wide, then the flag_mask field in
> cyg_can_callback_cfg_st must be expanded too. Must do a typedef (or a
> #define?) for flags instead of using cyg_uint32, I missed the
> flag_mask expansion because of this.
> 
> - in can_callbacks_t, describing the function pointers accessible to
> the low level drivers, xmt_msg is today:
> 
> void (*xmt_msg)(can_channel *chan, void *pdata)
> 
> I'd like to have instead:

[snip]

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).

  % ecosconfig list | grep DEVS_CAN
  Package CYGPKG_DEVS_CAN_AT91SAM7 (AT91SAM7 CAN device drivers):
  Package CYGPKG_DEVS_CAN_LOOP (Loop CAN device drivers):
  Package CYGPKG_DEVS_CAN_LPC2XXX (LPC2xxx CAN device drivers):
  Package CYGPKG_DEVS_CAN_MCF52xx_FLEXCAN (MCF52xx FlexCAN device drivers):

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (7 preceding siblings ...)
  2012-01-19 20:46 ` bugzilla-daemon
@ 2012-01-24 19:25 ` bugzilla-daemon
  2012-01-25 11:35 ` bugzilla-daemon
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-24 19:25 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #9 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-24 19:25:16 GMT ---
(In reply to comment #8)
>
> 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).
> 
>   % ecosconfig list | grep DEVS_CAN
>   Package CYGPKG_DEVS_CAN_AT91SAM7 (AT91SAM7 CAN device drivers):
>   Package CYGPKG_DEVS_CAN_LOOP (Loop CAN device drivers):
>   Package CYGPKG_DEVS_CAN_LPC2XXX (LPC2xxx CAN device drivers):
>   Package CYGPKG_DEVS_CAN_MCF52xx_FLEXCAN (MCF52xx FlexCAN device drivers):
> 
> Sergei

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.

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).

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

The loop driver does not require any change since it does not handle hardware.

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.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (8 preceding siblings ...)
  2012-01-24 19:25 ` bugzilla-daemon
@ 2012-01-25 11:35 ` bugzilla-daemon
  2012-01-25 11:39 ` bugzilla-daemon
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-25 11:35 UTC (permalink / raw)
  To: ecos-patches

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

Bernard Fouché <bernard.fouche@kuantic.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1517|0                           |1
        is obsolete|                            |

--- Comment #10 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-25 11:34:34 GMT ---
Created an attachment (id=1527)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1527)
Updated patch for CAN IO package

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (9 preceding siblings ...)
  2012-01-25 11:35 ` bugzilla-daemon
@ 2012-01-25 11:39 ` bugzilla-daemon
  2012-01-25 11:44 ` bugzilla-daemon
                   ` (11 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-25 11:39 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #11 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-25 11:38:50 GMT ---
Created an attachment (id=1528)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1528)
New config keys for CAN IO package

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (10 preceding siblings ...)
  2012-01-25 11:39 ` bugzilla-daemon
@ 2012-01-25 11:44 ` bugzilla-daemon
  2012-01-25 11:45 ` bugzilla-daemon
                   ` (10 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-25 11:44 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #12 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-25 11:44:23 GMT ---
Created an attachment (id=1529)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1529)
Reflect API changes in AT91SAM7 CAN driver

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (11 preceding siblings ...)
  2012-01-25 11:44 ` bugzilla-daemon
@ 2012-01-25 11:45 ` bugzilla-daemon
  2012-01-25 13:59 ` bugzilla-daemon
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-25 11:45 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #13 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-25 11:44:57 GMT ---
Created an attachment (id=1530)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1530)
Reflect API changes in MCF52XX CAN driver

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (12 preceding siblings ...)
  2012-01-25 11:45 ` bugzilla-daemon
@ 2012-01-25 13:59 ` bugzilla-daemon
  2012-01-25 20:07 ` bugzilla-daemon
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-25 13:59 UTC (permalink / raw)
  To: ecos-patches

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

Bernard Fouché <bernard.fouche@kuantic.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1529|0                           |1
        is obsolete|                            |
   Attachment #1530|0                           |1
        is obsolete|                            |

--- Comment #14 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-25 13:59:21 GMT ---
Created an attachment (id=1531)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1531)
Single patch for MCF52XX, AT91SAM7, LPC2XXX drivers

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (13 preceding siblings ...)
  2012-01-25 13:59 ` bugzilla-daemon
@ 2012-01-25 20:07 ` bugzilla-daemon
  2012-01-26  8:51 ` bugzilla-daemon
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-25 20:07 UTC (permalink / raw)
  To: ecos-patches

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (14 preceding siblings ...)
  2012-01-25 20:07 ` bugzilla-daemon
@ 2012-01-26  8:51 ` bugzilla-daemon
  2012-02-06 10:00 ` bugzilla-daemon
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-01-26  8:51 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #16 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-01-26 08:50:35 GMT ---
(In reply to comment #15)
> [snip]
> > 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.

Instead of updating the LPC2XXX CAN driver, I'll make a LPC17XX driver that can
still be compiled for LPC2XXX with a minimal amount of work. Anyway what stops
me to provide hardware drivers at this time is bug #1001456 since its solution
seems to be some API evolution regarding interrupt management, at least for
Cortex-M systems.

> 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.

Why not wait after the release of the new toolchain? Maybe other compilation
flags will have to be updated in different places?

> 
> 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.

There is still something missing: having the application to be able to get the
values of TXREC/RXREC but this means more modifications to all hw drivers and
I'm not sure how to do this properly. Ideally the counter values would be
provided in events having flags raised like ERR_ACTIVE/ERR_PASSIVE, or they
could be made available by a call to _get_config(). The first solution would
increase the event size (could be an option at the CAN IO package level), the
second solution returns values that are already obsolete by the time the
application get them. So I'll leave this to the next volunteer ;-)

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (15 preceding siblings ...)
  2012-01-26  8:51 ` bugzilla-daemon
@ 2012-02-06 10:00 ` bugzilla-daemon
  2012-02-06 15:57 ` bugzilla-daemon
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-02-06 10:00 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #17 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-02-06 10:00:22 GMT ---
*** Bug 1001435 has been marked as a duplicate of this bug. ***

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (16 preceding siblings ...)
  2012-02-06 10:00 ` bugzilla-daemon
@ 2012-02-06 15:57 ` bugzilla-daemon
  2012-02-06 20:30 ` bugzilla-daemon
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-02-06 15:57 UTC (permalink / raw)
  To: ecos-patches

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

Sergei Gavrikov <sergei.gavrikov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEEDINFO
     Ever Confirmed|0                           |1

--- Comment #18 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-02-06 15:56:24 GMT ---
Still need CA.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (17 preceding siblings ...)
  2012-02-06 15:57 ` bugzilla-daemon
@ 2012-02-06 20:30 ` bugzilla-daemon
  2012-02-07  6:13 ` bugzilla-daemon
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-02-06 20:30 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #19 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-02-06 20:30:17 GMT ---
(In reply to comment #18)
> Still need CA.

Hi Bernard

Could you specify when you sent your eCos Copyright Assignment(s) to
FSF? What have you sent and to whom?  You can drop me privately, if you
want.

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (18 preceding siblings ...)
  2012-02-06 20:30 ` bugzilla-daemon
@ 2012-02-07  6:13 ` bugzilla-daemon
  2012-02-09 21:46 ` bugzilla-daemon
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-02-07  6:13 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #20 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-02-07 06:12:55 GMT ---
(In reply to comment #19)

> Could you specify when you sent your eCos Copyright Assignment(s) to
> FSF? What have you sent and to whom?  You can drop me privately, if
> you want.

Bernard, yesterday I got an information that FSF signed and sent your
papers back to you. No need more information from you. So, now I can
process your patches "safely".

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (19 preceding siblings ...)
  2012-02-07  6:13 ` bugzilla-daemon
@ 2012-02-09 21:46 ` bugzilla-daemon
  2012-02-10 17:28 ` bugzilla-daemon
  2012-02-11  4:16 ` bugzilla-daemon
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-02-09 21:46 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #21 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-02-09 21:46:13 GMT ---
Now in CVS (a few small corrections was applied).

Thank you for your contribution to eCos.

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (20 preceding siblings ...)
  2012-02-09 21:46 ` bugzilla-daemon
@ 2012-02-10 17:28 ` bugzilla-daemon
  2012-02-11  4:16 ` bugzilla-daemon
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-02-10 17:28 UTC (permalink / raw)
  To: ecos-patches

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

--- Comment #22 from Bernard Fouché <bernard.fouche@kuantic.com> 2012-02-10 17:28:22 GMT ---
Thanks Sergei! IMHO this 'bug' can be closed...

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Bug 1001453] CAN IO package: wider flags field, flag to report return to 'error active' mode
  2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
                   ` (21 preceding siblings ...)
  2012-02-10 17:28 ` bugzilla-daemon
@ 2012-02-11  4:16 ` bugzilla-daemon
  22 siblings, 0 replies; 24+ messages in thread
From: bugzilla-daemon @ 2012-02-11  4:16 UTC (permalink / raw)
  To: ecos-patches

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

Sergei Gavrikov <sergei.gavrikov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |RESOLVED
         Resolution|                            |CURRENTRELEASE

--- Comment #23 from Sergei Gavrikov <sergei.gavrikov@gmail.com> 2012-02-11 04:15:53 GMT ---
Mark as RESOLVED.

-- 
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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2012-02-11  4:16 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-13 16:16 [Bug 1001453] New: CAN IO package: wider flags field, flag to report return to 'error active' mode bugzilla-daemon
2012-01-13 16:17 ` [Bug 1001453] " bugzilla-daemon
2012-01-14 18:51 ` bugzilla-daemon
2012-01-14 22:40 ` bugzilla-daemon
2012-01-15 13:56 ` bugzilla-daemon
2012-01-15 22:30 ` bugzilla-daemon
2012-01-16  8:16 ` bugzilla-daemon
2012-01-18 21:48 ` bugzilla-daemon
2012-01-19 20:46 ` bugzilla-daemon
2012-01-24 19:25 ` bugzilla-daemon
2012-01-25 11:35 ` bugzilla-daemon
2012-01-25 11:39 ` bugzilla-daemon
2012-01-25 11:44 ` bugzilla-daemon
2012-01-25 11:45 ` bugzilla-daemon
2012-01-25 13:59 ` bugzilla-daemon
2012-01-25 20:07 ` bugzilla-daemon
2012-01-26  8:51 ` bugzilla-daemon
2012-02-06 10:00 ` bugzilla-daemon
2012-02-06 15:57 ` bugzilla-daemon
2012-02-06 20:30 ` bugzilla-daemon
2012-02-07  6:13 ` bugzilla-daemon
2012-02-09 21:46 ` bugzilla-daemon
2012-02-10 17:28 ` bugzilla-daemon
2012-02-11  4:16 ` bugzilla-daemon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).