Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next-2.6 8/17 v3] can: EG20T PCH: Change Copyright and module description
From: Tomoya MORINAGA @ 2010-11-24 12:19 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Currently, Copyright and module description are not formal.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
>From 7e833aec12ab8199d33cad69a9aa6fea29f32fe5 Mon Sep 17 00:00:00 2001
From: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
Date: Thu, 18 Nov 2010 11:35:45 +0900
Subject: [PATCH] CAN : Change Copyright name Co to CO and module description

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   41 +++++++++++++++++++----------------------
 1 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 8985e16..3a39786 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 1999 - 2010 Intel Corporation.
- * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
+ * Copyright (C) 2010 OKI SEMICONDUCTOR CO., LTD.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -169,19 +169,16 @@ struct pch_can_regs {

 struct pch_can_priv {
 	struct can_priv can;
-	unsigned int can_num;
 	struct pci_dev *dev;
-	int tx_enable[PCH_TX_OBJ_END];
-	int rx_enable[PCH_TX_OBJ_END];
-	int rx_link[PCH_TX_OBJ_END];
-	unsigned int int_enables;
-	unsigned int int_stat;
+	u32 tx_enable[PCH_TX_OBJ_END];
+	u32 rx_enable[PCH_TX_OBJ_END];
+	u32 rx_link[PCH_TX_OBJ_END];
+	u32 int_enables;
 	struct net_device *ndev;
-	unsigned int msg_obj[PCH_TX_OBJ_END];
 	struct pch_can_regs __iomem *regs;
 	struct napi_struct napi;
-	unsigned int tx_obj;	/* Point next Tx Obj index */
-	unsigned int use_msi;
+	int tx_obj;	/* Point next Tx Obj index */
+	int use_msi;
 };

 static struct can_bittiming_const pch_can_bittiming_const = {
@@ -244,9 +241,9 @@ static void pch_can_set_optmode(struct pch_can_priv
*priv)
 	iowrite32(reg_val, &priv->regs->opt);
 }

-static void pch_can_rw_msg_obj(u32 __iomem *creq_addr, u32 num)
+static void pch_can_rw_msg_obj(void __iomem *creq_addr, u32 num)
 {
-	u32 counter = PCH_COUNTER_LIMIT;
+	int counter = PCH_COUNTER_LIMIT;
 	u32 ifx_creq;

 	iowrite32(num, creq_addr);
@@ -334,7 +331,7 @@ static void pch_can_set_tx_all(struct pch_can_priv
*priv, int set)
 		pch_can_set_rxtx(priv, i, set, 1);
 }

-static int pch_can_int_pending(struct pch_can_priv *priv)
+static u32 pch_can_int_pending(struct pch_can_priv *priv)
 {
 	return ioread32(&priv->regs->intr) & 0xffff;
 }
@@ -492,10 +489,10 @@ static void pch_can_int_clr(struct pch_can_priv
*priv, u32 mask)
 	}
 }

-static int pch_can_get_buffer_status(struct pch_can_priv *priv)
+static u32 pch_can_get_buffer_status(struct pch_can_priv *priv)
 {
 	return (ioread32(&priv->regs->treq1) & 0xffff) |
-	       ((ioread32(&priv->regs->treq2) & 0xffff) << 16);
+	       (ioread32(&priv->regs->treq2) << 16);
 }

 static void pch_can_reset(struct pch_can_priv *priv)
@@ -759,7 +756,7 @@ static void pch_can_tx_complete(struct net_device
*ndev, u32 int_stat)
 		netif_wake_queue(ndev);
 }

-static int pch_can_rx_poll(struct napi_struct *napi, int quota)
+static int pch_can_poll(struct napi_struct *napi, int quota)
 {
 	struct net_device *ndev = napi->dev;
 	struct pch_can_priv *priv = netdev_priv(ndev);
@@ -1032,10 +1029,10 @@ static void pch_can_set_int_custom(struct
pch_can_priv *priv)
 }

 /* This function retrieves interrupt enabled for the CAN device. */
-static void pch_can_get_int_enables(struct pch_can_priv *priv, u32
*enables)
+static u32 pch_can_get_int_enables(struct pch_can_priv *priv)
 {
 	/* Obtaining the status of IE, SIE and EIE interrupt bits. */
-	*enables = ((ioread32(&priv->regs->cont) & PCH_CTRL_IE_SIE_EIE) >> 1);
+	return (ioread32(&priv->regs->cont) & PCH_CTRL_IE_SIE_EIE) >> 1;
 }

 static u32 pch_can_get_rxtx_ir(struct pch_can_priv *priv, u32 buff_num,
u32 dir)
@@ -1094,7 +1091,7 @@ static int pch_can_suspend(struct pci_dev *pdev,
pm_message_t state)
 	int i;			/* Counter variable. */
 	int retval;		/* Return value. */
 	u32 buf_stat;	/* Variable for reading the transmit buffer status. */
-	u32 counter = 0xFFFFFF;
+	int counter = PCH_COUNTER_LIMIT;

 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct pch_can_priv *priv = netdev_priv(dev);
@@ -1117,7 +1114,7 @@ static int pch_can_suspend(struct pci_dev *pdev,
pm_message_t state)
 		dev_err(&pdev->dev, "%s -> Transmission time out.\n", __func__);

 	/* Save interrupt configuration and then disable them */
-	pch_can_get_int_enables(priv, &(priv->int_enables));
+	priv->int_enables = pch_can_get_int_enables(priv);
 	pch_can_set_int_enables(priv, PCH_CAN_DISABLE);

 	/* Save Tx buffer enable state */
@@ -1272,7 +1269,7 @@ static int __devinit pch_can_probe(struct pci_dev
*pdev,
 	ndev->netdev_ops = &pch_can_netdev_ops;
 	priv->can.clock.freq = PCH_CAN_CLK; /* Hz */

-	netif_napi_add(ndev, &priv->napi, pch_can_rx_poll, PCH_RX_OBJ_END);
+	netif_napi_add(ndev, &priv->napi, pch_can_poll, PCH_RX_OBJ_END);

 	rc = register_candev(ndev);
 	if (rc) {
@@ -1315,6 +1312,6 @@ static void __exit pch_can_pci_exit(void)
 }
 module_exit(pch_can_pci_exit);

-MODULE_DESCRIPTION("Controller Area Network Driver");
+MODULE_DESCRIPTION("Intel EG20T PCH CAN(Controller Area Network) Driver");
 MODULE_LICENSE("GPL v2");
 MODULE_VERSION("0.94");
-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 9/17 v3] can: EG20T PCH: Use netdev_dbg instead of dev_dbg
From: Tomoya MORINAGA @ 2010-11-24 12:20 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

For easy to readable, use netdev_dbg instead of dev_dbg partly

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 3a39786..e71817d 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -222,7 +222,7 @@ static void pch_can_set_run_mode(struct pch_can_priv
*priv,
 		break;

 	default:
-		dev_err(&priv->ndev->dev, "%s -> Invalid Mode.\n", __func__);
+		netdev_err(priv->ndev, "%s -> Invalid Mode.\n", __func__);
 		break;
 	}
 }
@@ -275,7 +275,7 @@ static void pch_can_set_int_enables(struct
pch_can_priv *priv,
 		break;

 	default:
-		dev_err(&priv->ndev->dev, "Invalid interrupt number.\n");
+		netdev_err(priv->ndev, "Invalid interrupt number.\n");
 		break;
 	}
 }
@@ -535,7 +535,7 @@ static void pch_can_error(struct net_device *ndev,
u32 status)
 			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
 		if ((errc & PCH_TEC) > 96)
 			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
-		dev_warn(&ndev->dev,
+		netdev_dbg(ndev,
 			"%s -> Error Counter is more than 96.\n", __func__);
 	}
 	/* Error passive interrupt. */
@@ -547,7 +547,7 @@ static void pch_can_error(struct net_device *ndev,
u32 status)
 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
 		if ((errc & PCH_TEC) > 127)
 			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
-		dev_err(&ndev->dev,
+		netdev_dbg(ndev,
 			"%s -> CAN controller is ERROR PASSIVE .\n", __func__);
 	}

@@ -877,10 +877,10 @@ static int pch_can_open(struct net_device *ndev)

 	retval = pci_enable_msi(priv->dev);
 	if (retval) {
-		dev_info(&ndev->dev, "PCH CAN opened without MSI\n");
+		netdev_err(ndev, "PCH CAN opened without MSI\n");
 		priv->use_msi = 0;
 	} else {
-		dev_info(&ndev->dev, "PCH CAN opened with MSI\n");
+		netdev_err(ndev, "PCH CAN opened with MSI\n");
 		priv->use_msi = 1;
 	}

@@ -888,14 +888,14 @@ static int pch_can_open(struct net_device *ndev)
 	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
 			     ndev->name, ndev);
 	if (retval) {
-		dev_err(&ndev->dev, "request_irq failed.\n");
+		netdev_err(ndev, "request_irq failed.\n");
 		goto req_irq_err;
 	}

 	/* Open common can device */
 	retval = open_candev(ndev);
 	if (retval) {
-		dev_err(ndev->dev.parent, "open_candev() failed %d\n", retval);
+		netdev_err(ndev, "open_candev() failed %d\n", retval);
 		goto err_open_candev;
 	}

-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 10/17 v3] can: EG20T PCH: Fix coding rule violation
From: Tomoya MORINAGA @ 2010-11-24 12:20 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Fix coding rule violation.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   48
++++++++++++++++++++------------------------
 1 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index e71817d..318eb1f 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -89,9 +89,11 @@

 #define PCH_CAN_CLK		50000000	/* 50MHz */

-/* Define the number of message object.
+/*
+ * Define the number of message object.
  * PCH CAN communications are done via Message RAM.
- * The Message RAM consists of 32 message objects. */
+ * The Message RAM consists of 32 message objects.
+ */
 #define PCH_RX_OBJ_NUM		26
 #define PCH_TX_OBJ_NUM		6
 #define PCH_RX_OBJ_START	1
@@ -126,7 +128,7 @@ enum pch_can_mode {
 	PCH_CAN_ALL,
 	PCH_CAN_NONE,
 	PCH_CAN_STOP,
-	PCH_CAN_RUN
+	PCH_CAN_RUN,
 };

 struct pch_can_if_regs {
@@ -290,21 +292,20 @@ static void pch_can_set_rxtx(struct pch_can_priv
*priv, u32 buff_num,
 	else
 		ie = PCH_IF_MCONT_RXIE;

-	/* Reading the receive buffer data from RAM to Interface1 registers */
+	/* Reading the receive buffer data from RAM to Interface1/2 registers */
 	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
 	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);

-	/* Setting the IF1MASK1 register to access MsgVal and RxIE bits */
+	/* Setting the IF1/2MASK1 register to access MsgVal and RxIE bits */
 	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_ARB | PCH_CMASK_CTRL,
 		  &priv->regs->ifregs[dir].cmask);

 	if (set) {
-		/* Setting the MsgVal and RxIE bits */
+		/* Setting the MsgVal and RxIE/TxIE bits */
 		pch_can_bit_set(&priv->regs->ifregs[dir].mcont, ie);
 		pch_can_bit_set(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
-
 	} else {
-		/* Resetting the MsgVal and RxIE bits */
+		/* Clearing the MsgVal and RxIE/TxIE bits */
 		pch_can_bit_clear(&priv->regs->ifregs[dir].mcont, ie);
 		pch_can_bit_clear(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
 	}
@@ -312,7 +313,6 @@ static void pch_can_set_rxtx(struct pch_can_priv
*priv, u32 buff_num,
 	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
 }

-
 static void pch_can_set_rx_all(struct pch_can_priv *priv, int set)
 {
 	int i;
@@ -328,7 +328,7 @@ static void pch_can_set_tx_all(struct pch_can_priv
*priv, int set)

 	/* Traversing to obtain the object configured as transmit object. */
 	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
-		pch_can_set_rxtx(priv, i, set, 1);
+		pch_can_set_rxtx(priv, i, set, PCH_TX_IFREG);
 }

 static u32 pch_can_int_pending(struct pch_can_priv *priv)
@@ -363,8 +363,7 @@ static void pch_can_config_rx_tx_buffers(struct
pch_can_priv *priv)
 	int i;

 	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++) {
-		iowrite32(PCH_CMASK_RX_TX_GET,
-			&priv->regs->ifregs[0].cmask);
+		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
 		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);

 		iowrite32(0x0, &priv->regs->ifregs[0].id1);
@@ -386,16 +385,14 @@ static void pch_can_config_rx_tx_buffers(struct
pch_can_priv *priv)
 				  0x1fff | PCH_MASK2_MDIR_MXTD);

 		/* Setting CMASK for writing */
