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