| Message ID | 1424763572-23317-1-git-send-email-achandran@mvista.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Le 24/02/2015 08:39, Arun Chandran a écrit : > This patch converts all __raw_readl and __raw_writel function calls > to their corresponding readl_relaxed and writel_relaxed variants. > > It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg > when the cpu is configured in big endian mode. > > Signed-off-by: Arun Chandran <achandran@mvista.com> > --- > This patch is tested on xilinx ZC702 evaluation board with > CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs > Added on the fly IP endianness detection according to > comments from Michal Simek. > --- > --- > drivers/net/ethernet/cadence/macb.c | 33 +++++++++++++++++++++++++++------ > drivers/net/ethernet/cadence/macb.h | 15 ++++++++------- > 2 files changed, 35 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c > index ad76b8e..1642911 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) > WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); > > for(; p < end; p++, reg++) > - *p += __raw_readl(reg); > + *p += readl_relaxed(reg); > } > > static int macb_halt_tx(struct macb *bp) > @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp) > static void macb_configure_dma(struct macb *bp) > { > u32 dmacfg; > + u32 tmp, ncr; > > if (macb_is_gem(bp)) { > dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L); > @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp) > if (bp->dma_burst_length) > dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg); > dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); > - dmacfg &= ~GEM_BIT(ENDIA); > + dmacfg &= ~GEM_BIT(ENDIA_PKT); I think this will fail... (see below) > + > + /* Here we use the loopback bit of net_ctrl register to detect > + * endianness on IP. save it first. Program swaped mode for > + * management descriptor accesses if writing to loop back bit > + * and reading it back brings no change in bit value. > + */ > + ncr = macb_readl(bp, NCR); > + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR); > + tmp = __raw_readl(bp->regs + MACB_NCR); > + > + if (tmp == MACB_BIT(LLB)) > + dmacfg &= ~GEM_BIT(ENDIA_DESC); Ditto > + else > + dmacfg |= GEM_BIT(ENDIA_DESC); Ditto > + > + /* Restore net_ctrl */ > + macb_writel(bp, NCR, ncr); > + > if (bp->dev->features & NETIF_F_HW_CSUM) > dmacfg |= GEM_BIT(TXCOEN); > else > @@ -1832,14 +1851,14 @@ static void gem_update_stats(struct macb *bp) > > for (i = 0; i < GEM_STATS_LEN; ++i, ++p) { > u32 offset = gem_statistics[i].offset; > - u64 val = __raw_readl(bp->regs + offset); > + u64 val = readl_relaxed(bp->regs + offset); > > bp->ethtool_stats[i] += val; > *p += val; > > if (offset == GEM_OCTTXL || offset == GEM_OCTRXL) { > /* Add GEM_OCTTXH, GEM_OCTRXH */ > - val = __raw_readl(bp->regs + offset + 4); > + val = readl_relaxed(bp->regs + offset + 4); > bp->ethtool_stats[i] += ((u64)val) << 32; > *(++p) += val; > } > @@ -2191,12 +2210,14 @@ static void macb_probe_queues(void __iomem *mem, > *num_queues = 1; > > /* is it macb or gem ? */ > - mid = __raw_readl(mem + MACB_MID); > + mid = readl_relaxed(mem + MACB_MID); > + > if (MACB_BFEXT(IDNUM, mid) != 0x2) > return; > > /* bit 0 is never set but queue 0 always exists */ > - *queue_mask = __raw_readl(mem + GEM_DCFG6) & 0xff; > + *queue_mask = readl_relaxed(mem + GEM_DCFG6) & 0xff; > + > *queue_mask |= 0x1; > > for (hw_q = 1; hw_q < MACB_MAX_QUEUES; ++hw_q) > diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h > index 31dc080..57f0a1a 100644 > --- a/drivers/net/ethernet/cadence/macb.h > +++ b/drivers/net/ethernet/cadence/macb.h > @@ -229,7 +229,8 @@ > /* Bitfields in DMACFG. */ > #define GEM_FBLDO_OFFSET 0 /* fixed burst length for DMA */ > #define GEM_FBLDO_SIZE 5 > -#define GEM_ENDIA_OFFSET 7 /* endian swap mode for packet data access */ > +#define GEM_ENDIA_DESC_OFFSET 6 /* endian swap mode for management descriptor access */ > +#define GEM_ENDIA_PKT_OFFSET 7 /* endian swap mode for packet data access */ here you would need to define as well: #define GEM_ENDIA_DESC_SIZE 1 #define GEM_ENDIA_PKT_SIZE 1 Otherwise, I suspect that the GEM_BIT() macro won't work well. > #define GEM_ENDIA_SIZE 1 And you can remove this one ^^^^. > #define GEM_RXBMS_OFFSET 8 /* RX packet buffer memory size select */ > #define GEM_RXBMS_SIZE 2 > @@ -423,17 +424,17 @@ > > /* Register access macros */ > #define macb_readl(port,reg) \ > - __raw_readl((port)->regs + MACB_##reg) > + readl_relaxed((port)->regs + MACB_##reg) > #define macb_writel(port,reg,value) \ > - __raw_writel((value), (port)->regs + MACB_##reg) > + writel_relaxed((value), (port)->regs + MACB_##reg) > #define gem_readl(port, reg) \ > - __raw_readl((port)->regs + GEM_##reg) > + readl_relaxed((port)->regs + GEM_##reg) > #define gem_writel(port, reg, value) \ > - __raw_writel((value), (port)->regs + GEM_##reg) > + writel_relaxed((value), (port)->regs + GEM_##reg) > #define queue_readl(queue, reg) \ > - __raw_readl((queue)->bp->regs + (queue)->reg) > + readl_relaxed((queue)->bp->regs + (queue)->reg) > #define queue_writel(queue, reg, value) \ > - __raw_writel((value), (queue)->bp->regs + (queue)->reg) > + writel_relaxed((value), (queue)->bp->regs + (queue)->reg) > > /* Conditional GEM/MACB macros. These perform the operation to the correct > * register dependent on whether the device is a GEM or a MACB. For registers >
On Tue, Feb 24, 2015 at 6:27 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote: > Le 24/02/2015 08:39, Arun Chandran a écrit : >> This patch converts all __raw_readl and __raw_writel function calls >> to their corresponding readl_relaxed and writel_relaxed variants. >> >> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg >> when the cpu is configured in big endian mode. >> >> Signed-off-by: Arun Chandran <achandran@mvista.com> >> --- >> This patch is tested on xilinx ZC702 evaluation board with >> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs >> Added on the fly IP endianness detection according to >> comments from Michal Simek. >> --- >> --- >> drivers/net/ethernet/cadence/macb.c | 33 +++++++++++++++++++++++++++------ >> drivers/net/ethernet/cadence/macb.h | 15 ++++++++------- >> 2 files changed, 35 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c >> index ad76b8e..1642911 100644 >> --- a/drivers/net/ethernet/cadence/macb.c >> +++ b/drivers/net/ethernet/cadence/macb.c >> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) >> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); >> >> for(; p < end; p++, reg++) >> - *p += __raw_readl(reg); >> + *p += readl_relaxed(reg); >> } >> >> static int macb_halt_tx(struct macb *bp) >> @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp) >> static void macb_configure_dma(struct macb *bp) >> { >> u32 dmacfg; >> + u32 tmp, ncr; >> >> if (macb_is_gem(bp)) { >> dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L); >> @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp) >> if (bp->dma_burst_length) >> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg); >> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); >> - dmacfg &= ~GEM_BIT(ENDIA); >> + dmacfg &= ~GEM_BIT(ENDIA_PKT); > > I think this will fail... (see below) > >> + >> + /* Here we use the loopback bit of net_ctrl register to detect >> + * endianness on IP. save it first. Program swaped mode for >> + * management descriptor accesses if writing to loop back bit >> + * and reading it back brings no change in bit value. >> + */ >> + ncr = macb_readl(bp, NCR); >> + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR); >> + tmp = __raw_readl(bp->regs + MACB_NCR); >> + >> + if (tmp == MACB_BIT(LLB)) >> + dmacfg &= ~GEM_BIT(ENDIA_DESC); > > Ditto > >> + else >> + dmacfg |= GEM_BIT(ENDIA_DESC); > > Ditto Actually GEM_BIT(ENDIA_DESC) won't fail as I have defined GEM_ENDIA_DESC_OFFSET. But I failed to notice others (GEM_BF*) I will send another version. Thank you for spotting it. --Arun -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/24/2015 08:39 AM, Arun Chandran wrote: > This patch converts all __raw_readl and __raw_writel function calls > to their corresponding readl_relaxed and writel_relaxed variants. > > It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg > when the cpu is configured in big endian mode. > > Signed-off-by: Arun Chandran <achandran@mvista.com> > --- > This patch is tested on xilinx ZC702 evaluation board with > CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs > Added on the fly IP endianness detection according to > comments from Michal Simek. > --- > --- > drivers/net/ethernet/cadence/macb.c | 33 +++++++++++++++++++++++++++------ > drivers/net/ethernet/cadence/macb.h | 15 ++++++++------- > 2 files changed, 35 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c > index ad76b8e..1642911 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) > WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); > > for(; p < end; p++, reg++) > - *p += __raw_readl(reg); > + *p += readl_relaxed(reg); > } > > static int macb_halt_tx(struct macb *bp) > @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp) > static void macb_configure_dma(struct macb *bp) > { > u32 dmacfg; > + u32 tmp, ncr; > > if (macb_is_gem(bp)) { > dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L); > @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp) > if (bp->dma_burst_length) > dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg); > dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); > - dmacfg &= ~GEM_BIT(ENDIA); > + dmacfg &= ~GEM_BIT(ENDIA_PKT); > + > + /* Here we use the loopback bit of net_ctrl register to detect > + * endianness on IP. save it first. Program swaped mode for > + * management descriptor accesses if writing to loop back bit > + * and reading it back brings no change in bit value. > + */ > + ncr = macb_readl(bp, NCR); > + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR); > + tmp = __raw_readl(bp->regs + MACB_NCR); I have tested this patch on zc702 le and be and it is working. The comment is not fully accurate. Sorry I thought that you are using a little bit different configuration than Zynq. The case I had in mind is that CPU and also IP can have different endianness. The code above is not checking endianness on IP itself but CPU endianness because IP is in little endian mode all the time. But the logic for detecting this on CPU is correct and this is what you need to do on Zynq. You are writing big endian value to little endian register and read it back to see if it was correctly written or not. Can you please fix that comment to reflect this? Thanks, Michal
On Wed, Feb 25, 2015 at 3:32 PM, Michal Simek <monstr@monstr.eu> wrote: > On 02/24/2015 08:39 AM, Arun Chandran wrote: >> This patch converts all __raw_readl and __raw_writel function calls >> to their corresponding readl_relaxed and writel_relaxed variants. >> >> It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg >> when the cpu is configured in big endian mode. >> >> Signed-off-by: Arun Chandran <achandran@mvista.com> >> --- >> This patch is tested on xilinx ZC702 evaluation board with >> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs >> Added on the fly IP endianness detection according to >> comments from Michal Simek. >> --- >> --- >> drivers/net/ethernet/cadence/macb.c | 33 +++++++++++++++++++++++++++------ >> drivers/net/ethernet/cadence/macb.h | 15 ++++++++------- >> 2 files changed, 35 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c >> index ad76b8e..1642911 100644 >> --- a/drivers/net/ethernet/cadence/macb.c >> +++ b/drivers/net/ethernet/cadence/macb.c >> @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) >> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); >> >> for(; p < end; p++, reg++) >> - *p += __raw_readl(reg); >> + *p += readl_relaxed(reg); >> } >> >> static int macb_halt_tx(struct macb *bp) >> @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp) >> static void macb_configure_dma(struct macb *bp) >> { >> u32 dmacfg; >> + u32 tmp, ncr; >> >> if (macb_is_gem(bp)) { >> dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L); >> @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp) >> if (bp->dma_burst_length) >> dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg); >> dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); >> - dmacfg &= ~GEM_BIT(ENDIA); >> + dmacfg &= ~GEM_BIT(ENDIA_PKT); >> + >> + /* Here we use the loopback bit of net_ctrl register to detect >> + * endianness on IP. save it first. Program swaped mode for >> + * management descriptor accesses if writing to loop back bit >> + * and reading it back brings no change in bit value. >> + */ >> + ncr = macb_readl(bp, NCR); >> + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR); >> + tmp = __raw_readl(bp->regs + MACB_NCR); > > > I have tested this patch on zc702 le and be and it is working. > The comment is not fully accurate. Sorry I thought that you are using a little bit different > configuration than Zynq. The case I had in mind is that CPU and also IP can have different endianness. > > The code above is not checking endianness on IP itself but CPU endianness because IP is in little Ok. I will change the comment. > endian mode all the time. But the logic for detecting this on CPU is correct and this is what > you need to do on Zynq. You are writing big endian value to little endian register and read it back > to see if it was correctly written or not. Just curious; will the same code work without change on a CPU with Big endian IP? (ie. If zynq hardware comes with only one change; IP configured in BE; will the driver be needing further changes?) --Arun -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> endian mode all the time. But the logic for detecting this on CPU is correct and this is what >> you need to do on Zynq. You are writing big endian value to little endian register and read it back >> to see if it was correctly written or not. > > Just curious; will the same code work without change on a CPU with Big > endian IP? > (ie. If zynq hardware comes with only one change; IP configured in BE; > will the driver be needing further changes?) Depends on HW setup and architecture which you want to use. Let's say test could be to have Microblaze in PL in BE mode (not supported by tools) and bridge to axi. Then depends on Linux kernel how IO functions are implemented. That's why it is hard to say yes/no. Thanks, Michal
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index ad76b8e..1642911 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c @@ -449,7 +449,7 @@ static void macb_update_stats(struct macb *bp) WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4); for(; p < end; p++, reg++) - *p += __raw_readl(reg); + *p += readl_relaxed(reg); } static int macb_halt_tx(struct macb *bp) @@ -1578,6 +1578,7 @@ static u32 macb_dbw(struct macb *bp) static void macb_configure_dma(struct macb *bp) { u32 dmacfg; + u32 tmp, ncr; if (macb_is_gem(bp)) { dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L); @@ -1585,7 +1586,25 @@ static void macb_configure_dma(struct macb *bp) if (bp->dma_burst_length) dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg); dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); - dmacfg &= ~GEM_BIT(ENDIA); + dmacfg &= ~GEM_BIT(ENDIA_PKT); + + /* Here we use the loopback bit of net_ctrl register to detect + * endianness on IP. save it first. Program swaped mode for + * management descriptor accesses if writing to loop back bit + * and reading it back brings no change in bit value. + */ + ncr = macb_readl(bp, NCR); + __raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR); + tmp = __raw_readl(bp->regs + MACB_NCR); + + if (tmp == MACB_BIT(LLB)) + dmacfg &= ~GEM_BIT(ENDIA_DESC); + else + dmacfg |= GEM_BIT(ENDIA_DESC); + + /* Restore net_ctrl */ + macb_writel(bp, NCR, ncr); + if (bp->dev->features & NETIF_F_HW_CSUM) dmacfg |= GEM_BIT(TXCOEN); else @@ -1832,14 +1851,14 @@ static void gem_update_stats(struct macb *bp) for (i = 0; i < GEM_STATS_LEN; ++i, ++p) { u32 offset = gem_statistics[i].offset; - u64 val = __raw_readl(bp->regs + offset); + u64 val = readl_relaxed(bp->regs + offset); bp->ethtool_stats[i] += val; *p += val; if (offset == GEM_OCTTXL || offset == GEM_OCTRXL) { /* Add GEM_OCTTXH, GEM_OCTRXH */ - val = __raw_readl(bp->regs + offset + 4); + val = readl_relaxed(bp->regs + offset + 4); bp->ethtool_stats[i] += ((u64)val) << 32; *(++p) += val; } @@ -2191,12 +2210,14 @@ static void macb_probe_queues(void __iomem *mem, *num_queues = 1; /* is it macb or gem ? */ - mid = __raw_readl(mem + MACB_MID); + mid = readl_relaxed(mem + MACB_MID); + if (MACB_BFEXT(IDNUM, mid) != 0x2) return; /* bit 0 is never set but queue 0 always exists */ - *queue_mask = __raw_readl(mem + GEM_DCFG6) & 0xff; + *queue_mask = readl_relaxed(mem + GEM_DCFG6) & 0xff; + *queue_mask |= 0x1; for (hw_q = 1; hw_q < MACB_MAX_QUEUES; ++hw_q) diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index 31dc080..57f0a1a 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -229,7 +229,8 @@ /* Bitfields in DMACFG. */ #define GEM_FBLDO_OFFSET 0 /* fixed burst length for DMA */ #define GEM_FBLDO_SIZE 5 -#define GEM_ENDIA_OFFSET 7 /* endian swap mode for packet data access */ +#define GEM_ENDIA_DESC_OFFSET 6 /* endian swap mode for management descriptor access */ +#define GEM_ENDIA_PKT_OFFSET 7 /* endian swap mode for packet data access */ #define GEM_ENDIA_SIZE 1 #define GEM_RXBMS_OFFSET 8 /* RX packet buffer memory size select */ #define GEM_RXBMS_SIZE 2 @@ -423,17 +424,17 @@ /* Register access macros */ #define macb_readl(port,reg) \ - __raw_readl((port)->regs + MACB_##reg) + readl_relaxed((port)->regs + MACB_##reg) #define macb_writel(port,reg,value) \ - __raw_writel((value), (port)->regs + MACB_##reg) + writel_relaxed((value), (port)->regs + MACB_##reg) #define gem_readl(port, reg) \ - __raw_readl((port)->regs + GEM_##reg) + readl_relaxed((port)->regs + GEM_##reg) #define gem_writel(port, reg, value) \ - __raw_writel((value), (port)->regs + GEM_##reg) + writel_relaxed((value), (port)->regs + GEM_##reg) #define queue_readl(queue, reg) \ - __raw_readl((queue)->bp->regs + (queue)->reg) + readl_relaxed((queue)->bp->regs + (queue)->reg) #define queue_writel(queue, reg, value) \ - __raw_writel((value), (queue)->bp->regs + (queue)->reg) + writel_relaxed((value), (queue)->bp->regs + (queue)->reg) /* Conditional GEM/MACB macros. These perform the operation to the correct * register dependent on whether the device is a GEM or a MACB. For registers
This patch converts all __raw_readl and __raw_writel function calls to their corresponding readl_relaxed and writel_relaxed variants. It also tells the driver to set ahb_endian_swp_mgmt_en bit in dma_cfg when the cpu is configured in big endian mode. Signed-off-by: Arun Chandran <achandran@mvista.com> --- This patch is tested on xilinx ZC702 evaluation board with CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs Added on the fly IP endianness detection according to comments from Michal Simek. --- --- drivers/net/ethernet/cadence/macb.c | 33 +++++++++++++++++++++++++++------ drivers/net/ethernet/cadence/macb.h | 15 ++++++++------- 2 files changed, 35 insertions(+), 13 deletions(-)