-		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
-			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
-			  &priv->regs->ifregs[0].cmask);
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
+			  PCH_CMASK_CTRL, &priv->regs->ifregs[0].cmask);

 		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
 	}

 	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
-		iowrite32(PCH_CMASK_RX_TX_GET,
-			&priv->regs->ifregs[1].cmask);
+		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[1].cmask);
 		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);

 		/* Resetting DIR bit for reception */
@@ -410,9 +407,8 @@ static void pch_can_config_rx_tx_buffers(struct
pch_can_priv *priv)
 		pch_can_bit_clear(&priv->regs->ifregs[1].mask2, 0x1fff);

 		/* Setting CMASK for writing */
-		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
-			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
-			  &priv->regs->ifregs[1].cmask);
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
+			  PCH_CMASK_CTRL, &priv->regs->ifregs[1].cmask);

 		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
 	}
@@ -471,8 +467,9 @@ static void pch_can_int_clr(struct pch_can_priv
*priv, u32 mask)

 		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, mask);
 	} else if ((mask >= PCH_TX_OBJ_START) && (mask <= PCH_TX_OBJ_END)) {
-		/* Setting CMASK for clearing interrupts for
-					 frame transmission. */
+		/*
+		 * Setting CMASK for clearing interrupts for frame transmission.
+		 */
 		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL | PCH_CMASK_ARB,
 			  &priv->regs->ifregs[1].cmask);

@@ -600,7 +597,6 @@ static irqreturn_t pch_can_interrupt(int irq, void
*dev_id)
 	struct pch_can_priv *priv = netdev_priv(ndev);

 	pch_can_set_int_enables(priv, PCH_CAN_NONE);
-
 	napi_schedule(&priv->napi);

 	return IRQ_HANDLED;
@@ -1048,11 +1044,11 @@ static u32 pch_can_get_rxtx_ir(struct
pch_can_priv *priv, u32 buff_num, u32 dir)
 	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);

 	if (((ioread32(&priv->regs->ifregs[dir].id2)) & PCH_ID_MSGVAL) &&
-			((ioread32(&priv->regs->ifregs[dir].mcont)) & ie)) {
+			((ioread32(&priv->regs->ifregs[dir].mcont)) & ie))
 		enable = 1;
-	} else {
+	else
 		enable = 0;
-	}
+
 	return enable;
 }

-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 11/17 v3] can: EG20T PCH: Delete unnecessary/redundant code
From: Tomoya MORINAGA @ 2010-11-24 12:21 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Delete unnecessary/redundant code

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   86
++++++++++++++++++++-------------------------
 1 files changed, 38 insertions(+), 48 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 318eb1f..7342030 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -447,11 +447,6 @@ static void pch_can_release(struct pch_can_priv *priv)
 /* This function clears interrupt(s) from the CAN device. */
 static void pch_can_int_clr(struct pch_can_priv *priv, u32 mask)
 {
-	if (mask == PCH_STATUS_INT) {
-		ioread32(&priv->regs->stat);
-		return;
-	}
-
 	/* Clear interrupt for transmit object */
 	if ((mask >= PCH_RX_OBJ_START) && (mask <= PCH_RX_OBJ_END)) {
 		/* Setting CMASK for clearing the reception interrupts. */
@@ -518,8 +513,6 @@ static void pch_can_error(struct net_device *ndev,
u32 status)
 		state = CAN_STATE_BUS_OFF;
 		cf->can_id |= CAN_ERR_BUSOFF;
 		can_bus_off(ndev);
-		pch_can_set_run_mode(priv, PCH_CAN_RUN);
-		dev_err(&ndev->dev, "%s -> Bus Off occurres.\n", __func__);
 	}

 	errc = ioread32(&priv->regs->errc);
@@ -658,7 +651,6 @@ static int pch_can_rx_normal(struct net_device
*ndev, u32 obj_num, int quota)
 	canid_t id;
 	int rcv_pkts = 0;
 	int rtn;
-	int next_flag = 0;
 	struct sk_buff *skb;
 	struct can_frame *cf;
 	struct pch_can_priv *priv = netdev_priv(ndev);
@@ -685,50 +677,48 @@ static int pch_can_rx_normal(struct net_device
*ndev, u32 obj_num, int quota)
 				return rtn;
 			rcv_pkts++;
 			quota--;
-			next_flag = 1;
-		} else if (!(reg & PCH_IF_MCONT_NEWDAT))
-			next_flag = 1;
-
-		if (!next_flag) {
-			skb = alloc_can_skb(priv->ndev, &cf);
-			if (!skb)
-				return -ENOMEM;
-
-			/* Get Received data */
-			id2 = ioread32(&priv->regs->ifregs[0].id2);
-			if (id2 & PCH_ID2_XTD) {
-				id = (ioread32(&priv->regs->ifregs[0].id1) &
-					       0xffff);
-				id |= (((id2) & 0x1fff) << 16);
-				cf->can_id = id | CAN_EFF_FLAG;
-			} else {
-				id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
-				cf->can_id = id;
-			}
-
-			if (id2 & PCH_ID2_DIR)
-				cf->can_id |= CAN_RTR_FLAG;
+			obj_num++;
+			continue;
+		} else if (!(reg & PCH_IF_MCONT_NEWDAT)) {
+			obj_num++;
+			continue;
+		}

-			cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
-						   ifregs[0].mcont)) & 0xF);
+		skb = alloc_can_skb(priv->ndev, &cf);
+		if (!skb)
+			return -ENOMEM;
+
+		/* Get Received data */
+		id2 = ioread32(&priv->regs->ifregs[0].id2);
+		if (id2 & PCH_ID2_XTD) {
+			id = (ioread32(&priv->regs->ifregs[0].id1) & 0xffff);
+			id |= (((id2) & 0x1fff) << 16);
+			cf->can_id = id | CAN_EFF_FLAG;
+		} else {
+			id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
+			cf->can_id = id;
+		}

-			for (i = 0; i < cf->can_dlc; i += 2) {
-				data_reg = ioread16(&priv->regs->ifregs[0].
-						    data[i / 2]);
-				cf->data[i] = data_reg & 0xff;
-				cf->data[i + 1] = (data_reg >> 8) & 0xff;
-			}
+		if (id2 & PCH_ID2_DIR)
+			cf->can_id |= CAN_RTR_FLAG;

-			netif_receive_skb(skb);
-			rcv_pkts++;
-			stats->rx_packets++;
-			quota--;
-			stats->rx_bytes += cf->can_dlc;
+		cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
+						    ifregs[0].mcont)) & 0xF);

-			pch_fifo_thresh(priv, obj_num);
+		for (i = 0; i < cf->can_dlc; i += 2) {
+			data_reg = ioread16(&priv->regs->ifregs[0].data[i / 2]);
+			cf->data[i] = data_reg & 0xff;
+			cf->data[i + 1] = data_reg >> 8;
 		}
+
+		netif_receive_skb(skb);
+		rcv_pkts++;
+		stats->rx_packets++;
+		quota--;
+		stats->rx_bytes += cf->can_dlc;
+
+		pch_fifo_thresh(priv, obj_num);
 		obj_num++;
-		next_flag = 0;
 	} while (quota > 0);

 	return rcv_pkts;
@@ -764,7 +754,7 @@ static int pch_can_poll(struct napi_struct *napi,
int quota)
 	if (!int_stat)
 		goto end;

-	if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
+	if (int_stat == PCH_STATUS_INT) {
 		reg_stat = ioread32(&priv->regs->stat);
 		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
 			if (reg_stat & PCH_BUS_OFF ||
@@ -932,7 +922,7 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb,
struct net_device *ndev)
 {
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	struct can_frame *cf = (struct can_frame *)skb->data;
-	int tx_obj_no = 0;
+	int tx_obj_no;
 	int i;
 	u32 id2;

-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 12/17 v3] can: EG20T PCH: Fix bit timing calculation issue
From: Tomoya MORINAGA @ 2010-11-24 12:22 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Fix bit timing calculation issue
Modify like use calculated value directly passed by CAN core module.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 7342030..466f011 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -800,17 +800,15 @@ static int pch_set_bittiming(struct net_device *ndev)
 	const struct can_bittiming *bt = &priv->can.bittiming;
 	u32 canbit;
 	u32 bepe;
-	u32 brp;

 	/* Setting the CCE bit for accessing the Can Timing register. */
 	pch_can_bit_set(&priv->regs->cont, PCH_CTRL_CCE);

-	brp = (bt->tq) / (1000000000/PCH_CAN_CLK) - 1;
-	canbit = brp & PCH_MSK_BITT_BRP;
+	canbit = (bt->brp - 1) & PCH_MSK_BITT_BRP;
 	canbit |= (bt->sjw - 1) << PCH_BIT_SJW_SHIFT;
 	canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << PCH_BIT_TSEG1_SHIFT;
 	canbit |= (bt->phase_seg2 - 1) << PCH_BIT_TSEG2_SHIFT;
-	bepe = (brp & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
+	bepe = ((bt->brp - 1) & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
 	iowrite32(canbit, &priv->regs->bitt);
 	iowrite32(bepe, &priv->regs->brpe);
 	pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_CCE);
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 13/17 v3] can: EG20T PCH: Fix miss-setting status issue
From: Tomoya MORINAGA @ 2010-11-24 12:22 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Modify miss-setting status issue at suspend.


Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 466f011..610cf3b 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -1084,7 +1084,7 @@ static int pch_can_suspend(struct pci_dev *pdev,
pm_message_t state)
 	pch_can_set_run_mode(priv, PCH_CAN_STOP);

 	/* Indicate that we are aboutto/in suspend */
-	priv->can.state = CAN_STATE_SLEEPING;
+	priv->can.state = CAN_STATE_STOPPED;

 	/* Waiting for all transmission to complete. */
 	while (counter) {
-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 14/17 v3] can: EG20T PCH: Comment optimization
From: Tomoya MORINAGA @ 2010-11-24 12:23 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Comment optimization

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 610cf3b..df14569 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -292,7 +292,7 @@ static void pch_can_set_rxtx(struct pch_can_priv
*priv, u32 buff_num,
 	else
 		ie = PCH_IF_MCONT_RXIE;

-	/* Reading the receive buffer data from RAM to Interface1/2 registers */
+	/* Reading the Msg buffer from Message RAM to IF1/2 registers. */
 	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
 	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);

@@ -868,7 +868,7 @@ static int pch_can_open(struct net_device *ndev)
 		priv->use_msi = 1;
 	}

-	/* Regsitering the interrupt. */
+	/* Regstering the interrupt. */
 	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
 			     ndev->name, ndev);
 	if (retval) {
@@ -972,7 +972,7 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb,
struct net_device *ndev)

 	can_put_echo_skb(skb, ndev, tx_obj_no - PCH_RX_OBJ_END - 1);

