linux-kernel - RE: [PATCH] can: xilinx: fix RX FIFO overflow error handling

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]

Message-ID: <C246CAC1457055469EF09E3A7AC4E11A4A4F2B66@XAP-PVEXMBX01.xlnx.xilinx.com>
Date:	Thu, 22 Oct 2015 04:46:54 +0000
From:	Appana Durga Kedareswara Rao <appana.durga.rao@...inx.com>
To:	'Andrea Scian - DAVE Embedded Systems' <andrea.scian@...e.eu>,
	"Michal Simek" <michals@...inx.com>
CC:	"mkl@...gutronix.de" <mkl@...gutronix.de>,
	"wg@...ndegger.com" <wg@...ndegger.com>,
	"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Anirudha Sarangi" <anirudh@...inx.com>
Subject: RE: [PATCH] can: xilinx: fix RX FIFO overflow error handling

Hi,

	Thanks for the patch. Sorry for the delay in the reply.
Patch looks good to me 

Reviewed-by: Kedareswara rao Appana <appanad@...inx.com>   

The bus off condition in the error interrupt handler also needs the same treatment sent the patch for the same.
Please resend your patch on top of my patch series.

Regards,
Kedar


> -----Original Message-----
> From: Andrea Scian - DAVE Embedded Systems
> [mailto:andrea.scian@...e.eu]
> Sent: Thursday, August 06, 2015 1:31 AM
> To: Michal Simek; Appana Durga Kedareswara Rao
> Cc: mkl@...gutronix.de; wg@...ndegger.com; linux-can@...r.kernel.org;
> linux-kernel@...r.kernel.org
> Subject: Re: [PATCH] can: xilinx: fix RX FIFO overflow error handling
> 
> 
> Michal,
> Kedar,
> 
> Any feedback regarding my patch?
> Have you ever experienced such a problem?
> 
> Andrea SCIAN
> 
> *DAVE Embedded Systems*
> via Talponedo 29/A 33080 Porcia (PN) - Italy
> Telephone: +39.0434.921215
> Telefax: +39.0434.1994030
> web: www.dave.eu <http://www.dave.eu>
> 
> 
> Il 24/07/2015 06:15, Michal Simek ha scritto:
> > + Kedar
> >
> > On 07/23/2015 11:13 PM, Andrea Scian wrote:
> >> Simply resetting the peripheral on RX FIFO overflow in not enough,
> >> because we also need to re-initialize the whole device.
> >> Also always enable RX FIFO overflow interrupt otherwise we may hang
> >> until another interrupt arrives (this happens if FIFO overrun and
> >> just read from CAN bus with candump)
> >>
> >> Signed-off-by: Andrea Scian <andrea.scian@...e.eu>
> >> ---
> >>
> >> You can reproduce the problem with the following test-bed
> >> * connect a Zynq based board to a PC with a CAN bus adapter (e.g.
> >> PCAN USB)
> >> * start sending lots of CAN messages to Zynq system
> >> * configure and start xilinx CAN driver
> >> ** canconfig can0 bitrate 1000000
> >> ** canconfig can0 start
> >> * try to send/receive messages (cansend/candump)
> >> * if you send a CAN message you'll get the RX FIFO error and
> >> additional messages will not be received
> >> * if you try to get messages you simple don't receive them (no
> >> interrupt triggered)
> >> * with canconfig stop/start the hang goes away (if the others peers
> >> stop sending send lots of messages ;-) )
> >>
> >> I tested the patch over xilinx-2014.04 kernel, but the patch applies
> >> cleanly to mainline too.
> >>
> >> If someone has a better understanding of Xilinx CAN driver, please
> >> let me know if it's better to handle the error in a different manner.
> >>
> >> Maybe the bus off handler need the same threadment too.
> >>
> >> TIA,
> >>
> >> Andrea Scian
> >>
> >> ---
> >>   drivers/net/can/xilinx_can.c |    5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/can/xilinx_can.c
> >> b/drivers/net/can/xilinx_can.c index 5e8b560..c278177 100644
> >> --- a/drivers/net/can/xilinx_can.c
> >> +++ b/drivers/net/can/xilinx_can.c
> >> @@ -100,6 +100,7 @@ enum xcan_reg {
> >>   #define XCAN_INTR_ALL		(XCAN_IXR_TXOK_MASK |
> XCAN_IXR_BSOFF_MASK |\
> >>   				 XCAN_IXR_WKUP_MASK |
> XCAN_IXR_SLP_MASK | \
> >>   				 XCAN_IXR_RXNEMP_MASK |
> XCAN_IXR_ERROR_MASK | \
> >> +				 XCAN_IXR_RXOFLW_MASK | \
> >>   				 XCAN_IXR_ARBLST_MASK |
> XCAN_IXR_RXOK_MASK)
> >>
> >>   /* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */ @@ -526,6
> >> +527,7 @@ static int xcan_rx(struct net_device *ndev)
> >>   	return 1;
> >>   }
> >>
> >> +static void xcan_chip_stop(struct net_device *ndev);
> >>   /**
> >>    * xcan_err_interrupt - error frame Isr
> >>    * @ndev:	net_device pointer
> >> @@ -597,7 +599,8 @@ static void xcan_err_interrupt(struct net_device
> *ndev, u32 isr)
> >>   	if (isr & XCAN_IXR_RXOFLW_MASK) {
> >>   		stats->rx_over_errors++;
> >>   		stats->rx_errors++;
> >> -		priv->write_reg(priv, XCAN_SRR_OFFSET,
> XCAN_SRR_RESET_MASK);
> >> +		xcan_chip_stop(ndev);
> >> +		xcan_chip_start(ndev);
> >>   		if (skb) {
> >>   			cf->can_id |= CAN_ERR_CRTL;
> >>   			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> >>
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/