* Re: [PATCH net-next-2.6 3/17 v3] can: EG20T PCH: Enumerate some macros
[not found] ` <4CED00A9.50006-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
@ 2010-11-24 13:07 ` Marc Kleine-Budde
0 siblings, 0 replies; 2+ messages in thread
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
[-- 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 [flat|nested] 2+ messages in thread
* [PATCH net-next-2.6 3/17 v3] can: EG20T PCH: Enumerate some macros
@ 2010-11-24 12:10 Tomoya MORINAGA
0 siblings, 0 replies; 2+ messages in thread
From: Tomoya MORINAGA @ 2010-11-24 12:10 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, LEC and interface register number macros are
replaced to enums.
Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
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,
+};
+
+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)
{
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;
+
priv->can.state = state;
netif_rx(skb);
--
1.6.0.6
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-11-24 13:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4CED00A9.50006@dsn.okisemi.com>
[not found] ` <4CED00A9.50006-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
2010-11-24 13:07 ` [PATCH net-next-2.6 3/17 v3] can: EG20T PCH: Enumerate some macros Marc Kleine-Budde
2010-11-24 12:10 Tomoya MORINAGA
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).