-	/* Updating the size of the data. */
+	/* Set the size of the data. Update if2_mcont */
 	iowrite32(cf->can_dlc | PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_TXRQXT |
 		  PCH_IF_MCONT_TXIE, &priv->regs->ifregs[1].mcont);

@@ -1072,8 +1072,8 @@ static u32 pch_can_get_rx_buffer_link(struct
pch_can_priv *priv, u32 buffer_num)

 static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 {
-	int i;			/* Counter variable. */
-	int retval;		/* Return value. */
+	int i;
+	int retval;
 	u32 buf_stat;	/* Variable for reading the transmit buffer status. */
 	int counter = PCH_COUNTER_LIMIT;

@@ -1130,8 +1130,8 @@ static int pch_can_suspend(struct pci_dev *pdev,
pm_message_t state)

 static int pch_can_resume(struct pci_dev *pdev)
 {
-	int i;			/* Counter variable. */
-	int retval;		/* Return variable. */
+	int i;
+	int retval;
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct pch_can_priv *priv = netdev_priv(dev);

-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 15/17 v3] can: EG20T PCH: Move MSI processing open/close to probe/remove
From: Tomoya MORINAGA @ 2010-11-24 12:24 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Move MSI processing to probe/remove processing.
Currently, in case this driver is integrated as module, and when this
module is re-installed, no interrupts is to be occurred.
For the above issue, move MSI processing to open/release processing.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   29 ++++++++++++++---------------
 1 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index df14569..c612a99 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -859,15 +859,6 @@ static int pch_can_open(struct net_device *ndev)
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	int retval;

-	retval = pci_enable_msi(priv->dev);
-	if (retval) {
-		netdev_err(ndev, "PCH CAN opened without MSI\n");
-		priv->use_msi = 0;
-	} else {
-		netdev_err(ndev, "PCH CAN opened with MSI\n");
-		priv->use_msi = 1;
-	}
-
 	/* Regstering the interrupt. */
 	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
 			     ndev->name, ndev);
@@ -893,9 +884,6 @@ static int pch_can_open(struct net_device *ndev)
 err_open_candev:
 	free_irq(priv->dev->irq, ndev);
 req_irq_err:
-	if (priv->use_msi)
-		pci_disable_msi(priv->dev);
-
 	pch_can_release(priv);

 	return retval;
@@ -909,8 +897,6 @@ static int pch_close(struct net_device *ndev)
 	napi_disable(&priv->napi);
 	pch_can_release(priv);
 	free_irq(priv->dev->irq, ndev);
-	if (priv->use_msi)
-		pci_disable_msi(priv->dev);
 	close_candev(ndev);
 	priv->can.state = CAN_STATE_STOPPED;
 	return 0;
@@ -993,12 +979,14 @@ static void __devexit pch_can_remove(struct
pci_dev *pdev)
 	struct pch_can_priv *priv = netdev_priv(ndev);

 	unregister_candev(priv->ndev);
-	free_candev(priv->ndev);
 	pci_iounmap(pdev, priv->regs);
+	if (priv->use_msi)
+		pci_disable_msi(priv->dev);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 	pch_can_reset(priv);
+	free_candev(priv->ndev);
 }

 #ifdef CONFIG_PM
@@ -1255,6 +1243,15 @@ static int __devinit pch_can_probe(struct pci_dev
*pdev,

 	netif_napi_add(ndev, &priv->napi, pch_can_poll, PCH_RX_OBJ_END);

+	rc = pci_enable_msi(priv->dev);
+	if (rc) {
+		netdev_err(ndev, "PCH CAN opened without MSI\n");
+		priv->use_msi = 0;
+	} else {
+		netdev_err(ndev, "PCH CAN opened with MSI\n");
+		priv->use_msi = 1;
+	}
+
 	rc = register_candev(ndev);
 	if (rc) {
 		dev_err(&pdev->dev, "Failed register_candev %d\n", rc);
@@ -1264,6 +1261,8 @@ static int __devinit pch_can_probe(struct pci_dev
*pdev,
 	return 0;

 probe_exit_reg_candev:
+	if (priv->use_msi)
+		pci_disable_msi(priv->dev);
 	free_candev(ndev);
 probe_exit_alloc_candev:
 	pci_iounmap(pdev, addr);
-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 16/17 v3] can: EG20T PCH: Fix incorrect return processing
From: Tomoya MORINAGA @ 2010-11-24 12:24 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Fix incorrect return processing

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index c612a99..48f4a2e 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -589,10 +589,12 @@ static irqreturn_t pch_can_interrupt(int irq, void
*dev_id)
 	struct net_device *ndev = (struct net_device *)dev_id;
 	struct pch_can_priv *priv = netdev_priv(ndev);

-	pch_can_set_int_enables(priv, PCH_CAN_NONE);
-	napi_schedule(&priv->napi);
-
-	return IRQ_HANDLED;
+	if ((pch_can_int_pending(priv) > 0) && (dev_id != NULL)) {
+		pch_can_set_int_enables(priv, PCH_CAN_NONE);
+		napi_schedule(&priv->napi);
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
 }

 static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
@@ -674,7 +676,7 @@ static int pch_can_rx_normal(struct net_device
*ndev, u32 obj_num, int quota)
 		if (reg & PCH_IF_MCONT_MSGLOST) {
 			rtn = pch_can_rx_msg_lost(ndev, obj_num);
 			if (!rtn)
-				return rtn;
+				return rcv_pkts;
 			rcv_pkts++;
 			quota--;
 			obj_num++;
@@ -777,10 +779,12 @@ static int pch_can_poll(struct napi_struct *napi,
int quota)
 		goto end;

 	if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
-		rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
-		quota -= rcv_pkts;
-		if (quota < 0)
+		rcv_pkts = pch_can_rx_normal(ndev, int_stat, quota);
+		if (rcv_pkts < 0) {
+			rcv_pkts = 0;
 			goto end;
+		}
+		quota -= rcv_pkts;
 	} else if ((int_stat >= PCH_TX_OBJ_START) &&
 		   (int_stat <= PCH_TX_OBJ_END)) {
 		/* Handle transmission interrupt */
-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 17/17 v3] can: EG20T PCH: Optimize "if" condition in rx/tx processing
From: Tomoya MORINAGA @ 2010-11-24 12:25 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

For reduce "if" condition, easy to read/understand the code,
optimize "if" condition in rx/tx processing.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   26 ++++++++++----------------
 1 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 48f4a2e..0ecb0d5 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -758,19 +758,16 @@ static int pch_can_poll(struct napi_struct *napi,
int quota)

 	if (int_stat == PCH_STATUS_INT) {
 		reg_stat = ioread32(&priv->regs->stat);
-		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
-			if (reg_stat & PCH_BUS_OFF ||
-			   (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
-				pch_can_error(ndev, reg_stat);
-				quota--;
-			}
-		}

-		if (reg_stat & PCH_TX_OK)
-			pch_can_bit_clear(&priv->regs->stat, PCH_TX_OK);
+		if ((reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) &&
+		   ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)) {
+			pch_can_error(ndev, reg_stat);
+			quota--;
+		}

-		if (reg_stat & PCH_RX_OK)
-			pch_can_bit_clear(&priv->regs->stat, PCH_RX_OK);
+		if (reg_stat & (PCH_TX_OK | PCH_RX_OK))
+			pch_can_bit_clear(&priv->regs->stat,
+					  reg_stat & (PCH_TX_OK | PCH_RX_OK));

 		int_stat = pch_can_int_pending(priv);
 	}
@@ -917,14 +914,13 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb,
struct net_device *ndev)
 	if (can_dropped_invalid_skb(ndev, skb))
 		return NETDEV_TX_OK;

+	tx_obj_no = priv->tx_obj;
 	if (priv->tx_obj == PCH_TX_OBJ_END) {
 		if (ioread32(&priv->regs->treq2) & PCH_TREQ2_TX_MASK)
 			netif_stop_queue(ndev);

-		tx_obj_no = priv->tx_obj;
 		priv->tx_obj = PCH_TX_OBJ_START;
 	} else {
-		tx_obj_no = priv->tx_obj;
 		priv->tx_obj++;
 	}

@@ -947,9 +943,7 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb,
struct net_device *ndev)
 	id2 |= PCH_ID_MSGVAL;

 	/* If remote frame has to be transmitted.. */
-	if (cf->can_id & CAN_RTR_FLAG)
-		id2 &= ~PCH_ID2_DIR;
-	else
+	if (!(cf->can_id & CAN_RTR_FLAG))
 		id2 |= PCH_ID2_DIR;

 	iowrite32(id2, &priv->regs->ifregs[1].id2);
-- 
1.6.0.6


^ permalink raw reply related

* Re: [PATCH net-next-2.6 8/17 v2] can: EG20T PCH: Change Copyright and module description
From: Tomoya MORINAGA @ 2010-11-24 12:32 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CECEE1E.9030201@pengutronix.de>

On Wednesday, November 24, 2010 7:51 PM, Marc Kleine-Budde wrote:
> Did your mailer spawn some extra spaces here?
Sorry for in convenience.
Once saving patch-mail temporary, it seems sometimes mail format is broken.
I can see the phonemenon.

I sent v3 patch series without temporary savings.

---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply

* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Marc Kleine-Budde @ 2010-11-24 12:34 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, qi.wang-ral2JQCrhuEAvxtiuMwx3w,
	David S. Miller, Christian Pellegrin, Wolfgang Grandegger
In-Reply-To: <000b01cb8b6b$d7542a90$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1795 bytes --]

On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>> I can't understand your saying.
>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>> busy-waiting _before_ accessing the if[1].
>>>
>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>> wait only function before accessing the if[1] in the TX path.
> 
> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
> complete yet.
> Thus, I think, the "busy waiting" is necessary.

Yes, it's necessary, but not where it is done currently.
Let me outline how I think the TX path should look like:

pch_xmit() {
	take_care_about_flow_control();
	prepare_can_frame_to_be_copied_to_tx_if();

	/* most likely we don't have to wait here */
	wait_until_tx_if_is_ready();

	copy_can_frame_to_tx_if();

	/* trigger tx in hardware */
	send_tx_if_but_dont_do_busywait();

	/* tx_if is busy now, but before we access it, we'll check tx_if is ready */
}

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* [PATCH 1/4] stmmac: tidy-up stmmac_priv structure
From: Peppe CAVALLARO @ 2010-11-24 12:37 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO

