* [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups
@ 2017-01-04 12:09 Sergei Shtylyov
2017-01-04 12:10 ` [PATCH 1/3] sh_eth: handle only enabled E-MAC interrupts Sergei Shtylyov
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-04 12:09 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc
Hello.
Here's a set of 3 patches against DaveM's 'net-next.git' repo. I'm cleaning
up the E-MAC interrupt handling with the main goal of factoring out the E-MAC
interrupt handler into a separate function.
[1/3] sh_eth: handle only enabled E-MAC interrupts
[2/3] sh_eth: no need for *else* after *goto*
[3/3] sh_eth: factor out sh_eth_emac_interrupt()
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] sh_eth: handle only enabled E-MAC interrupts
2017-01-04 12:09 [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups Sergei Shtylyov
@ 2017-01-04 12:10 ` Sergei Shtylyov
2017-01-04 12:10 ` [PATCH 2/3] sh_eth: no need for *else* after *goto* Sergei Shtylyov
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-04 12:10 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc
The driver should only handle the enabled E-MAC interrupts, like it does
for the E-DMAC interrupts since commit 3893b27345ac ("sh_eth: workaround
for spurious ECI interrupt"), so mask ECSR with ECSIPR when reading it
in sh_eth_error().
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: renesas/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- renesas.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ renesas/drivers/net/ethernet/renesas/sh_eth.c
@@ -1535,7 +1535,8 @@ static void sh_eth_error(struct net_devi
u32 mask;
if (intr_status & EESR_ECI) {
- felic_stat = sh_eth_read(ndev, ECSR);
+ felic_stat = sh_eth_read(ndev, ECSR) &
+ sh_eth_read(ndev, ECSIPR);
sh_eth_write(ndev, felic_stat, ECSR); /* clear int */
if (felic_stat & ECSR_ICD)
ndev->stats.tx_carrier_errors++;
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] sh_eth: no need for *else* after *goto*
2017-01-04 12:09 [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups Sergei Shtylyov
2017-01-04 12:10 ` [PATCH 1/3] sh_eth: handle only enabled E-MAC interrupts Sergei Shtylyov
@ 2017-01-04 12:10 ` Sergei Shtylyov
2017-01-04 12:11 ` [PATCH 3/3] sh_eth: factor out sh_eth_emac_interrupt() Sergei Shtylyov
2017-01-04 18:48 ` [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups David Miller
3 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-04 12:10 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc
Well, checkpatch.pl complains about *else* after *return* and *break* but
not after *goto*... and it probably should have complained about the code
in sh_eth_error(). Win couple LoCs by removing that *else*. :-)
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
Index: renesas/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- renesas.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ renesas/drivers/net/ethernet/renesas/sh_eth.c
@@ -1542,13 +1542,11 @@ static void sh_eth_error(struct net_devi
ndev->stats.tx_carrier_errors++;
if (felic_stat & ECSR_LCHNG) {
/* Link Changed */
- if (mdp->cd->no_psr || mdp->no_ether_link) {
+ if (mdp->cd->no_psr || mdp->no_ether_link)
goto ignore_link;
- } else {
- link_stat = (sh_eth_read(ndev, PSR));
- if (mdp->ether_link_active_low)
- link_stat = ~link_stat;
- }
+ link_stat = sh_eth_read(ndev, PSR);
+ if (mdp->ether_link_active_low)
+ link_stat = ~link_stat;
if (!(link_stat & PHY_ST_LINK)) {
sh_eth_rcv_snd_disable(ndev);
} else {
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] sh_eth: factor out sh_eth_emac_interrupt()
2017-01-04 12:09 [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups Sergei Shtylyov
2017-01-04 12:10 ` [PATCH 1/3] sh_eth: handle only enabled E-MAC interrupts Sergei Shtylyov
2017-01-04 12:10 ` [PATCH 2/3] sh_eth: no need for *else* after *goto* Sergei Shtylyov
@ 2017-01-04 12:11 ` Sergei Shtylyov
2017-01-04 12:13 ` Sergei Shtylyov
2017-01-04 18:48 ` [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups David Miller
3 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-04 12:11 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc
The E-MAC interrupt (EESR.ECI) is not always caused by an error condition,
so it really shouldn't be handled by sh_eth_error(). Factor out the E-MAC
interrupt handler, sh_eth_emac_interrupt(), removing the ECI bit from the
EESR's values throughout the driver...
Update Cogent Embedded's copyright and clean up the whitespace in Renesas'
copyright, while at it...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 102 +++++++++++++++++-----------------
drivers/net/ethernet/renesas/sh_eth.h | 2
2 files changed, 53 insertions(+), 51 deletions(-)
Index: renesas/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- renesas.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ renesas/drivers/net/ethernet/renesas/sh_eth.c
@@ -1,9 +1,9 @@
/* SuperH Ethernet device driver
*
- * Copyright (C) 2014 Renesas Electronics Corporation
+ * Copyright (C) 2014 Renesas Electronics Corporation
* Copyright (C) 2006-2012 Nobuhiro Iwamatsu
* Copyright (C) 2008-2014 Renesas Solutions Corp.
- * Copyright (C) 2013-2016 Cogent Embedded, Inc.
+ * Copyright (C) 2013-2017 Cogent Embedded, Inc.
* Copyright (C) 2014 Codethink Limited
*
* This program is free software; you can redistribute it and/or modify it
@@ -523,7 +523,7 @@ static struct sh_eth_cpu_data r7s72100_d
.tx_check = EESR_TC1 | EESR_FTC,
.eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
- EESR_TDE | EESR_ECI,
+ EESR_TDE,
.fdr_value = 0x0000070f,
.no_psr = 1,
@@ -562,7 +562,7 @@ static struct sh_eth_cpu_data r8a7740_da
.tx_check = EESR_TC1 | EESR_FTC,
.eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
- EESR_TDE | EESR_ECI,
+ EESR_TDE,
.fdr_value = 0x0000070f,
.apr = 1,
@@ -607,8 +607,7 @@ static struct sh_eth_cpu_data r8a777x_da
.tx_check = EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
.eesr_err_check = EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
- EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE |
- EESR_ECI,
+ EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
.fdr_value = 0x00000f0f,
.apr = 1,
@@ -630,8 +629,7 @@ static struct sh_eth_cpu_data r8a779x_da
.tx_check = EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
.eesr_err_check = EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
- EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE |
- EESR_ECI,
+ EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
.fdr_value = 0x00000f0f,
.trscer_err_mask = DESC_I_RINT8,
@@ -671,8 +669,7 @@ static struct sh_eth_cpu_data sh7724_dat
.tx_check = EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
.eesr_err_check = EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
- EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE |
- EESR_ECI,
+ EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
.apr = 1,
.mpr = 1,
@@ -707,8 +704,7 @@ static struct sh_eth_cpu_data sh7757_dat
.tx_check = EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
.eesr_err_check = EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
- EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE |
- EESR_ECI,
+ EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
.irq_flags = IRQF_SHARED,
.apr = 1,
@@ -776,7 +772,7 @@ static struct sh_eth_cpu_data sh7757_dat
.tx_check = EESR_TC1 | EESR_FTC,
.eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
- EESR_TDE | EESR_ECI,
+ EESR_TDE,
.fdr_value = 0x0000072f,
.irq_flags = IRQF_SHARED,
@@ -807,7 +803,7 @@ static struct sh_eth_cpu_data sh7734_dat
.tx_check = EESR_TC1 | EESR_FTC,
.eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
- EESR_TDE | EESR_ECI,
+ EESR_TDE,
.apr = 1,
.mpr = 1,
@@ -835,8 +831,7 @@ static struct sh_eth_cpu_data sh7763_dat
.tx_check = EESR_TC1 | EESR_FTC,
.eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
- EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE |
- EESR_ECI,
+ EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
.apr = 1,
.mpr = 1,
@@ -1526,43 +1521,44 @@ static void sh_eth_rcv_snd_enable(struct
sh_eth_modify(ndev, ECMR, ECMR_RE | ECMR_TE, ECMR_RE | ECMR_TE);
}
-/* error control function */
-static void sh_eth_error(struct net_device *ndev, u32 intr_status)
+/* E-MAC interrupt handler */
+static void sh_eth_emac_interrupt(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
u32 felic_stat;
u32 link_stat;
- u32 mask;
- if (intr_status & EESR_ECI) {
- felic_stat = sh_eth_read(ndev, ECSR) &
- sh_eth_read(ndev, ECSIPR);
- sh_eth_write(ndev, felic_stat, ECSR); /* clear int */
- if (felic_stat & ECSR_ICD)
- ndev->stats.tx_carrier_errors++;
- if (felic_stat & ECSR_LCHNG) {
- /* Link Changed */
- if (mdp->cd->no_psr || mdp->no_ether_link)
- goto ignore_link;
- link_stat = sh_eth_read(ndev, PSR);
- if (mdp->ether_link_active_low)
- link_stat = ~link_stat;
- if (!(link_stat & PHY_ST_LINK)) {
- sh_eth_rcv_snd_disable(ndev);
- } else {
- /* Link Up */
- sh_eth_modify(ndev, EESIPR, DMAC_M_ECI, 0);
- /* clear int */
- sh_eth_modify(ndev, ECSR, 0, 0);
- sh_eth_modify(ndev, EESIPR, DMAC_M_ECI,
- DMAC_M_ECI);
- /* enable tx and rx */
- sh_eth_rcv_snd_enable(ndev);
- }
+ felic_stat = sh_eth_read(ndev, ECSR) & sh_eth_read(ndev, ECSIPR);
+ sh_eth_write(ndev, felic_stat, ECSR); /* clear int */
+ if (felic_stat & ECSR_ICD)
+ ndev->stats.tx_carrier_errors++;
+ if (felic_stat & ECSR_LCHNG) {
+ /* Link Changed */
+ if (mdp->cd->no_psr || mdp->no_ether_link)
+ return;
+ link_stat = sh_eth_read(ndev, PSR);
+ if (mdp->ether_link_active_low)
+ link_stat = ~link_stat;
+ if (!(link_stat & PHY_ST_LINK)) {
+ sh_eth_rcv_snd_disable(ndev);
+ } else {
+ /* Link Up */
+ sh_eth_modify(ndev, EESIPR, DMAC_M_ECI, 0);
+ /* clear int */
+ sh_eth_modify(ndev, ECSR, 0, 0);
+ sh_eth_modify(ndev, EESIPR, DMAC_M_ECI, DMAC_M_ECI);
+ /* enable tx and rx */
+ sh_eth_rcv_snd_enable(ndev);
}
}
+}
+
+/* error control function */
+static void sh_eth_error(struct net_device *ndev, u32 intr_status)
+{
+ struct sh_eth_private *mdp = netdev_priv(ndev);
+ u32 mask;
-ignore_link:
if (intr_status & EESR_TWB) {
/* Unused write back interrupt */
if (intr_status & EESR_TABT) { /* Transmit Abort int */
@@ -1643,14 +1639,16 @@ static irqreturn_t sh_eth_interrupt(int
/* Get interrupt status */
intr_status = sh_eth_read(ndev, EESR);
- /* Mask it with the interrupt mask, forcing ECI interrupt to be always
- * enabled since it's the one that comes thru regardless of the mask,
- * and we need to fully handle it in sh_eth_error() in order to quench
- * it as it doesn't get cleared by just writing 1 to the ECI bit...
+ /* Mask it with the interrupt mask, forcing ECI interrupt to be always
+ * enabled since it's the one that comes thru regardless of the mask,
+ * and we need to fully handle it in sh_eth_emac_interrupt() in order
+ * to quench it as it doesn't get cleared by just writing 1 to the ECI
+ * bit...
*/
intr_enable = sh_eth_read(ndev, EESIPR);
intr_status &= intr_enable | DMAC_M_ECI;
- if (intr_status & (EESR_RX_CHECK | cd->tx_check | cd->eesr_err_check))
+ if (intr_status & (EESR_RX_CHECK | cd->tx_check | EESR_ECI |
+ cd->eesr_err_check))
ret = IRQ_HANDLED;
else
goto out;
@@ -1682,6 +1680,10 @@ static irqreturn_t sh_eth_interrupt(int
netif_wake_queue(ndev);
}
+ /* E-MAC interrupt */
+ if (intr_status & EESR_ECI)
+ sh_eth_emac_interrupt(ndev);
+
if (intr_status & cd->eesr_err_check) {
/* Clear error interrupts */
sh_eth_write(ndev, intr_status & cd->eesr_err_check, EESR);
Index: renesas/drivers/net/ethernet/renesas/sh_eth.h
===================================================================
--- renesas.orig/drivers/net/ethernet/renesas/sh_eth.h
+++ renesas/drivers/net/ethernet/renesas/sh_eth.h
@@ -265,7 +265,7 @@ enum EESR_BIT {
EESR_RTO)
#define DEFAULT_EESR_ERR_CHECK (EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE | \
EESR_RDE | EESR_RFRMER | EESR_ADE | \
- EESR_TFE | EESR_TDE | EESR_ECI)
+ EESR_TFE | EESR_TDE)
/* EESIPR */
enum DMAC_IM_BIT {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] sh_eth: factor out sh_eth_emac_interrupt()
2017-01-04 12:11 ` [PATCH 3/3] sh_eth: factor out sh_eth_emac_interrupt() Sergei Shtylyov
@ 2017-01-04 12:13 ` Sergei Shtylyov
2017-01-04 12:20 ` Sergei Shtylyov
0 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-04 12:13 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc
On 01/04/2017 03:11 PM, Sergei Shtylyov wrote:
> The E-MAC interrupt (EESR.ECI) is not always caused by an error condition,
> so it really shouldn't be handled by sh_eth_error(). Factor out the E-MAC
> interrupt handler, sh_eth_emac_interrupt(), removing the ECI bit from the
> EESR's values throughout the driver...
>
> Update Cogent Embedded's copyright and clean up the whitespace in Renesas'
> copyright, while at it...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Scrap it, I've just realized I never enable EESR.ECI...
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] sh_eth: factor out sh_eth_emac_interrupt()
2017-01-04 12:13 ` Sergei Shtylyov
@ 2017-01-04 12:20 ` Sergei Shtylyov
0 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-04 12:20 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc
On 01/04/2017 03:13 PM, Sergei Shtylyov wrote:
>> The E-MAC interrupt (EESR.ECI) is not always caused by an error condition,
>> so it really shouldn't be handled by sh_eth_error(). Factor out the E-MAC
>> interrupt handler, sh_eth_emac_interrupt(), removing the ECI bit from the
>> EESR's values throughout the driver...
>>
>> Update Cogent Embedded's copyright and clean up the whitespace in Renesas'
>> copyright, while at it...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Scrap it, I've just realized I never enable EESR.ECI...
Heh, hat was false alarm -- I didn't change the .eesipr_value in the
patch, so everything's OK, you can merge this series.
I'll look into avoiding bare numbers in the .eesipr_value initializers.
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups
2017-01-04 12:09 [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups Sergei Shtylyov
` (2 preceding siblings ...)
2017-01-04 12:11 ` [PATCH 3/3] sh_eth: factor out sh_eth_emac_interrupt() Sergei Shtylyov
@ 2017-01-04 18:48 ` David Miller
3 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2017-01-04 18:48 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, linux-renesas-soc
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 04 Jan 2017 15:09:36 +0300
> Here's a set of 3 patches against DaveM's 'net-next.git' repo. I'm cleaning
> up the E-MAC interrupt handling with the main goal of factoring out the E-MAC
> interrupt handler into a separate function.
>
> [1/3] sh_eth: handle only enabled E-MAC interrupts
> [2/3] sh_eth: no need for *else* after *goto*
> [3/3] sh_eth: factor out sh_eth_emac_interrupt()
Series applied to net-next, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-04 18:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-04 12:09 [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups Sergei Shtylyov
2017-01-04 12:10 ` [PATCH 1/3] sh_eth: handle only enabled E-MAC interrupts Sergei Shtylyov
2017-01-04 12:10 ` [PATCH 2/3] sh_eth: no need for *else* after *goto* Sergei Shtylyov
2017-01-04 12:11 ` [PATCH 3/3] sh_eth: factor out sh_eth_emac_interrupt() Sergei Shtylyov
2017-01-04 12:13 ` Sergei Shtylyov
2017-01-04 12:20 ` Sergei Shtylyov
2017-01-04 18:48 ` [PATCH 0/2] sh_eth: E-MAC interrupt handler cleanups David Miller
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).