public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public•gmane.org>
To: Bhupesh SHARMA <bhupesh.sharma-qxv4g6HH51o@public•gmane.org>
Cc: "socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public•gmane.org"
	<socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public•gmane.org>,
	"netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org"
	<netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
	Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public•gmane.org>
Subject: Re: [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for Bosch C_CAN controller
Date: Tue, 08 Feb 2011 12:28:08 +0100	[thread overview]
Message-ID: <4D5128C8.1060208@grandegger.com> (raw)
In-Reply-To: <D5ECB3C7A6F99444980976A8C6D896384DEE365EE8-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>

On 02/08/2011 11:45 AM, Bhupesh SHARMA wrote:
>> -----Original Message-----
>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public•gmane.org]
>> Sent: Tuesday, February 08, 2011 4:01 PM
>> To: Bhupesh SHARMA
>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public•gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org; Marc
>> Kleine-Budde
>> Subject: Re: [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for
>> Bosch C_CAN controller
>>
>> On 02/08/2011 10:04 AM, Bhupesh SHARMA wrote:
>>> Hi Wolfgang,
>>>
>>>>> + stats->rx_errors++;
>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>> + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
>>>>> +
>>>>> + switch (lec_type) {
>>>>> + case LEC_STUFF_ERROR:
>>>>> + netdev_dbg(dev, "stuff error\n");
>>>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>>>> + break;
>>>>> +
>>>>> + case LEC_FORM_ERROR:
>>>>> + netdev_dbg(dev, "form error\n");
>>>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>>>> + break;
>>>>> +
>>>>> + case LEC_ACK_ERROR:
>>>>> + netdev_dbg(dev, "ack error\n");
>>>>> + cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
>>>>> + CAN_ERR_PROT_LOC_ACK_DEL);
>>>>> + break;
>>>>> +
>>>>> + case LEC_BIT1_ERROR:
>>>>> + netdev_dbg(dev, "bit1 error\n");
>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
>>>>> + break;
>>>>> +
>>>>> + case LEC_BIT0_ERROR:
>>>>> + netdev_dbg(dev, "bit0 error\n");
>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT0;
>>>>> + break;
>>>>> +
>>>>> + case LEC_CRC_ERROR:
>>>>> + netdev_dbg(dev, "CRC error\n");
>>>>> + cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
>>>>> + CAN_ERR_PROT_LOC_CRC_DEL);
>>>>> + break;
>>>>> + }
>>>
>>> >From the C_CAN manual:
>>>>
>>>> "The LEC field holds a code which indicates the type of the last
>> error
>>>> to occur on the CAN bus. This field will be cleared to '0' when a
>>>> message has been transferred (reception or transmission) without
>> error.
>>>> The unused code '7' may be written by the CPU to check for updates."
>>>
>>>> Not sure if it's necessary to reset the lec at init and after an
>> error
>>>> to 0x7 and check it. More below...
>>>
>>> I worked on your suggestion and instead found that the follow algo
>> must be used
>>> for reading updated `lec` values:
>>> a. Init lec by 0x07 at start.
>>> b. In function `c_can_err` check if lec is 0x7 (no bus error since
>> this value was
>>> written by CPU on status register) or 0x0 (no error). If so, return
>> without
>>> sending an error frame on stack. Else, check for the lec error type
>> and
>>> submit error frame on stack accordingly.
>>> c. In case a lec error is found and served in `c_can_err` routine,
>> write
>>> lec value to 0x07 again in status reg to check for updated lec later.
>>>
>>> This is my understanding after reading the specs time and again and
>>> implementing/testing the logic.
>>>
>>> Do you think this is fine or do you have any better approach?
>>
>> That's what I remember from the CC770 driver. Search for lec in:
>>
>> http://svn.berlios.de/wsvn/socketcan/trunk/kernel/2.6/drivers/net/can/c
>> c770/cc770.c
>>
> 
> Seems similar. But step (c) mentioned above seems missing from cc770.c driver,
> i.e. "In case a lec error is found and served (by means of sending an error
> frame on the bus) write lec value to 0x07 again in status reg to check for updated
> lec later-on. In my view seems logical to add it also. 

It's done in cc770_status_interrupt():

        status = cc770_read_reg(priv, status);
        /* Reset the status register including RXOK and TXOK */
        cc770_write_reg(priv, status, STAT_LEC_MASK);

Wolfgang.

  parent reply	other threads:[~2011-02-08 11:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-08  9:04 [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for Bosch C_CAN controller Bhupesh SHARMA
     [not found] ` <D5ECB3C7A6F99444980976A8C6D896384DEE2BE326-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>
2011-02-08 10:30   ` Wolfgang Grandegger
     [not found]     ` <4D511B4E.1090001-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-02-08 10:45       ` Bhupesh SHARMA
     [not found]         ` <D5ECB3C7A6F99444980976A8C6D896384DEE365EE8-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>
2011-02-08 11:28           ` Wolfgang Grandegger [this message]
     [not found]             ` <4D5128C8.1060208-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-02-08 11:36               ` Bhupesh SHARMA
  -- strict thread matches above, loose matches on Subject: below --
2011-01-20  9:20 Bhupesh Sharma
     [not found] ` <1295515238-481-1-git-send-email-bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
2011-01-22 19:26   ` Wolfgang Grandegger
     [not found]     ` <4D3B2F66.8090603-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-01-25  4:36       ` Bhupesh SHARMA
     [not found]         ` <D5ECB3C7A6F99444980976A8C6D896384DEE082650-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>
2011-01-25  7:51           ` Wolfgang Grandegger
     [not found]             ` <4D3E8118.10904-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-01-25  8:28               ` Bhupesh SHARMA
2011-02-01  4:29               ` Bhupesh SHARMA
     [not found]                 ` <D5ECB3C7A6F99444980976A8C6D896384DEE0832F8-8vAmw3ZAcdzhJTuQ9jeba9BPR1lH4CV8@public.gmane.org>
2011-02-01  7:23                   ` Wolfgang Grandegger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D5128C8.1060208@grandegger.com \
    --to=wg-5yr1bzd7o62+xt7jha+gda@public$(echo .)gmane.org \
    --cc=bhupesh.sharma-qxv4g6HH51o@public$(echo .)gmane.org \
    --cc=mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public$(echo .)gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
    --cc=socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public$(echo .)gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox