netdev - RE: [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for Bosch C

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 8 Feb 2011 19:36:56 +0800
From:	Bhupesh SHARMA <bhupesh.sharma@...com>
To:	Wolfgang Grandegger <wg@...ndegger.com>
Cc:	"socketcan-core@...ts.berlios.de" <socketcan-core@...ts.berlios.de>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Marc Kleine-Budde <mkl@...gutronix.de>
Subject: RE: [PATCH net-next-2.6 v5 1/1] can: c_can: Added support for Bosch
 C_CAN controller



> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg@...ndegger.com]
> Sent: Tuesday, February 08, 2011 4:58 PM
> To: Bhupesh SHARMA
> Cc: socketcan-core@...ts.berlios.de; netdev@...r.kernel.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 11:45 AM, Bhupesh SHARMA wrote:
> >> -----Original Message-----
> >> From: Wolfgang Grandegger [mailto:wg@...ndegger.com]
> >> Sent: Tuesday, February 08, 2011 4:01 PM
> >> To: Bhupesh SHARMA
> >> Cc: socketcan-core@...ts.berlios.de; netdev@...r.kernel.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);
> 
Oops.. Sorry it seems I missed it.
Many Thanks for your help and comments.

Regards,
Bhupesh

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html