This patch tidies-up the stmmac_priv structure
that had many fileds alredy defined in the
plat_stmmacenet_data structure.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac.h         |   15 ++------
 drivers/net/stmmac/stmmac_ethtool.c |    4 +-
 drivers/net/stmmac/stmmac_main.c    |   69 ++++++++++++++++++-----------------
 drivers/net/stmmac/stmmac_mdio.c    |    8 ++--
 4 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 79bdc2e..3157567 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -37,7 +37,6 @@ struct stmmac_priv {
 	unsigned int cur_tx;
 	unsigned int dirty_tx;
 	unsigned int dma_tx_size;
-	int tx_coe;
 	int tx_coalesce;
 
 	struct dma_desc *dma_rx ;
@@ -48,7 +47,6 @@ struct stmmac_priv {
 	struct sk_buff_head rx_recycle;
 
 	struct net_device *dev;
-	int is_gmac;
 	dma_addr_t dma_rx_phy;
 	unsigned int dma_rx_size;
 	unsigned int dma_buf_sz;
@@ -60,14 +58,11 @@ struct stmmac_priv {
 	struct napi_struct napi;
 
 	phy_interface_t phy_interface;
-	int pbl;
-	int bus_id;
 	int phy_addr;
 	int phy_mask;
 	int (*phy_reset) (void *priv);
-	void (*fix_mac_speed) (void *priv, unsigned int speed);
-	void (*bus_setup)(void __iomem *ioaddr);
-	void *bsp_priv;
+	int rx_coe;
+	int no_csum_insertion;
 
 	int phy_irq;
 	struct phy_device *phydev;
@@ -77,7 +72,6 @@ struct stmmac_priv {
 	unsigned int flow_ctrl;
 	unsigned int pause;
 	struct mii_bus *mii;
-	int mii_clk_csr;
 
 	u32 msg_enable;
 	spinlock_t lock;
@@ -90,10 +84,7 @@ struct stmmac_priv {
 #ifdef STMMAC_VLAN_TAG_USED
 	struct vlan_group *vlgrp;
 #endif
-	int enh_desc;
-	int rx_coe;
-	int bugged_jumbo;
-	int no_csum_insertion;
+	struct plat_stmmacenet_data *plat;
 };
 
 #ifdef CONFIG_STM_DRIVERS
diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index 6d65482..f2695fd 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -94,7 +94,7 @@ static void stmmac_ethtool_getdrvinfo(struct net_device *dev,
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
 
-	if (!priv->is_gmac)
+	if (!priv->plat->has_gmac)
 		strcpy(info->driver, MAC100_ETHTOOL_NAME);
 	else
 		strcpy(info->driver, GMAC_ETHTOOL_NAME);
@@ -176,7 +176,7 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
 
 	memset(reg_space, 0x0, REG_SPACE_SIZE);
 
-	if (!priv->is_gmac) {
+	if (!priv->plat->has_gmac) {
 		/* MAC registers */
 		for (i = 0; i < 12; i++)
 			reg_space[i] = readl(priv->ioaddr + (i * 4));
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 06bc603..29ba286 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -186,6 +186,18 @@ static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 	return priv->dirty_tx + priv->dma_tx_size - priv->cur_tx - 1;
 }
 
+/* On some ST platforms, some HW system configuraton registers have to be
+ * set according to the link speed negotiated.
+ */
+static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
+{
+	struct phy_device *phydev = priv->phydev;
+
+	if (likely(priv->plat->fix_mac_speed))
+		priv->plat->fix_mac_speed(priv->plat->bsp_priv,
+					  phydev->speed);
+}
+
 /**
  * stmmac_adjust_link
  * @dev: net device structure
@@ -228,15 +240,13 @@ static void stmmac_adjust_link(struct net_device *dev)
 			new_state = 1;
 			switch (phydev->speed) {
 			case 1000:
-				if (likely(priv->is_gmac))
+				if (likely(priv->plat->has_gmac))
 					ctrl &= ~priv->hw->link.port;
-				if (likely(priv->fix_mac_speed))
-					priv->fix_mac_speed(priv->bsp_priv,
-							    phydev->speed);
+				stmmac_hw_fix_mac_speed(priv);
 				break;
 			case 100:
 			case 10:
-				if (priv->is_gmac) {
+				if (priv->plat->has_gmac) {
 					ctrl |= priv->hw->link.port;
 					if (phydev->speed == SPEED_100) {
 						ctrl |= priv->hw->link.speed;
@@ -246,9 +256,7 @@ static void stmmac_adjust_link(struct net_device *dev)
 				} else {
 					ctrl &= ~priv->hw->link.port;
 				}
-				if (likely(priv->fix_mac_speed))
-					priv->fix_mac_speed(priv->bsp_priv,
-							    phydev->speed);
+				stmmac_hw_fix_mac_speed(priv);
 				break;
 			default:
 				if (netif_msg_link(priv))
@@ -305,7 +313,7 @@ static int stmmac_init_phy(struct net_device *dev)
 		return 0;
 	}
 
-	snprintf(bus_id, MII_BUS_ID_SIZE, "%x", priv->bus_id);
+	snprintf(bus_id, MII_BUS_ID_SIZE, "%x", priv->plat->bus_id);
 	snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 		 priv->phy_addr);
 	pr_debug("stmmac_init_phy:  trying to attach to %s\n", phy_id);
@@ -552,7 +560,7 @@ static void free_dma_desc_resources(struct stmmac_priv *priv)
  */
 static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
 {
-	if (likely((priv->tx_coe) && (!priv->no_csum_insertion))) {
+	if (likely((priv->plat->tx_coe) && (!priv->no_csum_insertion))) {
 		/* In case of GMAC, SF mode has to be enabled
 		 * to perform the TX COE. This depends on:
 		 * 1) TX COE if actually supported
@@ -814,7 +822,7 @@ static int stmmac_open(struct net_device *dev)
 	init_dma_desc_rings(dev);
 
 	/* DMA initialization and SW reset */
-	if (unlikely(priv->hw->dma->init(priv->ioaddr, priv->pbl,
+	if (unlikely(priv->hw->dma->init(priv->ioaddr, priv->plat->pbl,
 					 priv->dma_tx_phy,
 					 priv->dma_rx_phy) < 0)) {
 
@@ -825,15 +833,15 @@ static int stmmac_open(struct net_device *dev)
 	/* Copy the MAC addr into the HW  */
 	priv->hw->mac->set_umac_addr(priv->ioaddr, dev->dev_addr, 0);
 	/* If required, perform hw setup of the bus. */
-	if (priv->bus_setup)
-		priv->bus_setup(priv->ioaddr);
+	if (priv->plat->bus_setup)
+		priv->plat->bus_setup(priv->ioaddr);
 	/* Initialize the MAC Core */
 	priv->hw->mac->core_init(priv->ioaddr);
 
 	priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr);
 	if (priv->rx_coe)
 		pr_info("stmmac: Rx Checksum Offload Engine supported\n");
-	if (priv->tx_coe)
+	if (priv->plat->tx_coe)
 		pr_info("\tTX Checksum insertion supported\n");
 
 	priv->shutdown = 0;
@@ -1042,7 +1050,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 		return stmmac_sw_tso(priv, skb);
 
 	if (likely((skb->ip_summed == CHECKSUM_PARTIAL))) {
-		if (unlikely((!priv->tx_coe) || (priv->no_csum_insertion)))
+		if (unlikely((!priv->plat->tx_coe) ||
+			     (priv->no_csum_insertion)))
 			skb_checksum_help(skb);
 		else
 			csum_insertion = 1;
@@ -1146,7 +1155,7 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv)
 					   DMA_FROM_DEVICE);
 
 			(p + entry)->des2 = priv->rx_skbuff_dma[entry];
-			if (unlikely(priv->is_gmac)) {
+			if (unlikely(priv->plat->has_gmac)) {
 				if (bfsize >= BUF_SIZE_8KiB)
 					(p + entry)->des3 =
 					    (p + entry)->des2 + BUF_SIZE_8KiB;
@@ -1356,7 +1365,7 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
 		return -EBUSY;
 	}
 
-	if (priv->is_gmac)
+	if (priv->plat->has_gmac)
 		max_mtu = JUMBO_LEN;
 	else
 		max_mtu = ETH_DATA_LEN;
@@ -1370,7 +1379,7 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
 	 * needs to have the Tx COE disabled for oversized frames
 	 * (due to limited buffer sizes). In this case we disable
 	 * the TX csum insertionin the TDES and not use SF. */
-	if ((priv->bugged_jumbo) && (priv->dev->mtu > ETH_DATA_LEN))
+	if ((priv->plat->bugged_jumbo) && (priv->dev->mtu > ETH_DATA_LEN))
 		priv->no_csum_insertion = 1;
 	else
 		priv->no_csum_insertion = 0;
@@ -1390,7 +1399,7 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	if (priv->is_gmac)
+	if (priv->plat->has_gmac)
 		/* To handle GMAC own interrupts */
 		priv->hw->mac->host_irq_status((void __iomem *) dev->base_addr);
 
@@ -1536,7 +1545,7 @@ static int stmmac_mac_device_setup(struct net_device *dev)
 
 	struct mac_device_info *device;
 
-	if (priv->is_gmac)
+	if (priv->plat->has_gmac)
 		device = dwmac1000_setup(priv->ioaddr);
 	else
 		device = dwmac100_setup(priv->ioaddr);
@@ -1544,7 +1553,7 @@ static int stmmac_mac_device_setup(struct net_device *dev)
 	if (!device)
 		return -ENOMEM;
 
-	if (priv->enh_desc) {
+	if (priv->plat->enh_desc) {
 		device->desc = &enh_desc_ops;
 		pr_info("\tEnhanced descriptor structure\n");
 	} else
@@ -1598,7 +1607,7 @@ static int stmmac_associate_phy(struct device *dev, void *data)
 		plat_dat->bus_id);
 
 	/* Check that this phy is for the MAC being initialised */
-	if (priv->bus_id != plat_dat->bus_id)
+	if (priv->plat->bus_id != plat_dat->bus_id)
 		return 0;
 
 	/* OK, this PHY is connected to the MAC.
@@ -1683,13 +1692,9 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
 	priv->device = &(pdev->dev);
 	priv->dev = ndev;
 	plat_dat = pdev->dev.platform_data;
-	priv->bus_id = plat_dat->bus_id;
-	priv->pbl = plat_dat->pbl;	/* TLI */
-	priv->mii_clk_csr = plat_dat->clk_csr;
-	priv->tx_coe = plat_dat->tx_coe;
-	priv->bugged_jumbo = plat_dat->bugged_jumbo;
-	priv->is_gmac = plat_dat->has_gmac;	/* GMAC is on board */
-	priv->enh_desc = plat_dat->enh_desc;
+
+	priv->plat = plat_dat;
+
 	priv->ioaddr = addr;
 
 	/* PMT module is not integrated in all the MAC devices. */
@@ -1727,16 +1732,12 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	priv->fix_mac_speed = plat_dat->fix_mac_speed;
-	priv->bus_setup = plat_dat->bus_setup;
-	priv->bsp_priv = plat_dat->bsp_priv;
-
 	pr_info("\t%s - (dev. name: %s - id: %d, IRQ #%d\n"
 	       "\tIO base addr: 0x%p)\n", ndev->name, pdev->name,
 	       pdev->id, ndev->irq, addr);
 
 	/* MDIO bus Registration */
-	pr_debug("\tMDIO bus (id: %d)...", priv->bus_id);
+	pr_debug("\tMDIO bus (id: %d)...", priv->plat->bus_id);
 	ret = stmmac_mdio_register(ndev);
 	if (ret < 0)
 		goto out;
diff --git a/drivers/net/stmmac/stmmac_mdio.c b/drivers/net/stmmac/stmmac_mdio.c
index d744161..234b406 100644
--- a/drivers/net/stmmac/stmmac_mdio.c
+++ b/drivers/net/stmmac/stmmac_mdio.c
@@ -53,7 +53,7 @@ static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
 	int data;
 	u16 regValue = (((phyaddr << 11) & (0x0000F800)) |
 			((phyreg << 6) & (0x000007C0)));
-	regValue |= MII_BUSY | ((priv->mii_clk_csr & 7) << 2);
+	regValue |= MII_BUSY | ((priv->plat->clk_csr & 7) << 2);
 
 	do {} while (((readl(priv->ioaddr + mii_address)) & MII_BUSY) == 1);
 	writel(regValue, priv->ioaddr + mii_address);
@@ -85,7 +85,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 	    (((phyaddr << 11) & (0x0000F800)) | ((phyreg << 6) & (0x000007C0)))
 	    | MII_WRITE;
 
-	value |= MII_BUSY | ((priv->mii_clk_csr & 7) << 2);
+	value |= MII_BUSY | ((priv->plat->clk_csr & 7) << 2);
 
 
 	/* Wait until any existing MII operation is complete */
@@ -114,7 +114,7 @@ static int stmmac_mdio_reset(struct mii_bus *bus)
 
 	if (priv->phy_reset) {
 		pr_debug("stmmac_mdio_reset: calling phy_reset\n");
-		priv->phy_reset(priv->bsp_priv);
+		priv->phy_reset(priv->plat->bsp_priv);
 	}
 
 	/* This is a workaround for problems with the STE101P PHY.
@@ -157,7 +157,7 @@ int stmmac_mdio_register(struct net_device *ndev)
 	new_bus->read = &stmmac_mdio_read;
 	new_bus->write = &stmmac_mdio_write;
 	new_bus->reset = &stmmac_mdio_reset;
-	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%x", priv->bus_id);
+	snprintf(new_bus->id, MII_BUS_ID_SIZE, "%x", priv->plat->bus_id);
 	new_bus->priv = ndev;
 	new_bus->irq = irqlist;
 	new_bus->phy_mask = priv->phy_mask;
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH 2/4] stmmac: add init/exit callback in plat_stmmacenet_data struct
From: Peppe CAVALLARO @ 2010-11-24 12:38 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO
In-Reply-To: <1290561646-9429-1-git-send-email-peppe.cavallaro@st.com>

This patch adds in the plat_stmmacenet_data
the init and exit callbacks that can be used
for invoking specific platform functions.
For example, on ST targets, these call the
PAD manager functions to set PIO lines and
syscfg registers.
The patch removes the stmmac_claim_resource
only used on STM Kernels as well.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac.h      |   22 ----------------------
 drivers/net/stmmac/stmmac_main.c |   18 +++++++++++++-----
 include/linux/stmmac.h           |    6 +++---
 3 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 3157567..8ae7650 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -87,28 +87,6 @@ struct stmmac_priv {
 	struct plat_stmmacenet_data *plat;
 };
 
-#ifdef CONFIG_STM_DRIVERS
-#include <linux/stm/pad.h>
-static inline int stmmac_claim_resource(struct platform_device *pdev)
-{
-	int ret = 0;
-	struct plat_stmmacenet_data *plat_dat = pdev->dev.platform_data;
-
-	/* Pad routing setup */
-	if (IS_ERR(devm_stm_pad_claim(&pdev->dev, plat_dat->pad_config,
-			dev_name(&pdev->dev)))) {
-		printk(KERN_ERR "%s: Failed to request pads!\n", __func__);
-		ret = -ENODEV;
-	}
-	return ret;
-}
-#else
-static inline int stmmac_claim_resource(struct platform_device *pdev)
-{
-	return 0;
-}
-#endif
-
 extern int stmmac_mdio_unregister(struct net_device *ndev);
 extern int stmmac_mdio_register(struct net_device *ndev);
 extern void stmmac_set_ethtool_ops(struct net_device *netdev);
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 29ba286..b806cd3 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1643,7 +1643,7 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *addr = NULL;
 	struct net_device *ndev = NULL;
-	struct stmmac_priv *priv;
+	struct stmmac_priv *priv = NULL;
 	struct plat_stmmacenet_data *plat_dat;
 
 	pr_info("STMMAC driver:\n\tplatform registration... ");
@@ -1708,10 +1708,12 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
 	/* Set the I/O base addr */
 	ndev->base_addr = (unsigned long)addr;
 
-	/* Verify embedded resource for the platform */
-	ret = stmmac_claim_resource(pdev);
-	if (ret < 0)
-		goto out;
+	/* Custom initialisation */
+	if (priv->plat->init) {
+		ret = priv->plat->init(pdev);
+		if (unlikely(ret))
+			goto out;
+	}
 
 	/* MAC HW revice detection */
 	ret = stmmac_mac_device_setup(ndev);
@@ -1745,6 +1747,9 @@ static int stmmac_dvr_probe(struct platform_device *pdev)
 
 out:
 	if (ret < 0) {
+		if (priv->plat->exit)
+			priv->plat->exit(pdev);
+
 		platform_set_drvdata(pdev, NULL);
 		release_mem_region(res->start, resource_size(res));
 		if (addr != NULL)
@@ -1778,6 +1783,9 @@ static int stmmac_dvr_remove(struct platform_device *pdev)
 
 	stmmac_mdio_unregister(ndev);
 
+	if (priv->plat->exit)
+		priv->plat->exit(pdev);
+
 	platform_set_drvdata(pdev, NULL);
 	unregister_netdev(ndev);
 
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index d66c617..e103529 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -40,9 +40,9 @@ struct plat_stmmacenet_data {
 	int pmt;
 	void (*fix_mac_speed)(void *priv, unsigned int speed);
 	void (*bus_setup)(void __iomem *ioaddr);
-#ifdef CONFIG_STM_DRIVERS
-	struct stm_pad_config *pad_config;
-#endif
+	int (*init)(struct platform_device *pdev);
+	void (*exit)(struct platform_device *pdev);
+	void *custom_cfg;
 	void *bsp_priv;
 };
 
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH 3/4] stmmac: convert to dev_pm_ops.
From: Peppe CAVALLARO @ 2010-11-24 12:38 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO
In-Reply-To: <1290561646-9429-2-git-send-email-peppe.cavallaro@st.com>

This patch updates the PM support using the dev_pm_ops
and reviews the hibernation support.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac.h      |    1 -
 drivers/net/stmmac/stmmac_main.c |  124 ++++++++++++++++++++++----------------
 2 files changed, 71 insertions(+), 54 deletions(-)

diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 8ae7650..eb258e2 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -77,7 +77,6 @@ struct stmmac_priv {
 	spinlock_t lock;
 	int wolopts;
 	int wolenabled;
-	int shutdown;
 #ifdef CONFIG_STMMAC_TIMER
 	struct stmmac_timer *tm;
 #endif
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index b806cd3..f1dbc18 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -844,8 +844,6 @@ static int stmmac_open(struct net_device *dev)
 	if (priv->plat->tx_coe)
 		pr_info("\tTX Checksum insertion supported\n");
 
-	priv->shutdown = 0;
-
 	/* Initialise the MMC (if present) to disable all interrupts. */
 	writel(0xffffffff, priv->ioaddr + MMC_HIGH_INTR_MASK);
 	writel(0xffffffff, priv->ioaddr + MMC_LOW_INTR_MASK);
@@ -1799,61 +1797,53 @@ static int stmmac_dvr_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int stmmac_suspend(struct platform_device *pdev, pm_message_t state)
+static int stmmac_suspend(struct device *dev)
 {
-	struct net_device *dev = platform_get_drvdata(pdev);
-	struct stmmac_priv *priv = netdev_priv(dev);
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
 	int dis_ic = 0;
 
-	if (!dev || !netif_running(dev))
+	if (!ndev || !netif_running(ndev))
 		return 0;
 
 	spin_lock(&priv->lock);
 
-	if (state.event == PM_EVENT_SUSPEND) {
-		netif_device_detach(dev);
-		netif_stop_queue(dev);
-		if (priv->phydev)
-			phy_stop(priv->phydev);
+	netif_device_detach(ndev);
+	netif_stop_queue(ndev);
+	if (priv->phydev)
+		phy_stop(priv->phydev);
 
 #ifdef CONFIG_STMMAC_TIMER
-		priv->tm->timer_stop();
-		if (likely(priv->tm->enable))
-			dis_ic = 1;
+	priv->tm->timer_stop();
+	if (likely(priv->tm->enable))
+		dis_ic = 1;
 #endif
-		napi_disable(&priv->napi);
-
-		/* Stop TX/RX DMA */
-		priv->hw->dma->stop_tx(priv->ioaddr);
-		priv->hw->dma->stop_rx(priv->ioaddr);
-		/* Clear the Rx/Tx descriptors */
-		priv->hw->desc->init_rx_desc(priv->dma_rx, priv->dma_rx_size,
-					     dis_ic);
-		priv->hw->desc->init_tx_desc(priv->dma_tx, priv->dma_tx_size);
-
-		/* Enable Power down mode by programming the PMT regs */
-		if (device_can_wakeup(priv->device))
-			priv->hw->mac->pmt(priv->ioaddr, priv->wolopts);
-		else
-			stmmac_disable_mac(priv->ioaddr);
-	} else {
-		priv->shutdown = 1;
-		/* Although this can appear slightly redundant it actually
-		 * makes fast the standby operation and guarantees the driver
-		 * working if hibernation is on media. */
-		stmmac_release(dev);
-	}
+	napi_disable(&priv->napi);
+
+	/* Stop TX/RX DMA */
+	priv->hw->dma->stop_tx(priv->ioaddr);
+	priv->hw->dma->stop_rx(priv->ioaddr);
+	/* Clear the Rx/Tx descriptors */
+	priv->hw->desc->init_rx_desc(priv->dma_rx, priv->dma_rx_size,
+				     dis_ic);
+	priv->hw->desc->init_tx_desc(priv->dma_tx, priv->dma_tx_size);
+
+	/* Enable Power down mode by programming the PMT regs */
+	if (device_may_wakeup(priv->device))
+		priv->hw->mac->pmt(priv->ioaddr, priv->wolopts);
+	else
+		stmmac_disable_mac(priv->ioaddr);
 
 	spin_unlock(&priv->lock);
 	return 0;
 }
 
-static int stmmac_resume(struct platform_device *pdev)
+static int stmmac_resume(struct device *dev)
 {
-	struct net_device *dev = platform_get_drvdata(pdev);
-	struct stmmac_priv *priv = netdev_priv(dev);
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
 
-	if (!netif_running(dev))
+	if (!netif_running(ndev))
 		return 0;
 
 	if (priv->shutdown) {
@@ -1870,10 +1860,10 @@ static int stmmac_resume(struct platform_device *pdev)
 	 * is received. Anyway, it's better to manually clear
 	 * this bit because it can generate problems while resuming
 	 * from another devices (e.g. serial console). */
-	if (device_can_wakeup(priv->device))
+	if (device_may_wakeup(priv->device))
 		priv->hw->mac->pmt(priv->ioaddr, 0);
 
-	netif_device_attach(dev);
+	netif_device_attach(ndev);
 
 	/* Enable the MAC and DMA */
 	stmmac_enable_mac(priv->ioaddr);
@@ -1881,31 +1871,59 @@ static int stmmac_resume(struct platform_device *pdev)
 	priv->hw->dma->start_rx(priv->ioaddr);
 
 #ifdef CONFIG_STMMAC_TIMER
-	priv->tm->timer_start(tmrate);
+	if (likely(priv->tm->enable))
+		priv->tm->timer_start(tmrate);
 #endif
 	napi_enable(&priv->napi);
 
 	if (priv->phydev)
 		phy_start(priv->phydev);
 
-	netif_start_queue(dev);
+	netif_start_queue(ndev);
 
 	spin_unlock(&priv->lock);
 	return 0;
 }
-#endif
 
-static struct platform_driver stmmac_driver = {
-	.driver = {
-		   .name = STMMAC_RESOURCE_NAME,
-		   },
-	.probe = stmmac_dvr_probe,
-	.remove = stmmac_dvr_remove,
-#ifdef CONFIG_PM
+static int stmmac_freeze(struct device *dev)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+
+	if (!ndev || !netif_running(ndev))
+		return 0;
+
+	return stmmac_release(ndev);
+}
+
+static int stmmac_restore(struct device *dev)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+
+	if (!ndev || !netif_running(ndev))
+		return 0;
+
+	return stmmac_open(ndev);
+}
+
+static const struct dev_pm_ops stmmac_pm_ops = {
 	.suspend = stmmac_suspend,
 	.resume = stmmac_resume,
-#endif
+	.freeze = stmmac_freeze,
+	.thaw = stmmac_restore,
+	.restore = stmmac_restore,
+};
+#else
+static const struct dev_pm_ops stmmac_pm_ops;
+#endif /* CONFIG_PM */
 
+static struct platform_driver stmmac_driver = {
+	.probe = stmmac_dvr_probe,
+	.remove = stmmac_dvr_remove,
+	.driver = {
+		.name = STMMAC_RESOURCE_NAME,
+		.owner = THIS_MODULE,
+		.pm = &stmmac_pm_ops,
+	},
 };
 
 /**
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH 4/4] stmmac: update the driver version
From: Peppe CAVALLARO @ 2010-11-24 12:38 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Peppe CAVALLARO
In-Reply-To: <1290561646-9429-3-git-send-email-peppe.cavallaro@st.com>

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index eb258e2..5f06c47 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -20,7 +20,7 @@
   Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
 *******************************************************************************/
 
-#define DRV_MODULE_VERSION	"Apr_2010"
+#define DRV_MODULE_VERSION	"Nov_2010"
 #include <linux/platform_device.h>
 #include <linux/stmmac.h>
 
-- 
1.5.5.6

^ permalink raw reply related

* Re: [PATCH net-2.6] net, ppp: Report correct error code if unit allocation failed
From: Jarek Poplawski @ 2010-11-24 12:49 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: LNML, Paul Mackerras, David Miller, linux-ppp, linux-kernel
In-Reply-To: <20101123214344.GB1839@lenovo>

On 2010-11-23 22:43, Cyrill Gorcunov wrote:
> Allocating unit from ird might return several error codes
> not only -EAGAIN, so it should not be changed and returned
> precisely. Same time unit release procedure should be invoked
> only if device is unregistering.

IMHO this unit release fix should be in a separate patch.

...
> @@ -2668,10 +2668,10 @@ static void ppp_shutdown_interface(struc
>  		ppp->closing = 1;
>  		ppp_unlock(ppp);
>  		unregister_netdev(ppp->dev);
> +		unit_put(&pn->units_idr, ppp->file.index);
>  	} else
>  		ppp_unlock(ppp);
>  
> -	unit_put(&pn->units_idr, ppp->file.index);
>  	ppp->file.dead = 1;
>  	ppp->owner = NULL;
>  	wake_up_interruptible(&ppp->file.rwait);

Btw, it seems these last 3 lines could be moved similarly.

Jarek P.

^ permalink raw reply

* Re: [PATCH net-next-2.6 12/17 v3] can: EG20T PCH: Fix bit timing calculation issue
From: Marc Kleine-Budde @ 2010-11-24 12:58 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED037F.6060306-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1912 bytes --]

On 11/24/2010 01:22 PM, Tomoya MORINAGA wrote:
> Fix bit timing calculation issue
> Modify like use calculated value directly passed by CAN core module.
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>

Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

> ---
>  drivers/net/can/pch_can.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 7342030..466f011 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -800,17 +800,15 @@ static int pch_set_bittiming(struct net_device *ndev)
>  	const struct can_bittiming *bt = &priv->can.bittiming;
>  	u32 canbit;
>  	u32 bepe;
> -	u32 brp;
> 
>  	/* Setting the CCE bit for accessing the Can Timing register. */
>  	pch_can_bit_set(&priv->regs->cont, PCH_CTRL_CCE);
> 
> -	brp = (bt->tq) / (1000000000/PCH_CAN_CLK) - 1;
> -	canbit = brp & PCH_MSK_BITT_BRP;
> +	canbit = (bt->brp - 1) & PCH_MSK_BITT_BRP;

Masking here shouldn't be necessary but won't hurt, better play safe.

>  	canbit |= (bt->sjw - 1) << PCH_BIT_SJW_SHIFT;
>  	canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << PCH_BIT_TSEG1_SHIFT;
>  	canbit |= (bt->phase_seg2 - 1) << PCH_BIT_TSEG2_SHIFT;
> -	bepe = (brp & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
> +	bepe = ((bt->brp - 1) & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;

dito

>  	iowrite32(canbit, &priv->regs->bitt);
>  	iowrite32(bepe, &priv->regs->brpe);
>  	pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_CCE);

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH net-next-2.6 5/17 v3] can: EG20T PCH: Delete unnecessary spin_lock
From: Marc Kleine-Budde @ 2010-11-24 13:01 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED0200.1030103-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 707 bytes --]

On 11/24/2010 01:16 PM, Tomoya MORINAGA wrote:
> Delete unnecessary spin_lock for accessing Message Object.
> Since all message objects are divided into tx/rx area completely,
> spin_lock processing is unnecessary.
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>

Looks good, however there're some wrapped lines in the patch. Add my
Acked-by in the fixed patch.

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH net-next-2.6 3/17 v3] can: EG20T PCH: Enumerate some macros
From: Marc Kleine-Budde @ 2010-11-24 13:07 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED00A9.50006-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 5184 bytes --]

On 11/24/2010 01:10 PM, Tomoya MORINAGA wrote:
> For easy to readable, LEC and interface register number macros are
> replaced to enums.

Your patch description is a bit misleading. You're not only replacing
defines by enums, you also add rx and tx error counters. Please change
the patch description or split into two patches. (see comments inline)

Wolfgang, can you say something to the generation of the error frames.

cheers, Marc

> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
>  drivers/net/can/pch_can.c |   90
> +++++++++++++++++++++++++--------------------
>  1 files changed, 50 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index b028ae4..5c21f5c 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -69,21 +69,12 @@
>  #define PCH_REC			0x00007f00
>  #define PCH_TEC			0x000000ff
> 
> +
>  #define PCH_TX_OK		BIT(3)
>  #define PCH_RX_OK		BIT(4)
>  #define PCH_EPASSIV		BIT(5)
>  #define PCH_EWARN		BIT(6)
>  #define PCH_BUS_OFF		BIT(7)
> -#define PCH_LEC0		BIT(0)
> -#define PCH_LEC1		BIT(1)
> -#define PCH_LEC2		BIT(2)
> -#define PCH_LEC_ALL		(PCH_LEC0 | PCH_LEC1 | PCH_LEC2)
> -#define PCH_STUF_ERR		PCH_LEC0
> -#define PCH_FORM_ERR		PCH_LEC1
> -#define PCH_ACK_ERR		(PCH_LEC0 | PCH_LEC1)
> -#define PCH_BIT1_ERR		PCH_LEC2
> -#define PCH_BIT0_ERR		(PCH_LEC0 | PCH_LEC2)
> -#define PCH_CRC_ERR		(PCH_LEC1 | PCH_LEC2)
> 
>  /* bit position of certain controller bits. */
>  #define PCH_BIT_BRP		0
> @@ -96,9 +87,6 @@
>  #define PCH_MSK_CTRL_IE_SIE_EIE	0x07
>  #define PCH_COUNTER_LIMIT	10
> 
> -#define PCH_RX_IFREG			0
> -#define PCH_TX_IFREG			1
> -
>  #define PCH_CAN_CLK		50000000	/* 50MHz */
> 
>  /* Define the number of message object.
> @@ -113,6 +101,21 @@
> 
>  #define PCH_FIFO_THRESH		16
> 
> +enum pch_ifreg {
> +	PCH_RX_IFREG,
> +	PCH_TX_IFREG,
> +};

please fold the "enum pch_ifreg" related changes into patch 1.

> +
> +enum pch_can_err {
> +	PCH_STUF_ERR = 1,
> +	PCH_FORM_ERR,
> +	PCH_ACK_ERR,
> +	PCH_BIT1_ERR,
> +	PCH_BIT0_ERR,
> +	PCH_CRC_ERR,
> +	PCH_LEC_ALL,
> +};
> +
>  enum pch_can_mode {
>  	PCH_CAN_ENABLE,
>  	PCH_CAN_DISABLE,
> @@ -302,7 +305,7 @@ static void pch_can_check_if_busy(u32 __iomem
> *creq_addr, u32 num)
>  }
> 
>  static void pch_can_set_rxtx(struct pch_can_priv *priv, u32 buff_num,
> -				  int set, u32 dir)
> +			     int set, enum pch_ifreg dir)

this should go into patch 1.

>  {
>  	unsigned long flags;
>  	u32 ie;
> @@ -616,7 +619,7 @@ static void pch_can_error(struct net_device *ndev,
> u32 status)
>  	struct sk_buff *skb;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  	struct can_frame *cf;
> -	u32 errc;
> +	u32 errc, lec;
>  	struct net_device_stats *stats = &(priv->ndev->stats);
>  	enum can_state state = priv->can.state;
> 
> @@ -661,35 +664,42 @@ static void pch_can_error(struct net_device *ndev,
> u32 status)
>  			"%s -> CAN controller is ERROR PASSIVE .\n", __func__);
>  	}
> 
> -	if (status & PCH_LEC_ALL) {
> +	lec = status & PCH_LEC_ALL;
> +	switch (lec) {
> +	case PCH_STUF_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_STUFF;
>  		priv->can.can_stats.bus_error++;
>  		stats->rx_errors++;
> -		switch (status & PCH_LEC_ALL) {
> -		case PCH_STUF_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_STUFF;
> -			break;
> -		case PCH_FORM_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_FORM;
> -			break;
> -		case PCH_ACK_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
> -				       CAN_ERR_PROT_LOC_ACK_DEL;
> -			break;
> -		case PCH_BIT1_ERR:
> -		case PCH_BIT0_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_BIT;
> -			break;
> -		case PCH_CRC_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> -				       CAN_ERR_PROT_LOC_CRC_DEL;
> -			break;
> -		default:
> -			iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
> -			break;
> -		}
> -
> +		break;
> +	case PCH_FORM_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_FORM;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_ACK_ERR:
> +		cf->can_id |= CAN_ERR_ACK;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_BIT1_ERR:
> +	case PCH_BIT0_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_BIT;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_CRC_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> +			       CAN_ERR_PROT_LOC_CRC_DEL;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_LEC_ALL: /* Written by CPU. No error status */
> +		break;
>  	}
> 
> +	cf->data[6] = errc & PCH_TEC;
> +	cf->data[7] = (errc & PCH_REC) >> 8;
> +

You patch description doesn't say anything about this

>  	priv->can.state = state;
>  	netif_rx(skb);
> 

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue
From: Marc Kleine-Budde @ 2010-11-24 13:31 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED023E.1070009-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 16311 bytes --]

On 11/24/2010 01:17 PM, Tomoya MORINAGA wrote:
> Fix endianness issue.
> there is endianness issue both Tx and Rx.
> Currently, data is set like below.
> Register:
>  MSB--LSB
>  x x D0 D1
>  x x D2 D3
>  x x D4 D5
>  x x D6 D7
> 
> But Data to be sent must be set like below.
> Register:
>  MSB--LSB
>  x x D1 D0
>  x x D3 D2
>  x x D5 D4
>  x x D7 D6  (x means reserved area.)

The endianess handling looks good now! (the & 0xff can be removed,
though, see linline)
> 
> 
> Modify netif_rx() to netif_receive_skb().

Please give a reason, I suppose because the function is used from NAPI.
(Frankly speaking this has nothing to do with endianess issues).

> For easy to read, some sub-functions are created.
> 
> 
> Modify complex "goto" to do~while.

What about the next_flag in the rx_normal function? Can you get rid of
it, too?

> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
> From 468cf576aadc0ce26496eaf13fbac4bb2670a9de Mon Sep 17 00:00:00 2001
> From: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> Date: Thu, 18 Nov 2010 10:23:32 +0900
> Subject: [PATCH] CAN : Improve RxTx processing
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
>  drivers/net/can/pch_can.c |  306
> +++++++++++++++++++++++----------------------
>  1 files changed, 156 insertions(+), 150 deletions(-)
> 
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 843bbce..fa47707 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -133,10 +133,7 @@ struct pch_can_if_regs {
>  	u32 id1;
>  	u32 id2;
>  	u32 mcont;
> -	u32 dataa1;
> -	u32 dataa2;
> -	u32 datab1;
> -	u32 datab2;
> +	u32 data[4];
>  	u32 rsv[13];
>  };
> 
> @@ -421,10 +418,10 @@ static void pch_can_clear_buffers(struct
> pch_can_priv *priv)
>  		iowrite32(0x0, &priv->regs->ifregs[0].id1);
>  		iowrite32(0x0, &priv->regs->ifregs[0].id2);
>  		iowrite32(0x0, &priv->regs->ifregs[0].mcont);
> -		iowrite32(0x0, &priv->regs->ifregs[0].dataa1);
> -		iowrite32(0x0, &priv->regs->ifregs[0].dataa2);
> -		iowrite32(0x0, &priv->regs->ifregs[0].datab1);
> -		iowrite32(0x0, &priv->regs->ifregs[0].datab2);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[0]);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[1]);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[2]);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[3]);
>  		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
>  			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
>  			  &priv->regs->ifregs[0].cmask);
> @@ -438,10 +435,10 @@ static void pch_can_clear_buffers(struct
> pch_can_priv *priv)
>  		iowrite32(0x0, &priv->regs->ifregs[1].id1);
>  		iowrite32(0x0, &priv->regs->ifregs[1].id2);
>  		iowrite32(0x0, &priv->regs->ifregs[1].mcont);
> -		iowrite32(0x0, &priv->regs->ifregs[1].dataa1);
> -		iowrite32(0x0, &priv->regs->ifregs[1].dataa2);
> -		iowrite32(0x0, &priv->regs->ifregs[1].datab1);
> -		iowrite32(0x0, &priv->regs->ifregs[1].datab2);
> +		iowrite32(0x0, &priv->regs->ifregs[1].data[0]);
> +		iowrite32(0x0, &priv->regs->ifregs[1].data[1]);
> +		iowrite32(0x0, &priv->regs->ifregs[1].data[2]);
> +		iowrite32(0x0, &priv->regs->ifregs[1].data[3]);
>  		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
>  			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
>  			  &priv->regs->ifregs[1].cmask);
> @@ -683,7 +680,7 @@ static void pch_can_error(struct net_device *ndev,
> u32 status)
>  	cf->data[7] = (errc & PCH_REC) >> 8;
> 
>  	priv->can.state = state;
> -	netif_rx(skb);
> +	netif_receive_skb(skb);

