* [PATCH] net: macb: Apply RXUBR workaround only to versions with errata
@ 2019-01-24 13:38 Harini Katakam
2019-01-25 10:28 ` Nicolas.Ferre
0 siblings, 1 reply; 3+ messages in thread
From: Harini Katakam @ 2019-01-24 13:38 UTC (permalink / raw)
To: nicolas.ferre, davem, claudiu.beznea, brandon.streiff
Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux,
harini.katakam
The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91 only. Subsequent versions do not need this
workaround. This workaround unecessarily resets RX whenever RX used
bit read is observed, which can be often under heavy traffic. Hence
introduce an CAPS mask and a check to enable this workaround.
Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
---
Changes from RFC:
- Use CAPS mask instead introducing and errata field.
- Use check only on RX reset part; ISR should still be cleared.
drivers/net/ethernet/cadence/macb.h | 1 +
drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++------
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c..2b412fa 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -643,6 +643,7 @@
#define MACB_CAPS_JUMBO 0x00000020
#define MACB_CAPS_GEM_HAS_PTP 0x00000040
#define MACB_CAPS_BD_RD_PREFETCH 0x00000080
+#define MACB_CAPS_NEEDS_RSTONUBR 0x00000100
#define MACB_CAPS_FIFO_MODE 0x10000000
#define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
#define MACB_CAPS_SG_DISABLED 0x40000000
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 66cc792..0bda1cd 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1416,10 +1416,12 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
* section 16.7.4 for details.
*/
if (status & MACB_BIT(RXUBR)) {
- ctrl = macb_readl(bp, NCR);
- macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
- wmb();
- macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+ if (bp->caps & MACB_CAPS_NEEDS_RSTONUBR) {
+ ctrl = macb_readl(bp, NCR);
+ macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+ wmb();
+ macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+ }
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
queue_writel(queue, ISR, MACB_BIT(RXUBR));
@@ -3864,7 +3866,8 @@ static int at91ether_init(struct platform_device *pdev)
}
static const struct macb_config at91sam9260_config = {
- .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
+ .caps = MACB_CAPS_USRIO_HAS_CLKEN |
+ MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII | MACB_CAPS_NEEDS_RSTONUBR,
.clk_init = macb_clk_init,
.init = macb_init,
};
@@ -3928,7 +3931,8 @@ static const struct macb_config zynqmp_config = {
};
static const struct macb_config zynq_config = {
- .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF,
+ .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
+ MACB_CAPS_NEEDS_RSTONUBR,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] net: macb: Apply RXUBR workaround only to versions with errata
2019-01-24 13:38 [PATCH] net: macb: Apply RXUBR workaround only to versions with errata Harini Katakam
@ 2019-01-25 10:28 ` Nicolas.Ferre
2019-01-25 11:03 ` Harini Katakam
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas.Ferre @ 2019-01-25 10:28 UTC (permalink / raw)
To: harini.katakam, davem, Claudiu.Beznea, brandon.streiff
Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux
On 24/01/2019 at 14:38, Harini Katakam wrote:
> The interrupt handler contains a workaround for RX hang applicable
> to Zynq and AT91 only. Subsequent versions do not need this
AT91RM9200 only. It's not the case for other AT91 SoCs (reading errata
list for them).
That being said I have to add a patch for making this perfectly clear in
the comment just above the flag's test.
> workaround. This workaround unecessarily resets RX whenever RX used
Typo: unnecessarily
> bit read is observed, which can be often under heavy traffic. Hence
> introduce an CAPS mask and a check to enable this workaround.
Nack for this one, see below...
> Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
> ---
> Changes from RFC:
> - Use CAPS mask instead introducing and errata field.
> - Use check only on RX reset part; ISR should still be cleared.
>
> drivers/net/ethernet/cadence/macb.h | 1 +
> drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++------
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 3d45f4c..2b412fa 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -643,6 +643,7 @@
> #define MACB_CAPS_JUMBO 0x00000020
> #define MACB_CAPS_GEM_HAS_PTP 0x00000040
> #define MACB_CAPS_BD_RD_PREFETCH 0x00000080
> +#define MACB_CAPS_NEEDS_RSTONUBR 0x00000100
> #define MACB_CAPS_FIFO_MODE 0x10000000
> #define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
> #define MACB_CAPS_SG_DISABLED 0x40000000
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 66cc792..0bda1cd 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1416,10 +1416,12 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
> * section 16.7.4 for details.
> */
> if (status & MACB_BIT(RXUBR)) {
> - ctrl = macb_readl(bp, NCR);
> - macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> - wmb();
> - macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> + if (bp->caps & MACB_CAPS_NEEDS_RSTONUBR) {
> + ctrl = macb_readl(bp, NCR);
> + macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> + wmb();
> + macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> + }
As this interrupt routine is not doing anything else than just being
called now, what about just removing the use of this status bit for
platforms which don't need this RX reset?
> if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> queue_writel(queue, ISR, MACB_BIT(RXUBR));
> @@ -3864,7 +3866,8 @@ static int at91ether_init(struct platform_device *pdev)
> }
>
> static const struct macb_config at91sam9260_config = {
> - .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
> + .caps = MACB_CAPS_USRIO_HAS_CLKEN |
> + MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII | MACB_CAPS_NEEDS_RSTONUBR,
> .clk_init = macb_clk_init,
> .init = macb_init,
Actually it seems not the proper configuration structure to modify: You
must change the emac_config instead.
> };
> @@ -3928,7 +3931,8 @@ static const struct macb_config zynqmp_config = {
> };
>
> static const struct macb_config zynq_config = {
> - .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF,
> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
> + MACB_CAPS_NEEDS_RSTONUBR,
> .dma_burst_length = 16,
> .clk_init = macb_clk_init,
> .init = macb_init,
Best regards,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] net: macb: Apply RXUBR workaround only to versions with errata
2019-01-25 10:28 ` Nicolas.Ferre
@ 2019-01-25 11:03 ` Harini Katakam
0 siblings, 0 replies; 3+ messages in thread
From: Harini Katakam @ 2019-01-25 11:03 UTC (permalink / raw)
To: Nicolas Ferre
Cc: Harini Katakam, David Miller, Claudiu Beznea, Brandon Streiff,
netdev, linux-kernel, Michal Simek
Hi Nicolas,
On Fri, Jan 25, 2019 at 3:58 PM <Nicolas.Ferre@microchip.com> wrote:
>
> On 24/01/2019 at 14:38, Harini Katakam wrote:
> > The interrupt handler contains a workaround for RX hang applicable
> > to Zynq and AT91 only. Subsequent versions do not need this
>
> AT91RM9200 only. It's not the case for other AT91 SoCs (reading errata
> list for them).
>
> That being said I have to add a patch for making this perfectly clear in
> the comment just above the flag's test.
>
> > workaround. This workaround unecessarily resets RX whenever RX used
>
> Typo: unnecessarily
>
> > bit read is observed, which can be often under heavy traffic. Hence
> > introduce an CAPS mask and a check to enable this workaround.
>
> Nack for this one, see below...
Thanks for the review. I dint realize it was AT91RM9200, hence I edited
the wrong config structure. Will fix.
<snip>
> > if (status & MACB_BIT(RXUBR)) {
> > - ctrl = macb_readl(bp, NCR);
> > - macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> > - wmb();
> > - macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> > + if (bp->caps & MACB_CAPS_NEEDS_RSTONUBR) {
> > + ctrl = macb_readl(bp, NCR);
> > + macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
> > + wmb();
> > + macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
> > + }
>
> As this interrupt routine is not doing anything else than just being
> called now, what about just removing the use of this status bit for
> platforms which don't need this RX reset?
OK, sure I'll try that. I left the interrupt enabled just in case there were
other users performing an action/using this interrupt.
Regards,
Harini
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-25 11:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-24 13:38 [PATCH] net: macb: Apply RXUBR workaround only to versions with errata Harini Katakam
2019-01-25 10:28 ` Nicolas.Ferre
2019-01-25 11:03 ` Harini Katakam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox