* [PATCH 2/3] can: c_can: fix an interrupt thrash issue with c_can driver
@ 2012-04-20 9:58 AnilKumar Ch
2012-04-24 8:01 ` Marc Kleine-Budde
0 siblings, 1 reply; 3+ messages in thread
From: AnilKumar Ch @ 2012-04-20 9:58 UTC (permalink / raw)
To: wg, mkl, linux-can; +Cc: anantgole, nsekhar, AnilKumar Ch
This patch fixes an interrupt thrash issue with c_can driver.
In c_can_isr() function interrupts are disabled and enabled only in
c_can_poll() function. c_can_isr() & c_can_poll() both read the
irqstatus flag. However, irqstatus is always read as 0 in c_can_poll()
because all C_CAN interrupts are disabled in c_can_isr(). This causes
all interrupts to be re-enabled in c_can_poll() which in turn causes
another interrupt since the event is not really handled. This keeps
happening causing a flood of interrupts.
To fix this, read the irqstatus register in isr and use the same cached
value in the poll function.
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
drivers/net/can/c_can/c_can.c | 19 ++++++++-----------
drivers/net/can/c_can/c_can.h | 1 +
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 9ac28df..09fcc50 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -946,18 +946,16 @@ static int c_can_handle_bus_err(struct net_device *dev,
static int c_can_poll(struct napi_struct *napi, int quota)
{
- u16 irqstatus;
int lec_type = 0;
int work_done = 0;
struct net_device *dev = napi->dev;
struct c_can_priv *priv = netdev_priv(dev);
- irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
- if (!irqstatus)
+ if (!priv->irqstatus)
goto end;
/* status events have the highest priority */
- if (irqstatus == STATUS_INTERRUPT) {
+ if (priv->irqstatus == STATUS_INTERRUPT) {
priv->current_status = priv->read_reg(priv,
&priv->regs->status);
@@ -1008,12 +1006,12 @@ static int c_can_poll(struct napi_struct *napi, int quota)
lec_type = c_can_has_and_handle_berr(priv);
if (lec_type)
work_done += c_can_handle_bus_err(dev, lec_type);
- } else if ((irqstatus >= C_CAN_MSG_OBJ_RX_FIRST) &&
- (irqstatus <= C_CAN_MSG_OBJ_RX_LAST)) {
+ } else if ((priv->irqstatus >= C_CAN_MSG_OBJ_RX_FIRST) &&
+ (priv->irqstatus <= C_CAN_MSG_OBJ_RX_LAST)) {
/* handle events corresponding to receive message objects */
work_done += c_can_do_rx_poll(dev, (quota - work_done));
- } else if ((irqstatus >= C_CAN_MSG_OBJ_TX_FIRST) &&
- (irqstatus <= C_CAN_MSG_OBJ_TX_LAST)) {
+ } else if ((priv->irqstatus >= C_CAN_MSG_OBJ_TX_FIRST) &&
+ (priv->irqstatus <= C_CAN_MSG_OBJ_TX_LAST)) {
/* handle events corresponding to transmit message objects */
c_can_do_tx(dev);
}
@@ -1030,12 +1028,11 @@ end:
static irqreturn_t c_can_isr(int irq, void *dev_id)
{
- u16 irqstatus;
struct net_device *dev = (struct net_device *)dev_id;
struct c_can_priv *priv = netdev_priv(dev);
- irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
- if (!irqstatus)
+ priv->irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
+ if (!priv->irqstatus)
return IRQ_NONE;
/* disable all interrupts and schedule the NAPI */
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 9b7fbef..5f32d34 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -76,6 +76,7 @@ struct c_can_priv {
unsigned int tx_next;
unsigned int tx_echo;
void *priv; /* for board-specific data */
+ u16 irqstatus;
};
struct net_device *alloc_c_can_dev(void);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/3] can: c_can: fix an interrupt thrash issue with c_can driver
2012-04-20 9:58 [PATCH 2/3] can: c_can: fix an interrupt thrash issue with c_can driver AnilKumar Ch
@ 2012-04-24 8:01 ` Marc Kleine-Budde
2012-04-24 11:27 ` AnilKumar, Chimata
0 siblings, 1 reply; 3+ messages in thread
From: Marc Kleine-Budde @ 2012-04-24 8:01 UTC (permalink / raw)
To: AnilKumar Ch; +Cc: wg, linux-can, anantgole, nsekhar
[-- Attachment #1: Type: text/plain, Size: 4056 bytes --]
On 04/20/2012 11:58 AM, AnilKumar Ch wrote:
> This patch fixes an interrupt thrash issue with c_can driver.
>
> In c_can_isr() function interrupts are disabled and enabled only in
> c_can_poll() function. c_can_isr() & c_can_poll() both read the
> irqstatus flag. However, irqstatus is always read as 0 in c_can_poll()
> because all C_CAN interrupts are disabled in c_can_isr(). This causes
> all interrupts to be re-enabled in c_can_poll() which in turn causes
> another interrupt since the event is not really handled. This keeps
> happening causing a flood of interrupts.
Do both c_can and d_can behave the same way?
> To fix this, read the irqstatus register in isr and use the same cached
> value in the poll function.
More comments inline:
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
> drivers/net/can/c_can/c_can.c | 19 ++++++++-----------
> drivers/net/can/c_can/c_can.h | 1 +
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 9ac28df..09fcc50 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -946,18 +946,16 @@ static int c_can_handle_bus_err(struct net_device *dev,
>
> static int c_can_poll(struct napi_struct *napi, int quota)
> {
> - u16 irqstatus;
> int lec_type = 0;
> int work_done = 0;
> struct net_device *dev = napi->dev;
> struct c_can_priv *priv = netdev_priv(dev);
>
> - irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
I suggest:
irqstatus = priv->irqstatus;
This would be a quite small patch, because you can keep using irqstatus
below.
> - if (!irqstatus)
> + if (!priv->irqstatus)
> goto end;
>
> /* status events have the highest priority */
> - if (irqstatus == STATUS_INTERRUPT) {
> + if (priv->irqstatus == STATUS_INTERRUPT) {
> priv->current_status = priv->read_reg(priv,
> &priv->regs->status);
>
> @@ -1008,12 +1006,12 @@ static int c_can_poll(struct napi_struct *napi, int quota)
> lec_type = c_can_has_and_handle_berr(priv);
> if (lec_type)
> work_done += c_can_handle_bus_err(dev, lec_type);
> - } else if ((irqstatus >= C_CAN_MSG_OBJ_RX_FIRST) &&
> - (irqstatus <= C_CAN_MSG_OBJ_RX_LAST)) {
> + } else if ((priv->irqstatus >= C_CAN_MSG_OBJ_RX_FIRST) &&
> + (priv->irqstatus <= C_CAN_MSG_OBJ_RX_LAST)) {
> /* handle events corresponding to receive message objects */
> work_done += c_can_do_rx_poll(dev, (quota - work_done));
> - } else if ((irqstatus >= C_CAN_MSG_OBJ_TX_FIRST) &&
> - (irqstatus <= C_CAN_MSG_OBJ_TX_LAST)) {
> + } else if ((priv->irqstatus >= C_CAN_MSG_OBJ_TX_FIRST) &&
> + (priv->irqstatus <= C_CAN_MSG_OBJ_TX_LAST)) {
> /* handle events corresponding to transmit message objects */
> c_can_do_tx(dev);
> }
> @@ -1030,12 +1028,11 @@ end:
>
> static irqreturn_t c_can_isr(int irq, void *dev_id)
> {
> - u16 irqstatus;
> struct net_device *dev = (struct net_device *)dev_id;
> struct c_can_priv *priv = netdev_priv(dev);
>
> - irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> - if (!irqstatus)
> + priv->irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> + if (!priv->irqstatus)
> return IRQ_NONE;
>
> /* disable all interrupts and schedule the NAPI */
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 9b7fbef..5f32d34 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -76,6 +76,7 @@ struct c_can_priv {
> unsigned int tx_next;
> unsigned int tx_echo;
> void *priv; /* for board-specific data */
> + u16 irqstatus;
> };
>
> struct net_device *alloc_c_can_dev(void);
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH 2/3] can: c_can: fix an interrupt thrash issue with c_can driver
2012-04-24 8:01 ` Marc Kleine-Budde
@ 2012-04-24 11:27 ` AnilKumar, Chimata
0 siblings, 0 replies; 3+ messages in thread
From: AnilKumar, Chimata @ 2012-04-24 11:27 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: wg, linux-can, Gole, Anant, Nori, Sekhar
On Tue, Apr 24, 2012 at 13:31:27, Marc Kleine-Budde wrote:
> On 04/20/2012 11:58 AM, AnilKumar Ch wrote:
> > This patch fixes an interrupt thrash issue with c_can driver.
> >
> > In c_can_isr() function interrupts are disabled and enabled only in
> > c_can_poll() function. c_can_isr() & c_can_poll() both read the
> > irqstatus flag. However, irqstatus is always read as 0 in c_can_poll()
> > because all C_CAN interrupts are disabled in c_can_isr(). This causes
> > all interrupts to be re-enabled in c_can_poll() which in turn causes
> > another interrupt since the event is not really handled. This keeps
> > happening causing a flood of interrupts.
>
> Do both c_can and d_can behave the same way?
I do not have c_can hardware to confirm, but from the specification
it looks like it should be applicable to c_can. On d_can, this is
definitely true from my testing.
[...]
> > static int c_can_poll(struct napi_struct *napi, int quota)
> > {
> > - u16 irqstatus;
> > int lec_type = 0;
> > int work_done = 0;
> > struct net_device *dev = napi->dev;
> > struct c_can_priv *priv = netdev_priv(dev);
> >
> > - irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
>
> I suggest:
> irqstatus = priv->irqstatus;
>
> This would be a quite small patch, because you can keep using irqstatus
> below.
Agree, I will change.
Regards
AnilKumar
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-24 11:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-20 9:58 [PATCH 2/3] can: c_can: fix an interrupt thrash issue with c_can driver AnilKumar Ch
2012-04-24 8:01 ` Marc Kleine-Budde
2012-04-24 11:27 ` AnilKumar, Chimata
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.