This is an unrelated fix. Please split into separate patch.

> 
>  	stats->rx_packets++;
>  	stats->rx_bytes += cf->can_dlc;
> @@ -701,190 +698,202 @@ static irqreturn_t pch_can_interrupt(int irq,
> void *dev_id)
>  	return IRQ_HANDLED;
>  }
> 
> -static int pch_can_rx_normal(struct net_device *ndev, u32 int_stat)
> +static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
> +{
> +	if (obj_id < PCH_FIFO_THRESH) {
> +		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL |
> +			  PCH_CMASK_ARB, &priv->regs->ifregs[0].cmask);
> +
> +		/* Clearing the Dir bit. */
> +		pch_can_bit_clear(&priv->regs->ifregs[0].id2, PCH_ID2_DIR);
> +
> +		/* Clearing NewDat & IntPnd */
> +		pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> +				  PCH_IF_MCONT_INTPND);
> +		pch_can_check_if_busy(&priv->regs->ifregs[0].creq, obj_id);
> +	} else if (obj_id > PCH_FIFO_THRESH) {
> +		pch_can_int_clr(priv, obj_id);
> +	} else if (obj_id == PCH_FIFO_THRESH) {
> +		int cnt;
> +		for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
> +			pch_can_int_clr(priv, cnt+1);
                                              ^^^^^

nitpick: there should be spaces around the "+".

> +	}
> +}
> +
> +static int pch_can_rx_msg_lost(struct net_device *ndev, int obj_id)
> +{
> +	struct pch_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &(priv->ndev->stats);
> +	struct sk_buff *skb;
> +	struct can_frame *cf;
> +
> +	netdev_dbg(priv->ndev, "Msg Obj is overwritten.\n");
> +	pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> +			  PCH_IF_MCONT_MSGLOST);
> +	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
> +		  &priv->regs->ifregs[0].cmask);
> +	pch_can_check_if_busy(&priv->regs->ifregs[0].creq, obj_id);
> +
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	cf->can_id |= CAN_ERR_CRTL;
> +	cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +	stats->rx_over_errors++;
> +	stats->rx_errors++;
> +
> +	netif_receive_skb(skb);
> +
> +	return 0;
> +}
> +
> +static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int
> quota)
>  {
>  	u32 reg;
>  	canid_t id;
> -	u32 ide;
> -	u32 rtr;
> -	int i, j, k;
>  	int rcv_pkts = 0;
> +	int rtn;
> +	int next_flag = 0;
>  	struct sk_buff *skb;
>  	struct can_frame *cf;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  	struct net_device_stats *stats = &(priv->ndev->stats);
> +	int i;
> +	u32 id2;
> +	u16 data_reg;
> 
> -	/* Reading the messsage object from the Message RAM */
> -	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> -	pch_can_check_if_busy(&priv->regs->ifregs[0].creq, int_stat);
> +	do {
> +		/* Reading the messsage object from the Message RAM */
> +		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> +		pch_can_check_if_busy(&priv->regs->ifregs[0].creq, obj_num);
> 
> -	/* Reading the MCONT register. */
> -	reg = ioread32(&priv->regs->ifregs[0].mcont);
> -	reg &= 0xffff;
> +		/* Reading the MCONT register. */
> +		reg = ioread32(&priv->regs->ifregs[0].mcont);
> +
> +		if (reg & PCH_IF_MCONT_EOB)
> +			break;
> 
> -	for (k = int_stat; !(reg & PCH_IF_MCONT_EOB); k++) {
>  		/* If MsgLost bit set. */
>  		if (reg & PCH_IF_MCONT_MSGLOST) {
> -			dev_err(&priv->ndev->dev, "Msg Obj is overwritten.\n");
> -			pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> -					  PCH_IF_MCONT_MSGLOST);
> -			iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
> -				  &priv->regs->ifregs[0].cmask);
> -			pch_can_check_if_busy(&priv->regs->ifregs[0].creq, k);
> -
> -			skb = alloc_can_err_skb(ndev, &cf);
> +			rtn = pch_can_rx_msg_lost(ndev, obj_num);
> +			if (!rtn)
> +				return rtn;

The function return "0" on success or -ENOMEM. The logic seems inverted.
IMHO just keep going if you had an OOM situation.

> +			rcv_pkts++;
> +			quota--;
> +			next_flag = 1;
> +		} else if (!(reg & PCH_IF_MCONT_NEWDAT))
> +			next_flag = 1;

Please get rid of the next_flag, rearrange your code and use "continue".

> +		if (!next_flag) {
> +			skb = alloc_can_skb(priv->ndev, &cf);
>  			if (!skb)
>  				return -ENOMEM;
> 
> -			priv->can.can_stats.error_passive++;
> -			priv->can.state = CAN_STATE_ERROR_PASSIVE;
> -			cf->can_id |= CAN_ERR_CRTL;
> -			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> -			cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> -			stats->rx_packets++;
> -			stats->rx_bytes += cf->can_dlc;
> +			/* Get Received data */
> +			id2 = ioread32(&priv->regs->ifregs[0].id2);
> +			if (id2 & PCH_ID2_XTD) {
> +				id = (ioread32(&priv->regs->ifregs[0].id1) &
> +					       0xffff);
> +				id |= (((id2) & 0x1fff) << 16);
> +				cf->can_id = id | CAN_EFF_FLAG;
> +			} else {
> +				id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
> +				cf->can_id = id;
> +			}
> +
> +			if (id2 & PCH_ID2_DIR)
> +				cf->can_id |= CAN_RTR_FLAG;
> +
> +			cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
> +						   ifregs[0].mcont)) & 0xF);
> +
> +			for (i = 0; i < cf->can_dlc; i += 2) {
> +				data_reg = ioread16(&priv->regs->ifregs[0].
> +						    data[i / 2]);
> +				cf->data[i] = data_reg & 0xff;
> +				cf->data[i + 1] = (data_reg >> 8) & 0xff;

the & 0xff are not needed, cf->data[] is 8bit only.

> +			}
> 
>  			netif_receive_skb(skb);
>  			rcv_pkts++;
> -			goto RX_NEXT;
> -		}
> -		if (!(reg & PCH_IF_MCONT_NEWDAT))
> -			goto RX_NEXT;
> -
> -		skb = alloc_can_skb(priv->ndev, &cf);
> -		if (!skb)
> -			return -ENOMEM;
> -
> -		/* Get Received data */
> -		ide = ((ioread32(&priv->regs->ifregs[0].id2)) & PCH_ID2_XTD) >>
> -									     14;
> -		if (ide) {
> -			id = (ioread32(&priv->regs->ifregs[0].id1) & 0xffff);
> -			id |= (((ioread32(&priv->regs->ifregs[0].id2)) &
> -					    0x1fff) << 16);
> -			cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
> -		} else {
> -			id = (((ioread32(&priv->regs->ifregs[0].id2)) &
> -						     (CAN_SFF_MASK << 2)) >> 2);
> -			cf->can_id = (id & CAN_SFF_MASK);
> -		}
> +			stats->rx_packets++;
> +			quota--;
> +			stats->rx_bytes += cf->can_dlc;
> 
> -		rtr = (ioread32(&priv->regs->ifregs[0].id2) &  PCH_ID2_DIR);
> -		if (rtr) {
> -			cf->can_dlc = 0;
> -			cf->can_id |= CAN_RTR_FLAG;
> -		} else {
> -			cf->can_dlc =
> -			      ((ioread32(&priv->regs->ifregs[0].mcont)) & 0x0f);
> +			pch_fifo_thresh(priv, obj_num);
>  		}
> +		obj_num++;
> +		next_flag = 0;
> +	} while (quota > 0);
> 
> -		for (i = 0, j = 0; i < cf->can_dlc; j++) {
> -			reg = ioread32(&priv->regs->ifregs[0].dataa1 + j*4);
> -			cf->data[i++] = cpu_to_le32(reg & 0xff);
> -			if (i == cf->can_dlc)
> -				break;
> -			cf->data[i++] = cpu_to_le32((reg >> 8) & 0xff);
> -		}
> +	return rcv_pkts;
> +}
> 
> -		netif_receive_skb(skb);
> -		rcv_pkts++;
> -		stats->rx_packets++;
> -		stats->rx_bytes += cf->can_dlc;
> -
> -		if (k < PCH_FIFO_THRESH) {
> -			iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL |
> -				  PCH_CMASK_ARB, &priv->regs->ifregs[0].cmask);
> -
> -			/* Clearing the Dir bit. */
> -			pch_can_bit_clear(&priv->regs->ifregs[0].id2,
> -					  PCH_ID2_DIR);
> -
> -			/* Clearing NewDat & IntPnd */
> -			pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> -					  PCH_IF_MCONT_INTPND);
> -			pch_can_check_if_busy(&priv->regs->ifregs[0].creq, k);
> -		} else if (k > PCH_FIFO_THRESH) {
> -			pch_can_int_clr(priv, k);
> -		} else if (k == PCH_FIFO_THRESH) {
> -			int cnt;
> -			for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
> -				pch_can_int_clr(priv, cnt+1);
> -		}
> -RX_NEXT:
> -		/* Reading the messsage object from the Message RAM */
> -		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> -		pch_can_check_if_busy(&priv->regs->ifregs[0].creq, k);
> -		reg = ioread32(&priv->regs->ifregs[0].mcont);
> -	}
> +static void pch_can_tx_complete(struct net_device *ndev, u32 int_stat)
> +{
> +	struct pch_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &(priv->ndev->stats);
> +	u32 dlc;
> 
> -	return rcv_pkts;
> +	can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_END - 1);
> +	iowrite32(PCH_CMASK_RX_TX_GET | PCH_CMASK_CLRINTPND,
> +		  &priv->regs->ifregs[1].cmask);
> +	dlc = get_can_dlc(ioread32(&priv->regs->ifregs[1].mcont) &
> +			  PCH_IF_MCONT_DLC);
> +	pch_can_check_if_busy(&priv->regs->ifregs[1].creq, int_stat);

