pull-request: can 2012-06-03

pull-request: can 2012-06-03

[Marc Kleine-Budde]

Hello David,

here is a series of patches intended for v3.5, targeting net/master.

The first three patches are by AnilKumar Ch and fix problems in the c_can
driver. While extending the c_can driver for the Bosch's d_can hardware
he found three problems in the existing driver: first a race condition in
the open() function, second a interrupt thrashing issue and third a
off-by-one when looking for transmitted packets.

The fourth patch is by Joe Perches, it fixes a misuse of | for & in the
cc770 driver.

regards, Marc

---

The following changes since commit 9ca3cc6f3026946ba655e863ca2096339e667639:

  fec_mpc52xx: fix timestamp filtering (2012-06-02 17:09:08 -0400)

are available in the git repository at:
  git@gitorious.org:linux-can/linux-can.git master

AnilKumar Ch (3):
      can: c_can: fix "BUG! echo_skb is occupied!" during transmit
      can: c_can: fix an interrupt thrash issue with c_can driver
      can: c_can: fix race condition in c_can_open()

Joe Perches (1):
      can: cc770: Fix likely misuse of | for &

 drivers/net/can/c_can/c_can.c          |   16 +++++++++-------
 drivers/net/can/c_can/c_can.h          |    1 +
 drivers/net/can/cc770/cc770_platform.c |    2 +-
 3 files changed, 11 insertions(+), 8 deletions(-)

[PATCH 1/4] can: c_can: fix "BUG! echo_skb is occupied!" during transmit

[Marc Kleine-Budde]

From: AnilKumar Ch <anilkumar@ti.com>

This patch fixes an issue with transmit routine, which causes
"can_put_echo_skb: BUG! echo_skb is occupied!" message when
using "cansequence -p" on D_CAN controller.

In c_can driver, while transmitting packets tx_echo flag holds
the no of can frames put for transmission into the hardware.

As the comment above c_can_do_tx() indicates, if we find any packet
which is not transmitted then we should stop looking for more.
In the current implementation this is not taken care of causing the
said message.

Also, fix the condition used to find if the packet is transmitted
or not. Current code skips the first tx message object and ends up
checking one extra invalid object.

While at it, fix the comment on top of c_can_do_tx() to use the
terminology "packet" instead of "package" since it is more
standard.

