Re: lost interrupts when running sabrelite images (v4.15+) in qemu
On 3/3/2018 1:12 PM, Guenter Roeck wrote: > On 03/03/2018 12:48 PM, Guenter Roeck wrote: >> On 03/03/2018 11:07 AM, Troy Kisky wrote: >>> On 3/3/2018 8:32 AM, Guenter Roeck wrote: >>>> Hi, >>>> >>>> since v4.15, I get the following runtime warning when running sabrelite >>>> images >>>> in qemu. >>>> >>>> irq 65: nobody cared (try booting with the "irqpoll" option) >>>> ... >>>> handlers: >>>> [<26292474>] fec_pps_interrupt >>>> Disabling IRQ #65 >>>> fec 2188000.ethernet (unnamed net_device) (uninitialized): MDIO read >>>> timeout >>>> >>>> Bisect points to commit 4ad1ceec05e491 ("net: fec: Let fec_ptp have its >>>> own interrupt routine"). Analysis shows that platform_irq_count() >>>> returns 2, which is reduced to 1 by fec_enet_get_irq_cnt(). >>>> If I let fec_enet_get_irq_cnt() return 2, the problem is gone. >>>> Reverting commit 4ad1ceec05e491 also fixes the problem. >>>> >>>> Bisect log is attached. >>>> >>> >>> Sounds like you found a bug with qemu. I just booted sabrelite over nfs >>> fine. >>> My interrupts look like this. >>> >>> >>> 64: 98767 0 0 0 GIC-0 150 Level >>> 2188000.ethernet >>> 65: 0 0 0 0 GIC-0 151 Level >>> 2188000.ethernet >>> ___ >>> Irq 65 is only for ptp interrrupts now. If qemu is signaling an tx/rx frame >>> interrupt on 65, >>> then qemu is wrong. Of course, I've never used qemu so feel free to ignore >>> me if I make no sense. >>> >> >> Thanks for checking with real hardware. >> >> This is what I see (with your patch reverted): >> >> 64: 0 GIC-0 150 Level 2188000.ethernet >> 65: 64 GIC-0 151 Level 2188000.ethernet >> >> Looking into the qemu source, I see: >> >> #define FSL_IMX6_ENET_MAC_1588_IRQ 118 >> #define FSL_IMX6_ENET_MAC_IRQ 119 >> >> FSL_IMX6_ENET_MAC_IRQ is then connected to fec interrupt index 0, and >> FSL_IMX6_ENET_MAC_1588_IRQ >> is connected to fec interrupt index 1. >> >> This may suggest that the defines are reversed. I'll see what happens if I >> swap them. >> > > Confirmed. If I swap the above defines, everything works fine. At the same > time, > the modified qemu works with older kernels. > > Thanks a lot for the hint, and sorry for the noise. > > Guenter > It definitely was not noise. I bet it helps people searching the mailing list in the future. Thanks for posting the resolution. BR Troy
Re: lost interrupts when running sabrelite images (v4.15+) in qemu
On 3/3/2018 8:32 AM, Guenter Roeck wrote: > Hi, > > since v4.15, I get the following runtime warning when running sabrelite images > in qemu. > > irq 65: nobody cared (try booting with the "irqpoll" option) > ... > handlers: > [<26292474>] fec_pps_interrupt > Disabling IRQ #65 > fec 2188000.ethernet (unnamed net_device) (uninitialized): MDIO read timeout > > Bisect points to commit 4ad1ceec05e491 ("net: fec: Let fec_ptp have its > own interrupt routine"). Analysis shows that platform_irq_count() > returns 2, which is reduced to 1 by fec_enet_get_irq_cnt(). > If I let fec_enet_get_irq_cnt() return 2, the problem is gone. > Reverting commit 4ad1ceec05e491 also fixes the problem. > > Bisect log is attached. > Sounds like you found a bug with qemu. I just booted sabrelite over nfs fine. My interrupts look like this. 64: 98767 0 0 0 GIC-0 150 Level 2188000.ethernet 65: 0 0 0 0 GIC-0 151 Level 2188000.ethernet ___ Irq 65 is only for ptp interrrupts now. If qemu is signaling an tx/rx frame interrupt on 65, then qemu is wrong. Of course, I've never used qemu so feel free to ignore me if I make no sense. BR Troy
Re: [PATCH v2 net,stable 1/2] net: fec: restore dev_id in the cases of probe error
On 1/2/2018 6:39 PM, Fugang Duan wrote: > The static variable dev_id always plus one before netdev registerred. > It should restore the dev_id value in the cases of probe error. > > Signed-off-by: Fugang Duan> --- > drivers/net/ethernet/freescale/fec_main.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index e17d10b..dae89bc 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -3576,6 +3576,7 @@ static int fec_enet_get_irq_cnt(struct platform_device > *pdev) > of_node_put(phy_node); > failed_ioremap: > free_netdev(ndev); > + dev_id--; This should be before failed_ioremap. > > return ret; > } >
[PATCH net v5 1/2] ARM: dts: imx: name the interrupts for the fec ethernet driver
imx7s/imx7d has the ptp interrupt newly added as well. For imx7, "int0" is the interrupt for queue 0 and ENET_MII "int1" is for queue 1 "int2" is for queue 2 For imx6sx, "int0" handles all 3 queues and ENET_MII And of course, the "pps" interrupt is for the PTP_CLOCK_PPS interrupts This will help document what each interrupt does. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: replaced empty names with "int0","int1", or "int2" reordered imx7 interrupts so that "int0", for queue 0, is first. v3: renamed "ptp" interrupt to "pps", added binding documentation for interrupt-names. v4: add blank, ie s/"int0","pps"/"int0", "pps"/ as suggested by Andrew Lunn v5: moved binding documentation to next patch --- arch/arm/boot/dts/imx6qdl.dtsi | 1 + arch/arm/boot/dts/imx6sx.dtsi | 2 ++ arch/arm/boot/dts/imx6ul.dtsi | 2 ++ arch/arm/boot/dts/imx7d.dtsi | 6 -- arch/arm/boot/dts/imx7s.dtsi | 6 -- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index 8884b4a3cafb..f97dee04b9be 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -1017,6 +1017,7 @@ fec: ethernet@02188000 { compatible = "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0", "pps"; interrupts-extended = < 0 118 IRQ_TYPE_LEVEL_HIGH>, < 0 119 IRQ_TYPE_LEVEL_HIGH>; diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index 6c7eb54be9e2..7f3773b88c47 100644 --- a/arch/arm/boot/dts/imx6sx.dtsi +++ b/arch/arm/boot/dts/imx6sx.dtsi @@ -861,6 +861,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0", "pps"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, @@ -970,6 +971,7 @@ fec2: ethernet@021b4000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x021b4000 0x4000>; + interrupt-names = "int0", "pps"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index f11a241a340d..5ac00ff959b2 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -476,6 +476,7 @@ fec2: ethernet@020b4000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x020b4000 0x4000>; + interrupt-names = "int0", "pps"; interrupts = , ; clocks = < IMX6UL_CLK_ENET>, @@ -775,6 +776,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0", "pps"; interrupts = , ; clocks = < IMX6UL_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi index 4d308d17f040..8b9c8c0695df 100644 --- a/arch/arm/boot/dts/imx7d.dtsi +++ b/arch/arm/boot/dts/imx7d.dtsi @@ -114,9 +114,11 @@ fec2: ethernet@30bf { compatible = "fsl,imx7d-fec", "fsl,imx6sx-fec"; reg = <0x30bf 0x1>; - interrupts = , + interrupt-names = "int0", "int1", "int2", "pps"; + interrupts = , + , , - ; + ; clocks = < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET2_TIME_ROOT_CLK>, diff --git a/arch/arm/boo
[PATCH net v5 2/2] net: fec: Let fec_ptp have its own interrupt routine
This is better for code locality and should slightly speed up normal interrupts. This also allows PPS clock output to start working for i.mx7. This is because i.mx7 was already using the limit of 3 interrupts, and needed another. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: made this change independent of any devicetree change so that old dtbs continue to work. Continue to register ptp clock if interrupt is not found. v3: renamed "ptp" interrupt to "pps" interrupt v4: no change v5: moving binding documentation to this patch as requested by Shawn Guo s/irq_index/irq_idx/ add function fec_enet_get_irq_cnt() to encapsulate if, as requested by Andy Duan --- Documentation/devicetree/bindings/net/fsl-fec.txt | 13 drivers/net/ethernet/freescale/fec.h | 3 +- drivers/net/ethernet/freescale/fec_main.c | 31 ++--- drivers/net/ethernet/freescale/fec_ptp.c | 82 +-- 4 files changed, 84 insertions(+), 45 deletions(-) diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt index 6f55bdd52f8a..f0dc94409107 100644 --- a/Documentation/devicetree/bindings/net/fsl-fec.txt +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt @@ -34,6 +34,19 @@ Optional properties: - fsl,err006687-workaround-present: If present indicates that the system has the hardware workaround for ERR006687 applied and does not need a software workaround. + -interrupt-names: names of the interrupts listed in interrupts property in + the same order. The defaults if not specified are + __Number of interrupts__ __Default__ + 1 "int0" + 2 "int0", "pps" + 3 "int0", "int1", "int2" + 4 "int0", "int1", "int2", "pps" + The order may be changed as long as they correspond to the interrupts + property. Currently, only i.mx7 uses "int1" and "int2". They correspond to + tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts. + For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse + per second interrupt associated with 1588 precision time protocol(PTP). + Optional subnodes: - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ede1876a9a19..0af58991ca8f 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -582,12 +582,11 @@ struct fec_enet_private { u64 ethtool_stats[0]; }; -void fec_ptp_init(struct platform_device *pdev); +void fec_ptp_init(struct platform_device *pdev, int irq_idx); void fec_ptp_stop(struct platform_device *pdev); void fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private *fep); // #endif /* FEC_H */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3dc2d771a222..610573855213 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) ret = IRQ_HANDLED; complete(>mdio_done); } - - if (fep->ptp_clock) - if (fec_ptp_check_pps_event(fep)) - ret = IRQ_HANDLED; return ret; } @@ -3312,6 +3308,19 @@ fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, int *num_rx) } +static int fec_enet_get_irq_cnt(struct platform_device *pdev) +{ + int irq_cnt = platform_irq_count(pdev); + + if (irq_cnt > FEC_IRQ_NUM) + irq_cnt = FEC_IRQ_NUM; /* last for pps */ + else if (irq_cnt == 2) + irq_cnt = 1;/* last for pps */ + else if (irq_cnt <= 0) + irq_cnt = 1;/* At least 1 irq is needed */ + return irq_cnt; +} + static int fec_probe(struct platform_device *pdev) { @@ -3325,6 +3334,8 @@ fec_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node, *phy_node; int num_tx_qs; int num_rx_qs; + char irq_name[8]; + int irq_cnt; fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); @@ -3465,18 +3476,20 @@ fec_probe(struct platform_device *pdev) if (ret) goto failed_reset; + irq_cnt = fec_enet_get_irq_cnt(pdev); if (fep->bufdesc_ex) -
Re: [PATCH net v4 2/2] net: fec: Let fec_ptp have its own interrupt routine
On 10/31/2017 7:43 PM, Andy Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Wednesday, November > 01, 2017 4:17 AM >> This is better for code locality and should slightly speed up normal >> interrupts. >> >> This also allows PPS clock output to start working for i.mx7. This is because >> i.mx7 was already using the limit of 3 interrupts, and needed another. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> >> --- >> >> v2: made this change independent of any devicetree change so that old dtbs >> continue to work. >> >> Continue to register ptp clock if interrupt is not found. >> >> v3: renamed "ptp" interrupt to "pps" interrupt >> >> v4: no change >> --- >> drivers/net/ethernet/freescale/fec.h | 3 +- >> drivers/net/ethernet/freescale/fec_main.c | 25 ++ >> drivers/net/ethernet/freescale/fec_ptp.c | 82 ++ >> - >> 3 files changed, 65 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec.h >> b/drivers/net/ethernet/freescale/fec.h >> index ede1876a9a19..be56ac1f1ac4 100644 >> --- a/drivers/net/ethernet/freescale/fec.h >> +++ b/drivers/net/ethernet/freescale/fec.h >> @@ -582,12 +582,11 @@ struct fec_enet_private { >> u64 ethtool_stats[0]; >> }; >> >> -void fec_ptp_init(struct platform_device *pdev); >> +void fec_ptp_init(struct platform_device *pdev, int irq_index); > > Seems change irq_index to irq_idx much better. no problem >> @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev) >> if (ret) >> goto failed_reset; >> >> +irq_cnt = platform_irq_count(pdev); >> +if (irq_cnt > FEC_IRQ_NUM) >> +irq_cnt = FEC_IRQ_NUM; /* last for pps */ >> +else if (irq_cnt == 2) >> +irq_cnt = 1;/* last for pps */ >> +else if (irq_cnt <= 0) >> +irq_cnt = 1;/* Let the for loop fail */ >> + > > Do some parse on probe function seems uncomfortable...can you encapsulate > these code into one api ? > Do you mean something like irq_cnt = limit_irq(pdev); int limit_irq(struct platform_device *pdev) { int irq_cnt = platform_irq_count(pdev); if (irq_cnt > FEC_IRQ_NUM) irq_cnt = FEC_IRQ_NUM; /* last for pps */ else if (irq_cnt == 2) irq_cnt = 1;/* last for pps */ else if (irq_cnt <= 0) irq_cnt = 1;/* At least 1 irq is needed */ return irq_cnt; } ___ Can you give some code to your idea? I don't think I understand. > Others seems fine to me. > Thanks. >
[PATCH net v4 1/2] imx7s/imx7d has the ptp interrupt newly added as well.
For imx7, "int0" is the interrupt for queue 0 and ENET_MII "int1" is for queue 1 "int2" is for queue 2 For imx6sx, "int0" handles all 3 queues and ENET_MII And of course, the "pps" interrupt is for the PTP_CLOCK_PPS interrupts This will help document what each interrupt does. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: replaced empty names with "int0","int1", or "int2" reordered imx7 interrupts so that "int0", for queue 0, is first. v3: renamed "ptp" interrupt to "pps", added binding documentation for interrupt-names. v4: add blank, ie s/"int0","pps"/"int0", "pps"/ as suggested by Andrew Lunn --- Documentation/devicetree/bindings/net/fsl-fec.txt | 13 + arch/arm/boot/dts/imx6qdl.dtsi| 1 + arch/arm/boot/dts/imx6sx.dtsi | 2 ++ arch/arm/boot/dts/imx6ul.dtsi | 2 ++ arch/arm/boot/dts/imx7d.dtsi | 6 -- arch/arm/boot/dts/imx7s.dtsi | 6 -- 6 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt index 6f55bdd52f8a..f0dc94409107 100644 --- a/Documentation/devicetree/bindings/net/fsl-fec.txt +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt @@ -34,6 +34,19 @@ Optional properties: - fsl,err006687-workaround-present: If present indicates that the system has the hardware workaround for ERR006687 applied and does not need a software workaround. + -interrupt-names: names of the interrupts listed in interrupts property in + the same order. The defaults if not specified are + __Number of interrupts__ __Default__ + 1 "int0" + 2 "int0", "pps" + 3 "int0", "int1", "int2" + 4 "int0", "int1", "int2", "pps" + The order may be changed as long as they correspond to the interrupts + property. Currently, only i.mx7 uses "int1" and "int2". They correspond to + tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts. + For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse + per second interrupt associated with 1588 precision time protocol(PTP). + Optional subnodes: - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index 8884b4a3cafb..f97dee04b9be 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -1017,6 +1017,7 @@ fec: ethernet@02188000 { compatible = "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0", "pps"; interrupts-extended = < 0 118 IRQ_TYPE_LEVEL_HIGH>, < 0 119 IRQ_TYPE_LEVEL_HIGH>; diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index 6c7eb54be9e2..7f3773b88c47 100644 --- a/arch/arm/boot/dts/imx6sx.dtsi +++ b/arch/arm/boot/dts/imx6sx.dtsi @@ -861,6 +861,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0", "pps"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, @@ -970,6 +971,7 @@ fec2: ethernet@021b4000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x021b4000 0x4000>; + interrupt-names = "int0", "pps"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index f11a241a340d..5ac00ff959b2 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -476,6 +476,7 @@ fec2: ethernet@020b4000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x020b4000 0x4000>; +
[PATCH net v4 2/2] net: fec: Let fec_ptp have its own interrupt routine
This is better for code locality and should slightly speed up normal interrupts. This also allows PPS clock output to start working for i.mx7. This is because i.mx7 was already using the limit of 3 interrupts, and needed another. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: made this change independent of any devicetree change so that old dtbs continue to work. Continue to register ptp clock if interrupt is not found. v3: renamed "ptp" interrupt to "pps" interrupt v4: no change --- drivers/net/ethernet/freescale/fec.h | 3 +- drivers/net/ethernet/freescale/fec_main.c | 25 ++ drivers/net/ethernet/freescale/fec_ptp.c | 82 ++- 3 files changed, 65 insertions(+), 45 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ede1876a9a19..be56ac1f1ac4 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -582,12 +582,11 @@ struct fec_enet_private { u64 ethtool_stats[0]; }; -void fec_ptp_init(struct platform_device *pdev); +void fec_ptp_init(struct platform_device *pdev, int irq_index); void fec_ptp_stop(struct platform_device *pdev); void fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private *fep); // #endif /* FEC_H */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3dc2d771a222..18fc136817c9 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) ret = IRQ_HANDLED; complete(>mdio_done); } - - if (fep->ptp_clock) - if (fec_ptp_check_pps_event(fep)) - ret = IRQ_HANDLED; return ret; } @@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node, *phy_node; int num_tx_qs; int num_rx_qs; + char irq_name[8]; + int irq_cnt; fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev) if (ret) goto failed_reset; + irq_cnt = platform_irq_count(pdev); + if (irq_cnt > FEC_IRQ_NUM) + irq_cnt = FEC_IRQ_NUM; /* last for pps */ + else if (irq_cnt == 2) + irq_cnt = 1;/* last for pps */ + else if (irq_cnt <= 0) + irq_cnt = 1;/* Let the for loop fail */ + if (fep->bufdesc_ex) - fec_ptp_init(pdev); + fec_ptp_init(pdev, irq_cnt); ret = fec_enet_init(ndev); if (ret) goto failed_init; - for (i = 0; i < FEC_IRQ_NUM; i++) { - irq = platform_get_irq(pdev, i); + for (i = 0; i < irq_cnt; i++) { + sprintf(irq_name, "int%d", i); + irq = platform_get_irq_byname(pdev, irq_name); + if (irq < 0) + irq = platform_get_irq(pdev, i); if (irq < 0) { - if (i) - break; ret = irq; goto failed_irq; } diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 6ebad3fac81d..0f59b0ea1001 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -549,6 +549,37 @@ static void fec_time_keep(struct work_struct *work) schedule_delayed_work(>time_keep, HZ); } +/* This function checks the pps event and reloads the timer compare counter. */ +static irqreturn_t fec_pps_interrupt(int irq, void *dev_id) +{ + struct net_device *ndev = dev_id; + struct fec_enet_private *fep = netdev_priv(ndev); + u32 val; + u8 channel = fep->pps_channel; + struct ptp_clock_event event; + + val = readl(fep->hwp + FEC_TCSR(channel)); + if (val & FEC_T_TF_MASK) { + /* Write the next next compare(not the next according the spec) +* value to the register +*/ + writel(fep->next_counter, fep->hwp + FEC_TCCR(channel)); + do { + writel(val, fep->hwp + FEC_TCSR(channel)); + } while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK); + + /* Update the counter; */ + fep->next_counter = (fep->next_counter + fep->reload_period) & + fep->cc.mask; + + ev
[PATCH net v3 2/2] net: fec: Let fec_ptp have its own interrupt routine
This is better for code locality and should slightly speed up normal interrupts. This also allows PPS clock output to start working for i.mx7. This is because i.mx7 was already using the limit of 3 interrupts, and needed another. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: made this change independent of any devicetree change so that old dtbs continue to work. Continue to register ptp clock if interrupt is not found. v3: renamed "ptp" interrupt to "pps" interrupt --- drivers/net/ethernet/freescale/fec.h | 3 +- drivers/net/ethernet/freescale/fec_main.c | 25 ++ drivers/net/ethernet/freescale/fec_ptp.c | 82 ++- 3 files changed, 65 insertions(+), 45 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ede1876a9a19..be56ac1f1ac4 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -582,12 +582,11 @@ struct fec_enet_private { u64 ethtool_stats[0]; }; -void fec_ptp_init(struct platform_device *pdev); +void fec_ptp_init(struct platform_device *pdev, int irq_index); void fec_ptp_stop(struct platform_device *pdev); void fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private *fep); // #endif /* FEC_H */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3dc2d771a222..18fc136817c9 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) ret = IRQ_HANDLED; complete(>mdio_done); } - - if (fep->ptp_clock) - if (fec_ptp_check_pps_event(fep)) - ret = IRQ_HANDLED; return ret; } @@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node, *phy_node; int num_tx_qs; int num_rx_qs; + char irq_name[8]; + int irq_cnt; fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev) if (ret) goto failed_reset; + irq_cnt = platform_irq_count(pdev); + if (irq_cnt > FEC_IRQ_NUM) + irq_cnt = FEC_IRQ_NUM; /* last for pps */ + else if (irq_cnt == 2) + irq_cnt = 1;/* last for pps */ + else if (irq_cnt <= 0) + irq_cnt = 1;/* Let the for loop fail */ + if (fep->bufdesc_ex) - fec_ptp_init(pdev); + fec_ptp_init(pdev, irq_cnt); ret = fec_enet_init(ndev); if (ret) goto failed_init; - for (i = 0; i < FEC_IRQ_NUM; i++) { - irq = platform_get_irq(pdev, i); + for (i = 0; i < irq_cnt; i++) { + sprintf(irq_name, "int%d", i); + irq = platform_get_irq_byname(pdev, irq_name); + if (irq < 0) + irq = platform_get_irq(pdev, i); if (irq < 0) { - if (i) - break; ret = irq; goto failed_irq; } diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 6ebad3fac81d..0f59b0ea1001 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -549,6 +549,37 @@ static void fec_time_keep(struct work_struct *work) schedule_delayed_work(>time_keep, HZ); } +/* This function checks the pps event and reloads the timer compare counter. */ +static irqreturn_t fec_pps_interrupt(int irq, void *dev_id) +{ + struct net_device *ndev = dev_id; + struct fec_enet_private *fep = netdev_priv(ndev); + u32 val; + u8 channel = fep->pps_channel; + struct ptp_clock_event event; + + val = readl(fep->hwp + FEC_TCSR(channel)); + if (val & FEC_T_TF_MASK) { + /* Write the next next compare(not the next according the spec) +* value to the register +*/ + writel(fep->next_counter, fep->hwp + FEC_TCCR(channel)); + do { + writel(val, fep->hwp + FEC_TCSR(channel)); + } while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK); + + /* Update the counter; */ + fep->next_counter = (fep->next_counter + fep->reload_period) & + fep->cc.mask; + + ev
[PATCH net v3 1/2] ARM: dts: imx: name the interrupts for the fec ethernet driver
imx7s/imx7d has the ptp interrupt newly added as well. For imx7, "int0" is the interrupt for queue 0 and ENET_MII "int1" is for queue 1 "int2" is for queue 2 For imx6sx, "int0" handles all 3 queues and ENET_MII And of course, the "pps" interrupt is for the PTP_CLOCK_PPS interrupts This will help document what each interrupt does. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: replaced empty names with "int0","int1", or "int2" reordered imx7 interrupts so that "int0", for queue 0, is first. v3: renamed "ptp" interrupt to "pps", added binding documentation for interrupt-names. --- Documentation/devicetree/bindings/net/fsl-fec.txt | 13 + arch/arm/boot/dts/imx6qdl.dtsi| 1 + arch/arm/boot/dts/imx6sx.dtsi | 2 ++ arch/arm/boot/dts/imx6ul.dtsi | 2 ++ arch/arm/boot/dts/imx7d.dtsi | 6 -- arch/arm/boot/dts/imx7s.dtsi | 6 -- 6 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt index 6f55bdd52f8a..f0dc94409107 100644 --- a/Documentation/devicetree/bindings/net/fsl-fec.txt +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt @@ -34,6 +34,19 @@ Optional properties: - fsl,err006687-workaround-present: If present indicates that the system has the hardware workaround for ERR006687 applied and does not need a software workaround. + -interrupt-names: names of the interrupts listed in interrupts property in + the same order. The defaults if not specified are + __Number of interrupts__ __Default__ + 1 "int0" + 2 "int0", "pps" + 3 "int0", "int1", "int2" + 4 "int0", "int1", "int2", "pps" + The order may be changed as long as they correspond to the interrupts + property. Currently, only i.mx7 uses "int1" and "int2". They correspond to + tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts. + For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse + per second interrupt associated with 1588 precision time protocol(PTP). + Optional subnodes: - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index 8884b4a3cafb..070e856860f3 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -1017,6 +1017,7 @@ fec: ethernet@02188000 { compatible = "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0","pps"; interrupts-extended = < 0 118 IRQ_TYPE_LEVEL_HIGH>, < 0 119 IRQ_TYPE_LEVEL_HIGH>; diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index 6c7eb54be9e2..d31fc9917142 100644 --- a/arch/arm/boot/dts/imx6sx.dtsi +++ b/arch/arm/boot/dts/imx6sx.dtsi @@ -861,6 +861,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0","pps"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, @@ -970,6 +971,7 @@ fec2: ethernet@021b4000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x021b4000 0x4000>; + interrupt-names = "int0","pps"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index f11a241a340d..5f7611041439 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -476,6 +476,7 @@ fec2: ethernet@020b4000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x020b4000 0x4000>; + interrupt-names = "int0","pps";
Re: [PATCH net v2 2/2] net: fec: Let fec_ptp have its own interrupt routine
On 10/18/2017 11:10 AM, Troy Kisky wrote: > On 10/17/2017 7:30 PM, Andy Duan wrote: >> From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Wednesday, October >> 18, 2017 5:34 AM >>>>> This is better for code locality and should slightly speed up normal >>> interrupts. >>>>> >>>>> This also allows PPS clock output to start working for i.mx7. This is >>>>> because >>>>> i.mx7 was already using the limit of 3 interrupts, and needed another. >>>>> >>>>> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >>>>> >>>>> --- >>>>> >>>>> v2: made this change independent of any devicetree change so that old >>>>> dtbs continue to work. >>>>> >>>>> Continue to register ptp clock if interrupt is not found. >>>>> --- >>>>> drivers/net/ethernet/freescale/fec.h | 3 +- >>>>> drivers/net/ethernet/freescale/fec_main.c | 25 ++ >>>>> drivers/net/ethernet/freescale/fec_ptp.c | 82 >>>>> ++ >>>>> - >>>>> 3 files changed, 65 insertions(+), 45 deletions(-) >>>>> >>>>> diff --git a/drivers/net/ethernet/freescale/fec.h >>>>> b/drivers/net/ethernet/freescale/fec.h >>>>> index ede1876a9a19..be56ac1f1ac4 100644 >>>>> --- a/drivers/net/ethernet/freescale/fec.h >>>>> +++ b/drivers/net/ethernet/freescale/fec.h >>>>> @@ -582,12 +582,11 @@ struct fec_enet_private { >>>>> u64 ethtool_stats[0]; >>>>> }; >>>>> >>>>> -void fec_ptp_init(struct platform_device *pdev); >>>>> +void fec_ptp_init(struct platform_device *pdev, int irq_index); >>>>> void fec_ptp_stop(struct platform_device *pdev); void >>>>> fec_ptp_start_cyclecounter(struct net_device *ndev); int >>>>> fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int >>>>> fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint >>>>> fec_ptp_check_pps_event(struct fec_enet_private *fep); >>>>> >>>>> >>>>> >>> /** >>>>> **/ >>>>> #endif /* FEC_H */ >>>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c >>>>> b/drivers/net/ethernet/freescale/fec_main.c >>>>> index 3dc2d771a222..21afabbc560f 100644 >>>>> --- a/drivers/net/ethernet/freescale/fec_main.c >>>>> +++ b/drivers/net/ethernet/freescale/fec_main.c >>>>> @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) >>>>> ret = IRQ_HANDLED; >>>>> complete(>mdio_done); >>>>> } >>>>> - >>>>> - if (fep->ptp_clock) >>>>> - if (fec_ptp_check_pps_event(fep)) >>>>> - ret = IRQ_HANDLED; >>>>> return ret; >>>>> } >>>>> >>>>> @@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev) >>>>> struct device_node *np = pdev->dev.of_node, *phy_node; >>>>> int num_tx_qs; >>>>> int num_rx_qs; >>>>> + char irq_name[8]; >>>>> + int irq_cnt; >>>>> >>>>> fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); >>>>> >>>>> @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev) >>>>> if (ret) >>>>> goto failed_reset; >>>>> >>>>> + irq_cnt = platform_irq_count(pdev); >>>>> + if (irq_cnt > FEC_IRQ_NUM) >>>>> + irq_cnt = FEC_IRQ_NUM; /* last for ptp */ >>>>> + else if (irq_cnt == 2) >>>>> + irq_cnt = 1;/* last for ptp */ >>>>> + else if (irq_cnt <= 0) >>>>> + irq_cnt = 1;/* Let the for loop fail */ >>>> >>>> Don't do like this. Don't suppose pps interrupt is the last one. >>> >>> >>> I don't. If the pps interrupt is named, the named interrupt will be used. >>> If it is >>> NOT named, the last interrupt is used, if 2 interrupts, or >3 interrupt are >>> provided. >>> Otherwise, no pps interrupt is assumed. >>> Fortunately this seems to be true currently. >>> >> If pps interrupt is not
Re: [PATCH net v2 2/2] net: fec: Let fec_ptp have its own interrupt routine
On 10/17/2017 7:30 PM, Andy Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Wednesday, October > 18, 2017 5:34 AM >>>> This is better for code locality and should slightly speed up normal >> interrupts. >>>> >>>> This also allows PPS clock output to start working for i.mx7. This is >>>> because >>>> i.mx7 was already using the limit of 3 interrupts, and needed another. >>>> >>>> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >>>> >>>> --- >>>> >>>> v2: made this change independent of any devicetree change so that old >>>> dtbs continue to work. >>>> >>>> Continue to register ptp clock if interrupt is not found. >>>> --- >>>> drivers/net/ethernet/freescale/fec.h | 3 +- >>>> drivers/net/ethernet/freescale/fec_main.c | 25 ++ >>>> drivers/net/ethernet/freescale/fec_ptp.c | 82 >>>> ++ >>>> - >>>> 3 files changed, 65 insertions(+), 45 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/freescale/fec.h >>>> b/drivers/net/ethernet/freescale/fec.h >>>> index ede1876a9a19..be56ac1f1ac4 100644 >>>> --- a/drivers/net/ethernet/freescale/fec.h >>>> +++ b/drivers/net/ethernet/freescale/fec.h >>>> @@ -582,12 +582,11 @@ struct fec_enet_private { >>>>u64 ethtool_stats[0]; >>>> }; >>>> >>>> -void fec_ptp_init(struct platform_device *pdev); >>>> +void fec_ptp_init(struct platform_device *pdev, int irq_index); >>>> void fec_ptp_stop(struct platform_device *pdev); void >>>> fec_ptp_start_cyclecounter(struct net_device *ndev); int >>>> fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int >>>> fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint >>>> fec_ptp_check_pps_event(struct fec_enet_private *fep); >>>> >>>> >>>> >> /** >>>> **/ >>>> #endif /* FEC_H */ >>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c >>>> b/drivers/net/ethernet/freescale/fec_main.c >>>> index 3dc2d771a222..21afabbc560f 100644 >>>> --- a/drivers/net/ethernet/freescale/fec_main.c >>>> +++ b/drivers/net/ethernet/freescale/fec_main.c >>>> @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) >>>>ret = IRQ_HANDLED; >>>>complete(>mdio_done); >>>>} >>>> - >>>> - if (fep->ptp_clock) >>>> - if (fec_ptp_check_pps_event(fep)) >>>> - ret = IRQ_HANDLED; >>>>return ret; >>>> } >>>> >>>> @@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev) >>>>struct device_node *np = pdev->dev.of_node, *phy_node; >>>>int num_tx_qs; >>>>int num_rx_qs; >>>> + char irq_name[8]; >>>> + int irq_cnt; >>>> >>>>fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); >>>> >>>> @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev) >>>>if (ret) >>>>goto failed_reset; >>>> >>>> + irq_cnt = platform_irq_count(pdev); >>>> + if (irq_cnt > FEC_IRQ_NUM) >>>> + irq_cnt = FEC_IRQ_NUM; /* last for ptp */ >>>> + else if (irq_cnt == 2) >>>> + irq_cnt = 1;/* last for ptp */ >>>> + else if (irq_cnt <= 0) >>>> + irq_cnt = 1;/* Let the for loop fail */ >>> >>> Don't do like this. Don't suppose pps interrupt is the last one. >> >> >> I don't. If the pps interrupt is named, the named interrupt will be used. If >> it is >> NOT named, the last interrupt is used, if 2 interrupts, or >3 interrupt are >> provided. >> Otherwise, no pps interrupt is assumed. >> Fortunately this seems to be true currently. >> > If pps interrupt is not named, then it limit the last one is pps. > We cannot get the pps interrupt based on current chip interrupt define, we > never know the future chip how to define interrupt. > Although your current implementation can work with current chips, but it is > not really good solution. > >> >>> And if irq_cnt is 1 like imx28/imx5x, the patch will break fec interrupt >
Re: [PATCH net v2 2/2] net: fec: Let fec_ptp have its own interrupt routine
On 10/15/2017 8:41 PM, Andy Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Saturday, October 14, > 2017 10:10 AM >> This is better for code locality and should slightly speed up normal >> interrupts. >> >> This also allows PPS clock output to start working for i.mx7. This is because >> i.mx7 was already using the limit of 3 interrupts, and needed another. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> >> --- >> >> v2: made this change independent of any devicetree change so that old dtbs >> continue to work. >> >> Continue to register ptp clock if interrupt is not found. >> --- >> drivers/net/ethernet/freescale/fec.h | 3 +- >> drivers/net/ethernet/freescale/fec_main.c | 25 ++ >> drivers/net/ethernet/freescale/fec_ptp.c | 82 ++ >> - >> 3 files changed, 65 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec.h >> b/drivers/net/ethernet/freescale/fec.h >> index ede1876a9a19..be56ac1f1ac4 100644 >> --- a/drivers/net/ethernet/freescale/fec.h >> +++ b/drivers/net/ethernet/freescale/fec.h >> @@ -582,12 +582,11 @@ struct fec_enet_private { >> u64 ethtool_stats[0]; >> }; >> >> -void fec_ptp_init(struct platform_device *pdev); >> +void fec_ptp_init(struct platform_device *pdev, int irq_index); >> void fec_ptp_stop(struct platform_device *pdev); void >> fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct >> net_device *ndev, struct ifreq *ifr); int fec_ptp_get(struct net_device >> *ndev, >> struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private >> *fep); >> >> >> /** >> **/ >> #endif /* FEC_H */ >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 3dc2d771a222..21afabbc560f 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) >> ret = IRQ_HANDLED; >> complete(>mdio_done); >> } >> - >> -if (fep->ptp_clock) >> -if (fec_ptp_check_pps_event(fep)) >> -ret = IRQ_HANDLED; >> return ret; >> } >> >> @@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev) >> struct device_node *np = pdev->dev.of_node, *phy_node; >> int num_tx_qs; >> int num_rx_qs; >> +char irq_name[8]; >> +int irq_cnt; >> >> fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); >> >> @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev) >> if (ret) >> goto failed_reset; >> >> +irq_cnt = platform_irq_count(pdev); >> +if (irq_cnt > FEC_IRQ_NUM) >> +irq_cnt = FEC_IRQ_NUM; /* last for ptp */ >> +else if (irq_cnt == 2) >> +irq_cnt = 1;/* last for ptp */ >> +else if (irq_cnt <= 0) >> +irq_cnt = 1;/* Let the for loop fail */ > > Don't do like this. Don't suppose pps interrupt is the last one. I don't. If the pps interrupt is named, the named interrupt will be used. If it is NOT named, the last interrupt is used, if 2 interrupts, or >3 interrupt are provided. Otherwise, no pps interrupt is assumed. Fortunately this seems to be true currently. > And if irq_cnt is 1 like imx28/imx5x, the patch will break fec interrupt > function. How ? fec_ptp_init will not be called as bufdesc_ex is 0. Also, if only 1 interrupt is provided, it is assumed there is no unnamed pps interrupt. > > I suggest to use .platform_get_irq_byname() to get pps(ptp) interrupt like > your v1 logic check. > >> + >> if (fep->bufdesc_ex) >> -fec_ptp_init(pdev); >> +fec_ptp_init(pdev, irq_cnt); >> >> ret = fec_enet_init(ndev); >> if (ret) >> goto failed_init; >> >> -for (i = 0; i < FEC_IRQ_NUM; i++) { >> -irq = platform_get_irq(pdev, i); >> +for (i = 0; i < irq_cnt; i++) { >> +sprintf(irq_name, "int%d", i); >> +irq = platform_get_irq_byname(pdev, irq_name); >> +if (irq < 0) >> +irq = platform_get_irq(pdev, i); >> if (irq < 0) { >> -if (i) >> -
[PATCH net v2 2/2] net: fec: Let fec_ptp have its own interrupt routine
This is better for code locality and should slightly speed up normal interrupts. This also allows PPS clock output to start working for i.mx7. This is because i.mx7 was already using the limit of 3 interrupts, and needed another. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: made this change independent of any devicetree change so that old dtbs continue to work. Continue to register ptp clock if interrupt is not found. --- drivers/net/ethernet/freescale/fec.h | 3 +- drivers/net/ethernet/freescale/fec_main.c | 25 ++ drivers/net/ethernet/freescale/fec_ptp.c | 82 ++- 3 files changed, 65 insertions(+), 45 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ede1876a9a19..be56ac1f1ac4 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -582,12 +582,11 @@ struct fec_enet_private { u64 ethtool_stats[0]; }; -void fec_ptp_init(struct platform_device *pdev); +void fec_ptp_init(struct platform_device *pdev, int irq_index); void fec_ptp_stop(struct platform_device *pdev); void fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private *fep); // #endif /* FEC_H */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3dc2d771a222..21afabbc560f 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) ret = IRQ_HANDLED; complete(>mdio_done); } - - if (fep->ptp_clock) - if (fec_ptp_check_pps_event(fep)) - ret = IRQ_HANDLED; return ret; } @@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node, *phy_node; int num_tx_qs; int num_rx_qs; + char irq_name[8]; + int irq_cnt; fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev) if (ret) goto failed_reset; + irq_cnt = platform_irq_count(pdev); + if (irq_cnt > FEC_IRQ_NUM) + irq_cnt = FEC_IRQ_NUM; /* last for ptp */ + else if (irq_cnt == 2) + irq_cnt = 1;/* last for ptp */ + else if (irq_cnt <= 0) + irq_cnt = 1;/* Let the for loop fail */ + if (fep->bufdesc_ex) - fec_ptp_init(pdev); + fec_ptp_init(pdev, irq_cnt); ret = fec_enet_init(ndev); if (ret) goto failed_init; - for (i = 0; i < FEC_IRQ_NUM; i++) { - irq = platform_get_irq(pdev, i); + for (i = 0; i < irq_cnt; i++) { + sprintf(irq_name, "int%d", i); + irq = platform_get_irq_byname(pdev, irq_name); + if (irq < 0) + irq = platform_get_irq(pdev, i); if (irq < 0) { - if (i) - break; ret = irq; goto failed_irq; } diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 6ebad3fac81d..3abeee0d16dd 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -549,6 +549,37 @@ static void fec_time_keep(struct work_struct *work) schedule_delayed_work(>time_keep, HZ); } +/* This function checks the pps event and reloads the timer compare counter. */ +static irqreturn_t fec_ptp_interrupt(int irq, void *dev_id) +{ + struct net_device *ndev = dev_id; + struct fec_enet_private *fep = netdev_priv(ndev); + u32 val; + u8 channel = fep->pps_channel; + struct ptp_clock_event event; + + val = readl(fep->hwp + FEC_TCSR(channel)); + if (val & FEC_T_TF_MASK) { + /* Write the next next compare(not the next according the spec) +* value to the register +*/ + writel(fep->next_counter, fep->hwp + FEC_TCCR(channel)); + do { + writel(val, fep->hwp + FEC_TCSR(channel)); + } while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK); + + /* Update the counter; */ + fep->next_counter = (fep->next_counter + fep->reload_period) & + fep->cc.mask; + + event.type = PTP_CLOCK_PPS; + ptp_clock_event
[PATCH net v2 1/2] ARM: dts: imx: name the interrupts for the fec ethernet driver
imx7s/imx7d has the ptp interrupt newly added as well. For imx7, "int0" is the interrupt for queue 0 and ENET_MII "int1" is for queue 1 "int2" is for queue 2 For imx6sx, "int0" handles all 3 queues and ENET_MII And of course, the "ptp" interrupt is for the PTP_CLOCK_PPS interrupts This will help document what each interrupt does. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v2: replaced empty names with "int0","int1", or "int2" reordered imx7 interrupts so that "int0", for queue 0, is first. --- arch/arm/boot/dts/imx6qdl.dtsi | 1 + arch/arm/boot/dts/imx6sx.dtsi | 2 ++ arch/arm/boot/dts/imx6ul.dtsi | 2 ++ arch/arm/boot/dts/imx7d.dtsi | 6 -- arch/arm/boot/dts/imx7s.dtsi | 6 -- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index 8884b4a3cafb..b78f7e7a0869 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -1017,6 +1017,7 @@ fec: ethernet@02188000 { compatible = "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0","ptp"; interrupts-extended = < 0 118 IRQ_TYPE_LEVEL_HIGH>, < 0 119 IRQ_TYPE_LEVEL_HIGH>; diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index 6c7eb54be9e2..ed148775f991 100644 --- a/arch/arm/boot/dts/imx6sx.dtsi +++ b/arch/arm/boot/dts/imx6sx.dtsi @@ -861,6 +861,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0","ptp"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, @@ -970,6 +971,7 @@ fec2: ethernet@021b4000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x021b4000 0x4000>; + interrupt-names = "int0","ptp"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index f11a241a340d..b624b5fc2d99 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -476,6 +476,7 @@ fec2: ethernet@020b4000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x020b4000 0x4000>; + interrupt-names = "int0","ptp"; interrupts = , ; clocks = < IMX6UL_CLK_ENET>, @@ -775,6 +776,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "int0","ptp"; interrupts = , ; clocks = < IMX6UL_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi index f46814a7ea44..312d24ff106e 100644 --- a/arch/arm/boot/dts/imx7d.dtsi +++ b/arch/arm/boot/dts/imx7d.dtsi @@ -114,9 +114,11 @@ fec2: ethernet@30bf { compatible = "fsl,imx7d-fec", "fsl,imx6sx-fec"; reg = <0x30bf 0x1>; - interrupts = , + interrupt-names = "int0","int1","int2","ptp"; + interrupts = , + , , - ; + ; clocks = < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET2_TIME_ROOT_CLK>, diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi index 82ad26e766eb..b00a31a50771 100644 --- a/arch/arm
Re: [PATCH net v1 1/2] ARM: dts: imx: let's name the ptp interrupt for the fec ethernet driver
On 10/2/2017 5:51 PM, Andrew Lunn wrote: > On Mon, Oct 02, 2017 at 05:04:41PM -0700, Troy Kisky wrote: >> imx7s/imx7d has the ptp interrupt newly added as well. >> This will allow the ptp interrupt to have its own interrupt routine. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> arch/arm/boot/dts/imx6qdl.dtsi | 1 + >> arch/arm/boot/dts/imx6sx.dtsi | 2 ++ >> arch/arm/boot/dts/imx6ul.dtsi | 2 ++ >> arch/arm/boot/dts/imx7d.dtsi | 4 +++- >> arch/arm/boot/dts/imx7s.dtsi | 4 +++- >> 5 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi >> index 8884b4a3cafb..d848d2bfe8e2 100644 >> --- a/arch/arm/boot/dts/imx6qdl.dtsi >> +++ b/arch/arm/boot/dts/imx6qdl.dtsi >> @@ -1017,6 +1017,7 @@ >> fec: ethernet@02188000 { >> compatible = "fsl,imx6q-fec"; >> reg = <0x02188000 0x4000>; >> +interrupt-names = "","ptp"; > > Hi Troy > > The "" looks a bit odd. Can you use a name here? > > Andrew > Sure. Can I use "q0","q1","q2", and look them up by name in fec_main as well ? Should I worrying about compatiblity with old dtbs ? I could look up by number if name fails. Maybe with a deprecated warning ? Thanks Troy
[PATCH net v1 1/2] ARM: dts: imx: let's name the ptp interrupt for the fec ethernet driver
imx7s/imx7d has the ptp interrupt newly added as well. This will allow the ptp interrupt to have its own interrupt routine. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- arch/arm/boot/dts/imx6qdl.dtsi | 1 + arch/arm/boot/dts/imx6sx.dtsi | 2 ++ arch/arm/boot/dts/imx6ul.dtsi | 2 ++ arch/arm/boot/dts/imx7d.dtsi | 4 +++- arch/arm/boot/dts/imx7s.dtsi | 4 +++- 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index 8884b4a3cafb..d848d2bfe8e2 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -1017,6 +1017,7 @@ fec: ethernet@02188000 { compatible = "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "","ptp"; interrupts-extended = < 0 118 IRQ_TYPE_LEVEL_HIGH>, < 0 119 IRQ_TYPE_LEVEL_HIGH>; diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index 6c7eb54be9e2..d64ae33c0b41 100644 --- a/arch/arm/boot/dts/imx6sx.dtsi +++ b/arch/arm/boot/dts/imx6sx.dtsi @@ -861,6 +861,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "","ptp"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, @@ -970,6 +971,7 @@ fec2: ethernet@021b4000 { compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec"; reg = <0x021b4000 0x4000>; + interrupt-names = "","ptp"; interrupts = , ; clocks = < IMX6SX_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index f11a241a340d..fe093f8531b0 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -476,6 +476,7 @@ fec2: ethernet@020b4000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x020b4000 0x4000>; + interrupt-names = "","ptp"; interrupts = , ; clocks = < IMX6UL_CLK_ENET>, @@ -775,6 +776,7 @@ fec1: ethernet@02188000 { compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec"; reg = <0x02188000 0x4000>; + interrupt-names = "","ptp"; interrupts = , ; clocks = < IMX6UL_CLK_ENET>, diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi index f46814a7ea44..e8fc76483236 100644 --- a/arch/arm/boot/dts/imx7d.dtsi +++ b/arch/arm/boot/dts/imx7d.dtsi @@ -114,9 +114,11 @@ fec2: ethernet@30bf { compatible = "fsl,imx7d-fec", "fsl,imx6sx-fec"; reg = <0x30bf 0x1>; + interrupt-names = "","","","ptp"; interrupts = , , - ; + , + ; clocks = < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET2_TIME_ROOT_CLK>, diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi index 82ad26e766eb..fc1376079494 100644 --- a/arch/arm/boot/dts/imx7s.dtsi +++ b/arch/arm/boot/dts/imx7s.dtsi @@ -1007,9 +1007,11 @@ fec1: ethernet@30be { compatible = "fsl,imx7d-fec", "fsl,imx6sx-fec"; reg = <0x30be 0x1>; + interrupt-names = "","","","ptp"; interrupts = , , - ; + , + ; clocks = < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET_AXI_ROOT_CLK>, < IMX7D_ENET1_TIME_ROOT_CLK>, -- 2.11.0
[PATCH net v1 2/2] net: fec: Let fec_ptp have its own interrupt routine
This is better for code locality and should slightly speed up normal interrupts. This also allows PPS clock output to start working for i.mx7. This is because i.mx7 was already using the limit of 3 interrupts, and needed another. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- If this patch is taken before the corresponding dtb file patch than time stamping will be momentarily broken. --- drivers/net/ethernet/freescale/fec.h | 1 - drivers/net/ethernet/freescale/fec_main.c | 15 +++--- drivers/net/ethernet/freescale/fec_ptp.c | 77 ++- 3 files changed, 52 insertions(+), 41 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ede1876a9a19..782509041102 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -587,7 +587,6 @@ void fec_ptp_stop(struct platform_device *pdev); void fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr); int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private *fep); // #endif /* FEC_H */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3dc2d771a222..80fe04165ba0 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id) ret = IRQ_HANDLED; complete(>mdio_done); } - - if (fep->ptp_clock) - if (fec_ptp_check_pps_event(fep)) - ret = IRQ_HANDLED; return ret; } @@ -3325,6 +3321,7 @@ fec_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node, *phy_node; int num_tx_qs; int num_rx_qs; + int ptp_irq, j; fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); @@ -3472,20 +3469,24 @@ fec_probe(struct platform_device *pdev) if (ret) goto failed_init; - for (i = 0; i < FEC_IRQ_NUM; i++) { - irq = platform_get_irq(pdev, i); + ptp_irq = platform_get_irq_byname(pdev, "ptp"); + i = j = 0; + while (i < FEC_IRQ_NUM) { + irq = platform_get_irq(pdev, j++); if (irq < 0) { if (i) break; ret = irq; goto failed_irq; } + if (irq == ptp_irq) + continue; ret = devm_request_irq(>dev, irq, fec_enet_interrupt, 0, pdev->name, ndev); if (ret) goto failed_irq; - fep->irq[i] = irq; + fep->irq[i++] = irq; } init_completion(>mdio_done); diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 6ebad3fac81d..20e01a3ca453 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -549,6 +549,38 @@ static void fec_time_keep(struct work_struct *work) schedule_delayed_work(>time_keep, HZ); } +/* + * This function checks the pps event and reloads the timer compare counter. + */ +static irqreturn_t fec_ptp_interrupt(int irq, void *dev_id) +{ + struct net_device *ndev = dev_id; + struct fec_enet_private *fep = netdev_priv(ndev); + u32 val; + u8 channel = fep->pps_channel; + struct ptp_clock_event event; + + val = readl(fep->hwp + FEC_TCSR(channel)); + if (val & FEC_T_TF_MASK) { + /* Write the next next compare(not the next according the spec) +* value to the register +*/ + writel(fep->next_counter, fep->hwp + FEC_TCCR(channel)); + do { + writel(val, fep->hwp + FEC_TCSR(channel)); + } while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK); + + /* Update the counter; */ + fep->next_counter = (fep->next_counter + fep->reload_period) & fep->cc.mask; + + event.type = PTP_CLOCK_PPS; + ptp_clock_event(fep->ptp_clock, ); + return IRQ_HANDLED; + } + + return IRQ_NONE; +} + /** * fec_ptp_init * @ndev: The FEC network adapter @@ -562,6 +594,8 @@ void fec_ptp_init(struct platform_device *pdev) { struct net_device *ndev = platform_get_drvdata(pdev); struct fec_enet_private *fep = netdev_priv(ndev); + int irq; + int ret; fep->ptp_caps.owner = THIS_MODULE; snprintf(fep->ptp_caps.name, 16, "fec ptp&qu
[PATCH v4 3/3] net: fec: return IRQ_HANDLED if fec_ptp_check_pps_event handled it
fec_ptp_check_pps_event will return 1 if FEC_T_TF_MASK caused an interrupt. Don't return IRQ_NONE in this case. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> Acked-by: Fugang Duan <fugang.d...@nxp.com> --- v3: New patch, came from feedback from another patch. v4: Added Acked-by Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 464055f..3dc2d77 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1604,8 +1604,8 @@ fec_enet_interrupt(int irq, void *dev_id) } if (fep->ptp_clock) - fec_ptp_check_pps_event(fep); - + if (fec_ptp_check_pps_event(fep)) + ret = IRQ_HANDLED; return ret; } -- 2.7.4
[PATCH v4 2/3] net: fec: remove unused interrupt FEC_ENET_TS_TIMER
FEC_ENET_TS_TIMER is not checked in the interrupt routine so there is no need to enable it. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> Acked-by: Fugang Duan <fugang.d...@nxp.com> --- v4: Added Acked-by Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 38c7b21..ede1876 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -374,8 +374,8 @@ struct bufdesc_ex { #define FEC_ENET_TS_AVAIL ((uint)0x0001) #define FEC_ENET_TS_TIMER ((uint)0x8000) -#define FEC_DEFAULT_IMASK (FEC_ENET_TXF | FEC_ENET_RXF | FEC_ENET_MII | FEC_ENET_TS_TIMER) -#define FEC_NAPI_IMASK (FEC_ENET_MII | FEC_ENET_TS_TIMER) +#define FEC_DEFAULT_IMASK (FEC_ENET_TXF | FEC_ENET_RXF | FEC_ENET_MII) +#define FEC_NAPI_IMASK FEC_ENET_MII #define FEC_RX_DISABLED_IMASK (FEC_DEFAULT_IMASK & (~FEC_ENET_RXF)) /* ENET interrupt coalescing macro define */ -- 2.7.4
[PATCH v4 1/3] net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set
Before queue 0 was always checked if any queue caused an interrupt. It is better to just mark queue 0 if queue 0 has caused an interrupt. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> Acked-by: Fugang Duan <fugang.d...@nxp.com> --- v3: add Acked-by v4: no change Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 56f56d6..464055f 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1559,14 +1559,14 @@ fec_enet_collect_events(struct fec_enet_private *fep, uint int_events) if (int_events == 0) return false; - if (int_events & FEC_ENET_RXF) + if (int_events & FEC_ENET_RXF_0) fep->work_rx |= (1 << 2); if (int_events & FEC_ENET_RXF_1) fep->work_rx |= (1 << 0); if (int_events & FEC_ENET_RXF_2) fep->work_rx |= (1 << 1); - if (int_events & FEC_ENET_TXF) + if (int_events & FEC_ENET_TXF_0) fep->work_tx |= (1 << 2); if (int_events & FEC_ENET_TXF_1) fep->work_tx |= (1 << 0); -- 2.7.4
[PATCH net v2 1/1] net: fec: update dirty_tx even if no skb
If dirty_tx isn't updated, then dma_unmap_single can be called twice. This fixes a [ 58.420980] [ cut here ] [ 58.425667] WARNING: CPU: 0 PID: 377 at /home/schurig/d/mkarm/linux-4.5/lib/dma-debug.c:1096 check_unmap+0x9d0/0xab8() [ 58.436405] fec 2188000.ethernet: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x] [size=66 bytes] encountered by Holger Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> Tested-by: <holgerschu...@gmail.com> Acked-by: Fugang Duan <fugang.d...@nxp.com> --- v2: add ack --- drivers/net/ethernet/freescale/fec_main.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index ca2..3c0255e 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1197,10 +1197,8 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) fec16_to_cpu(bdp->cbd_datlen), DMA_TO_DEVICE); bdp->cbd_bufaddr = cpu_to_fec32(0); - if (!skb) { - bdp = fec_enet_get_nextdesc(bdp, >bd); - continue; - } + if (!skb) + goto skb_done; /* Check for errors. */ if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | @@ -1239,7 +1237,7 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) /* Free the sk buffer associated with this last transmit */ dev_kfree_skb_any(skb); - +skb_done: /* Make sure the update to bdp and tx_skbuff are performed * before dirty_tx */ -- 2.5.0
Re: [PATCH net 1/1] net: fec: update dirty_tx even if no skb
On 4/21/2016 10:59 PM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Friday, April 22, > 2016 10:01 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; Fugang Duan >> <fugang.d...@nxp.com>; lzn...@gmail.com >> Cc: Fabio Estevam <fabio.este...@nxp.com>; l.st...@pengutronix.de; >> and...@lunn.ch; trem...@gmail.com; g...@uclinux.org; linux-arm- >> ker...@lists.infradead.org; johan...@sipsolutions.net; >> stillcompil...@gmail.com; sergei.shtyl...@cogentembedded.com; >> a...@arndb.de; holgerschu...@gmail.com; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net 1/1] net: fec: update dirty_tx even if no skb >> >> If dirty_tx isn't updated, then dma_unmap_single will be called twice. >> >> This fixes a >> [ 58.420980] [ cut here ] >> [ 58.425667] WARNING: CPU: 0 PID: 377 at /home/schurig/d/mkarm/linux- >> 4.5/lib/dma-debug.c:1096 check_unmap+0x9d0/0xab8() >> [ 58.436405] fec 2188000.ethernet: DMA-API: device driver tries to free DMA >> memory it has not allocated [device address=0x] [size=66 >> bytes] >> >> encountered by Holger >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> Tested-by: <holgerschu...@gmail.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 8 +++- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 08243c2..b71654c 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -1197,10 +1197,8 @@ fec_enet_tx_queue(struct net_device *ndev, u16 >> queue_id) >> fec16_to_cpu(bdp->cbd_datlen), >> DMA_TO_DEVICE); >> bdp->cbd_bufaddr = cpu_to_fec32(0); >> -if (!skb) { >> -bdp = fec_enet_get_nextdesc(bdp, >bd); >> -continue; >> -} >> +if (!skb) >> +goto skb_done; >> >> /* Check for errors. */ >> if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | @@ -1239,7 >> +1237,7 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) >> >> /* Free the sk buffer associated with this last transmit */ >> dev_kfree_skb_any(skb); >> - >> +skb_done: >> /* Make sure the update to bdp and tx_skbuff are performed >> * before dirty_tx >> */ >> -- >> 2.5.0 > > The patch is fine for me. > Can you review below patch that also fix the issue. It can take much effort > due to less rmb() and READ_ONCE() operation that is very sensitive for duplex > Gbps test for i.MX6SX/i.MX7d SOC. (i.MX6SX can reach at 1.4Gbps, i.MX7D can > reach at 1.8Gbps.) > Am I supposed to do anything else to get this patch into net ?
Re: [PATCH net 1/1] net: fec: update dirty_tx even if no skb
On 4/21/2016 10:59 PM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Friday, April 22, > 2016 10:01 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; Fugang Duan >> <fugang.d...@nxp.com>; lzn...@gmail.com >> Cc: Fabio Estevam <fabio.este...@nxp.com>; l.st...@pengutronix.de; >> and...@lunn.ch; trem...@gmail.com; g...@uclinux.org; linux-arm- >> ker...@lists.infradead.org; johan...@sipsolutions.net; >> stillcompil...@gmail.com; sergei.shtyl...@cogentembedded.com; >> a...@arndb.de; holgerschu...@gmail.com; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net 1/1] net: fec: update dirty_tx even if no skb >> >> If dirty_tx isn't updated, then dma_unmap_single will be called twice. >> >> This fixes a >> [ 58.420980] [ cut here ] >> [ 58.425667] WARNING: CPU: 0 PID: 377 at /home/schurig/d/mkarm/linux- >> 4.5/lib/dma-debug.c:1096 check_unmap+0x9d0/0xab8() >> [ 58.436405] fec 2188000.ethernet: DMA-API: device driver tries to free DMA >> memory it has not allocated [device address=0x] [size=66 >> bytes] >> >> encountered by Holger >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> Tested-by: <holgerschu...@gmail.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 8 +++- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 08243c2..b71654c 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -1197,10 +1197,8 @@ fec_enet_tx_queue(struct net_device *ndev, u16 >> queue_id) >> fec16_to_cpu(bdp->cbd_datlen), >> DMA_TO_DEVICE); >> bdp->cbd_bufaddr = cpu_to_fec32(0); >> -if (!skb) { >> -bdp = fec_enet_get_nextdesc(bdp, >bd); >> -continue; >> -} >> +if (!skb) >> +goto skb_done; >> >> /* Check for errors. */ >> if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | @@ -1239,7 >> +1237,7 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) >> >> /* Free the sk buffer associated with this last transmit */ >> dev_kfree_skb_any(skb); >> - >> +skb_done: >> /* Make sure the update to bdp and tx_skbuff are performed >> * before dirty_tx >> */ >> -- >> 2.5.0 > > The patch is fine for me. > Can you review below patch that also fix the issue. It can take much effort > due to less rmb() and READ_ONCE() operation that is very sensitive for duplex > Gbps test for i.MX6SX/i.MX7d SOC. (i.MX6SX can reach at 1.4Gbps, i.MX7D can > reach at 1.8Gbps.) If "READ_ONCE(bdp->cbd_sc)" is really that expensive, then you should skip the 1st read as well and only do it after skb presence is verified. But some numbers would really help sell the patch. Also, I comment as to why the code is reorganized would be warranted > > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1160,12 +1160,13 @@ static void > fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) > { > struct fec_enet_private *fep; > - struct bufdesc *bdp; > + struct bufdesc *bdp, *bdp_t; > unsigned short status; > struct sk_buff *skb; > struct fec_enet_priv_tx_q *txq; > struct netdev_queue *nq; > int index = 0; > + int i, bdnum; > int entries_free; > > fep = netdev_priv(ndev); > @@ -1187,20 +1188,28 @@ fec_enet_tx_queue(struct net_device *ndev, u16 > queue_id) > if (status & BD_ENET_TX_READY) > break; > > - index = fec_enet_get_bd_index(bdp, >bd); > - > + bdp_t = bdp; > + bdnum = 1; > + index = fec_enet_get_bd_index(txq->tx_bd_base, bdp_t, fep); > skb = txq->tx_skbuff[index]; > - txq->tx_skbuff[index] = NULL; > - if (!IS_TSO_HEADER(txq, fec32_to_cpu(bdp->cbd_bufaddr))) > - dma_unmap_single(>pdev->dev, > -fec32_to_cpu(bdp->cbd_bufaddr), > -
[PATCH net 1/1] net: fec: update dirty_tx even if no skb
If dirty_tx isn't updated, then dma_unmap_single will be called twice. This fixes a [ 58.420980] [ cut here ] [ 58.425667] WARNING: CPU: 0 PID: 377 at /home/schurig/d/mkarm/linux-4.5/lib/dma-debug.c:1096 check_unmap+0x9d0/0xab8() [ 58.436405] fec 2188000.ethernet: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x] [size=66 bytes] encountered by Holger Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> Tested-by: <holgerschu...@gmail.com> --- drivers/net/ethernet/freescale/fec_main.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 08243c2..b71654c 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1197,10 +1197,8 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) fec16_to_cpu(bdp->cbd_datlen), DMA_TO_DEVICE); bdp->cbd_bufaddr = cpu_to_fec32(0); - if (!skb) { - bdp = fec_enet_get_nextdesc(bdp, >bd); - continue; - } + if (!skb) + goto skb_done; /* Check for errors. */ if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | @@ -1239,7 +1237,7 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) /* Free the sk buffer associated with this last transmit */ dev_kfree_skb_any(skb); - +skb_done: /* Make sure the update to bdp and tx_skbuff are performed * before dirty_tx */ -- 2.5.0
Re: [PATCH net-next V3 00/16] net: fec: cleanup and fixes
On 4/14/2016 3:13 AM, Holger Schurig wrote: > Do you guys that work with the FEC driver ever run with > CONFIG_DMA_API_DEBUG enabled? > > I ask this Because I get this error when it's turned on when I do some > "rsync" transfer to my device: > > [ 58.420980] [ cut here ] > [ 58.425667] WARNING: CPU: 0 PID: 377 at > /home/schurig/d/mkarm/linux-4.5/lib/dma-debug.c:1096 check_unmap+0x9d0/0xab8() > [ 58.436405] fec 2188000.ethernet: DMA-API: device driver tries to free DMA > memory it has not allocated [device address=0x] [size=66 > bytes] > [ 58.450248] Modules linked in: bnep usbhid imx_sdma flexcan btusb btrtl > btbcm btintel smsc95xx usbnet mii bluetooth > [ 58.460882] CPU: 0 PID: 377 Comm: sshd Tainted: GW 4.5.1 #3 > [ 58.467671] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > [ 58.474199] Backtrace: > [ 58.476675] [] (dump_backtrace) from [] > (show_stack+0x18/0x1c) > [ 58.484244] r6:6113 r5:c05a96c0 r4: r3: > [ 58.489964] [] (show_stack) from [] > (dump_stack+0x9c/0xb0) > [ 58.497197] [] (dump_stack) from [] > (warn_slowpath_common+0x8c/0xbc) > [ 58.505286] r6:c01f9c74 r5:0009 r4:ee9f17f8 r3:c0596da4 > [ 58.511002] [] (warn_slowpath_common) from [] > (warn_slowpath_fmt+0x38/0x40) > [ 58.519698] r8:0042 r7:0001 r6: r5: r4:c050c020 > [ 58.526470] [] (warn_slowpath_fmt) from [] > (check_unmap+0x9d0/0xab8) > [ 58.534559] r3:c0520e6c r2:c050c020 > [ 58.538159] r4: > [ 58.540710] [] (check_unmap) from [] > (debug_dma_unmap_page+0x84/0x8c) > [ 58.548886] r10:ef2ec000 r9:f09e5fa0 r8:ef0ef810 r7:0001 r6: > r5:0042 > [ 58.556780] r4:0001 > [ 58.559336] [] (debug_dma_unmap_page) from [] > (fec_txq+0x140/0x31c) > [ 58.567338] r8:ef0ef810 r7: r6: r5: r4:ef2c6000 > [ 58.574108] [] (fec_txq) from [] > (fec_enet_napi_q1+0x98/0xe8) > [ 58.581589] r10:0800 r9:ef2ec580 r8: r7:0040 r6:0000 > r5:ef2ec000 I think I've already fixed this, but I've only submitted once. commit 466cb4a2e5583d2e18470f30d5948edcf4b947f5 Author: Troy Kisky <troy.ki...@boundarydevices.com> Date: Wed Jan 20 12:52:10 2016 -0700 net: fec: update dirty_tx even if no skb If dirty_tx isn't updated, then dma_unmap_single will be called twice. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 452be9c..150a90a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1243,10 +1243,8 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_priv_tx_q *txq) fec16_to_cpu(bdp->cbd_datlen), DMA_TO_DEVICE); bdp->cbd_bufaddr = cpu_to_fec32(0); - if (!skb) { - bdp = fec_enet_get_nextdesc(bdp, >bd); - continue; - } + if (!skb) + goto skb_done; /* Check for errors. */ if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | @@ -1285,7 +1283,7 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_priv_tx_q *txq) /* Free the sk buffer associated with this last transmit */ dev_kfree_skb_any(skb); - +skb_done: /* Make sure the update to bdp and tx_skbuff are performed * before dirty_tx */
Re: [PATCH net-next V3 05/16] net: fec: reduce interrupts
On 4/6/2016 8:57 PM, David Miller wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> > Date: Wed, 6 Apr 2016 17:42:47 -0700 > >> Sure, that's an easy change. But if a TX int is what caused the >> interrupt and masks them, and then a RX packet happens before napi >> runs, do you want the TX serviced 1st, or RX ? > > If you properly split your driver up into seperate interrupt/poll > instances, one for TX one for RX, you wouldn't need to ask me > that question now would you? > > :-) > I absolutely claim no ownership :-)
Re: [PATCH net-next V3 00/16] net: fec: cleanup and fixes
On 4/6/2016 8:58 PM, David Miller wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> > Date: Wed, 6 Apr 2016 18:09:17 -0700 > >> On 4/6/2016 2:20 PM, David Miller wrote: >>> >>> This is a way too large patch series. >>> >>> Please split it up into smaller, more logical, pieces. >>> >>> Thanks. >>> >> >> If you apply the 1st 3 that have been acked, I'll be at 13. >> >> Would I then send the next 5 for V4, and when that is applied >> send another V4 with the next 8 that have been already been acked? > > What other reasonable option is there? I can't think of any. > A V1 for the next 8 would not be too unreasonable.
Re: [PATCH net-next V3 00/16] net: fec: cleanup and fixes
On 4/6/2016 2:20 PM, David Miller wrote: > > This is a way too large patch series. > > Please split it up into smaller, more logical, pieces. > > Thanks. > If you apply the 1st 3 that have been acked, I'll be at 13. Would I then send the next 5 for V4, and when that is applied send another V4 with the next 8 that have been already been acked? Thanks Troy
Re: [PATCH net-next V3 05/16] net: fec: reduce interrupts
On 4/6/2016 2:20 PM, David Miller wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> > Date: Tue, 5 Apr 2016 19:25:51 -0700 > >> By clearing the NAPI interrupts in the NAPI routine >> and not in the interrupt handler, we can reduce the >> number of interrupts. We also don't need any status >> variables as the registers are still valid. >> >> Also, notice that if budget pkts are received, the >> next call to fec_enet_rx_napi will now continue to >> receive the previously pending packets. >> >> To test that this actually reduces interrupts, try >> this command before/after patch >> >> cat /proc/interrupts |grep ether; \ >> ping -s2800 192.168.0.201 -f -c1000 ; \ >> cat /proc/interrupts |grep ether >> >> For me, before this patch is 2996 interrupts. >> After patch is 2010 interrupts. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> > > I really don't think this is a good idea at all. > > I would instead really rather see you stash away the > status register values into some piece of software state, > and then re-read them before you are about to finish a > NAPI poll cycle. > > Sure, that's an easy change. But if a TX int is what caused the interrupt and masks them, and then a RX packet happens before napi runs, do you want the TX serviced 1st, or RX ?
Re: [PATCH net-next V3 00/16] net: fec: cleanup and fixes
On 4/6/2016 9:43 AM, Troy Kisky wrote: > On 4/6/2016 1:51 AM, Fugang Duan wrote: >> From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Wednesday, April 06, >> 2016 10:26 AM >>> To: netdev@vger.kernel.org; da...@davemloft.net; Fugang Duan >>> <fugang.d...@nxp.com>; lzn...@gmail.com >>> Cc: Fabio Estevam <fabio.este...@nxp.com>; l.st...@pengutronix.de; >>> and...@lunn.ch; trem...@gmail.com; g...@uclinux.org; linux-arm- >>> ker...@lists.infradead.org; johan...@sipsolutions.net; >>> stillcompil...@gmail.com; sergei.shtyl...@cogentembedded.com; >>> a...@arndb.de; Troy Kisky <troy.ki...@boundarydevices.com> >>> Subject: [PATCH net-next V3 00/16] net: fec: cleanup and fixes >>> >>> V3 has >>> >>> 1 dropped patch "net: fec: print more debug info in fec_timeout" >>> 2 new patches >>> 0002-net-fec-remove-unused-interrupt-FEC_ENET_TS_TIMER.patch >>> 0003-net-fec-return-IRQ_HANDLED-if-fec_ptp_check_pps_even.patch >>> >>> 1 combined patch >>> 0004-net-fec-pass-rxq-txq-to-fec_enet_rx-tx_queue-instead.patch >>> >>> The changes are noted on individual patches >>> >>> My measured performance of this series is >>> >>> before patch set >>> 365 Mbits/sec Tx/407 RX >>> >>> after patch set >>> 374 Tx/427 Rx >>> >> >> I doubt the performance data, I validate it on i.MX6q sabresd board on the >> latest commit(4da46cebbd3b) in net tree. > > > > I was doing UDP tests, as outlined in my V2 cover letter. Also, my cpu is 1G. > Is yours 1.2G? > > > > >> root@imx6qdlsolo:~# uname -a >> Linux imx6qdlsolo 4.6.0-rc1-00318-g4da46ce #180 SMP Wed Apr 6 16:24:09 CST >> 2016 armv7l GNU/Linux > > > This is the V2 patch that I dropped. Sorry, your right. It is the current head, without this series. > > I will force update my local net-next_master branch, to make testing this > series easier. > Note that my local net-next_master branch has about 19 patches on top of this > series. > so, > > tkisky@office-server2:~/linux-imx6$ git reset --hard HEAD~19 > HEAD is now at a125da7 net: fec: don't set cbd_bufaddr unless no mapping error > > >> >> TCP RX performance is 602Mbps, TX is only 325Mbps, TX path has some >> performance issue in net tree. >> I will dig out it. >> >> > > > More testing is always better. Thanks > > >>> >>> Troy Kisky (16): >>> net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set >>> net: fec: remove unused interrupt FEC_ENET_TS_TIMER >>> net: fec: return IRQ_HANDLED if fec_ptp_check_pps_event handled it >>> net: fec: pass rxq/txq to fec_enet_rx/tx_queue instead of queue_id >>> net: fec: reduce interrupts >>> net: fec: split off napi routine with 3 queues >>> net: fec: don't clear all rx queue bits when just one is being checked >>> net: fec: set cbd_sc without relying on previous value >>> net: fec: eliminate calls to fec_enet_get_prevdesc >>> net: fec: move restart test for efficiency >>> net: fec: clear cbd_sc after transmission to help with debugging >>> net: fec: dump all tx queues in fec_dump >>> net: fec: detect tx int lost >>> net: fec: create subroutine reset_tx_queue >>> net: fec: call dma_unmap_single on mapped tx buffers at restart >>> net: fec: don't set cbd_bufaddr unless no mapping error >>> >>> drivers/net/ethernet/freescale/fec.h | 10 +- >>> drivers/net/ethernet/freescale/fec_main.c | 410 >>> >>> -- >>> 2 files changed, 218 insertions(+), 202 deletions(-) >>> >>> -- >>> 2.5.0 >>
Re: [PATCH net-next V3 00/16] net: fec: cleanup and fixes
On 4/6/2016 1:51 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Wednesday, April 06, > 2016 10:26 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; Fugang Duan >> <fugang.d...@nxp.com>; lzn...@gmail.com >> Cc: Fabio Estevam <fabio.este...@nxp.com>; l.st...@pengutronix.de; >> and...@lunn.ch; trem...@gmail.com; g...@uclinux.org; linux-arm- >> ker...@lists.infradead.org; johan...@sipsolutions.net; >> stillcompil...@gmail.com; sergei.shtyl...@cogentembedded.com; >> a...@arndb.de; Troy Kisky <troy.ki...@boundarydevices.com> >> Subject: [PATCH net-next V3 00/16] net: fec: cleanup and fixes >> >> V3 has >> >> 1 dropped patch "net: fec: print more debug info in fec_timeout" >> 2 new patches >> 0002-net-fec-remove-unused-interrupt-FEC_ENET_TS_TIMER.patch >> 0003-net-fec-return-IRQ_HANDLED-if-fec_ptp_check_pps_even.patch >> >> 1 combined patch >> 0004-net-fec-pass-rxq-txq-to-fec_enet_rx-tx_queue-instead.patch >> >> The changes are noted on individual patches >> >> My measured performance of this series is >> >> before patch set >> 365 Mbits/sec Tx/407 RX >> >> after patch set >> 374 Tx/427 Rx >> > > I doubt the performance data, I validate it on i.MX6q sabresd board on the > latest commit(4da46cebbd3b) in net tree. I was doing UDP tests, as outlined in my V2 cover letter. Also, my cpu is 1G. Is yours 1.2G? > root@imx6qdlsolo:~# uname -a > Linux imx6qdlsolo 4.6.0-rc1-00318-g4da46ce #180 SMP Wed Apr 6 16:24:09 CST > 2016 armv7l GNU/Linux This is the V2 patch that I dropped. I will force update my local net-next_master branch, to make testing this series easier. Note that my local net-next_master branch has about 19 patches on top of this series. so, tkisky@office-server2:~/linux-imx6$ git reset --hard HEAD~19 HEAD is now at a125da7 net: fec: don't set cbd_bufaddr unless no mapping error > > TCP RX performance is 602Mbps, TX is only 325Mbps, TX path has some > performance issue in net tree. > I will dig out it. > > More testing is always better. Thanks >> >> Troy Kisky (16): >> net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set >> net: fec: remove unused interrupt FEC_ENET_TS_TIMER >> net: fec: return IRQ_HANDLED if fec_ptp_check_pps_event handled it >> net: fec: pass rxq/txq to fec_enet_rx/tx_queue instead of queue_id >> net: fec: reduce interrupts >> net: fec: split off napi routine with 3 queues >> net: fec: don't clear all rx queue bits when just one is being checked >> net: fec: set cbd_sc without relying on previous value >> net: fec: eliminate calls to fec_enet_get_prevdesc >> net: fec: move restart test for efficiency >> net: fec: clear cbd_sc after transmission to help with debugging >> net: fec: dump all tx queues in fec_dump >> net: fec: detect tx int lost >> net: fec: create subroutine reset_tx_queue >> net: fec: call dma_unmap_single on mapped tx buffers at restart >> net: fec: don't set cbd_bufaddr unless no mapping error >> >> drivers/net/ethernet/freescale/fec.h | 10 +- >> drivers/net/ethernet/freescale/fec_main.c | 410 >> -- >> 2 files changed, 218 insertions(+), 202 deletions(-) >> >> -- >> 2.5.0 >
[PATCH net-next V3 13/16] net: fec: detect tx int lost
If a tx int is lost, no need to reset the fec. Just mark the event and call napi_schedule. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: no change --- drivers/net/ethernet/freescale/fec_main.c | 38 ++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index be875fd..445443d 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1094,14 +1094,50 @@ fec_stop(struct net_device *ndev) } } +static const uint txint_flags[] = { + FEC_ENET_TXF_0, FEC_ENET_TXF_1, FEC_ENET_TXF_2 +}; static void fec_timeout(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); + struct bufdesc *bdp; + unsigned short status; + int i; + uint events = 0; - fec_dump(ndev); + for (i = 0; i < fep->num_tx_queues; i++) { + struct fec_enet_priv_tx_q *txq = fep->tx_queue[i]; + int index; + struct sk_buff *skb = NULL; + bdp = txq->dirty_tx; + while (1) { + bdp = fec_enet_get_nextdesc(bdp, >bd); + if (bdp == txq->bd.cur) + break; + index = fec_enet_get_bd_index(bdp, >bd); + skb = txq->tx_skbuff[index]; + if (skb) { + status = fec16_to_cpu(bdp->cbd_sc); + if ((status & BD_ENET_TX_READY) == 0) + events |= txint_flags[i]; + break; + } + } + } + if (events) { + fep->events |= events; + /* Disable the RX/TX interrupt */ + writel(FEC_NAPI_IMASK, fep->hwp + FEC_IMASK); + napi_schedule(>napi); + netif_wake_queue(fep->netdev); + pr_err("%s: tx int lost\n", __func__); + return; + } + + fec_dump(ndev); ndev->stats.tx_errors++; schedule_work(>tx_timeout_work); -- 2.5.0
[PATCH net-next V3 09/16] net: fec: eliminate calls to fec_enet_get_prevdesc
Eliminating calls to fec_enet_get_prevdesc shrinks the code a little. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: Change commit message s/unsigned status/unsigned int status/ as requested --- drivers/net/ethernet/freescale/fec_main.c | 37 +-- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 21d2cd0..349fda1 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -758,6 +758,7 @@ static void fec_enet_bd_init(struct net_device *dev) struct bufdesc *bdp; unsigned int i; unsigned int q; + unsigned int status; for (q = 0; q < fep->num_rx_queues; q++) { /* Initialize the receive buffer descriptors. */ @@ -765,19 +766,13 @@ static void fec_enet_bd_init(struct net_device *dev) bdp = rxq->bd.base; for (i = 0; i < rxq->bd.ring_size; i++) { - /* Initialize the BD for every fragment in the page. */ - if (bdp->cbd_bufaddr) - bdp->cbd_sc = cpu_to_fec16(BD_ENET_RX_EMPTY); - else - bdp->cbd_sc = cpu_to_fec16(0); + status = bdp->cbd_bufaddr ? BD_ENET_RX_EMPTY : 0; + if (bdp == rxq->bd.last) + status |= BD_SC_WRAP; + bdp->cbd_sc = cpu_to_fec16(status); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap */ - bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); - rxq->bd.cur = rxq->bd.base; } @@ -789,18 +784,16 @@ static void fec_enet_bd_init(struct net_device *dev) for (i = 0; i < txq->bd.ring_size; i++) { /* Initialize the BD for every fragment in the page. */ - bdp->cbd_sc = cpu_to_fec16(0); if (txq->tx_skbuff[i]) { dev_kfree_skb_any(txq->tx_skbuff[i]); txq->tx_skbuff[i] = NULL; } bdp->cbd_bufaddr = cpu_to_fec32(0); + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap */ bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); txq->dirty_tx = bdp; } } @@ -2717,19 +2710,16 @@ fec_enet_alloc_rxq_buffers(struct net_device *ndev, unsigned int queue) } rxq->rx_skbuff[i] = skb; - bdp->cbd_sc = cpu_to_fec16(BD_ENET_RX_EMPTY); if (fep->bufdesc_ex) { struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; ebdp->cbd_esc = cpu_to_fec32(BD_ENET_RX_INT); } + bdp->cbd_sc = cpu_to_fec16(BD_ENET_RX_EMPTY | + ((bdp == rxq->bd.last) ? BD_SC_WRAP : 0)); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap. */ - bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); return 0; err_alloc: @@ -2752,21 +2742,16 @@ fec_enet_alloc_txq_buffers(struct net_device *ndev, unsigned int queue) if (!txq->tx_bounce[i]) goto err_alloc; - bdp->cbd_sc = cpu_to_fec16(0); bdp->cbd_bufaddr = cpu_to_fec32(0); if (fep->bufdesc_ex) { struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; ebdp->cbd_esc = cpu_to_fec32(BD_ENET_TX_INT); } - + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap. */ - bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); - return 0; err_alloc: -- 2.5.0
[PATCH net-next V3 14/16] net: fec: create subroutine reset_tx_queue
Create subroutine reset_tx_queue to have one place to release any queued tx skbs. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: change commit message --- drivers/net/ethernet/freescale/fec_main.c | 50 +++ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 445443d..a38acf2 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -752,12 +752,33 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) return NETDEV_TX_OK; } +static void reset_tx_queue(struct fec_enet_private *fep, + struct fec_enet_priv_tx_q *txq) +{ + struct bufdesc *bdp = txq->bd.base; + unsigned int i; + + txq->bd.cur = bdp; + for (i = 0; i < txq->bd.ring_size; i++) { + /* Initialize the BD for every fragment in the page. */ + if (txq->tx_skbuff[i]) { + dev_kfree_skb_any(txq->tx_skbuff[i]); + txq->tx_skbuff[i] = NULL; + } + bdp->cbd_bufaddr = cpu_to_fec32(0); + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); + bdp = fec_enet_get_nextdesc(bdp, >bd); + } + bdp = fec_enet_get_prevdesc(bdp, >bd); + txq->dirty_tx = bdp; +} + /* Init RX & TX buffer descriptors */ static void fec_enet_bd_init(struct net_device *dev) { struct fec_enet_private *fep = netdev_priv(dev); - struct fec_enet_priv_tx_q *txq; struct fec_enet_priv_rx_q *rxq; struct bufdesc *bdp; unsigned int i; @@ -780,26 +801,8 @@ static void fec_enet_bd_init(struct net_device *dev) rxq->bd.cur = rxq->bd.base; } - for (q = 0; q < fep->num_tx_queues; q++) { - /* ...and the same for transmit */ - txq = fep->tx_queue[q]; - bdp = txq->bd.base; - txq->bd.cur = bdp; - - for (i = 0; i < txq->bd.ring_size; i++) { - /* Initialize the BD for every fragment in the page. */ - if (txq->tx_skbuff[i]) { - dev_kfree_skb_any(txq->tx_skbuff[i]); - txq->tx_skbuff[i] = NULL; - } - bdp->cbd_bufaddr = cpu_to_fec32(0); - bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? - BD_SC_WRAP : 0); - bdp = fec_enet_get_nextdesc(bdp, >bd); - } - bdp = fec_enet_get_prevdesc(bdp, >bd); - txq->dirty_tx = bdp; - } + for (q = 0; q < fep->num_tx_queues; q++) + reset_tx_queue(fep, fep->tx_queue[q]); } static void fec_enet_active_rxring(struct net_device *ndev) @@ -2648,13 +2651,10 @@ static void fec_enet_free_buffers(struct net_device *ndev) for (q = 0; q < fep->num_tx_queues; q++) { txq = fep->tx_queue[q]; - bdp = txq->bd.base; + reset_tx_queue(fep, txq); for (i = 0; i < txq->bd.ring_size; i++) { kfree(txq->tx_bounce[i]); txq->tx_bounce[i] = NULL; - skb = txq->tx_skbuff[i]; - txq->tx_skbuff[i] = NULL; - dev_kfree_skb(skb); } } } -- 2.5.0
[PATCH net-next V3 05/16] net: fec: reduce interrupts
By clearing the NAPI interrupts in the NAPI routine and not in the interrupt handler, we can reduce the number of interrupts. We also don't need any status variables as the registers are still valid. Also, notice that if budget pkts are received, the next call to fec_enet_rx_napi will now continue to receive the previously pending packets. To test that this actually reduces interrupts, try this command before/after patch cat /proc/interrupts |grep ether; \ ping -s2800 192.168.0.201 -f -c1000 ; \ cat /proc/interrupts |grep ether For me, before this patch is 2996 interrupts. After patch is 2010 interrupts. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: Fix introduced bug of checking for FEC_ENET_TS_TIMER before calling fec_ptp_check_pps_event Changed commit message to show measured changes. Used netdev_info instead of pr_info. Fugang Duan suggested splitting TX and RX into two NAPI contexts, but that should be a separate patch as it is unrelated to what this patch does. --- drivers/net/ethernet/freescale/fec.h | 6 +- drivers/net/ethernet/freescale/fec_main.c | 118 +++--- 2 files changed, 45 insertions(+), 79 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 6dd0ba8..9d5bdc6 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -505,11 +505,7 @@ struct fec_enet_private { unsigned int total_tx_ring_size; unsigned int total_rx_ring_size; - - unsigned long work_tx; - unsigned long work_rx; - unsigned long work_ts; - unsigned long work_mdio; + uintevents; struct platform_device *pdev; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index b4d46f8..918ac82 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -70,8 +70,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev); #define DRIVER_NAME"fec" -#define FEC_ENET_GET_QUQUE(_x) ((_x == 0) ? 1 : ((_x == 1) ? 2 : 0)) - /* Pause frame feild and FIFO threshold */ #define FEC_ENET_FCE (1 << 5) #define FEC_ENET_RSEM_V0x84 @@ -1257,21 +1255,6 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_priv_tx_q *txq) writel(0, txq->bd.reg_desc_active); } -static void -fec_enet_tx(struct net_device *ndev) -{ - struct fec_enet_private *fep = netdev_priv(ndev); - struct fec_enet_priv_tx_q *txq; - u16 queue_id; - /* First process class A queue, then Class B and Best Effort queue */ - for_each_set_bit(queue_id, >work_tx, FEC_ENET_MAX_TX_QS) { - clear_bit(queue_id, >work_tx); - txq = fep->tx_queue[FEC_ENET_GET_QUQUE(queue_id)]; - fec_txq(ndev, txq); - } - return; -} - static int fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff *skb) { @@ -1505,70 +1488,34 @@ rx_processing_done: return pkt_received; } -static int -fec_enet_rx(struct net_device *ndev, int budget) -{ - int pkt_received = 0; - u16 queue_id; - struct fec_enet_private *fep = netdev_priv(ndev); - struct fec_enet_priv_rx_q *rxq; - - for_each_set_bit(queue_id, >work_rx, FEC_ENET_MAX_RX_QS) { - clear_bit(queue_id, >work_rx); - rxq = fep->rx_queue[FEC_ENET_GET_QUQUE(queue_id)]; - pkt_received += fec_rxq(ndev, rxq, budget - pkt_received); - } - return pkt_received; -} - -static bool -fec_enet_collect_events(struct fec_enet_private *fep, uint int_events) -{ - if (int_events == 0) - return false; - - if (int_events & FEC_ENET_RXF_0) - fep->work_rx |= (1 << 2); - if (int_events & FEC_ENET_RXF_1) - fep->work_rx |= (1 << 0); - if (int_events & FEC_ENET_RXF_2) - fep->work_rx |= (1 << 1); - - if (int_events & FEC_ENET_TXF_0) - fep->work_tx |= (1 << 2); - if (int_events & FEC_ENET_TXF_1) - fep->work_tx |= (1 << 0); - if (int_events & FEC_ENET_TXF_2) - fep->work_tx |= (1 << 1); - - return true; -} - static irqreturn_t fec_enet_interrupt(int irq, void *dev_id) { struct net_device *ndev = dev_id; struct fec_enet_private *fep = netdev_priv(ndev); - uint int_events; irqreturn_t ret = IRQ_NONE; + uint eir = readl(fep->hwp + FEC_IEVENT); + uint int_events = eir & readl(fep->hwp + FEC_IMASK); - int_events = readl(fep->hwp + FEC_IEVENT); - writel(int_events, fep->hwp + FEC_IEVENT); - fec_enet_collect_events(fep, int_events); - - if ((fep->work_tx || fep->work_rx) && f
[PATCH net-next V3 10/16] net: fec: move restart test for efficiency
Move restart test to earlier in fec_txq() which saves one comparison. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: change commit message --- drivers/net/ethernet/freescale/fec_main.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 349fda1..a2a9dca 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1157,8 +1157,13 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_priv_tx_q *txq) /* Order the load of bd.cur and cbd_sc */ rmb(); status = fec16_to_cpu(READ_ONCE(bdp->cbd_sc)); - if (status & BD_ENET_TX_READY) + if (status & BD_ENET_TX_READY) { + if (!readl(txq->bd.reg_desc_active)) { + /* ERR006358 has hit, restart tx */ + writel(0, txq->bd.reg_desc_active); + } break; + } index = fec_enet_get_bd_index(bdp, >bd); @@ -1230,11 +1235,6 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_priv_tx_q *txq) netif_tx_wake_queue(nq); } } - - /* ERR006538: Keep the transmitter going */ - if (bdp != txq->bd.cur && - readl(txq->bd.reg_desc_active) == 0) - writel(0, txq->bd.reg_desc_active); } static int -- 2.5.0
[PATCH net-next V3 11/16] net: fec: clear cbd_sc after transmission to help with debugging
When the tx queue is dumped, it is easier to see that this entry is idle if cbd_sc is cleared after transmission. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: change commit message --- drivers/net/ethernet/freescale/fec_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index a2a9dca..f96ea97 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1164,6 +1164,8 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_priv_tx_q *txq) } break; } + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); index = fec_enet_get_bd_index(bdp, >bd); -- 2.5.0
[PATCH net-next V3 12/16] net: fec: dump all tx queues in fec_dump
Dump all tx queues, not just queue 0. Also, disable fec interrupts first. The interrupts will be reenabled in fec_restart. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: no change --- drivers/net/ethernet/freescale/fec_main.c | 40 +-- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index f96ea97..be875fd 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -271,28 +271,32 @@ static void swap_buffer2(void *dst_buf, void *src_buf, int len) static void fec_dump(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - struct bufdesc *bdp; - struct fec_enet_priv_tx_q *txq; - int index = 0; + int i; + /* Disable all FEC interrupts */ + writel(0, fep->hwp + FEC_IMASK); netdev_info(ndev, "TX ring dump\n"); pr_info("Nr SC addr len SKB\n"); - txq = fep->tx_queue[0]; - bdp = txq->bd.base; - - do { - pr_info("%3u %c%c 0x%04x 0x%08x %4u %p\n", - index, - bdp == txq->bd.cur ? 'S' : ' ', - bdp == txq->dirty_tx ? 'H' : ' ', - fec16_to_cpu(bdp->cbd_sc), - fec32_to_cpu(bdp->cbd_bufaddr), - fec16_to_cpu(bdp->cbd_datlen), - txq->tx_skbuff[index]); - bdp = fec_enet_get_nextdesc(bdp, >bd); - index++; - } while (bdp != txq->bd.base); + for (i = 0; i < fep->num_tx_queues; i++) { + struct fec_enet_priv_tx_q *txq = fep->tx_queue[i]; + struct bufdesc *bdp = txq->bd.base; + int index = 0; + + pr_info("tx queue %d\n", i); + do { + pr_info("%3u %c%c 0x%04x 0x%08x %4u %p\n", + index, + bdp == txq->bd.cur ? 'S' : ' ', + bdp == txq->dirty_tx ? 'H' : ' ', + fec16_to_cpu(bdp->cbd_sc), + fec32_to_cpu(bdp->cbd_bufaddr), + fec16_to_cpu(bdp->cbd_datlen), + txq->tx_skbuff[index]); + bdp = fec_enet_get_nextdesc(bdp, >bd); + index++; + } while (bdp != txq->bd.base); + } } static inline bool is_ipv4_pkt(struct sk_buff *skb) -- 2.5.0
[PATCH net-next V3 03/16] net: fec: return IRQ_HANDLED if fec_ptp_check_pps_event handled it
fec_ptp_check_pps_event will return 1 if FEC_T_TF_MASK caused an interrupt. Don't return IRQ_NONE in this case. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: New patch, came from feedback from another patch. --- drivers/net/ethernet/freescale/fec_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index a011719..7993040 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1579,8 +1579,8 @@ fec_enet_interrupt(int irq, void *dev_id) } if (fep->ptp_clock) - fec_ptp_check_pps_event(fep); - + if (fec_ptp_check_pps_event(fep)) + ret = IRQ_HANDLED; return ret; } -- 2.5.0
[PATCH net-next V3 16/16] net: fec: don't set cbd_bufaddr unless no mapping error
Not assigning cbd_bufaddr on error will prevent trying to unmap the error in case the FEC is reset. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: no change --- drivers/net/ethernet/freescale/fec_main.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 101d820..c2ed8be 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -600,7 +600,7 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq, int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb); struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc); void *bufaddr; - unsigned long dmabuf; + dma_addr_t dmabuf; unsigned int estatus = 0; bufaddr = txq->tso_hdrs + index * TSO_HEADER_SIZE; @@ -1295,17 +1295,21 @@ fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff { struct fec_enet_private *fep = netdev_priv(ndev); int off; + dma_addr_t dmabuf; off = ((unsigned long)skb->data) & fep->rx_align; if (off) skb_reserve(skb, fep->rx_align + 1 - off); - bdp->cbd_bufaddr = cpu_to_fec32(dma_map_single(>pdev->dev, skb->data, FEC_ENET_RX_FRSIZE - fep->rx_align, DMA_FROM_DEVICE)); - if (dma_mapping_error(>pdev->dev, fec32_to_cpu(bdp->cbd_bufaddr))) { + dmabuf = dma_map_single(>pdev->dev, skb->data, + FEC_ENET_RX_FRSIZE - fep->rx_align, + DMA_FROM_DEVICE); + if (dma_mapping_error(>pdev->dev, dmabuf)) { if (net_ratelimit()) netdev_err(ndev, "Rx DMA memory map failed\n"); return -ENOMEM; } + bdp->cbd_bufaddr = cpu_to_fec32(dmabuf); return 0; } -- 2.5.0
[PATCH net-next V3 15/16] net: fec: call dma_unmap_single on mapped tx buffers at restart
Make sure any pending tx buffers are unmapped when the fec is restarted. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: no change --- drivers/net/ethernet/freescale/fec_main.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index a38acf2..101d820 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -404,6 +404,7 @@ dma_mapping_error: bdp = fec_enet_get_nextdesc(bdp, >bd); dma_unmap_single(>pdev->dev, fec32_to_cpu(bdp->cbd_bufaddr), fec16_to_cpu(bdp->cbd_datlen), DMA_TO_DEVICE); + bdp->cbd_bufaddr = cpu_to_fec32(0); } return ERR_PTR(-ENOMEM); } @@ -761,11 +762,18 @@ static void reset_tx_queue(struct fec_enet_private *fep, txq->bd.cur = bdp; for (i = 0; i < txq->bd.ring_size; i++) { /* Initialize the BD for every fragment in the page. */ + if (bdp->cbd_bufaddr) { + if (!IS_TSO_HEADER(txq, fec32_to_cpu(bdp->cbd_bufaddr))) + dma_unmap_single(>pdev->dev, +fec32_to_cpu(bdp->cbd_bufaddr), +fec16_to_cpu(bdp->cbd_datlen), +DMA_TO_DEVICE); + bdp->cbd_bufaddr = cpu_to_fec32(0); + } if (txq->tx_skbuff[i]) { dev_kfree_skb_any(txq->tx_skbuff[i]); txq->tx_skbuff[i] = NULL; } - bdp->cbd_bufaddr = cpu_to_fec32(0); bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? BD_SC_WRAP : 0); bdp = fec_enet_get_nextdesc(bdp, >bd); @@ -2643,6 +2651,7 @@ static void fec_enet_free_buffers(struct net_device *ndev) fec32_to_cpu(bdp->cbd_bufaddr), FEC_ENET_RX_FRSIZE - fep->rx_align, DMA_FROM_DEVICE); + bdp->cbd_bufaddr = cpu_to_fec32(0); dev_kfree_skb(skb); } bdp = fec_enet_get_nextdesc(bdp, >bd); -- 2.5.0
[PATCH net-next V3 07/16] net: fec: don't clear all rx queue bits when just one is being checked
FEC_ENET_RXF is 3 separate bits, we only check one queue at a time. So, when the last queue is being checked, it is bad to remove the interrupt on the 1st queue. Also, since tx/rx interrupts are now cleared in the napi routine and not the interrupt, it is not needed here any longer. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: change commit message --- drivers/net/ethernet/freescale/fec_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 17140ea..3cd0cdf 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1339,8 +1339,6 @@ static int fec_rxq(struct net_device *ndev, struct fec_enet_priv_rx_q *rxq, break; pkt_received++; - writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); - /* Check for errors. */ status ^= BD_ENET_RX_LAST; if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO | -- 2.5.0
[PATCH net-next V3 08/16] net: fec: set cbd_sc without relying on previous value
Relying on the wrap bit of cdb_sc to stay valid once initialized when the controller also writes to this byte seems undesirable since we can easily know what the value should be. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: change commit message --- drivers/net/ethernet/freescale/fec_main.c | 38 +-- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3cd0cdf..21d2cd0 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -340,9 +340,8 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, bdp = fec_enet_get_nextdesc(bdp, >bd); ebdp = (struct bufdesc_ex *)bdp; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); + status = BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0); frag_len = skb_shinfo(skb)->frags[frag].size; /* Handle the last BD specially */ @@ -436,8 +435,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, /* Fill in a Tx ring entry */ bdp = txq->bd.cur; last_bdp = bdp; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; /* Set buffer length and buffer pointer */ bufaddr = skb->data; @@ -462,6 +459,8 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, return NETDEV_TX_OK; } + status = BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0); if (nr_frags) { last_bdp = fec_enet_txq_submit_frag_skb(txq, skb, ndev); if (IS_ERR(last_bdp)) { @@ -512,7 +511,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, /* Send it on its way. Tell FEC it's ready, interrupt when done, * it's the last BD of the frame, and to put the CRC on the end. */ - status |= (BD_ENET_TX_READY | BD_ENET_TX_TC); bdp->cbd_sc = cpu_to_fec16(status); /* If this was the last BD in the ring, start at the beginning again. */ @@ -544,11 +542,6 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, unsigned int estatus = 0; dma_addr_t addr; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; - - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); - if (((unsigned long) data) & fep->tx_align || fep->quirks & FEC_QUIRK_SWAP_FRAME) { memcpy(txq->tx_bounce[index], data, size); @@ -578,15 +571,16 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, ebdp->cbd_esc = cpu_to_fec32(estatus); } + status = BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0); /* Handle the last BD specially */ if (last_tcp) - status |= (BD_ENET_TX_LAST | BD_ENET_TX_TC); + status |= BD_ENET_TX_LAST; if (is_last) { status |= BD_ENET_TX_INTR; if (fep->bufdesc_ex) ebdp->cbd_esc |= cpu_to_fec32(BD_ENET_TX_INT); } - bdp->cbd_sc = cpu_to_fec16(status); return 0; @@ -602,13 +596,8 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq, struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc); void *bufaddr; unsigned long dmabuf; - unsigned short status; unsigned int estatus = 0; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); - bufaddr = txq->tso_hdrs + index * TSO_HEADER_SIZE; dmabuf = txq->tso_hdrs_dma + index * TSO_HEADER_SIZE; if (((unsigned long)bufaddr) & fep->tx_align || @@ -641,8 +630,8 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq, ebdp->cbd_esc = cpu_to_fec32(estatus); } - bdp->cbd_sc = cpu_to_fec16(status); - + bdp->cbd_sc = cpu_to_fec16(BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0)); return 0; } @@ -1454,12 +1443,6 @@ static int fec_rxq(struct net_device *ndev, struct fec_enet_priv_rx_q *rxq, } rx_processing_done: - /* Clear the status flags for this buffer */ - status &= ~BD_ENET_RX_STATS; - - /* Mark the buffer empty */ - status |= BD_ENET_RX_EMPTY; - if (fep->bufdesc_ex) {
[PATCH net-next V3 02/16] net: fec: remove unused interrupt FEC_ENET_TS_TIMER
FEC_ENET_TS_TIMER is not checked in the interrupt routine so there is no need to enable it. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: New patch Frank Li said "TS_TIMER should never be triggered." when discussing another patch. --- drivers/net/ethernet/freescale/fec.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 195122e..6dd0ba8 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -374,8 +374,8 @@ struct bufdesc_ex { #define FEC_ENET_TS_AVAIL ((uint)0x0001) #define FEC_ENET_TS_TIMER ((uint)0x8000) -#define FEC_DEFAULT_IMASK (FEC_ENET_TXF | FEC_ENET_RXF | FEC_ENET_MII | FEC_ENET_TS_TIMER) -#define FEC_NAPI_IMASK (FEC_ENET_MII | FEC_ENET_TS_TIMER) +#define FEC_DEFAULT_IMASK (FEC_ENET_TXF | FEC_ENET_RXF | FEC_ENET_MII) +#define FEC_NAPI_IMASK FEC_ENET_MII #define FEC_RX_DISABLED_IMASK (FEC_DEFAULT_IMASK & (~FEC_ENET_RXF)) /* ENET interrupt coalescing macro define */ -- 2.5.0
[PATCH net-next V3 06/16] net: fec: split off napi routine with 3 queues
If we only have 1 tx/rx queue, we need not check the other queues. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- v3: rebase changes only, fep is no longer passed as a parameter to fec_rxq/fec_txq --- drivers/net/ethernet/freescale/fec_main.c | 39 +-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 918ac82..17140ea 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1524,7 +1524,7 @@ fec_enet_interrupt(int irq, void *dev_id) return ret; } -static int fec_enet_rx_napi(struct napi_struct *napi, int budget) +static int fec_enet_napi_q3(struct napi_struct *napi, int budget) { struct net_device *ndev = napi->dev; struct fec_enet_private *fep = netdev_priv(ndev); @@ -1564,6 +1564,39 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget) return pkts; } +static int fec_enet_napi_q1(struct napi_struct *napi, int budget) +{ + struct net_device *ndev = napi->dev; + struct fec_enet_private *fep = netdev_priv(ndev); + int pkts = 0; + uint events; + + do { + events = readl(fep->hwp + FEC_IEVENT); + if (fep->events) { + events |= fep->events; + fep->events = 0; + } + events &= FEC_ENET_RXF_0 | FEC_ENET_TXF_0; + if (!events) { + if (budget) { + napi_complete(napi); + writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK); + } + return pkts; + } + + writel(events, fep->hwp + FEC_IEVENT); + if (events & FEC_ENET_RXF_0) + pkts += fec_rxq(ndev, fep->rx_queue[0], + budget - pkts); + if (events & FEC_ENET_TXF_0) + fec_txq(ndev, fep->tx_queue[0]); + } while (pkts < budget); + fep->events |= FEC_ENET_RXF_0; /* save for next callback */ + return pkts; +} + /* - */ static void fec_get_mac(struct net_device *ndev) { @@ -3123,7 +3156,9 @@ static int fec_enet_init(struct net_device *ndev) ndev->ethtool_ops = _enet_ethtool_ops; writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK); - netif_napi_add(ndev, >napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT); + netif_napi_add(ndev, >napi, (fep->num_rx_queues | + fep->num_tx_queues) == 1 ? fec_enet_napi_q1 : + fec_enet_napi_q3, NAPI_POLL_WEIGHT); if (fep->quirks & FEC_QUIRK_HAS_VLAN) /* enable hw VLAN support */ -- 2.5.0
[PATCH net-next V3 00/16] net: fec: cleanup and fixes
V3 has 1 dropped patch "net: fec: print more debug info in fec_timeout" 2 new patches 0002-net-fec-remove-unused-interrupt-FEC_ENET_TS_TIMER.patch 0003-net-fec-return-IRQ_HANDLED-if-fec_ptp_check_pps_even.patch 1 combined patch 0004-net-fec-pass-rxq-txq-to-fec_enet_rx-tx_queue-instead.patch The changes are noted on individual patches My measured performance of this series is before patch set 365 Mbits/sec Tx/407 RX after patch set 374 Tx/427 Rx Troy Kisky (16): net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set net: fec: remove unused interrupt FEC_ENET_TS_TIMER net: fec: return IRQ_HANDLED if fec_ptp_check_pps_event handled it net: fec: pass rxq/txq to fec_enet_rx/tx_queue instead of queue_id net: fec: reduce interrupts net: fec: split off napi routine with 3 queues net: fec: don't clear all rx queue bits when just one is being checked net: fec: set cbd_sc without relying on previous value net: fec: eliminate calls to fec_enet_get_prevdesc net: fec: move restart test for efficiency net: fec: clear cbd_sc after transmission to help with debugging net: fec: dump all tx queues in fec_dump net: fec: detect tx int lost net: fec: create subroutine reset_tx_queue net: fec: call dma_unmap_single on mapped tx buffers at restart net: fec: don't set cbd_bufaddr unless no mapping error drivers/net/ethernet/freescale/fec.h | 10 +- drivers/net/ethernet/freescale/fec_main.c | 410 -- 2 files changed, 218 insertions(+), 202 deletions(-) -- 2.5.0
[PATCH net-next V3 04/16] net: fec: pass rxq/txq to fec_enet_rx/tx_queue instead of queue_id
The queue_id is the qid member of struct bufdesc_prop. Passing rxq/txq will allow the macro FEC_ENET_GET_QUQUE to be removed in the next patch. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> Acked-by: Fugang Duan <fugang.d...@nxp.com> --- v3: add Acked-by combine with "net: fec: pass txq to fec_enet_tx_queue instead of queue_id" reverted change that passed fep as a parameter --- drivers/net/ethernet/freescale/fec_main.c | 29 +++-- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 7993040..b4d46f8 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1156,25 +1156,18 @@ fec_enet_hwtstamp(struct fec_enet_private *fep, unsigned ts, hwtstamps->hwtstamp = ns_to_ktime(ns); } -static void -fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) +static void fec_txq(struct net_device *ndev, struct fec_enet_priv_tx_q *txq) { - struct fec_enet_private *fep; + struct fec_enet_private *fep = netdev_priv(ndev); struct bufdesc *bdp; unsigned short status; struct sk_buff *skb; - struct fec_enet_priv_tx_q *txq; struct netdev_queue *nq; int index = 0; int entries_free; - fep = netdev_priv(ndev); - - queue_id = FEC_ENET_GET_QUQUE(queue_id); - - txq = fep->tx_queue[queue_id]; /* get next bdp of dirty_tx */ - nq = netdev_get_tx_queue(ndev, queue_id); + nq = netdev_get_tx_queue(ndev, txq->bd.qid); bdp = txq->dirty_tx; /* get next bdp of dirty_tx */ @@ -1268,11 +1261,13 @@ static void fec_enet_tx(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); + struct fec_enet_priv_tx_q *txq; u16 queue_id; /* First process class A queue, then Class B and Best Effort queue */ for_each_set_bit(queue_id, >work_tx, FEC_ENET_MAX_TX_QS) { clear_bit(queue_id, >work_tx); - fec_enet_tx_queue(ndev, queue_id); + txq = fep->tx_queue[FEC_ENET_GET_QUQUE(queue_id)]; + fec_txq(ndev, txq); } return; } @@ -1328,11 +1323,10 @@ static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb, * not been given to the system, we just set the empty indicator, * effectively tossing the packet. */ -static int -fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id) +static int fec_rxq(struct net_device *ndev, struct fec_enet_priv_rx_q *rxq, + int budget) { struct fec_enet_private *fep = netdev_priv(ndev); - struct fec_enet_priv_rx_q *rxq; struct bufdesc *bdp; unsigned short status; struct sk_buff *skb_new = NULL; @@ -1350,8 +1344,6 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id) #ifdef CONFIG_M532x flush_cache_all(); #endif - queue_id = FEC_ENET_GET_QUQUE(queue_id); - rxq = fep->rx_queue[queue_id]; /* First, grab all of the stats for the incoming packet. * These get messed up if we get called due to a busy condition. @@ -1519,11 +1511,12 @@ fec_enet_rx(struct net_device *ndev, int budget) int pkt_received = 0; u16 queue_id; struct fec_enet_private *fep = netdev_priv(ndev); + struct fec_enet_priv_rx_q *rxq; for_each_set_bit(queue_id, >work_rx, FEC_ENET_MAX_RX_QS) { clear_bit(queue_id, >work_rx); - pkt_received += fec_enet_rx_queue(ndev, - budget - pkt_received, queue_id); + rxq = fep->rx_queue[FEC_ENET_GET_QUQUE(queue_id)]; + pkt_received += fec_rxq(ndev, rxq, budget - pkt_received); } return pkt_received; } -- 2.5.0
[PATCH net-next V3 01/16] net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set
Before queue 0 was always checked if any queue caused an interrupt. It is better to just mark queue 0 if queue 0 has caused an interrupt. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> Acked-by: Fugang Duan <fugang.d...@nxp.com> --- v3: add Acked-by --- drivers/net/ethernet/freescale/fec_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 08243c2..a011719 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1534,14 +1534,14 @@ fec_enet_collect_events(struct fec_enet_private *fep, uint int_events) if (int_events == 0) return false; - if (int_events & FEC_ENET_RXF) + if (int_events & FEC_ENET_RXF_0) fep->work_rx |= (1 << 2); if (int_events & FEC_ENET_RXF_1) fep->work_rx |= (1 << 0); if (int_events & FEC_ENET_RXF_2) fep->work_rx |= (1 << 1); - if (int_events & FEC_ENET_TXF) + if (int_events & FEC_ENET_TXF_0) fep->work_tx |= (1 << 2); if (int_events & FEC_ENET_TXF_1) fep->work_tx |= (1 << 0); -- 2.5.0
Re: [PATCH] net: fec: stop the "rcv is not +last, " error messages
On 3/29/2016 8:24 PM, Greg Ungerer wrote: > Hi Troy, > > Commit 55cd48c8 ('net: fec: stop the "rcv is not +last, " error > messages') adds a write to a register that is not present in all > implementations of the FEC hardware module. None of the ColdFire > SoC parts with the FEC module have the FTRL (0x1b0) register. > > Does this need a quirk flag to key access to this register of? > Or can you piggyback on the FEC_QUIRK_HAS_RACC flag? > > Regards > Greg I'm no expert on what hardware has which registers, but piggybacking works for all the processors that I use. Let's see what Freescale says, but would you like to submit a patch to move it inside the quirk's "if", or do you want me to do it?
Re: [PATCH net-next V2 13/16] net: fec: print more debug info in fec_timeout
On 3/4/2016 10:35 AM, Joe Perches wrote: > On Fri, 2016-03-04 at 09:05 -0700, Troy Kisky wrote: >> On 3/4/2016 3:06 AM, Fugang Duan wrote: >>> From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, February >>> 25, 2016 8:37 AM > [] >>>> Print the current interrupt flags and mask and the interrupt state during >>>> the last >>>> interrupt in fec_timeout. > [] >>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c > [] >>>> @@ -1107,6 +1107,9 @@ fec_timeout(struct net_device *ndev) >>>>int i; >>>>uint events = 0; >>>> >>>> + pr_err("%s: last=%x %x, mask %x\n", __func__, fep->last_ievents, >>>> + readl(fep->hwp + FEC_IEVENT), readl(fep->hwp + FEC_IMASK)); >>>> + >>> pr_err() -> netdev_err() >> Sounds good > > This seems like debugging information rather than > an error a user can do anything with and if there's > a timeout, how likely is it that the hardware is > hosed and this would repetitively and unnecessarily > fill up logs? > > So maybe netdev_dbg and net_ratelimit() too. > > if (net_ratelimit() > netdev_(etc...) > > This patch hasn't been helpful to me in quite a while. I'll just drop it. I know where to get it if I need it again. Thanks Troy
Re: [PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits when just one is being checked
On 3/4/2016 9:38 AM, Russell King - ARM Linux wrote: > On Fri, Mar 04, 2016 at 09:18:19AM -0700, Troy Kisky wrote: >> On 3/4/2016 2:11 AM, Fugang Duan wrote: >>> From: Troy Kisky <troy.ki...@boundarydevices.com>Sent: Thursday, February >>> 25, 2016 8:37 AM >>>> To: netdev@vger.kernel.org; da...@davemloft.net; b38...@freescale.com >>>> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >>>> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >>>> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >>>> johan...@sipsolutions.net; stillcompil...@gmail.com; >>>> sergei.shtyl...@cogentembedded.com; a...@arndb.de; Troy Kisky >>>> <troy.ki...@boundarydevices.com> >>>> Subject: [PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits >>>> when >>>> just one is being checked >>>> >>>> FEC_ENET_RXF is 3 separate bits, we only check one queue at a time. So, >>>> when >>>> the last queue is being checked, it is bad to remove the interrupt on the >>>> 1st >>>> queue. >>>> >>>> Also, since this is now done in the napi routine and not the interrupt, it >>>> is not >>>> needed. >>>> >>>> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >>>> --- >>>> drivers/net/ethernet/freescale/fec_main.c | 2 -- >>>> 1 file changed, 2 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c >>>> b/drivers/net/ethernet/freescale/fec_main.c >>>> index 610cf6c..791f385 100644 >>>> --- a/drivers/net/ethernet/freescale/fec_main.c >>>> +++ b/drivers/net/ethernet/freescale/fec_main.c >>>> @@ -1338,8 +1338,6 @@ static int fec_rxq(struct net_device *ndev, struct >>>> fec_enet_private *fep, >>>>break; >>>>pkt_received++; >>>> >>>> - writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); >>>> - >>> >>> We should clear the related rx queue ievent, not remove the code. >>> Pls see commit: db3421c114cf that was submitted by Russell King. >>> >>> No ack the patch. >> >> >> This is now done in patch #4 "net: fec: reduce interrupts" and you could >> argue >> that it should be squashed into that patch. But I like separating changes >> as much as possible. >> >> >> Russell, this patch and patch #4 will likely need your ack before it will be >> applied. >> Can you take a look please? > > I stopped caring about the FEC ethernet driver about 18 months ago, > after I ended up dropping a significant pile of fixes on the floor > through the huge number of conflicts and the shere effort of > constantly trying to move them forward. > > My patch series tend to be large because I put concentrated effort > into something for a month, which then gives a problem if conflicts > come up later and the series has to be effectively rewritten from > scratch. It was after the second or third time of facing an almost > total rewrite that happened that I just gave up. > > I've toyed with the idea of forking the driver, but I wouldn't have > time to maintain such a thing. So, right now I just put up with all > the bad quirks, and reset/power cycle the boards when things go wrong. > Right now, I just disable runtime PM support on the FEC to get > stability here. :) > > Sorry, but I can't be of more help. > I can sympathize, I've been almost ready to post my patches numerous times when a huge patch set would hit, and conflict everywhere. Including once about 18 months ago :) That's why I got trigger happy, and first posted my too large set before net-next was opened. It didn't help though, there was already a conflict in net. Troy
Re: [PATCH net-next V2 03/16] net: fec: pass txq to fec_enet_tx_queue instead of queue_id
On 3/4/2016 12:41 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, March 03, > 2016 12:14 AM >> To: Fugang Duan <fugang.d...@nxp.com>; netdev@vger.kernel.org; >> da...@davemloft.net; b38...@freescale.com >> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >> johan...@sipsolutions.net; stillcompil...@gmail.com; >> sergei.shtyl...@cogentembedded.com; a...@arndb.de >> Subject: Re: [PATCH net-next V2 03/16] net: fec: pass txq to >> fec_enet_tx_queue instead of queue_id >> >> On 3/2/2016 8:16 AM, Fugang Duan wrote: >>> From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, >>> February 25, 2016 8:37 AM >>>> To: netdev@vger.kernel.org; da...@davemloft.net; >> b38...@freescale.com >>>> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; >>>> and...@lunn.ch; trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >>>> ker...@lists.infradead.org; l...@boundarydevices.com; >>>> shawn...@kernel.org; johan...@sipsolutions.net; >>>> stillcompil...@gmail.com; sergei.shtyl...@cogentembedded.com; >>>> a...@arndb.de; Troy Kisky <troy.ki...@boundarydevices.com> >>>> Subject: [PATCH net-next V2 03/16] net: fec: pass txq to >>>> fec_enet_tx_queue instead of queue_id >>>> >>>> queue_id is the qid member of struct bufdesc_prop. >>>> >>>> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >>>> --- >>>> drivers/net/ethernet/freescale/fec_main.c | 17 ++--- >>>> 1 file changed, 6 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/freescale/fec_main.c >>>> b/drivers/net/ethernet/freescale/fec_main.c >>>> index 9619b9e..c517194 100644 >>>> --- a/drivers/net/ethernet/freescale/fec_main.c >>>> +++ b/drivers/net/ethernet/freescale/fec_main.c >>>> @@ -1156,25 +1156,18 @@ fec_enet_hwtstamp(struct fec_enet_private >>>> *fep, unsigned ts, >>>>hwtstamps->hwtstamp = ns_to_ktime(ns); } >>>> >>>> -static void >>>> -fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) >>>> +static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, >>>> + struct fec_enet_priv_tx_q *txq) >>>> { >>>> - struct fec_enet_private *fep; >>>>struct bufdesc *bdp; >>>>unsigned short status; >>>>struct sk_buff *skb; >>>> - struct fec_enet_priv_tx_q *txq; >>>>struct netdev_queue *nq; >>>>int index = 0; >>>>int entries_free; >>>> >>>> - fep = netdev_priv(ndev); >>>> - >>>> - queue_id = FEC_ENET_GET_QUQUE(queue_id); >>>> - >>>> - txq = fep->tx_queue[queue_id]; >>>>/* get next bdp of dirty_tx */ >>>> - nq = netdev_get_tx_queue(ndev, queue_id); >>>> + nq = netdev_get_tx_queue(ndev, txq->bd.qid); >>>>bdp = txq->dirty_tx; >>>> >>>>/* get next bdp of dirty_tx */ >>>> @@ -1268,11 +1261,13 @@ static void >>>> fec_enet_tx(struct net_device *ndev) { >>>>struct fec_enet_private *fep = netdev_priv(ndev); >>>> + struct fec_enet_priv_tx_q *txq; >>>>u16 queue_id; >>>>/* First process class A queue, then Class B and Best Effort queue */ >>>>for_each_set_bit(queue_id, >work_tx, FEC_ENET_MAX_TX_QS) >> { >>>>clear_bit(queue_id, >work_tx); >>>> - fec_enet_tx_queue(ndev, queue_id); >>>> + txq = fep->tx_queue[FEC_ENET_GET_QUQUE(queue_id)]; >>>> + fec_txq(ndev, fep, txq); >>>>} >>>>return; >>>> } >>>> -- >>>> 2.5.0 >>> >>> The patch should merge with patch#1. >>> >> >> >> Why ? That would only hide the change in patch #1. > > Hi Troy Kisky, > > Sorry, I mean patch#2 net: fec: pass rxq to fec_enet_rx_queue instead of > queue_id. It is not necessary to separate them. > I'll happily squash them together, that is easy, and I will. I don't agree they should be, but it is just not at all important to me. Sincere thanks for reviewing this series though. Troy
Re: [PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits when just one is being checked
On 3/4/2016 2:11 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com>Sent: Thursday, February 25, > 2016 8:37 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; b38...@freescale.com >> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >> johan...@sipsolutions.net; stillcompil...@gmail.com; >> sergei.shtyl...@cogentembedded.com; a...@arndb.de; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits >> when >> just one is being checked >> >> FEC_ENET_RXF is 3 separate bits, we only check one queue at a time. So, when >> the last queue is being checked, it is bad to remove the interrupt on the 1st >> queue. >> >> Also, since this is now done in the napi routine and not the interrupt, it >> is not >> needed. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 610cf6c..791f385 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -1338,8 +1338,6 @@ static int fec_rxq(struct net_device *ndev, struct >> fec_enet_private *fep, >> break; >> pkt_received++; >> >> -writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); >> - > > We should clear the related rx queue ievent, not remove the code. > Pls see commit: db3421c114cf that was submitted by Russell King. > > No ack the patch. This is now done in patch #4 "net: fec: reduce interrupts" and you could argue that it should be squashed into that patch. But I like separating changes as much as possible. Russell, this patch and patch #4 will likely need your ack before it will be applied. Can you take a look please? http://www.spinics.net/lists/netdev/msg361927.html Thanks Troy
Re: [PATCH net-next V2 13/16] net: fec: print more debug info in fec_timeout
On 3/4/2016 3:06 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, February > 25, 2016 8:37 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; b38...@freescale.com >> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >> johan...@sipsolutions.net; stillcompil...@gmail.com; >> sergei.shtyl...@cogentembedded.com; a...@arndb.de; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net-next V2 13/16] net: fec: print more debug info in >> fec_timeout >> >> Print the current interrupt flags and mask and the interrupt state during >> the last >> interrupt in fec_timeout. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec.h | 1 + >> drivers/net/ethernet/freescale/fec_main.c | 4 >> 2 files changed, 5 insertions(+) >> >> diff --git a/drivers/net/ethernet/freescale/fec.h >> b/drivers/net/ethernet/freescale/fec.h >> index 001200b..615cca1 100644 >> --- a/drivers/net/ethernet/freescale/fec.h >> +++ b/drivers/net/ethernet/freescale/fec.h >> @@ -506,6 +506,7 @@ struct fec_enet_private { >> unsigned int total_tx_ring_size; >> unsigned int total_rx_ring_size; >> uintevents; >> +uintlast_ievents; >> >> struct platform_device *pdev; >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index afd4060..9a3136b 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -1107,6 +1107,9 @@ fec_timeout(struct net_device *ndev) >> int i; >> uint events = 0; >> >> +pr_err("%s: last=%x %x, mask %x\n", __func__, fep->last_ievents, >> + readl(fep->hwp + FEC_IEVENT), readl(fep->hwp + FEC_IMASK)); >> + > pr_err() -> netdev_err() > Sounds good
Re: [PATCH net-next V2 07/16] net: fec: set cbd_sc without relying on previous value
On 3/4/2016 2:29 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, February > 25, 2016 8:37 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; b38...@freescale.com >> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >> johan...@sipsolutions.net; stillcompil...@gmail.com; >> sergei.shtyl...@cogentembedded.com; a...@arndb.de; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net-next V2 07/16] net: fec: set cbd_sc without relying on >> previous value >> >> Relying on the wrap bit to stay valid once initialized when the controller >> also >> writes to this byte seems undesirable since we can easily know what the value >> should be. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 38 >> +-- >> 1 file changed, 11 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 791f385..6ceb5f9 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -340,9 +340,8 @@ fec_enet_txq_submit_frag_skb(struct >> fec_enet_priv_tx_q *txq, >> bdp = fec_enet_get_nextdesc(bdp, >bd); >> ebdp = (struct bufdesc_ex *)bdp; >> >> -status = fec16_to_cpu(bdp->cbd_sc); >> -status &= ~BD_ENET_TX_STATS; >> -status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); >> +status = BD_ENET_TX_TC | BD_ENET_TX_READY | >> +((bdp == txq->bd.last) ? BD_SC_WRAP : 0); >> frag_len = skb_shinfo(skb)->frags[frag].size; >> >> /* Handle the last BD specially */ >> @@ -436,8 +435,6 @@ static int fec_enet_txq_submit_skb(struct >> fec_enet_priv_tx_q *txq, >> /* Fill in a Tx ring entry */ >> bdp = txq->bd.cur; >> last_bdp = bdp; >> -status = fec16_to_cpu(bdp->cbd_sc); >> -status &= ~BD_ENET_TX_STATS; >> >> /* Set buffer length and buffer pointer */ >> bufaddr = skb->data; >> @@ -462,6 +459,8 @@ static int fec_enet_txq_submit_skb(struct >> fec_enet_priv_tx_q *txq, >> return NETDEV_TX_OK; >> } >> >> +status = BD_ENET_TX_TC | BD_ENET_TX_READY | >> +((bdp == txq->bd.last) ? BD_SC_WRAP : 0); >> if (nr_frags) { >> last_bdp = fec_enet_txq_submit_frag_skb(txq, skb, ndev); >> if (IS_ERR(last_bdp)) { >> @@ -512,7 +511,6 @@ static int fec_enet_txq_submit_skb(struct >> fec_enet_priv_tx_q *txq, >> /* Send it on its way. Tell FEC it's ready, interrupt when done, >> * it's the last BD of the frame, and to put the CRC on the end. >> */ >> -status |= (BD_ENET_TX_READY | BD_ENET_TX_TC); > > This is completely error. We have to prepare all BDs for frag skb, and then > enable "READY" and "TC" bit for the first BD, otherwise uDMA copy un-correct > data to fifo. > I don't follow. Please read patch again. Thanks Troy
Re: [PATCH net-next V2 08/16] net: fec: eliminate calls to fec_enet_get_prevdesc
On 3/4/2016 2:33 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, February > 25, 2016 8:37 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; b38...@freescale.com >> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >> johan...@sipsolutions.net; stillcompil...@gmail.com; >> sergei.shtyl...@cogentembedded.com; a...@arndb.de; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net-next V2 08/16] net: fec: eliminate calls to >> fec_enet_get_prevdesc >> >> This shrinks the code a little. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 37 >> +-- >> 1 file changed, 11 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 6ceb5f9..b5ed287 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -758,6 +758,7 @@ static void fec_enet_bd_init(struct net_device *dev) >> struct bufdesc *bdp; >> unsigned int i; >> unsigned int q; >> +unsigned status; > > Should be unsigned int status; > Fine Thanks
Re: [PATCH net-next V2 04/16] net: fec: reduce interrupts
On 3/2/2016 9:47 AM, Zhi Li wrote: > On Wed, Mar 2, 2016 at 10:12 AM, Troy Kisky > <troy.ki...@boundarydevices.com> wrote: >> On 3/2/2016 8:13 AM, Fugang Duan wrote: >>> From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, February >>> 25, 2016 8:37 AM >>>> >>>> -if (fep->ptp_clock) >>>> +if ((int_events & FEC_ENET_TS_TIMER) && fep->ptp_clock) >>>> fec_ptp_check_pps_event(fep); >>>> - >>> This is error in here. FEC compare timer event is not TS timer. >>> >>> >> >> >> So when should fec_ptp_check_pps_event be called ? On every interrupt ? > > Compare event is not showed in EIR register. Need check TCSR, please > see below code. > > uint fec_ptp_check_pps_event(struct fec_enet_private *fep) > { > xx > val = readl(fep->hwp + FEC_TCSR(channel)); > if (val & FEC_T_TF_MASK) { > } So, should FEC_ENET_TS_TIMER be removed from FEC_DEFAULT_IMASK, since the interrupt routine never checks it ?
Re: [PATCH net-next V2 04/16] net: fec: reduce interrupts
On 3/2/2016 8:13 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, February > 25, 2016 8:37 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; b38...@freescale.com >> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >> johan...@sipsolutions.net; stillcompil...@gmail.com; >> sergei.shtyl...@cogentembedded.com; a...@arndb.de; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net-next V2 04/16] net: fec: reduce interrupts >> >> By clearing the NAPI interrupts in the NAPI routine and not in the interrupt >> handler, we can reduce the number of interrupts. We also don't need any >> status variables as the registers are still valid. >> >> Also, notice that if budget pkts are received, the next call to >> fec_enet_rx_napi >> will now continue to receive the previously pending packets. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec.h | 6 +- >> drivers/net/ethernet/freescale/fec_main.c | 127 >> -- >> 2 files changed, 50 insertions(+), 83 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec.h >> b/drivers/net/ethernet/freescale/fec.h >> index 195122e..001200b 100644 >> --- a/drivers/net/ethernet/freescale/fec.h >> +++ b/drivers/net/ethernet/freescale/fec.h >> @@ -505,11 +505,7 @@ struct fec_enet_private { >> >> unsigned int total_tx_ring_size; >> unsigned int total_rx_ring_size; >> - >> -unsigned long work_tx; >> -unsigned long work_rx; >> -unsigned long work_ts; >> -unsigned long work_mdio; >> +uintevents; >> >> struct platform_device *pdev; >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index c517194..4a218b9 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -70,8 +70,6 @@ static void fec_enet_itr_coal_init(struct net_device >> *ndev); >> >> #define DRIVER_NAME "fec" >> >> -#define FEC_ENET_GET_QUQUE(_x) ((_x == 0) ? 1 : ((_x == 1) ? 2 : 0)) >> - >> /* Pause frame feild and FIFO threshold */ >> #define FEC_ENET_FCE(1 << 5) >> #define FEC_ENET_RSEM_V 0x84 >> @@ -1257,21 +1255,6 @@ static void fec_txq(struct net_device *ndev, struct >> fec_enet_private *fep, >> writel(0, txq->bd.reg_desc_active); >> } >> >> -static void >> -fec_enet_tx(struct net_device *ndev) >> -{ >> -struct fec_enet_private *fep = netdev_priv(ndev); >> -struct fec_enet_priv_tx_q *txq; >> -u16 queue_id; >> -/* First process class A queue, then Class B and Best Effort queue */ >> -for_each_set_bit(queue_id, >work_tx, FEC_ENET_MAX_TX_QS) >> { >> -clear_bit(queue_id, >work_tx); >> -txq = fep->tx_queue[FEC_ENET_GET_QUQUE(queue_id)]; >> -fec_txq(ndev, fep, txq); >> -} >> -return; >> -} >> - >> static int >> fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct >> sk_buff *skb) { @@ -1504,92 +1487,80 @@ rx_processing_done: >> return pkt_received; >> } >> >> -static int >> -fec_enet_rx(struct net_device *ndev, int budget) -{ >> -int pkt_received = 0; >> -u16 queue_id; >> -struct fec_enet_private *fep = netdev_priv(ndev); >> -struct fec_enet_priv_rx_q *rxq; >> - >> -for_each_set_bit(queue_id, >work_rx, FEC_ENET_MAX_RX_QS) >> { >> -clear_bit(queue_id, >work_rx); >> -rxq = fep->rx_queue[FEC_ENET_GET_QUQUE(queue_id)]; >> -pkt_received += fec_rxq(ndev, fep, rxq, budget - >> pkt_received); >> -} >> -return pkt_received; >> -} >> - >> -static bool >> -fec_enet_collect_events(struct fec_enet_private *fep, uint int_events) -{ >> -if (int_events == 0) >> -return false; >> - >> -if (int_events & FEC_ENET_RXF_0) >> -fep->work_rx |= (1 << 2); >> -if (int_events & FEC_ENET_RXF_1) >> -fep->work_rx |= (1 << 0); >> -if (int_events & FEC_ENET_RXF_2) >> -
Re: [PATCH net-next V2 03/16] net: fec: pass txq to fec_enet_tx_queue instead of queue_id
On 3/2/2016 8:16 AM, Fugang Duan wrote: > From: Troy Kisky <troy.ki...@boundarydevices.com> Sent: Thursday, February > 25, 2016 8:37 AM >> To: netdev@vger.kernel.org; da...@davemloft.net; b38...@freescale.com >> Cc: fabio.este...@freescale.com; l.st...@pengutronix.de; and...@lunn.ch; >> trem...@gmail.com; li...@arm.linux.org.uk; linux-arm- >> ker...@lists.infradead.org; l...@boundarydevices.com; shawn...@kernel.org; >> johan...@sipsolutions.net; stillcompil...@gmail.com; >> sergei.shtyl...@cogentembedded.com; a...@arndb.de; Troy Kisky >> <troy.ki...@boundarydevices.com> >> Subject: [PATCH net-next V2 03/16] net: fec: pass txq to fec_enet_tx_queue >> instead of queue_id >> >> queue_id is the qid member of struct bufdesc_prop. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 17 ++--- >> 1 file changed, 6 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 9619b9e..c517194 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -1156,25 +1156,18 @@ fec_enet_hwtstamp(struct fec_enet_private *fep, >> unsigned ts, >> hwtstamps->hwtstamp = ns_to_ktime(ns); } >> >> -static void >> -fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) >> +static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, >> +struct fec_enet_priv_tx_q *txq) >> { >> -struct fec_enet_private *fep; >> struct bufdesc *bdp; >> unsigned short status; >> struct sk_buff *skb; >> -struct fec_enet_priv_tx_q *txq; >> struct netdev_queue *nq; >> int index = 0; >> int entries_free; >> >> -fep = netdev_priv(ndev); >> - >> -queue_id = FEC_ENET_GET_QUQUE(queue_id); >> - >> -txq = fep->tx_queue[queue_id]; >> /* get next bdp of dirty_tx */ >> -nq = netdev_get_tx_queue(ndev, queue_id); >> +nq = netdev_get_tx_queue(ndev, txq->bd.qid); >> bdp = txq->dirty_tx; >> >> /* get next bdp of dirty_tx */ >> @@ -1268,11 +1261,13 @@ static void >> fec_enet_tx(struct net_device *ndev) >> { >> struct fec_enet_private *fep = netdev_priv(ndev); >> +struct fec_enet_priv_tx_q *txq; >> u16 queue_id; >> /* First process class A queue, then Class B and Best Effort queue */ >> for_each_set_bit(queue_id, >work_tx, FEC_ENET_MAX_TX_QS) >> { >> clear_bit(queue_id, >work_tx); >> -fec_enet_tx_queue(ndev, queue_id); >> +txq = fep->tx_queue[FEC_ENET_GET_QUQUE(queue_id)]; >> +fec_txq(ndev, fep, txq); >> } >> return; >> } >> -- >> 2.5.0 > > The patch should merge with patch#1. > Why ? That would only hide the change in patch #1.
Re: [PATCH net-next V2 03/16] net: fec: pass txq to fec_enet_tx_queue instead of queue_id
On 3/1/2016 3:26 PM, Zhi Li wrote: > On Tue, Mar 1, 2016 at 3:51 PM, Troy Kisky > <troy.ki...@boundarydevices.com> wrote: >> True, but fec_txq/fec_rxq is called in a loop. > > netdev_priv(ndev) is that pointer move. > dev + ALIGN(sizeof(struct net_device), NETDEV_ALIGN) > > Modem compiler can handle it greatly. > > You can't get any valuable performance gain by that. > > The main time of CPU is call dma_map_xxx. > > best regards > Frank Li > Ok, I'll revert that part.
Re: [PATCH net-next V2 03/16] net: fec: pass txq to fec_enet_tx_queue instead of queue_id
On 3/1/2016 2:06 PM, Zhi Li wrote: > On Wed, Feb 24, 2016 at 6:36 PM, Troy Kisky > <troy.ki...@boundarydevices.com> wrote: >> queue_id is the qid member of struct bufdesc_prop. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 17 ++--- >> 1 file changed, 6 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 9619b9e..c517194 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -1156,25 +1156,18 @@ fec_enet_hwtstamp(struct fec_enet_private *fep, >> unsigned ts, >> hwtstamps->hwtstamp = ns_to_ktime(ns); >> } >> >> -static void >> -fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) >> +static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, >> + struct fec_enet_priv_tx_q *txq) > > you can get fep from ndev. > True, but fec_txq/fec_rxq is called in a loop. Why not pass it, rather than look it up again?
Re: [PATCH] net: fec: Add "phy-reset-active-low" property to DT
We need that for a custom hardware that needs the reverse reset sequence. Signed-off-by: Bernhard Walle--- Documentation/devicetree/bindings/net/fsl-fec.txt | 3 +++ drivers/net/ethernet/freescale/fec_main.c | 8 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt index a9eb611..a4799ff 100644 --- a/Documentation/devicetree/bindings/net/fsl-fec.txt +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt @@ -12,6 +12,9 @@ Optional properties: only if property "phy-reset-gpios" is available. Missing the property will have the duration be 1 millisecond. Numbers greater than 1000 are invalid and 1 millisecond will be used instead. +- phy-reset-active-low : If present then the reset sequence using the GPIO + specified in the "phy-reset-gpios" property is reversed (H=reset state, + L=operation state). Shouldn't this be named phy-reset-active-high, as you are making the reset active high H=reset, L= normal operation Troy
Re: [PATCH net-next V2 00/16] net: fec: cleanup and fixes
On 2/24/2016 7:52 PM, Joshua Clayton wrote: > Hello Troy, > I'm replying here instead of to a particular commit because several of > the commit messages seem inadequate. > > The first line summaries all look good. > > The descriptions should each also include the "user visible impact" of > the patch and the justification for it (i.e. why you made the change). > > For instance, patch 3 doesn't include either what will change > (nothing, I'm guessing?) or why we now pass in the structures > instead of a queue_id. I can add to the commit message, that this is in preparation for patch 4 which depends on it. Or I could squash patches 2/3/4 together, but I think it is easier to review smaller patches. > > You've also got a few (e.g. patch 9, patch 14) where the substance > of the patch is in the summary, > > but missing from the message. > > These kind of descriptions are very hard to review since the expression > is split between the subject of the email and the body of the email, which > are not close > together in some email programs. > > Better to reiterate or elaborate on the summary in the message. > In patch 9, for instance, it would be more clear to say: > > Move restart test to earlier in fec_txq() which saves one comparison. I can do this. And change patch 14 to read Create subroutine reset_tx_queue to have one place to release any queued tx skbs. Any other commit messages you'd like to improve? > P.S I'm a little confused, as I came looking for a v3 of the first 8 patches > and found these instead. I'll try to give your first 8 a look when they show > up. The 1st 8 patches have already been applied. I added a patch to address your review there at the end of the series. So, that patch will show up in my next set. Thanks for the review Troy
Re: [PATCH net-next V2 00/16] net: fec: cleanup and fixes
On 2/25/2016 1:39 AM, Holger Schurig wrote: > Hi Troy, > > what is the general aim of your patches? Stability? Speed? Cleanup? > 1. Stability 2. performance 3. easier to read 4. more debug info The 2nd goal is very hard to measure. It seems function alignment changes swamp most any other improvements. I think that if the same measurement that I did were done with a different compiler, you would see different patches increased/decreased the BPS. But at least the overall trend on the patch set is positive. And each individual patch has been tested. I would like someone to test on a machine with 3 queues though. If you have a more accurate way to measure performance, please let me know. Also, if you know why freescale's bsp has so much better performance that would be a very welcome patch. Troy
[PATCH net-next V2 03/16] net: fec: pass txq to fec_enet_tx_queue instead of queue_id
queue_id is the qid member of struct bufdesc_prop. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 9619b9e..c517194 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1156,25 +1156,18 @@ fec_enet_hwtstamp(struct fec_enet_private *fep, unsigned ts, hwtstamps->hwtstamp = ns_to_ktime(ns); } -static void -fec_enet_tx_queue(struct net_device *ndev, u16 queue_id) +static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, + struct fec_enet_priv_tx_q *txq) { - struct fec_enet_private *fep; struct bufdesc *bdp; unsigned short status; struct sk_buff *skb; - struct fec_enet_priv_tx_q *txq; struct netdev_queue *nq; int index = 0; int entries_free; - fep = netdev_priv(ndev); - - queue_id = FEC_ENET_GET_QUQUE(queue_id); - - txq = fep->tx_queue[queue_id]; /* get next bdp of dirty_tx */ - nq = netdev_get_tx_queue(ndev, queue_id); + nq = netdev_get_tx_queue(ndev, txq->bd.qid); bdp = txq->dirty_tx; /* get next bdp of dirty_tx */ @@ -1268,11 +1261,13 @@ static void fec_enet_tx(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); + struct fec_enet_priv_tx_q *txq; u16 queue_id; /* First process class A queue, then Class B and Best Effort queue */ for_each_set_bit(queue_id, >work_tx, FEC_ENET_MAX_TX_QS) { clear_bit(queue_id, >work_tx); - fec_enet_tx_queue(ndev, queue_id); + txq = fep->tx_queue[FEC_ENET_GET_QUQUE(queue_id)]; + fec_txq(ndev, fep, txq); } return; } -- 2.5.0
[PATCH net-next V2 01/16] net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set
Before queue 0 was always checked if any queue caused an interrupt. It is better to just mark queue 0 if queue 0 has caused an interrupt. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index bad0ba2..dbac975 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1534,14 +1534,14 @@ fec_enet_collect_events(struct fec_enet_private *fep, uint int_events) if (int_events == 0) return false; - if (int_events & FEC_ENET_RXF) + if (int_events & FEC_ENET_RXF_0) fep->work_rx |= (1 << 2); if (int_events & FEC_ENET_RXF_1) fep->work_rx |= (1 << 0); if (int_events & FEC_ENET_RXF_2) fep->work_rx |= (1 << 1); - if (int_events & FEC_ENET_TXF) + if (int_events & FEC_ENET_TXF_0) fep->work_tx |= (1 << 2); if (int_events & FEC_ENET_TXF_1) fep->work_tx |= (1 << 0); -- 2.5.0
[PATCH net-next V2 11/16] net: fec: dump all tx queues in fec_dump
Dump all tx queues, not just queue 0. Also, disable fec interrupts first. The interrupts will be reenabled in fec_restart. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 40 +-- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 96a0394..9b24669 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -271,28 +271,32 @@ static void swap_buffer2(void *dst_buf, void *src_buf, int len) static void fec_dump(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - struct bufdesc *bdp; - struct fec_enet_priv_tx_q *txq; - int index = 0; + int i; + /* Disable all FEC interrupts */ + writel(0, fep->hwp + FEC_IMASK); netdev_info(ndev, "TX ring dump\n"); pr_info("Nr SC addr len SKB\n"); - txq = fep->tx_queue[0]; - bdp = txq->bd.base; - - do { - pr_info("%3u %c%c 0x%04x 0x%08x %4u %p\n", - index, - bdp == txq->bd.cur ? 'S' : ' ', - bdp == txq->dirty_tx ? 'H' : ' ', - fec16_to_cpu(bdp->cbd_sc), - fec32_to_cpu(bdp->cbd_bufaddr), - fec16_to_cpu(bdp->cbd_datlen), - txq->tx_skbuff[index]); - bdp = fec_enet_get_nextdesc(bdp, >bd); - index++; - } while (bdp != txq->bd.base); + for (i = 0; i < fep->num_tx_queues; i++) { + struct fec_enet_priv_tx_q *txq = fep->tx_queue[i]; + struct bufdesc *bdp = txq->bd.base; + int index = 0; + + pr_info("tx queue %d\n", i); + do { + pr_info("%3u %c%c 0x%04x 0x%08x %4u %p\n", + index, + bdp == txq->bd.cur ? 'S' : ' ', + bdp == txq->dirty_tx ? 'H' : ' ', + fec16_to_cpu(bdp->cbd_sc), + fec32_to_cpu(bdp->cbd_bufaddr), + fec16_to_cpu(bdp->cbd_datlen), + txq->tx_skbuff[index]); + bdp = fec_enet_get_nextdesc(bdp, >bd); + index++; + } while (bdp != txq->bd.base); + } } static inline bool is_ipv4_pkt(struct sk_buff *skb) -- 2.5.0
[PATCH net-next V2 07/16] net: fec: set cbd_sc without relying on previous value
Relying on the wrap bit to stay valid once initialized when the controller also writes to this byte seems undesirable since we can easily know what the value should be. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 38 +-- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 791f385..6ceb5f9 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -340,9 +340,8 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, bdp = fec_enet_get_nextdesc(bdp, >bd); ebdp = (struct bufdesc_ex *)bdp; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); + status = BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0); frag_len = skb_shinfo(skb)->frags[frag].size; /* Handle the last BD specially */ @@ -436,8 +435,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, /* Fill in a Tx ring entry */ bdp = txq->bd.cur; last_bdp = bdp; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; /* Set buffer length and buffer pointer */ bufaddr = skb->data; @@ -462,6 +459,8 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, return NETDEV_TX_OK; } + status = BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0); if (nr_frags) { last_bdp = fec_enet_txq_submit_frag_skb(txq, skb, ndev); if (IS_ERR(last_bdp)) { @@ -512,7 +511,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, /* Send it on its way. Tell FEC it's ready, interrupt when done, * it's the last BD of the frame, and to put the CRC on the end. */ - status |= (BD_ENET_TX_READY | BD_ENET_TX_TC); bdp->cbd_sc = cpu_to_fec16(status); /* If this was the last BD in the ring, start at the beginning again. */ @@ -544,11 +542,6 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, unsigned int estatus = 0; dma_addr_t addr; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; - - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); - if (((unsigned long) data) & fep->tx_align || fep->quirks & FEC_QUIRK_SWAP_FRAME) { memcpy(txq->tx_bounce[index], data, size); @@ -578,15 +571,16 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, ebdp->cbd_esc = cpu_to_fec32(estatus); } + status = BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0); /* Handle the last BD specially */ if (last_tcp) - status |= (BD_ENET_TX_LAST | BD_ENET_TX_TC); + status |= BD_ENET_TX_LAST; if (is_last) { status |= BD_ENET_TX_INTR; if (fep->bufdesc_ex) ebdp->cbd_esc |= cpu_to_fec32(BD_ENET_TX_INT); } - bdp->cbd_sc = cpu_to_fec16(status); return 0; @@ -602,13 +596,8 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq, struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc); void *bufaddr; unsigned long dmabuf; - unsigned short status; unsigned int estatus = 0; - status = fec16_to_cpu(bdp->cbd_sc); - status &= ~BD_ENET_TX_STATS; - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); - bufaddr = txq->tso_hdrs + index * TSO_HEADER_SIZE; dmabuf = txq->tso_hdrs_dma + index * TSO_HEADER_SIZE; if (((unsigned long)bufaddr) & fep->tx_align || @@ -641,8 +630,8 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq, ebdp->cbd_esc = cpu_to_fec32(estatus); } - bdp->cbd_sc = cpu_to_fec16(status); - + bdp->cbd_sc = cpu_to_fec16(BD_ENET_TX_TC | BD_ENET_TX_READY | + ((bdp == txq->bd.last) ? BD_SC_WRAP : 0)); return 0; } @@ -1453,12 +1442,6 @@ static int fec_rxq(struct net_device *ndev, struct fec_enet_private *fep, } rx_processing_done: - /* Clear the status flags for this buffer */ - status &= ~BD_ENET_RX_STATS; - - /* Mark the buffer empty */ - status |= BD_ENET_RX_EMPTY; - if (fep->bufdesc_ex) { struct bufde
[PATCH net-next V2 06/16] net: fec: don't clear all rx queue bits when just one is being checked
FEC_ENET_RXF is 3 separate bits, we only check one queue at a time. So, when the last queue is being checked, it is bad to remove the interrupt on the 1st queue. Also, since this is now done in the napi routine and not the interrupt, it is not needed. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 610cf6c..791f385 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1338,8 +1338,6 @@ static int fec_rxq(struct net_device *ndev, struct fec_enet_private *fep, break; pkt_received++; - writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); - /* Check for errors. */ status ^= BD_ENET_RX_LAST; if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO | -- 2.5.0
[PATCH net-next V2 10/16] net: fec: clear cbd_sc after transmission to help with debugging
When the tx queue is dumped, it is easier to see that this entry is idle. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 1902897..96a0394 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1164,6 +1164,8 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, } break; } + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); index = fec_enet_get_bd_index(bdp, >bd); -- 2.5.0
[PATCH net-next V2 12/16] net: fec: detect tx int lost
If a tx int is lost, no need to reset the fec. Just mark the event and call napi_schedule. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 38 ++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 9b24669..afd4060 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1094,14 +1094,50 @@ fec_stop(struct net_device *ndev) } } +static const uint txint_flags[] = { + FEC_ENET_TXF_0, FEC_ENET_TXF_1, FEC_ENET_TXF_2 +}; static void fec_timeout(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); + struct bufdesc *bdp; + unsigned short status; + int i; + uint events = 0; - fec_dump(ndev); + for (i = 0; i < fep->num_tx_queues; i++) { + struct fec_enet_priv_tx_q *txq = fep->tx_queue[i]; + int index; + struct sk_buff *skb = NULL; + bdp = txq->dirty_tx; + while (1) { + bdp = fec_enet_get_nextdesc(bdp, >bd); + if (bdp == txq->bd.cur) + break; + index = fec_enet_get_bd_index(bdp, >bd); + skb = txq->tx_skbuff[index]; + if (skb) { + status = fec16_to_cpu(bdp->cbd_sc); + if ((status & BD_ENET_TX_READY) == 0) + events |= txint_flags[i]; + break; + } + } + } + if (events) { + fep->events |= events; + /* Disable the RX/TX interrupt */ + writel(FEC_NAPI_IMASK, fep->hwp + FEC_IMASK); + napi_schedule(>napi); + netif_wake_queue(fep->netdev); + pr_err("%s: tx int lost\n", __func__); + return; + } + + fec_dump(ndev); ndev->stats.tx_errors++; schedule_work(>tx_timeout_work); -- 2.5.0
[PATCH net-next V2 13/16] net: fec: print more debug info in fec_timeout
Print the current interrupt flags and mask and the interrupt state during the last interrupt in fec_timeout. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 4 2 files changed, 5 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 001200b..615cca1 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -506,6 +506,7 @@ struct fec_enet_private { unsigned int total_tx_ring_size; unsigned int total_rx_ring_size; uintevents; + uintlast_ievents; struct platform_device *pdev; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index afd4060..9a3136b 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1107,6 +1107,9 @@ fec_timeout(struct net_device *ndev) int i; uint events = 0; + pr_err("%s: last=%x %x, mask %x\n", __func__, fep->last_ievents, + readl(fep->hwp + FEC_IEVENT), readl(fep->hwp + FEC_IMASK)); + for (i = 0; i < fep->num_tx_queues; i++) { struct fec_enet_priv_tx_q *txq = fep->tx_queue[i]; int index; @@ -1514,6 +1517,7 @@ fec_enet_interrupt(int irq, void *dev_id) if (!int_events) return IRQ_NONE; + fep->last_ievents = int_events; if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) { if (napi_schedule_prep(>napi)) { -- 2.5.0
[PATCH net-next V2 05/16] net: fec: split off napi routine with 3 queues
If we only have 1 tx/rx queue, we need not check the other queues. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 39 +-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 4a218b9..610cf6c 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1521,7 +1521,7 @@ fec_enet_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } -static int fec_enet_rx_napi(struct napi_struct *napi, int budget) +static int fec_enet_napi_q3(struct napi_struct *napi, int budget) { struct net_device *ndev = napi->dev; struct fec_enet_private *fep = netdev_priv(ndev); @@ -1564,6 +1564,39 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget) return pkts; } +static int fec_enet_napi_q1(struct napi_struct *napi, int budget) +{ + struct net_device *ndev = napi->dev; + struct fec_enet_private *fep = netdev_priv(ndev); + int pkts = 0; + uint events; + + do { + events = readl(fep->hwp + FEC_IEVENT); + if (fep->events) { + events |= fep->events; + fep->events = 0; + } + events &= FEC_ENET_RXF_0 | FEC_ENET_TXF_0; + if (!events) { + if (budget) { + napi_complete(napi); + writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK); + } + return pkts; + } + + writel(events, fep->hwp + FEC_IEVENT); + if (events & FEC_ENET_RXF_0) + pkts += fec_rxq(ndev, fep, fep->rx_queue[0], + budget - pkts); + if (events & FEC_ENET_TXF_0) + fec_txq(ndev, fep, fep->tx_queue[0]); + } while (pkts < budget); + fep->events |= FEC_ENET_RXF_0; /* save for next callback */ + return pkts; +} + /* - */ static void fec_get_mac(struct net_device *ndev) { @@ -3123,7 +3156,9 @@ static int fec_enet_init(struct net_device *ndev) ndev->ethtool_ops = _enet_ethtool_ops; writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK); - netif_napi_add(ndev, >napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT); + netif_napi_add(ndev, >napi, (fep->num_rx_queues | + fep->num_tx_queues) == 1 ? fec_enet_napi_q1 : + fec_enet_napi_q3, NAPI_POLL_WEIGHT); if (fep->quirks & FEC_QUIRK_HAS_VLAN) /* enable hw VLAN support */ -- 2.5.0
[PATCH net-next V2 14/16] net: fec: create subroutine reset_tx_queue
This creates one place to release any queued tx skbs. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 50 +++ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 9a3136b..8711196 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -752,12 +752,33 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) return NETDEV_TX_OK; } +static void reset_tx_queue(struct fec_enet_private *fep, + struct fec_enet_priv_tx_q *txq) +{ + struct bufdesc *bdp = txq->bd.base; + unsigned int i; + + txq->bd.cur = bdp; + for (i = 0; i < txq->bd.ring_size; i++) { + /* Initialize the BD for every fragment in the page. */ + if (txq->tx_skbuff[i]) { + dev_kfree_skb_any(txq->tx_skbuff[i]); + txq->tx_skbuff[i] = NULL; + } + bdp->cbd_bufaddr = cpu_to_fec32(0); + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); + bdp = fec_enet_get_nextdesc(bdp, >bd); + } + bdp = fec_enet_get_prevdesc(bdp, >bd); + txq->dirty_tx = bdp; +} + /* Init RX & TX buffer descriptors */ static void fec_enet_bd_init(struct net_device *dev) { struct fec_enet_private *fep = netdev_priv(dev); - struct fec_enet_priv_tx_q *txq; struct fec_enet_priv_rx_q *rxq; struct bufdesc *bdp; unsigned int i; @@ -780,26 +801,8 @@ static void fec_enet_bd_init(struct net_device *dev) rxq->bd.cur = rxq->bd.base; } - for (q = 0; q < fep->num_tx_queues; q++) { - /* ...and the same for transmit */ - txq = fep->tx_queue[q]; - bdp = txq->bd.base; - txq->bd.cur = bdp; - - for (i = 0; i < txq->bd.ring_size; i++) { - /* Initialize the BD for every fragment in the page. */ - if (txq->tx_skbuff[i]) { - dev_kfree_skb_any(txq->tx_skbuff[i]); - txq->tx_skbuff[i] = NULL; - } - bdp->cbd_bufaddr = cpu_to_fec32(0); - bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? - BD_SC_WRAP : 0); - bdp = fec_enet_get_nextdesc(bdp, >bd); - } - bdp = fec_enet_get_prevdesc(bdp, >bd); - txq->dirty_tx = bdp; - } + for (q = 0; q < fep->num_tx_queues; q++) + reset_tx_queue(fep, fep->tx_queue[q]); } static void fec_enet_active_rxring(struct net_device *ndev) @@ -2652,13 +2655,10 @@ static void fec_enet_free_buffers(struct net_device *ndev) for (q = 0; q < fep->num_tx_queues; q++) { txq = fep->tx_queue[q]; - bdp = txq->bd.base; + reset_tx_queue(fep, txq); for (i = 0; i < txq->bd.ring_size; i++) { kfree(txq->tx_bounce[i]); txq->tx_bounce[i] = NULL; - skb = txq->tx_skbuff[i]; - txq->tx_skbuff[i] = NULL; - dev_kfree_skb(skb); } } } -- 2.5.0
[PATCH net-next V2 08/16] net: fec: eliminate calls to fec_enet_get_prevdesc
This shrinks the code a little. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 37 +-- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 6ceb5f9..b5ed287 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -758,6 +758,7 @@ static void fec_enet_bd_init(struct net_device *dev) struct bufdesc *bdp; unsigned int i; unsigned int q; + unsigned status; for (q = 0; q < fep->num_rx_queues; q++) { /* Initialize the receive buffer descriptors. */ @@ -765,19 +766,13 @@ static void fec_enet_bd_init(struct net_device *dev) bdp = rxq->bd.base; for (i = 0; i < rxq->bd.ring_size; i++) { - /* Initialize the BD for every fragment in the page. */ - if (bdp->cbd_bufaddr) - bdp->cbd_sc = cpu_to_fec16(BD_ENET_RX_EMPTY); - else - bdp->cbd_sc = cpu_to_fec16(0); + status = bdp->cbd_bufaddr ? BD_ENET_RX_EMPTY : 0; + if (bdp == rxq->bd.last) + status |= BD_SC_WRAP; + bdp->cbd_sc = cpu_to_fec16(status); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap */ - bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); - rxq->bd.cur = rxq->bd.base; } @@ -789,18 +784,16 @@ static void fec_enet_bd_init(struct net_device *dev) for (i = 0; i < txq->bd.ring_size; i++) { /* Initialize the BD for every fragment in the page. */ - bdp->cbd_sc = cpu_to_fec16(0); if (txq->tx_skbuff[i]) { dev_kfree_skb_any(txq->tx_skbuff[i]); txq->tx_skbuff[i] = NULL; } bdp->cbd_bufaddr = cpu_to_fec32(0); + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap */ bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); txq->dirty_tx = bdp; } } @@ -2717,19 +2710,16 @@ fec_enet_alloc_rxq_buffers(struct net_device *ndev, unsigned int queue) } rxq->rx_skbuff[i] = skb; - bdp->cbd_sc = cpu_to_fec16(BD_ENET_RX_EMPTY); if (fep->bufdesc_ex) { struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; ebdp->cbd_esc = cpu_to_fec32(BD_ENET_RX_INT); } + bdp->cbd_sc = cpu_to_fec16(BD_ENET_RX_EMPTY | + ((bdp == rxq->bd.last) ? BD_SC_WRAP : 0)); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap. */ - bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); return 0; err_alloc: @@ -2752,21 +2742,16 @@ fec_enet_alloc_txq_buffers(struct net_device *ndev, unsigned int queue) if (!txq->tx_bounce[i]) goto err_alloc; - bdp->cbd_sc = cpu_to_fec16(0); bdp->cbd_bufaddr = cpu_to_fec32(0); if (fep->bufdesc_ex) { struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; ebdp->cbd_esc = cpu_to_fec32(BD_ENET_TX_INT); } - + bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? + BD_SC_WRAP : 0); bdp = fec_enet_get_nextdesc(bdp, >bd); } - - /* Set the last buffer to wrap. */ - bdp = fec_enet_get_prevdesc(bdp, >bd); - bdp->cbd_sc |= cpu_to_fec16(BD_SC_WRAP); - return 0; err_alloc: -- 2.5.0
[PATCH net-next V2 15/16] net: fec: call dma_unmap_single on mapped tx buffers at restart
Make sure any pending tx buffers are unmapped when the fec is restarted. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 8711196..1d71b2e 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -404,6 +404,7 @@ dma_mapping_error: bdp = fec_enet_get_nextdesc(bdp, >bd); dma_unmap_single(>pdev->dev, fec32_to_cpu(bdp->cbd_bufaddr), fec16_to_cpu(bdp->cbd_datlen), DMA_TO_DEVICE); + bdp->cbd_bufaddr = cpu_to_fec32(0); } return ERR_PTR(-ENOMEM); } @@ -761,11 +762,18 @@ static void reset_tx_queue(struct fec_enet_private *fep, txq->bd.cur = bdp; for (i = 0; i < txq->bd.ring_size; i++) { /* Initialize the BD for every fragment in the page. */ + if (bdp->cbd_bufaddr) { + if (!IS_TSO_HEADER(txq, fec32_to_cpu(bdp->cbd_bufaddr))) + dma_unmap_single(>pdev->dev, +fec32_to_cpu(bdp->cbd_bufaddr), +fec16_to_cpu(bdp->cbd_datlen), +DMA_TO_DEVICE); + bdp->cbd_bufaddr = cpu_to_fec32(0); + } if (txq->tx_skbuff[i]) { dev_kfree_skb_any(txq->tx_skbuff[i]); txq->tx_skbuff[i] = NULL; } - bdp->cbd_bufaddr = cpu_to_fec32(0); bdp->cbd_sc = cpu_to_fec16((bdp == txq->bd.last) ? BD_SC_WRAP : 0); bdp = fec_enet_get_nextdesc(bdp, >bd); @@ -2647,6 +2655,7 @@ static void fec_enet_free_buffers(struct net_device *ndev) fec32_to_cpu(bdp->cbd_bufaddr), FEC_ENET_RX_FRSIZE - fep->rx_align, DMA_FROM_DEVICE); + bdp->cbd_bufaddr = cpu_to_fec32(0); dev_kfree_skb(skb); } bdp = fec_enet_get_nextdesc(bdp, >bd); -- 2.5.0
[PATCH net-next V2 16/16] net: fec: don't set cbd_bufaddr unless no mapping error
Not assigning cbd_bufaddr on error will prevent trying to unmap the error in case the FEC is reset. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 1d71b2e..c443a96 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -600,7 +600,7 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq, int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb); struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc); void *bufaddr; - unsigned long dmabuf; + dma_addr_t dmabuf; unsigned int estatus = 0; bufaddr = txq->tso_hdrs + index * TSO_HEADER_SIZE; @@ -1298,17 +1298,21 @@ fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff { struct fec_enet_private *fep = netdev_priv(ndev); int off; + dma_addr_t dmabuf; off = ((unsigned long)skb->data) & fep->rx_align; if (off) skb_reserve(skb, fep->rx_align + 1 - off); - bdp->cbd_bufaddr = cpu_to_fec32(dma_map_single(>pdev->dev, skb->data, FEC_ENET_RX_FRSIZE - fep->rx_align, DMA_FROM_DEVICE)); - if (dma_mapping_error(>pdev->dev, fec32_to_cpu(bdp->cbd_bufaddr))) { + dmabuf = dma_map_single(>pdev->dev, skb->data, + FEC_ENET_RX_FRSIZE - fep->rx_align, + DMA_FROM_DEVICE); + if (dma_mapping_error(>pdev->dev, dmabuf)) { if (net_ratelimit()) netdev_err(ndev, "Rx DMA memory map failed\n"); return -ENOMEM; } + bdp->cbd_bufaddr = cpu_to_fec32(dmabuf); return 0; } -- 2.5.0
[PATCH net-next V2 09/16] net: fec: move restart test for efficiency
This saves 1 comparison. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index b5ed287..1902897 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1157,8 +1157,13 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, /* Order the load of bd.cur and cbd_sc */ rmb(); status = fec16_to_cpu(READ_ONCE(bdp->cbd_sc)); - if (status & BD_ENET_TX_READY) + if (status & BD_ENET_TX_READY) { + if (!readl(txq->bd.reg_desc_active)) { + /* ERR006358 has hit, restart tx */ + writel(0, txq->bd.reg_desc_active); + } break; + } index = fec_enet_get_bd_index(bdp, >bd); @@ -1230,11 +1235,6 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, netif_tx_wake_queue(nq); } } - - /* ERR006538: Keep the transmitter going */ - if (bdp != txq->bd.cur && - readl(txq->bd.reg_desc_active) == 0) - writel(0, txq->bd.reg_desc_active); } static int -- 2.5.0
[PATCH net-next V2 04/16] net: fec: reduce interrupts
By clearing the NAPI interrupts in the NAPI routine and not in the interrupt handler, we can reduce the number of interrupts. We also don't need any status variables as the registers are still valid. Also, notice that if budget pkts are received, the next call to fec_enet_rx_napi will now continue to receive the previously pending packets. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 6 +- drivers/net/ethernet/freescale/fec_main.c | 127 -- 2 files changed, 50 insertions(+), 83 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 195122e..001200b 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -505,11 +505,7 @@ struct fec_enet_private { unsigned int total_tx_ring_size; unsigned int total_rx_ring_size; - - unsigned long work_tx; - unsigned long work_rx; - unsigned long work_ts; - unsigned long work_mdio; + uintevents; struct platform_device *pdev; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index c517194..4a218b9 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -70,8 +70,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev); #define DRIVER_NAME"fec" -#define FEC_ENET_GET_QUQUE(_x) ((_x == 0) ? 1 : ((_x == 1) ? 2 : 0)) - /* Pause frame feild and FIFO threshold */ #define FEC_ENET_FCE (1 << 5) #define FEC_ENET_RSEM_V0x84 @@ -1257,21 +1255,6 @@ static void fec_txq(struct net_device *ndev, struct fec_enet_private *fep, writel(0, txq->bd.reg_desc_active); } -static void -fec_enet_tx(struct net_device *ndev) -{ - struct fec_enet_private *fep = netdev_priv(ndev); - struct fec_enet_priv_tx_q *txq; - u16 queue_id; - /* First process class A queue, then Class B and Best Effort queue */ - for_each_set_bit(queue_id, >work_tx, FEC_ENET_MAX_TX_QS) { - clear_bit(queue_id, >work_tx); - txq = fep->tx_queue[FEC_ENET_GET_QUQUE(queue_id)]; - fec_txq(ndev, fep, txq); - } - return; -} - static int fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff *skb) { @@ -1504,92 +1487,80 @@ rx_processing_done: return pkt_received; } -static int -fec_enet_rx(struct net_device *ndev, int budget) -{ - int pkt_received = 0; - u16 queue_id; - struct fec_enet_private *fep = netdev_priv(ndev); - struct fec_enet_priv_rx_q *rxq; - - for_each_set_bit(queue_id, >work_rx, FEC_ENET_MAX_RX_QS) { - clear_bit(queue_id, >work_rx); - rxq = fep->rx_queue[FEC_ENET_GET_QUQUE(queue_id)]; - pkt_received += fec_rxq(ndev, fep, rxq, budget - pkt_received); - } - return pkt_received; -} - -static bool -fec_enet_collect_events(struct fec_enet_private *fep, uint int_events) -{ - if (int_events == 0) - return false; - - if (int_events & FEC_ENET_RXF_0) - fep->work_rx |= (1 << 2); - if (int_events & FEC_ENET_RXF_1) - fep->work_rx |= (1 << 0); - if (int_events & FEC_ENET_RXF_2) - fep->work_rx |= (1 << 1); - - if (int_events & FEC_ENET_TXF_0) - fep->work_tx |= (1 << 2); - if (int_events & FEC_ENET_TXF_1) - fep->work_tx |= (1 << 0); - if (int_events & FEC_ENET_TXF_2) - fep->work_tx |= (1 << 1); - - return true; -} - static irqreturn_t fec_enet_interrupt(int irq, void *dev_id) { struct net_device *ndev = dev_id; struct fec_enet_private *fep = netdev_priv(ndev); - uint int_events; - irqreturn_t ret = IRQ_NONE; + uint eir = readl(fep->hwp + FEC_IEVENT); + uint int_events = eir & readl(fep->hwp + FEC_IMASK); - int_events = readl(fep->hwp + FEC_IEVENT); - writel(int_events, fep->hwp + FEC_IEVENT); - fec_enet_collect_events(fep, int_events); - - if ((fep->work_tx || fep->work_rx) && fep->link) { - ret = IRQ_HANDLED; + if (!int_events) + return IRQ_NONE; + if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) { if (napi_schedule_prep(>napi)) { /* Disable the NAPI interrupts */ writel(FEC_NAPI_IMASK, fep->hwp + FEC_IMASK); __napi_schedule(>napi); + int_events &= ~(FEC_ENET_RXF | FEC_ENET_TXF); + if (!int_events) + return IRQ_HANDLED; + } else { +
[PATCH net-next V2 02/16] net: fec: pass rxq to fec_enet_rx_queue instead of queue_id
The queue_id is the qid member of struct bufdesc_prop. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index dbac975..9619b9e 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1328,11 +1328,9 @@ static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb, * not been given to the system, we just set the empty indicator, * effectively tossing the packet. */ -static int -fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id) +static int fec_rxq(struct net_device *ndev, struct fec_enet_private *fep, + struct fec_enet_priv_rx_q *rxq, int budget) { - struct fec_enet_private *fep = netdev_priv(ndev); - struct fec_enet_priv_rx_q *rxq; struct bufdesc *bdp; unsigned short status; struct sk_buff *skb_new = NULL; @@ -1350,8 +1348,6 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id) #ifdef CONFIG_M532x flush_cache_all(); #endif - queue_id = FEC_ENET_GET_QUQUE(queue_id); - rxq = fep->rx_queue[queue_id]; /* First, grab all of the stats for the incoming packet. * These get messed up if we get called due to a busy condition. @@ -1519,11 +1515,12 @@ fec_enet_rx(struct net_device *ndev, int budget) int pkt_received = 0; u16 queue_id; struct fec_enet_private *fep = netdev_priv(ndev); + struct fec_enet_priv_rx_q *rxq; for_each_set_bit(queue_id, >work_rx, FEC_ENET_MAX_RX_QS) { clear_bit(queue_id, >work_rx); - pkt_received += fec_enet_rx_queue(ndev, - budget - pkt_received, queue_id); + rxq = fep->rx_queue[FEC_ENET_GET_QUQUE(queue_id)]; + pkt_received += fec_rxq(ndev, fep, rxq, budget - pkt_received); } return pkt_received; } -- 2.5.0
[PATCH net-next V2 00/16] net: fec: cleanup and fixes
V2 is a rebase on top of johannes endian-safe patch and this set is only the next 16 patches. The testing for this series was done on a nitrogen6x. The base commit was commit f5461c27631672b9e95282812ee521c53f502eca Merge branch 'dsa-pass-bridge-to-drivers' Testing showed no change in performance. Testing used imx_v6_v7_defconfig + CONFIG_MICREL_PHY. The processor was running at 996Mhz. The following commands were used to get the transfer rates. On an x86 ubunto system, iperf -s -i.5 -u On a nitrogen6x board, running via SD Card. I first stopped some background processes stop cron stop upstart-file-bridge stop upstart-socket-bridge stop upstart-udev-bridge stop rsyslog stop dbus killall dhclient echo performance >/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor taskset 0x2 iperf -c 192.168.0.201 -u -t60 -b500M -r There is a branch available on github with this series, and the rest of my fec patches, for those who would like to test it. https://github.com:boundarydevices/linux-imx6.git branch net-next_master Troy Kisky (16): SDCard TX/RX 379/402 Before any patches 376/402 net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set 379/399 net: fec: pass rxq to fec_enet_rx_queue instead of queue_id 379/401 net: fec: pass txq to fec_enet_tx_queue instead of queue_id 389/409 net: fec: reduce interrupts 393/411 net: fec: split off napi routine with 3 queues 386/415 net: fec: don't clear all rx queue bits when just one is being checked 387/412 net: fec: set cbd_sc without relying on previous value 385/415 net: fec: eliminate calls to fec_enet_get_prevdesc 389/414 net: fec: move restart test for efficiency 386/412 net: fec: clear cbd_sc after transmission to help with debugging 387/415 net: fec: dump all tx queues in fec_dump 380/418 net: fec: detect tx int lost 385/417 net: fec: print more debug info in fec_timeout 388/417 net: fec: create subroutine reset_tx_queue 389/418 net: fec: call dma_unmap_single on mapped tx buffers at restart 381/414 net: fec: don't set cbd_bufaddr unless no mapping error drivers/net/ethernet/freescale/fec.h | 7 +- drivers/net/ethernet/freescale/fec_main.c | 420 -- 2 files changed, 224 insertions(+), 203 deletions(-) -- 2.5.0
Re: [PATCH net-next V2 7/8] net: fec: don't transfer ownership until descriptor write is complete
On 2/6/2016 4:52 AM, Sergei Shtylyov wrote: > Hello. > > On 2/6/2016 12:52 AM, Troy Kisky wrote: > >> If you don't own it, you shouldn't write to it. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 14 +- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index ca2708d..97ca72a 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -390,6 +390,10 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q >> *txq, >> >> bdp->cbd_bufaddr = cpu_to_fec32(addr); >> bdp->cbd_datlen = cpu_to_fec16(frag_len); >> +/* Make sure the updates to rest of the descriptor are >> + * performed before transferring ownership. >> + */ >> +wmb(); >> bdp->cbd_sc = cpu_to_fec16(status); >> } >> >> @@ -499,6 +503,10 @@ static int fec_enet_txq_submit_skb(struct >> fec_enet_priv_tx_q *txq, >> >> bdp->cbd_datlen = cpu_to_fec16(buflen); >> bdp->cbd_bufaddr = cpu_to_fec32(addr); >> +/* Make sure the updates to rest of the descriptor are performed before >> + * transferring ownership. >> + */ >> +wmb(); >> >> /* Send it on its way. Tell FEC it's ready, interrupt when done, >>* it's the last BD of the frame, and to put the CRC on the end. > [...] >> @@ -1484,6 +1491,11 @@ rx_processing_done: >> ebdp->cbd_prot = 0; >> ebdp->cbd_bdu = 0; >> } >> +/* Make sure the updates to rest of the descriptor are >> + * performed before transferring ownership. >> + */ >> +wmb(); >> +bdp->cbd_sc = cpu_to_fec16(status); > >I think you can use "ligter" dma_wmb() in this case. > Thanks for the review, I added a patch to the end of the series to address this. Troy
Re: [PATCH net-next V2 7/8] net: fec: don't transfer ownership until descriptor write is complete
On 2/5/2016 6:03 PM, Joshua Clayton wrote: > On Fri, 5 Feb 2016 14:52:49 -0700 > Troy Kisky <troy.ki...@boundarydevices.com> wrote: > >> If you don't own it, you shouldn't write to it. >> >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 14 +- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c index ca2708d..97ca72a >> 100644 --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -390,6 +390,10 @@ fec_enet_txq_submit_frag_skb(struct >> fec_enet_priv_tx_q *txq, >> bdp->cbd_bufaddr = cpu_to_fec32(addr); >> bdp->cbd_datlen = cpu_to_fec16(frag_len); >> +/* Make sure the updates to rest of the descriptor >> are >> + * performed before transferring ownership. >> + */ >> +wmb(); >> bdp->cbd_sc = cpu_to_fec16(status); > You use almost exactly the same code in each place. > I'd prefer to have wmb(); bdp->cbd_sc = cpu_to_fec16(status) wrapped in > a function or function like macro, and put the comment in one place. > Thanks for the review. I added a patch to the end of the series to create a common "trigger_tx" subroutine which should address this.
[PATCH net-next V2 5/8] net: fec: add variable reg_desc_active to speed things up
There is no need for complex macros every time we need to activate a queue. Also, no need to call skb_get_queue_mapping when we already know which queue it is using. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 7 + drivers/net/ethernet/freescale/fec_main.c | 44 +-- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 53ec04f..bedd28a 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -310,12 +310,6 @@ struct bufdesc_ex { #define FEC_R_BUFF_SIZE(X) (((X) == 1) ? FEC_R_BUFF_SIZE_1 : \ (((X) == 2) ? \ FEC_R_BUFF_SIZE_2 : FEC_R_BUFF_SIZE_0)) -#define FEC_R_DES_ACTIVE(X)(((X) == 1) ? FEC_R_DES_ACTIVE_1 : \ - (((X) == 2) ? \ - FEC_R_DES_ACTIVE_2 : FEC_R_DES_ACTIVE_0)) -#define FEC_X_DES_ACTIVE(X)(((X) == 1) ? FEC_X_DES_ACTIVE_1 : \ - (((X) == 2) ? \ - FEC_X_DES_ACTIVE_2 : FEC_X_DES_ACTIVE_0)) #define FEC_DMA_CFG(X) (((X) == 2) ? FEC_DMA_CFG_2 : FEC_DMA_CFG_1) @@ -454,6 +448,7 @@ struct bufdesc_prop { struct bufdesc *base; struct bufdesc *last; struct bufdesc *cur; + void __iomem*reg_desc_active; dma_addr_t dma; unsigned short ring_size; unsigned char dsize; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index b039288..712e3bb 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -328,7 +328,6 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, struct bufdesc *bdp = txq->bd.cur; struct bufdesc_ex *ebdp; int nr_frags = skb_shinfo(skb)->nr_frags; - unsigned short queue = skb_get_queue_mapping(skb); int frag, frag_len; unsigned short status; unsigned int estatus = 0; @@ -361,7 +360,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, if (fep->bufdesc_ex) { if (fep->quirks & FEC_QUIRK_HAS_AVB) - estatus |= FEC_TX_BD_FTYPE(queue); + estatus |= FEC_TX_BD_FTYPE(txq->bd.qid); if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; ebdp->cbd_bdu = 0; @@ -415,7 +414,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, dma_addr_t addr; unsigned short status; unsigned short buflen; - unsigned short queue; unsigned int estatus = 0; unsigned int index; int entries_free; @@ -444,7 +442,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, bufaddr = skb->data; buflen = skb_headlen(skb); - queue = skb_get_queue_mapping(skb); index = fec_enet_get_bd_index(bdp, >bd); if (((unsigned long) bufaddr) & fep->tx_align || fep->quirks & FEC_QUIRK_SWAP_FRAME) { @@ -487,7 +484,7 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; if (fep->quirks & FEC_QUIRK_HAS_AVB) - estatus |= FEC_TX_BD_FTYPE(queue); + estatus |= FEC_TX_BD_FTYPE(txq->bd.qid); if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; @@ -521,7 +518,7 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, txq->bd.cur = bdp; /* Trigger transmission start */ - writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue)); + writel(0, txq->bd.reg_desc_active); return 0; } @@ -534,7 +531,6 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, { struct fec_enet_private *fep = netdev_priv(ndev); struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc); - unsigned short queue = skb_get_queue_mapping(skb); unsigned short status; unsigned int estatus = 0; dma_addr_t addr; @@ -566,7 +562,7 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, if (fep->bufdesc_ex) { if (fep->quirks & FEC_QUIRK_HAS_AVB) - estatus |= FEC_TX_BD_FTYPE(queue); + estatus |= FEC_TX_BD_FTYPE(txq->bd.qid); if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_
[PATCH net-next V2 3/8] net: fec: fix fec_enet_get_free_txdesc_num
When first initialized, cur_tx points to the 1st entry in the queue, and dirty_tx points to the last. At this point, fec_enet_get_free_txdesc_num will return tx_ring_size -2. If tx_ring_size -2 entries are now queued, then fec_enet_get_free_txdesc_num should return 0, but it returns tx_ring_size instead. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 162fa59..adbddfd 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -296,7 +296,7 @@ static int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep, entries = ((const char *)txq->dirty_tx - (const char *)txq->cur_tx) / fep->bufdesc_size - 1; - return entries > 0 ? entries : entries + txq->tx_ring_size; + return entries >= 0 ? entries : entries + txq->tx_ring_size; } static void swap_buffer(void *bufaddr, int len) -- 2.5.0
[PATCH net-next V2 2/8] net: fec: fix rx error counts
On an overrun, the other flags are not valid, so don't check them. Also, don't pass bad frames up the stack. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 36 +-- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3e5b24a..162fa59 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1408,37 +1408,31 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id) break; pkt_received++; - /* Since we have allocated space to hold a complete frame, -* the last indicator should be set. -*/ - if ((status & BD_ENET_RX_LAST) == 0) - netdev_err(ndev, "rcv is not +last\n"); - writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); /* Check for errors. */ + status ^= BD_ENET_RX_LAST; if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO | - BD_ENET_RX_CR | BD_ENET_RX_OV)) { + BD_ENET_RX_CR | BD_ENET_RX_OV | BD_ENET_RX_LAST | + BD_ENET_RX_CL)) { ndev->stats.rx_errors++; - if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH)) { + if (status & BD_ENET_RX_OV) { + /* FIFO overrun */ + ndev->stats.rx_fifo_errors++; + goto rx_processing_done; + } + if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH + | BD_ENET_RX_LAST)) { /* Frame too long or too short. */ ndev->stats.rx_length_errors++; + if (status & BD_ENET_RX_LAST) + netdev_err(ndev, "rcv is not +last\n"); } - if (status & BD_ENET_RX_NO) /* Frame alignment */ - ndev->stats.rx_frame_errors++; if (status & BD_ENET_RX_CR) /* CRC Error */ ndev->stats.rx_crc_errors++; - if (status & BD_ENET_RX_OV) /* FIFO overrun */ - ndev->stats.rx_fifo_errors++; - } - - /* Report late collisions as a frame error. -* On this error, the BD is closed, but we don't know what we -* have in the buffer. So, just drop this frame on the floor. -*/ - if (status & BD_ENET_RX_CL) { - ndev->stats.rx_errors++; - ndev->stats.rx_frame_errors++; + /* Report late collisions as a frame error. */ + if (status & (BD_ENET_RX_NO | BD_ENET_RX_CL)) + ndev->stats.rx_frame_errors++; goto rx_processing_done; } -- 2.5.0
[PATCH net-next V2 8/8] net: fec: improve error handling
Unmap initial buffer on error. Don't free skb until it has been unmapped. Move cbd_bufaddr assignment closer to the mapping function. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 97ca72a..ef18ca5 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -382,7 +382,6 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, addr = dma_map_single(>pdev->dev, bufaddr, frag_len, DMA_TO_DEVICE); if (dma_mapping_error(>pdev->dev, addr)) { - dev_kfree_skb_any(skb); if (net_ratelimit()) netdev_err(ndev, "Tx DMA memory map failed\n"); goto dma_mapping_error; @@ -467,8 +466,12 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, if (nr_frags) { last_bdp = fec_enet_txq_submit_frag_skb(txq, skb, ndev); - if (IS_ERR(last_bdp)) + if (IS_ERR(last_bdp)) { + dma_unmap_single(>pdev->dev, addr, +buflen, DMA_TO_DEVICE); + dev_kfree_skb_any(skb); return NETDEV_TX_OK; + } } else { status |= (BD_ENET_TX_INTR | BD_ENET_TX_LAST); if (fep->bufdesc_ex) { @@ -478,6 +481,8 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, estatus |= BD_ENET_TX_TS; } } + bdp->cbd_bufaddr = cpu_to_fec32(addr); + bdp->cbd_datlen = cpu_to_fec16(buflen); if (fep->bufdesc_ex) { @@ -501,8 +506,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, /* Save skb pointer */ txq->tx_skbuff[index] = skb; - bdp->cbd_datlen = cpu_to_fec16(buflen); - bdp->cbd_bufaddr = cpu_to_fec32(addr); /* Make sure the updates to rest of the descriptor are performed before * transferring ownership. */ -- 2.5.0
[PATCH net-next V2 4/8] net: fec: add struct bufdesc_prop
This reduces code and gains speed. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 29 ++- drivers/net/ethernet/freescale/fec_main.c | 288 -- 2 files changed, 132 insertions(+), 185 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index cc9677a..53ec04f 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -448,33 +448,34 @@ struct bufdesc_ex { /* Controller supports RACC register */ #define FEC_QUIRK_HAS_RACC (1 << 12) +struct bufdesc_prop { + int qid; + /* Address of Rx and Tx buffers */ + struct bufdesc *base; + struct bufdesc *last; + struct bufdesc *cur; + dma_addr_t dma; + unsigned short ring_size; + unsigned char dsize; + unsigned char dsize_log2; +}; + struct fec_enet_priv_tx_q { - int index; + struct bufdesc_prop bd; unsigned char *tx_bounce[TX_RING_SIZE]; struct sk_buff *tx_skbuff[TX_RING_SIZE]; - dma_addr_t bd_dma; - struct bufdesc *tx_bd_base; - uint tx_ring_size; - unsigned short tx_stop_threshold; unsigned short tx_wake_threshold; - struct bufdesc *cur_tx; struct bufdesc *dirty_tx; char *tso_hdrs; dma_addr_t tso_hdrs_dma; }; struct fec_enet_priv_rx_q { - int index; + struct bufdesc_prop bd; struct sk_buff *rx_skbuff[RX_RING_SIZE]; - - dma_addr_t bd_dma; - struct bufdesc *rx_bd_base; - uint rx_ring_size; - - struct bufdesc *cur_rx; }; /* The FEC buffer descriptors track the ring buffers. The rx_bd_base and @@ -514,8 +515,6 @@ struct fec_enet_private { unsigned long work_ts; unsigned long work_mdio; - unsigned short bufdesc_size; - struct platform_device *pdev; int dev_id; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index adbddfd..b039288 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -217,86 +217,38 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); #define IS_TSO_HEADER(txq, addr) \ ((addr >= txq->tso_hdrs_dma) && \ - (addr < txq->tso_hdrs_dma + txq->tx_ring_size * TSO_HEADER_SIZE)) + (addr < txq->tso_hdrs_dma + txq->bd.ring_size * TSO_HEADER_SIZE)) static int mii_cnt; -static inline -struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, - struct fec_enet_private *fep, - int queue_id) -{ - struct bufdesc *new_bd = bdp + 1; - struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp + 1; - struct fec_enet_priv_tx_q *txq = fep->tx_queue[queue_id]; - struct fec_enet_priv_rx_q *rxq = fep->rx_queue[queue_id]; - struct bufdesc_ex *ex_base; - struct bufdesc *base; - int ring_size; - - if (bdp >= txq->tx_bd_base) { - base = txq->tx_bd_base; - ring_size = txq->tx_ring_size; - ex_base = (struct bufdesc_ex *)txq->tx_bd_base; - } else { - base = rxq->rx_bd_base; - ring_size = rxq->rx_ring_size; - ex_base = (struct bufdesc_ex *)rxq->rx_bd_base; - } - - if (fep->bufdesc_ex) - return (struct bufdesc *)((ex_new_bd >= (ex_base + ring_size)) ? - ex_base : ex_new_bd); - else - return (new_bd >= (base + ring_size)) ? - base : new_bd; -} - -static inline -struct bufdesc *fec_enet_get_prevdesc(struct bufdesc *bdp, - struct fec_enet_private *fep, - int queue_id) -{ - struct bufdesc *new_bd = bdp - 1; - struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp - 1; - struct fec_enet_priv_tx_q *txq = fep->tx_queue[queue_id]; - struct fec_enet_priv_rx_q *rxq = fep->rx_queue[queue_id]; - struct bufdesc_ex *ex_base; - struct bufdesc *base; - int ring_size; - - if (bdp >= txq->tx_bd_base) { - base = txq->tx_bd_base; - ring_size = txq->tx_ring_size; - ex_base = (struct bufdesc_ex *)txq->tx_bd_base; - } else { - base = rxq->rx_bd_base; - ring_size = rxq->rx_ring_size; - ex_base = (struct bufdesc_ex *)rxq->rx_bd_base; - } +static struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, +struct bufdesc_prop *bd) +{ + return (bdp >= bd->last) ? bd->base + : (struct bufde
[PATCH net-next V2 0/8] net: fec: cleanup/fixes
V2 is a rebase on top of johannes endian-safe patch and is only the 1st eight patches. The testing for this series was done on a nitrogen6x. The base commit was commit b45efa30a626e915192a6c548cd8642379cd47cc Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net Testing showed no change in performance. Testing used imx_v6_v7_defconfig + CONFIG_MICREL_PHY. The processor was running at 996Mhz. The following commands were used to get the transfer rates. On an x86 ubunto system, iperf -s -i.5 -u On a nitrogen6x board, running via SD Card. I first stopped some background processes stop cron stop upstart-file-bridge stop upstart-socket-bridge stop upstart-udev-bridge stop rsyslog stop dbus killall dhclient echo performance >/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor taskset 0x2 iperf -c 192.168.0.201 -u -t60 -b500M -r There is a branch available on github with this series, and the rest of my fec patches, for those who would like to test it. https://github.com:boundarydevices/linux-imx6.git branch net-next_master Troy Kisky (8): SDCard TX/RX 378/405 before any patches 379/407 net: fec: stop the "rcv is not +last, " error messages 378/405 net: fec: fix rx error counts 379/396 net: fec: fix fec_enet_get_free_txdesc_num 379/403 net: fec: add struct bufdesc_prop 390/396 net: fec: add variable reg_desc_active to speed things up 388/396 net: fec: don't disable FEC_ENET_TS_TIMER interrupt 382/403 net: fec: don't transfer ownership until descriptor write is complete 378/403 net: fec: improve error handling drivers/net/ethernet/freescale/fec.h | 38 ++- drivers/net/ethernet/freescale/fec_main.c | 396 ++ 2 files changed, 196 insertions(+), 238 deletions(-) -- 2.5.0
[PATCH net-next V2 7/8] net: fec: don't transfer ownership until descriptor write is complete
If you don't own it, you shouldn't write to it. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index ca2708d..97ca72a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -390,6 +390,10 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, bdp->cbd_bufaddr = cpu_to_fec32(addr); bdp->cbd_datlen = cpu_to_fec16(frag_len); + /* Make sure the updates to rest of the descriptor are +* performed before transferring ownership. +*/ + wmb(); bdp->cbd_sc = cpu_to_fec16(status); } @@ -499,6 +503,10 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, bdp->cbd_datlen = cpu_to_fec16(buflen); bdp->cbd_bufaddr = cpu_to_fec32(addr); + /* Make sure the updates to rest of the descriptor are performed before +* transferring ownership. +*/ + wmb(); /* Send it on its way. Tell FEC it's ready, interrupt when done, * it's the last BD of the frame, and to put the CRC on the end. @@ -1475,7 +1483,6 @@ rx_processing_done: /* Mark the buffer empty */ status |= BD_ENET_RX_EMPTY; - bdp->cbd_sc = cpu_to_fec16(status); if (fep->bufdesc_ex) { struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; @@ -1484,6 +1491,11 @@ rx_processing_done: ebdp->cbd_prot = 0; ebdp->cbd_bdu = 0; } + /* Make sure the updates to rest of the descriptor are +* performed before transferring ownership. +*/ + wmb(); + bdp->cbd_sc = cpu_to_fec16(status); /* Update BD pointer to next entry */ bdp = fec_enet_get_nextdesc(bdp, >bd); -- 2.5.0
[PATCH net-next V2 6/8] net: fec: don't disable FEC_ENET_TS_TIMER interrupt
Only the interrupt routine processes this condition. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index bedd28a..195122e 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -375,6 +375,7 @@ struct bufdesc_ex { #define FEC_ENET_TS_TIMER ((uint)0x8000) #define FEC_DEFAULT_IMASK (FEC_ENET_TXF | FEC_ENET_RXF | FEC_ENET_MII | FEC_ENET_TS_TIMER) +#define FEC_NAPI_IMASK (FEC_ENET_MII | FEC_ENET_TS_TIMER) #define FEC_RX_DISABLED_IMASK (FEC_DEFAULT_IMASK & (~FEC_ENET_RXF)) /* ENET interrupt coalescing macro define */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 712e3bb..ca2708d 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1553,7 +1553,7 @@ fec_enet_interrupt(int irq, void *dev_id) if (napi_schedule_prep(>napi)) { /* Disable the NAPI interrupts */ - writel(FEC_ENET_MII, fep->hwp + FEC_IMASK); + writel(FEC_NAPI_IMASK, fep->hwp + FEC_IMASK); __napi_schedule(>napi); } } -- 2.5.0
[PATCH net-next V2 1/8] net: fec: stop the "rcv is not +last, " error messages
Setting the FTRL register will stop the fec from trying to use multiple receive buffers. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 2106d72..cc9677a 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -64,6 +64,7 @@ #define FEC_R_FIFO_RSEM0x194 /* Receive FIFO section empty threshold */ #define FEC_R_FIFO_RAEM0x198 /* Receive FIFO almost empty threshold */ #define FEC_R_FIFO_RAFL0x19c /* Receive FIFO almost full threshold */ +#define FEC_FTRL 0x1b0 /* Frame truncation receive length*/ #define FEC_RACC 0x1c4 /* Receive Accelerator function */ #define FEC_RCMR_1 0x1c8 /* Receive classification match ring 1 */ #define FEC_RCMR_2 0x1cc /* Receive classification match ring 2 */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 41c81f6..3e5b24a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -988,6 +988,7 @@ fec_restart(struct net_device *ndev) val &= ~FEC_RACC_OPTIONS; writel(val, fep->hwp + FEC_RACC); } + writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_FTRL); #endif /* -- 2.5.0
Re: [PATCH net-next 08/40] net: fec: move cbd_bufaddr assignment closer to the mapping function
On 1/28/2016 3:02 PM, Arnd Bergmann wrote: > On Thursday 28 January 2016 14:25:32 Troy Kisky wrote: >> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> >> --- >> drivers/net/ethernet/freescale/fec_main.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > [note: missing changelog?] > >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index b87f80d..15a93f90 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -476,6 +476,8 @@ static int fec_enet_txq_submit_skb(struct >> fec_enet_priv_tx_q *txq, >> estatus |= BD_ENET_TX_TS; >> } >> } >> +bdp->cbd_bufaddr = addr; >> +bdp->cbd_datlen = buflen; >> >> if (fep->bufdesc_ex) { >> >> @@ -499,8 +501,6 @@ static int fec_enet_txq_submit_skb(struct >> fec_enet_priv_tx_q *txq, >> /* Save skb pointer */ >> txq->tx_skbuff[index] = skb; >> >> -bdp->cbd_datlen = buflen; >> -bdp->cbd_bufaddr = addr; >> /* Make sure the updates to rest of the descriptor are performed before >> * transferring ownership. >> */ >> > > This patch and others in the series conflicts with the bugfix "net: fec: make > driver endian-safe" that Johannes sent this week. Can you include his fix > in your series and ensure that all descriptor accesses are done in > an endian-safe way? > > Arnd Sure, I'll rebase and send the first few when net-next is updated. Thanks Troy
[PATCH net-next 04/40] net: fec: add struct bufdesc_prop
This reduces code and gains speed. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 29 ++- drivers/net/ethernet/freescale/fec_main.c | 288 -- 2 files changed, 132 insertions(+), 185 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 5144e73..6018d0e4 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -432,33 +432,34 @@ struct bufdesc_ex { /* Controller supports RACC register */ #define FEC_QUIRK_HAS_RACC (1 << 12) +struct bufdesc_prop { + int qid; + /* Address of Rx and Tx buffers */ + struct bufdesc *base; + struct bufdesc *last; + struct bufdesc *cur; + dma_addr_t dma; + unsigned short ring_size; + unsigned char dsize; + unsigned char dsize_log2; +}; + struct fec_enet_priv_tx_q { - int index; + struct bufdesc_prop bd; unsigned char *tx_bounce[TX_RING_SIZE]; struct sk_buff *tx_skbuff[TX_RING_SIZE]; - dma_addr_t bd_dma; - struct bufdesc *tx_bd_base; - uint tx_ring_size; - unsigned short tx_stop_threshold; unsigned short tx_wake_threshold; - struct bufdesc *cur_tx; struct bufdesc *dirty_tx; char *tso_hdrs; dma_addr_t tso_hdrs_dma; }; struct fec_enet_priv_rx_q { - int index; + struct bufdesc_prop bd; struct sk_buff *rx_skbuff[RX_RING_SIZE]; - - dma_addr_t bd_dma; - struct bufdesc *rx_bd_base; - uint rx_ring_size; - - struct bufdesc *cur_rx; }; /* The FEC buffer descriptors track the ring buffers. The rx_bd_base and @@ -498,8 +499,6 @@ struct fec_enet_private { unsigned long work_ts; unsigned long work_mdio; - unsigned short bufdesc_size; - struct platform_device *pdev; int dev_id; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 8e1f2a6..feff466 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -217,86 +217,38 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); #define IS_TSO_HEADER(txq, addr) \ ((addr >= txq->tso_hdrs_dma) && \ - (addr < txq->tso_hdrs_dma + txq->tx_ring_size * TSO_HEADER_SIZE)) + (addr < txq->tso_hdrs_dma + txq->bd.ring_size * TSO_HEADER_SIZE)) static int mii_cnt; -static inline -struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, - struct fec_enet_private *fep, - int queue_id) -{ - struct bufdesc *new_bd = bdp + 1; - struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp + 1; - struct fec_enet_priv_tx_q *txq = fep->tx_queue[queue_id]; - struct fec_enet_priv_rx_q *rxq = fep->rx_queue[queue_id]; - struct bufdesc_ex *ex_base; - struct bufdesc *base; - int ring_size; - - if (bdp >= txq->tx_bd_base) { - base = txq->tx_bd_base; - ring_size = txq->tx_ring_size; - ex_base = (struct bufdesc_ex *)txq->tx_bd_base; - } else { - base = rxq->rx_bd_base; - ring_size = rxq->rx_ring_size; - ex_base = (struct bufdesc_ex *)rxq->rx_bd_base; - } - - if (fep->bufdesc_ex) - return (struct bufdesc *)((ex_new_bd >= (ex_base + ring_size)) ? - ex_base : ex_new_bd); - else - return (new_bd >= (base + ring_size)) ? - base : new_bd; -} - -static inline -struct bufdesc *fec_enet_get_prevdesc(struct bufdesc *bdp, - struct fec_enet_private *fep, - int queue_id) -{ - struct bufdesc *new_bd = bdp - 1; - struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp - 1; - struct fec_enet_priv_tx_q *txq = fep->tx_queue[queue_id]; - struct fec_enet_priv_rx_q *rxq = fep->rx_queue[queue_id]; - struct bufdesc_ex *ex_base; - struct bufdesc *base; - int ring_size; - - if (bdp >= txq->tx_bd_base) { - base = txq->tx_bd_base; - ring_size = txq->tx_ring_size; - ex_base = (struct bufdesc_ex *)txq->tx_bd_base; - } else { - base = rxq->rx_bd_base; - ring_size = rxq->rx_ring_size; - ex_base = (struct bufdesc_ex *)rxq->rx_bd_base; - } +static struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, +struct bufdesc_prop *bd) +{ + return (bdp >= bd->last) ? bd->base + : (struct bufde
[PATCH net-next 00/40] net: fec: cleanup/fixes
The testing for this series was done on a nitrogen6x. The base commit was commit 7a26019fdecdb45ff784ae4e3b7e0cc9045100ca Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net Testing used imx_v6_v7_defconfig + CONFIG_MICREL_PHY. The processor was running at 996Mhz. The last 2 patches are independent from this series, but I included here to have the bench marks in one place. The following commands were used to get the transfer rates. On an x86 ubunto system, iperf -s -i.5 -u On a nitrogen6x board, running via NFS/Sd card. I first stopped some background processes stop cron stop upstart-file-bridge stop upstart-socket-bridge stop upstart-udev-bridge stop rsyslog stop dbus killall dhclient echo performance >/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor taskset 0x2 iperf -c 192.168.0.201 -u -t60 -b500M -r There is a branch available on github with this series https://github.com:boundarydevices/linux-imx6.git branch net-next_master Troy Kisky (40): NFS SDCARD TX/RX TX/RX Mbits/sec 377/406 379/407 before any patches 377/409 379/406 net: fec: stop the "rcv is not +last, " error messages 374/406 374/407 net: fec: fix rx error counts 373/410 377/407 net: fec: fix fec_enet_get_free_txdesc_num 374/408 379/413 net: fec: add struct bufdesc_prop 375/410 379/412 net: fec: add variable reg_desc_active to speed things up 380/413 383/416 net: fec: don't disable FEC_ENET_TS_TIMER interrupt 375/406 379/413 net: fec: don't transfer ownership until descriptor write is complete 373/411 378/414 net: fec: move cbd_bufaddr assignment closer to the mapping function 372/411 377/414 net: fec: only check queue 0 if RXF_0/TXF_0 interrupt is set 375/410 375/412 net: fec: pass rxq to fec_enet_rx_queue instead of queue_id 373/414 377/413 net: fec: pass txq to fec_enet_tx_queue instead of queue_id 380/426 382/425 net: fec: reduce interrupts 378/418 379/417 net: fec: split off napi routine with 3 queues 379/417 379/418 net: fec: don't clear all rx queue bits when just one is being checked 379/420 379/420 net: fec: set cbd_sc without relying on previous value 379/422 379/422 net: fec: eliminate calls to fec_enet_get_prevdesc 379/425 379/424 net: fec: move restart test for efficiency 379/415 379/419 net: fec: clear cbd_sc after transmission to help with debugging 373/420 379/421 net: fec: dump all tx queues in fec_dump 378/425 379/427 net: fec: detect tx int lost 379/416 379/418 net: fec: print more debug info in fec_timeout 379/415 380/415 net: fec: call dma_unmap_single on mapped tx buffers at restart 382/419 384/418 net: fec: don't set cbd_bufaddr unless no mapping error 379/417 380/415 net: fec: return NETDEV_TX_BUSY, when not enough space in ring 379/417 379/417 net: fec: don't free skb until it has been unmapped 378/421 379/425 net: fec: set cbd_esc only once 379/413 380/411 net: fec: enable interrupt on very last descriptor only 381/411 381/411 net: fec: unmap initial buffer on error 379/419 387/417 net: fec: don't free skb when returning busy 384/419 386/418 net: fec: move last_bdp assignment for symmetry 383/420 387/419 net: fec: don't transfer ownership until entire tso is queued 381/424 384/424 net: fec: fix err_release in fec_enet_txq_submit_tso 382/423 383/422 net: fec: shrink the window for 'tx int lost' 387/418 388/420 net: fec: update dirty_tx even if no skb 387/423 390/423 net: fec: rename dirty_tx to pending_tx 388/426 389/423 net: fec: use mac set by bootloader before fuses 379/425 380/432 net: fec: add events device file 379/432 381/429 net: fec: recover from lost rxf_0 interrupt 381/432 381/432 ARM: dts: imx6qdl-nitrogen6x: add phy interrupt to eliminate polling 380/432 380/432 ARM: dts: imx6qdl-nitrogen6_max: add phy interrupt to eliminate polling The same patch set on Freescale's 3.14 release gives 458/619 Mbits/sec with the command modified to taskset 0x2 iperf -c 192.168.0.201 -u -t60 -b600M -r The huge difference may be related to the cpuidle driver which isn't enabled in imx_v6_v7_defconfig and doesn't seem to work if it is enabled. arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi | 9 + arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi| 11 + drivers/net/ethernet/freescale/fec.h | 59 +- drivers/net/ethernet/freescale/fec_main.c| 921 ++- 4 files changed, 525 insertions(+), 475 deletions(-) -- 2.5.0
[PATCH net-next 02/40] net: fec: fix rx error counts
On an overrun, the other flags are not valid, so don't check them. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 36 +-- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 2c7c845..620834f 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1403,37 +1403,31 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id) break; pkt_received++; - /* Since we have allocated space to hold a complete frame, -* the last indicator should be set. -*/ - if ((status & BD_ENET_RX_LAST) == 0) - netdev_err(ndev, "rcv is not +last\n"); - writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); /* Check for errors. */ + status ^= BD_ENET_RX_LAST; if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO | - BD_ENET_RX_CR | BD_ENET_RX_OV)) { + BD_ENET_RX_CR | BD_ENET_RX_OV | BD_ENET_RX_LAST | + BD_ENET_RX_CL)) { ndev->stats.rx_errors++; - if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH)) { + if (status & BD_ENET_RX_OV) { + /* FIFO overrun */ + ndev->stats.rx_fifo_errors++; + goto rx_processing_done; + } + if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH + | BD_ENET_RX_LAST)) { /* Frame too long or too short. */ ndev->stats.rx_length_errors++; + if (status & BD_ENET_RX_LAST) + netdev_err(ndev, "rcv is not +last\n"); } - if (status & BD_ENET_RX_NO) /* Frame alignment */ - ndev->stats.rx_frame_errors++; if (status & BD_ENET_RX_CR) /* CRC Error */ ndev->stats.rx_crc_errors++; - if (status & BD_ENET_RX_OV) /* FIFO overrun */ - ndev->stats.rx_fifo_errors++; - } - - /* Report late collisions as a frame error. -* On this error, the BD is closed, but we don't know what we -* have in the buffer. So, just drop this frame on the floor. -*/ - if (status & BD_ENET_RX_CL) { - ndev->stats.rx_errors++; - ndev->stats.rx_frame_errors++; + /* Report late collisions as a frame error. */ + if (status & (BD_ENET_RX_NO | BD_ENET_RX_CL)) + ndev->stats.rx_frame_errors++; goto rx_processing_done; } -- 2.5.0
[PATCH net-next 05/40] net: fec: add variable reg_desc_active to speed things up
There is no need for complex macros every time we need to activate a queue. Also, no need to call skb_get_queue_mapping when we already know which queue it is using. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 7 + drivers/net/ethernet/freescale/fec_main.c | 44 +-- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 6018d0e4..f8d7fdb 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -294,12 +294,6 @@ struct bufdesc_ex { #define FEC_R_BUFF_SIZE(X) (((X) == 1) ? FEC_R_BUFF_SIZE_1 : \ (((X) == 2) ? \ FEC_R_BUFF_SIZE_2 : FEC_R_BUFF_SIZE_0)) -#define FEC_R_DES_ACTIVE(X)(((X) == 1) ? FEC_R_DES_ACTIVE_1 : \ - (((X) == 2) ? \ - FEC_R_DES_ACTIVE_2 : FEC_R_DES_ACTIVE_0)) -#define FEC_X_DES_ACTIVE(X)(((X) == 1) ? FEC_X_DES_ACTIVE_1 : \ - (((X) == 2) ? \ - FEC_X_DES_ACTIVE_2 : FEC_X_DES_ACTIVE_0)) #define FEC_DMA_CFG(X) (((X) == 2) ? FEC_DMA_CFG_2 : FEC_DMA_CFG_1) @@ -438,6 +432,7 @@ struct bufdesc_prop { struct bufdesc *base; struct bufdesc *last; struct bufdesc *cur; + void __iomem*reg_desc_active; dma_addr_t dma; unsigned short ring_size; unsigned char dsize; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index feff466..0e8c8b6 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -326,7 +326,6 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, struct bufdesc *bdp = txq->bd.cur; struct bufdesc_ex *ebdp; int nr_frags = skb_shinfo(skb)->nr_frags; - unsigned short queue = skb_get_queue_mapping(skb); int frag, frag_len; unsigned short status; unsigned int estatus = 0; @@ -359,7 +358,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq, if (fep->bufdesc_ex) { if (fep->quirks & FEC_QUIRK_HAS_AVB) - estatus |= FEC_TX_BD_FTYPE(queue); + estatus |= FEC_TX_BD_FTYPE(txq->bd.qid); if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; ebdp->cbd_bdu = 0; @@ -413,7 +412,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, dma_addr_t addr; unsigned short status; unsigned short buflen; - unsigned short queue; unsigned int estatus = 0; unsigned int index; int entries_free; @@ -442,7 +440,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, bufaddr = skb->data; buflen = skb_headlen(skb); - queue = skb_get_queue_mapping(skb); index = fec_enet_get_bd_index(bdp, >bd); if (((unsigned long) bufaddr) & fep->tx_align || fep->quirks & FEC_QUIRK_SWAP_FRAME) { @@ -485,7 +482,7 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; if (fep->quirks & FEC_QUIRK_HAS_AVB) - estatus |= FEC_TX_BD_FTYPE(queue); + estatus |= FEC_TX_BD_FTYPE(txq->bd.qid); if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; @@ -519,7 +516,7 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq, txq->bd.cur = bdp; /* Trigger transmission start */ - writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue)); + writel(0, txq->bd.reg_desc_active); return 0; } @@ -532,7 +529,6 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, { struct fec_enet_private *fep = netdev_priv(ndev); struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc); - unsigned short queue = skb_get_queue_mapping(skb); unsigned short status; unsigned int estatus = 0; dma_addr_t addr; @@ -564,7 +560,7 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb, if (fep->bufdesc_ex) { if (fep->quirks & FEC_QUIRK_HAS_AVB) - estatus |= FEC_TX_BD_FTYPE(queue); + estatus |= FEC_TX_BD_FTYPE(txq->bd.qid); if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_E
[PATCH net-next 13/40] net: fec: split off napi routine with 3 queues
If we only have 1 tx/rx queue, we need not check the other queues. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec_main.c | 39 +-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 8bebf01..04b1fb7 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1510,7 +1510,7 @@ fec_enet_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } -static int fec_enet_rx_napi(struct napi_struct *napi, int budget) +static int fec_enet_napi_q3(struct napi_struct *napi, int budget) { struct net_device *ndev = napi->dev; struct fec_enet_private *fep = netdev_priv(ndev); @@ -1553,6 +1553,39 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget) return pkts; } +static int fec_enet_napi_q1(struct napi_struct *napi, int budget) +{ + struct net_device *ndev = napi->dev; + struct fec_enet_private *fep = netdev_priv(ndev); + int pkts = 0; + uint events; + + do { + events = readl(fep->hwp + FEC_IEVENT); + if (fep->events) { + events |= fep->events; + fep->events = 0; + } + events &= FEC_ENET_RXF_0 | FEC_ENET_TXF_0; + if (!events) { + if (budget) { + napi_complete(napi); + writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK); + } + return pkts; + } + + writel(events, fep->hwp + FEC_IEVENT); + if (events & FEC_ENET_RXF_0) + pkts += fec_rxq(ndev, fep, fep->rx_queue[0], + budget - pkts); + if (events & FEC_ENET_TXF_0) + fec_txq(ndev, fep, fep->tx_queue[0]); + } while (pkts < budget); + fep->events |= FEC_ENET_RXF_0; /* save for next callback */ + return pkts; +} + /* - */ static void fec_get_mac(struct net_device *ndev) { @@ -3113,7 +3146,9 @@ static int fec_enet_init(struct net_device *ndev) ndev->ethtool_ops = _enet_ethtool_ops; writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK); - netif_napi_add(ndev, >napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT); + netif_napi_add(ndev, >napi, (fep->num_rx_queues | + fep->num_tx_queues) == 1 ? fec_enet_napi_q1 : + fec_enet_napi_q3, NAPI_POLL_WEIGHT); if (fep->quirks & FEC_QUIRK_HAS_VLAN) /* enable hw VLAN support */ -- 2.5.0
[PATCH net-next 06/40] net: fec: don't disable FEC_ENET_TS_TIMER interrupt
Only the interrupt routine processes this condition. Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com> --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index f8d7fdb..684fe91f 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -359,6 +359,7 @@ struct bufdesc_ex { #define FEC_ENET_TS_TIMER ((uint)0x8000) #define FEC_DEFAULT_IMASK (FEC_ENET_TXF | FEC_ENET_RXF | FEC_ENET_MII | FEC_ENET_TS_TIMER) +#define FEC_NAPI_IMASK (FEC_ENET_MII | FEC_ENET_TS_TIMER) #define FEC_RX_DISABLED_IMASK (FEC_DEFAULT_IMASK & (~FEC_ENET_RXF)) /* ENET interrupt coalescing macro define */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 0e8c8b6..e8f35fb 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1545,7 +1545,7 @@ fec_enet_interrupt(int irq, void *dev_id) if (napi_schedule_prep(>napi)) { /* Disable the NAPI interrupts */ - writel(FEC_ENET_MII, fep->hwp + FEC_IMASK); + writel(FEC_NAPI_IMASK, fep->hwp + FEC_IMASK); __napi_schedule(>napi); } } -- 2.5.0