This is the TX-Complete interrupt handler, right? What does the
pch_can_check_if_busy trigger here?

> +	stats->tx_bytes += dlc;
> +	stats->tx_packets++;
> +	if (int_stat == PCH_TX_OBJ_END)
> +		netif_wake_queue(ndev);
>  }
> +
>  static int pch_can_rx_poll(struct napi_struct *napi, int quota)
>  {
>  	struct net_device *ndev = napi->dev;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
> -	struct net_device_stats *stats = &(priv->ndev->stats);
> -	u32 dlc;
>  	u32 int_stat;
>  	int rcv_pkts = 0;
>  	u32 reg_stat;
> 
>  	int_stat = pch_can_int_pending(priv);
>  	if (!int_stat)
> -		return 0;
> +		goto end;
> 
> -INT_STAT:
> -	if (int_stat == PCH_STATUS_INT) {
> +	if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
>  		reg_stat = ioread32(&priv->regs->stat);
>  		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
> -			if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)
> +			if (reg_stat & PCH_BUS_OFF ||
> +			   (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
>  				pch_can_error(ndev, reg_stat);
> +				quota--;
> +			}
>  		}
> 
> -		if (reg_stat & PCH_TX_OK) {
> -			iowrite32(PCH_CMASK_RX_TX_GET,
> -				  &priv->regs->ifregs[1].cmask);
> -			pch_can_check_if_busy(&priv->regs->ifregs[1].creq,
> -					       ioread32(&priv->regs->intr));
> +		if (reg_stat & PCH_TX_OK)
>  			pch_can_bit_clear(&priv->regs->stat, PCH_TX_OK);
> -		}
> 
>  		if (reg_stat & PCH_RX_OK)
>  			pch_can_bit_clear(&priv->regs->stat, PCH_RX_OK);
> 
>  		int_stat = pch_can_int_pending(priv);
> -		if (int_stat == PCH_STATUS_INT)
> -			goto INT_STAT;
>  	}
> 
> -MSG_OBJ:
> +	if (quota == 0)
> +		goto end;
> +
>  	if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
> -		rcv_pkts = pch_can_rx_normal(ndev, int_stat);
> -		if (rcv_pkts < 0)
> -			return 0;
> +		rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
> +		quota -= rcv_pkts;
> +		if (quota < 0)
> +			goto end;
>  	} else if ((int_stat >= PCH_TX_OBJ_START) &&
>  		   (int_stat <= PCH_TX_OBJ_END)) {
>  		/* Handle transmission interrupt */
> -		can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_END - 1);
> -		iowrite32(PCH_CMASK_RX_TX_GET | PCH_CMASK_CLRINTPND,
> -			  &priv->regs->ifregs[1].cmask);
> -		dlc = ioread32(&priv->regs->ifregs[1].mcont) &
> -			       PCH_IF_MCONT_DLC;
> -		pch_can_check_if_busy(&priv->regs->ifregs[1].creq, int_stat);
> -		if (dlc > 8)
> -			dlc = 8;
> -		stats->tx_bytes += dlc;
> -		stats->tx_packets++;
> -		if (int_stat == PCH_TX_OBJ_END)
> -			netif_wake_queue(ndev);
> +		pch_can_tx_complete(ndev, int_stat);
>  	}
> 
> -	int_stat = pch_can_int_pending(priv);
> -	if (int_stat == PCH_STATUS_INT)
> -		goto INT_STAT;
> -	else if (int_stat >= 1 && int_stat <= 32)
> -		goto MSG_OBJ;
> -
> +end:
>  	napi_complete(napi);
>  	pch_can_set_int_enables(priv, PCH_CAN_ALL);
> 
> @@ -1017,10 +1026,10 @@ static int pch_close(struct net_device *ndev)
> 
>  static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
>  {
> -	int i, j;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  	struct can_frame *cf = (struct can_frame *)skb->data;
>  	int tx_obj_no = 0;
> +	int i;
> 
>  	if (can_dropped_invalid_skb(ndev, skb))
>  		return NETDEV_TX_OK;
> @@ -1061,13 +1070,10 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb,
> struct net_device *ndev)
>  	if (cf->can_id & CAN_RTR_FLAG)
>  		pch_can_bit_clear(&priv->regs->ifregs[1].id2, PCH_ID2_DIR);
> 
> -	for (i = 0, j = 0; i < cf->can_dlc; j++) {
> -		iowrite32(le32_to_cpu(cf->data[i++]),
> -			 (&priv->regs->ifregs[1].dataa1) + j*4);
> -		if (i == cf->can_dlc)
> -			break;
> -		iowrite32(le32_to_cpu(cf->data[i++] << 8),
> -			 (&priv->regs->ifregs[1].dataa1) + j*4);
> +	/* Copy data to register */
> +	for (i = 0; i < cf->can_dlc; i += 2) {
> +		iowrite16(cf->data[i] | (cf->data[i + 1] << 8),
> +			  &priv->regs->ifregs[1].data[i / 2]);
>  	}
> 
>  	can_put_echo_skb(skb, ndev, tx_obj_no - PCH_RX_OBJ_END - 1);

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH net-next-2.6 8/17 v3] can: EG20T PCH: Change Copyright and module description
From: Marc Kleine-Budde @ 2010-11-24 13:35 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED02C5.4080504-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 526 bytes --]