Cc: stable@kernel.org # 2.6.39+
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 536bda0..9ac28df 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -686,7 +686,7 @@ static int c_can_get_berr_counter(const struct net_device *dev,
  *
  * We iterate from priv->tx_echo to priv->tx_next and check if the
  * packet has been transmitted, echo it back to the CAN framework.
- * If we discover a not yet transmitted package, stop looking for more.
+ * If we discover a not yet transmitted packet, stop looking for more.
  */
 static void c_can_do_tx(struct net_device *dev)
 {
@@ -698,7 +698,7 @@ static void c_can_do_tx(struct net_device *dev)
 	for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv->tx_echo++) {
 		msg_obj_no = get_tx_echo_msg_obj(priv);
 		val = c_can_read_reg32(priv, &priv->regs->txrqst1);
-		if (!(val & (1 << msg_obj_no))) {
+		if (!(val & (1 << (msg_obj_no - 1)))) {
 			can_get_echo_skb(dev,
 					msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
 			stats->tx_bytes += priv->read_reg(priv,
@@ -706,6 +706,8 @@ static void c_can_do_tx(struct net_device *dev)
 					& IF_MCONT_DLC_MASK;
 			stats->tx_packets++;
 			c_can_inval_msg_object(dev, 0, msg_obj_no);
+		} else {
+			break;
 		}
 	}
 
-- 
1.7.4.1

[PATCH 2/4] can: c_can: fix an interrupt thrash issue with c_can driver

[Marc Kleine-Budde]

From: AnilKumar Ch <anilkumar@ti.com>

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.

Cc: stable@kernel.org # 2.6.39+
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c |    7 +++----
 drivers/net/can/c_can/c_can.h |    1 +
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 9ac28df..fa01621 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -952,7 +952,7 @@ static int c_can_poll(struct napi_struct *napi, int quota)
 	struct net_device *dev = napi->dev;
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
+	irqstatus = priv->irqstatus;
 	if (!irqstatus)
 		goto end;
 
@@ -1030,12 +1030,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.4.1

[PATCH 3/4] can: c_can: fix race condition in c_can_open()

[Marc Kleine-Budde]

From: AnilKumar Ch <anilkumar@ti.com>

Fix the issue of C_CAN interrupts getting disabled forever when canconfig
utility is used multiple times. According to NAPI usage we disable all
the hardware interrupts in ISR and re-enable them in poll(). Current
implementation calls napi_enable() after hardware interrupts are enabled.
If we get any interrupts between these two steps then we do not process
those interrupts because napi is not enabled. Mostly these interrupts
come because of STATUS is not 0x7 or ERROR interrupts. If napi_enable()
happens before HW interrupts enabled then c_can_poll() function will be
called eventual re-enabling.

This patch moves the napi_enable() call before interrupts enabled.

Cc: stable@kernel.org # 2.6.39+
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index fa01621..8dc84d6 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -1064,10 +1064,11 @@ static int c_can_open(struct net_device *dev)
 		goto exit_irq_fail;
 	}
 
+	napi_enable(&priv->napi);
+
 	/* start the c_can controller */
 	c_can_start(dev);
 
-	napi_enable(&priv->napi);
 	netif_start_queue(dev);
 
 	return 0;
-- 
1.7.4.1

[PATCH 4/4] can: cc770: Fix likely misuse of | for &

[Marc Kleine-Budde]

From: Joe Perches <joe@perches.com>

Using | with a constant is always true.
Likely this should have be &.

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/cc770/cc770_platform.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c
index 53115ee..688371c 100644
--- a/drivers/net/can/cc770/cc770_platform.c
+++ b/drivers/net/can/cc770/cc770_platform.c
@@ -154,7 +154,7 @@ static int __devinit cc770_get_platform_data(struct platform_device *pdev,
 	struct cc770_platform_data *pdata = pdev->dev.platform_data;
 
 	priv->can.clock.freq = pdata->osc_freq;
-	if (priv->cpu_interface | CPUIF_DSC)
+	if (priv->cpu_interface & CPUIF_DSC)
 		priv->can.clock.freq /= 2;
 	priv->clkout = pdata->cor;
 	priv->bus_config = pdata->bcr;
-- 
1.7.4.1

Re: pull-request: can 2012-06-03

[David Miller]

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon,  4 Jun 2012 00:21:56 +0200

> git@gitorious.org:linux-can/linux-can.git master

I can't pull from that tree.

[davem@bql net]$ git pull git@gitorious.org:linux-can/linux-can.git master
The authenticity of host 'gitorious.org (87.238.52.168)' can't be established.
RSA key fingerprint is 7e:af:8d:ec:f0:39:5e:ba:52:16:ce:19:fa:d4:b8:7d.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added 'gitorious.org,87.238.52.168' (RSA) to the list of known hosts.
Permission denied (publickey).
fatal: The remote end hung up unexpectedly

Re: pull-request: can 2012-06-03

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]

On 06/04/2012 05:44 PM, David Miller wrote:
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Date: Mon,  4 Jun 2012 00:21:56 +0200
> 
>> git@gitorious.org:linux-can/linux-can.git master
> 
> I can't pull from that tree.
> 
> [davem@bql net]$ git pull git@gitorious.org:linux-can/linux-can.git master
> The authenticity of host 'gitorious.org (87.238.52.168)' can't be established.
> RSA key fingerprint is 7e:af:8d:ec:f0:39:5e:ba:52:16:ce:19:fa:d4:b8:7d.
> Are you sure you want to continue connecting (yes/no)? yes
> Warning: Permanently added 'gitorious.org,87.238.52.168' (RSA) to the list of known hosts.
> Permission denied (publickey).
> fatal: The remote end hung up unexpectedly

Doh,

git://gitorious.org/linux-can/linux-can.git master

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 --]

Re: pull-request: can 2012-06-03

[David Miller]

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 04 Jun 2012 17:46:55 +0200

> On 06/04/2012 05:44 PM, David Miller wrote:
>> From: Marc Kleine-Budde <mkl@pengutronix.de>
>> Date: Mon,  4 Jun 2012 00:21:56 +0200
>> 
>>> git@gitorious.org:linux-can/linux-can.git master
>> 
>> I can't pull from that tree.
>> 
>> [davem@bql net]$ git pull git@gitorious.org:linux-can/linux-can.git master
>> The authenticity of host 'gitorious.org (87.238.52.168)' can't be established.
>> RSA key fingerprint is 7e:af:8d:ec:f0:39:5e:ba:52:16:ce:19:fa:d4:b8:7d.
>> Are you sure you want to continue connecting (yes/no)? yes
>> Warning: Permanently added 'gitorious.org,87.238.52.168' (RSA) to the list of known hosts.
>> Permission denied (publickey).
>> fatal: The remote end hung up unexpectedly
> 
> Doh,
> 
> git://gitorious.org/linux-can/linux-can.git master

Pulled, thanks.