On 11/24/2010 01:19 PM, Tomoya MORINAGA wrote:
> Currently, Copyright and module description are not formal.

As David pointed out in an older version of this series, this patch does
more than the subject indicates. Please fix.

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH net-next-2.6 16/17 v3] can: EG20T PCH: Fix incorrect return processing
From: Marc Kleine-Budde @ 2010-11-24 13:39 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED0412.60305-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 2831 bytes --]

On 11/24/2010 01:24 PM, Tomoya MORINAGA wrote:
> Fix incorrect return processing

see comments inline

> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
>  drivers/net/can/pch_can.c |   20 ++++++++++++--------
>  1 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index c612a99..48f4a2e 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -589,10 +589,12 @@ static irqreturn_t pch_can_interrupt(int irq, void
> *dev_id)
>  	struct net_device *ndev = (struct net_device *)dev_id;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
> 
> -	pch_can_set_int_enables(priv, PCH_CAN_NONE);
> -	napi_schedule(&priv->napi);
> -
> -	return IRQ_HANDLED;
> +	if ((pch_can_int_pending(priv) > 0) && (dev_id != NULL)) {
> +		pch_can_set_int_enables(priv, PCH_CAN_NONE);
> +		napi_schedule(&priv->napi);
> +		return IRQ_HANDLED;
> +	}
> +	return IRQ_NONE;

My comment from the first review still applied here:

dev_id is always != NULL, because you registered your IRQ handler with
it. (BTW: dev_id has already been dereferenced in netdev_priv(), so if
this code is executed, dev_if is != NULL)

Just write:

if (!pch_can_int_pending(priv))
	return IRQ_NONE;

>  }
> 
>  static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
> @@ -674,7 +676,7 @@ static int pch_can_rx_normal(struct net_device
> *ndev, u32 obj_num, int quota)
>  		if (reg & PCH_IF_MCONT_MSGLOST) {
>  			rtn = pch_can_rx_msg_lost(ndev, obj_num);
>  			if (!rtn)
> -				return rtn;
> +				return rcv_pkts;
>  			rcv_pkts++;
>  			quota--;
>  			obj_num++;
> @@ -777,10 +779,12 @@ static int pch_can_poll(struct napi_struct *napi,
> int quota)
>  		goto end;
> 
>  	if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
> -		rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
> -		quota -= rcv_pkts;
> -		if (quota < 0)
> +		rcv_pkts = pch_can_rx_normal(ndev, int_stat, quota);
> +		if (rcv_pkts < 0) {
> +			rcv_pkts = 0;
>  			goto end;
> +		}
> +		quota -= rcv_pkts;

you introduced the problem in patch "[PATCH net-next-2.6 6/17 v3] can:
EG20T PCH: Fix endianness issue", please don't do this in the first place.

( This is an example why you should split your patches and give them
proper subject.)

>  	} else if ((int_stat >= PCH_TX_OBJ_START) &&
>  		   (int_stat <= PCH_TX_OBJ_END)) {
>  		/* Handle transmission interrupt */

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* [PATCH net-next] bnx2x: Disable local BHes to prevent a dead-lock situation
From: Vladislav Zolotarov @ 2010-11-24 13:41 UTC (permalink / raw)
  To: Dave Miller; +Cc: Eilon Greenstein, netdev list

From: Eric Dumazet <eric.dumazet@gmail.com>

Disable local BHes to prevent a dead-lock situation between sch_direct_xmit()
(Soft_IRQ context) and bnx2x_tx_int (called by bnx2x_run_loopback() - syscall
context), as both are taking a netif_tx_lock().

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/bnx2x/bnx2x_ethtool.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index d02ffbd..0301278 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -1499,8 +1499,15 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode, u8 link_up)
 	 * updates that have been performed while interrupts were
 	 * disabled.
 	 */
-	if (bp->common.int_block == INT_BLOCK_IGU)
+	if (bp->common.int_block == INT_BLOCK_IGU) {
+		/* Disable local BHes to prevent a dead-lock situation between
+		 * sch_direct_xmit() and bnx2x_run_loopback() (calling
+		 * bnx2x_tx_int()), as both are taking netif_tx_lock().
+		 */
+		local_bh_disable();
 		bnx2x_tx_int(fp_tx);
+		local_bh_enable();
+	}
 
 	rx_idx = le16_to_cpu(*fp_rx->rx_cons_sb);
 	if (rx_idx != rx_start_idx + num_pkts)
-- 
1.7.0.4





^ permalink raw reply related

* [PATCH net-next] bnx2x: Disable local BHes to prevent a dead-lock situation
From: Vladislav Zolotarov @ 2010-11-24 13:45 UTC (permalink / raw)
  To: Dave Miller; +Cc: Eilon Greenstein, netdev list, Eric Dumazet

From: Eric Dumazet <eric.dumazet@gmail.com>

According to Eric's suggestion:
Disable local BHes to prevent a dead-lock situation between sch_direct_xmit()
(Soft_IRQ context) and bnx2x_tx_int (called by bnx2x_run_loopback() - syscall
context), as both are taking a netif_tx_lock().

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
CCing Eric... :)

 drivers/net/bnx2x/bnx2x_ethtool.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index d02ffbd..0301278 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -1499,8 +1499,15 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode, u8 link_up)
 	 * updates that have been performed while interrupts were
 	 * disabled.
 	 */
-	if (bp->common.int_block == INT_BLOCK_IGU)
+	if (bp->common.int_block == INT_BLOCK_IGU) {
+		/* Disable local BHes to prevent a dead-lock situation between
+		 * sch_direct_xmit() and bnx2x_run_loopback() (calling
+		 * bnx2x_tx_int()), as both are taking netif_tx_lock().
+		 */
+		local_bh_disable();
 		bnx2x_tx_int(fp_tx);
+		local_bh_enable();
+	}
 
 	rx_idx = le16_to_cpu(*fp_rx->rx_cons_sb);
 	if (rx_idx != rx_start_idx + num_pkts)
-- 
1.7.0.4





^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox