2015-02-18 11:30:00

by Arun Chandran

[permalink] [raw]
Subject: [PATCH] net: macb: Add big endian CPU support

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 <[email protected]>
---
This patch is tested on xilinx ZC702 evaluation board with
CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
---
---
drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++------
drivers/net/ethernet/cadence/macb.h | 15 ++++++++-------
2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index ad76b8e..05fb36d 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)
@@ -1585,7 +1585,11 @@ 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);
+ /* Tell the chip to byteswap descriptors on big-endian hosts */
+#ifdef __BIG_ENDIAN
+ dmacfg |= GEM_BIT(ENDIA_DESC);
+#endif
if (bp->dev->features & NETIF_F_HW_CSUM)
dmacfg |= GEM_BIT(TXCOEN);
else
@@ -1832,14 +1836,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 +2195,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
--
1.9.1


2015-02-19 12:16:39

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

Le 18/02/2015 12:29, 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 <[email protected]>
> ---
> This patch is tested on xilinx ZC702 evaluation board with
> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs

It seems okay:
Acked-by: Nicolas Ferre <[email protected]>

> ---
> ---
> drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++------
> drivers/net/ethernet/cadence/macb.h | 15 ++++++++-------
> 2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index ad76b8e..05fb36d 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)
> @@ -1585,7 +1585,11 @@ 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);
> + /* Tell the chip to byteswap descriptors on big-endian hosts */
> +#ifdef __BIG_ENDIAN
> + dmacfg |= GEM_BIT(ENDIA_DESC);
> +#endif
> if (bp->dev->features & NETIF_F_HW_CSUM)
> dmacfg |= GEM_BIT(TXCOEN);
> else
> @@ -1832,14 +1836,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 +2195,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
>


--
Nicolas Ferre

2015-02-19 12:23:09

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

On 02/18/2015 12:29 PM, 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 <[email protected]>
> ---
> This patch is tested on xilinx ZC702 evaluation board with
> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
> ---
> ---
> drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++------
> drivers/net/ethernet/cadence/macb.h | 15 ++++++++-------
> 2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index ad76b8e..05fb36d 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)
> @@ -1585,7 +1585,11 @@ 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);
> + /* Tell the chip to byteswap descriptors on big-endian hosts */
> +#ifdef __BIG_ENDIAN
> + dmacfg |= GEM_BIT(ENDIA_DESC);
> +#endif

I don't think this is the best way what you should do.
Instead of having this ifdef here you should find out any reg and detect if the IP
is in big endian or little endian mode. I have done it for some xilinx IPs which
can run on big or little endian system.
In general find reg which some field which has some meaning - write there 1
and read expected value and based on that decide if you are on big or little endian system.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (198.00 B)
OpenPGP digital signature

2015-02-20 11:45:08

by Arun Chandran

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <[email protected]> wrote:
>
> On 02/18/2015 12:29 PM, 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 <[email protected]>
> > ---
> > This patch is tested on xilinx ZC702 evaluation board with
> > CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
> > ---
> > ---
> > drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++------
> > drivers/net/ethernet/cadence/macb.h | 15 ++++++++-------
> > 2 files changed, 20 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> > index ad76b8e..05fb36d 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)
> > @@ -1585,7 +1585,11 @@ 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);
> > + /* Tell the chip to byteswap descriptors on big-endian hosts */
> > +#ifdef __BIG_ENDIAN
> > + dmacfg |= GEM_BIT(ENDIA_DESC);
> > +#endif
>
> I don't think this is the best way what you should do.
> Instead of having this ifdef here you should find out any reg and detect if the IP
> is in big endian or little endian mode. I have done it for some xilinx IPs which
> can run on big or little endian system.
> In general find reg which some field which has some meaning - write there 1
> and read expected value and based on that decide if you are on big or little endian system.

Hi Michal,

I was not able to find any such registers for GEM in the TRM
http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf

I tried writing to dma_cfg (0x00000010) and reading from design_cfg5
(0x00000290)
Its not reflecting anything(design_cfg5 always reads same value); It
is not helping.

The only way (I don't think its not right way either) I can think of
eliminating that
#ifdef __BIG_ENDIAN is reading some register in ARM cpu to identify its
current endianness and write dma_cfg accordingly.

--Arun

2015-02-23 14:10:55

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

Hi Arun,

On 02/20/2015 12:45 PM, Arun Chandran wrote:
> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <[email protected]> wrote:
>>
>> On 02/18/2015 12:29 PM, 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 <[email protected]>
>>> ---
>>> This patch is tested on xilinx ZC702 evaluation board with
>>> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
>>> ---
>>> ---
>>> drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++------
>>> drivers/net/ethernet/cadence/macb.h | 15 ++++++++-------
>>> 2 files changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>>> index ad76b8e..05fb36d 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)
>>> @@ -1585,7 +1585,11 @@ 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);
>>> + /* Tell the chip to byteswap descriptors on big-endian hosts */
>>> +#ifdef __BIG_ENDIAN
>>> + dmacfg |= GEM_BIT(ENDIA_DESC);
>>> +#endif
>>
>> I don't think this is the best way what you should do.
>> Instead of having this ifdef here you should find out any reg and detect if the IP
>> is in big endian or little endian mode. I have done it for some xilinx IPs which
>> can run on big or little endian system.
>> In general find reg which some field which has some meaning - write there 1
>> and read expected value and based on that decide if you are on big or little endian system.
>
> Hi Michal,
>
> I was not able to find any such registers for GEM in the TRM
> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
>
> I tried writing to dma_cfg (0x00000010) and reading from design_cfg5
> (0x00000290)
> Its not reflecting anything(design_cfg5 always reads same value); It
> is not helping.
>
> The only way (I don't think its not right way either) I can think of
> eliminating that
> #ifdef __BIG_ENDIAN is reading some register in ARM cpu to identify its
> current endianness and write dma_cfg accordingly.

Definitely no to detect cpu endianess.

What about this?
writel(0x2, 0xE000b000); //write little endian
if (readl(0xE000b000) == 0x2) { //read little endian
printf("little endian\n")
disable 0x2 bit (loopback)
} else {
printf("big endian\n");

definitely good to check writing 0x2 here that IP is
in big endian and reacts - if not BUG()
}


I have written this to spi-xilinx.c and you can use similar construction
for macb too (of course with checking above).

/*
* Detect endianess on the IP via loop bit in CR. Detection
* must be done before reset is sent because incorrect reset
* value generates error interrupt.
* Setup little endian helper functions first and try to use them
* and check if bit was correctly setup or not.
*/
xspi->read_fn = xspi_read32;
xspi->write_fn = xspi_write32;

xspi->write_fn(XSPI_CR_LOOP, xspi->regs + XSPI_CR_OFFSET);
tmp = xspi->read_fn(xspi->regs + XSPI_CR_OFFSET);
tmp &= XSPI_CR_LOOP;
if (tmp != XSPI_CR_LOOP) {
xspi->read_fn = xspi_read32_be;
xspi->write_fn = xspi_write32_be;
}

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (198.00 B)
OpenPGP digital signature

2015-02-23 14:38:19

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

Le 20/02/2015 12:45, Arun Chandran a écrit :
> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <[email protected]> wrote:
>>
>> On 02/18/2015 12:29 PM, 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 <[email protected]>
>>> ---
>>> This patch is tested on xilinx ZC702 evaluation board with
>>> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
>>> ---
>>> ---
>>> drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++------
>>> drivers/net/ethernet/cadence/macb.h | 15 ++++++++-------
>>> 2 files changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>>> index ad76b8e..05fb36d 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)
>>> @@ -1585,7 +1585,11 @@ 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);
>>> + /* Tell the chip to byteswap descriptors on big-endian hosts */
>>> +#ifdef __BIG_ENDIAN
>>> + dmacfg |= GEM_BIT(ENDIA_DESC);
>>> +#endif
>>
>> I don't think this is the best way what you should do.
>> Instead of having this ifdef here you should find out any reg and detect if the IP
>> is in big endian or little endian mode. I have done it for some xilinx IPs which
>> can run on big or little endian system.
>> In general find reg which some field which has some meaning - write there 1
>> and read expected value and based on that decide if you are on big or little endian system.
>
> Hi Michal,
>
> I was not able to find any such registers for GEM in the TRM
> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
>
> I tried writing to dma_cfg (0x00000010) and reading from design_cfg5
> (0x00000290)
> Its not reflecting anything(design_cfg5 always reads same value); It
> is not helping.

0kay, here you mean that the design_cfg5 (@0x290) and its bit 16:15
"gem_endian_swap_def" doesn't change depending on the big/little endian
configuration you use?

I was also thinking about this configuration bit... too bad...

Bye,


> The only way (I don't think its not right way either) I can think of
> eliminating that
> #ifdef __BIG_ENDIAN is reading some register in ARM cpu to identify its
> current endianness and write dma_cfg accordingly.
>
> --Arun
>
>


--
Nicolas Ferre

2015-02-24 07:27:49

by Arun Chandran

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

On Mon, Feb 23, 2015 at 8:08 PM, Nicolas Ferre <[email protected]> wrote:
> Le 20/02/2015 12:45, Arun Chandran a écrit :
>> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <[email protected]> wrote:
>>>
>>> On 02/18/2015 12:29 PM, 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 <[email protected]>
>>>> ---
>>>> This patch is tested on xilinx ZC702 evaluation board with
>>>> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
>>>> ---
>>>> ---
>>>> drivers/net/ethernet/cadence/macb.c | 18 ++++++++++++------
>>>> drivers/net/ethernet/cadence/macb.h | 15 ++++++++-------
>>>> 2 files changed, 20 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>>>> index ad76b8e..05fb36d 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)
>>>> @@ -1585,7 +1585,11 @@ 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);
>>>> + /* Tell the chip to byteswap descriptors on big-endian hosts */
>>>> +#ifdef __BIG_ENDIAN
>>>> + dmacfg |= GEM_BIT(ENDIA_DESC);
>>>> +#endif
>>>
>>> I don't think this is the best way what you should do.
>>> Instead of having this ifdef here you should find out any reg and detect if the IP
>>> is in big endian or little endian mode. I have done it for some xilinx IPs which
>>> can run on big or little endian system.
>>> In general find reg which some field which has some meaning - write there 1
>>> and read expected value and based on that decide if you are on big or little endian system.
>>
>> Hi Michal,
>>
>> I was not able to find any such registers for GEM in the TRM
>> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
>>
>> I tried writing to dma_cfg (0x00000010) and reading from design_cfg5
>> (0x00000290)
>> Its not reflecting anything(design_cfg5 always reads same value); It
>> is not helping.
>
> 0kay, here you mean that the design_cfg5 (@0x290) and its bit 16:15
> "gem_endian_swap_def" doesn't change depending on the big/little endian
> configuration you use?

Yes. That register always reads the same.

-- Arun

2015-02-24 07:40:08

by Arun Chandran

[permalink] [raw]
Subject: [PATCH] net: macb: Add big endian CPU support

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 <[email protected]>
---
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);
+
+ 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
--
1.9.1

2015-02-24 12:57:11

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

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 <[email protected]>
> ---
> 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
>


--
Nicolas Ferre

2015-02-24 17:57:54

by Arun Chandran

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

On Tue, Feb 24, 2015 at 6:27 PM, Nicolas Ferre <[email protected]> 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 <[email protected]>
>> ---
>> 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

2015-02-25 10:02:35

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

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 <[email protected]>
> ---
> 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
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (198.00 B)
OpenPGP digital signature

2015-02-25 10:56:49

by Arun Chandran

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

On Wed, Feb 25, 2015 at 3:32 PM, Michal Simek <[email protected]> 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 <[email protected]>
>> ---
>> 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

2015-02-25 11:50:49

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

>> 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

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (198.00 B)
OpenPGP digital signature

2015-02-26 10:44:50

by Arun Chandran

[permalink] [raw]
Subject: [PATCH] net: macb: Add big endian CPU support

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 <[email protected]>
---
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.
Corrected GEM_* macros as per comments from Nicolas Ferre
---
---
drivers/net/ethernet/cadence/macb.c | 32 ++++++++++++++++++++++++++------
drivers/net/ethernet/cadence/macb.h | 18 ++++++++++--------
2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index ad76b8e..1fe8b94 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,24 @@ 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);
+
+ /* Find the CPU endianness by using the loopback bit of net_ctrl
+ * register. save it first. When the CPU is in big endian we
+ * need to program swaped mode for management descriptor access.
+ */
+ 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); /* CPU in big endian */
+
+ /* Restore net_ctrl */
+ macb_writel(bp, NCR, ncr);
+
if (bp->dev->features & NETIF_F_HW_CSUM)
dmacfg |= GEM_BIT(TXCOEN);
else
@@ -1832,14 +1850,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 +2209,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..83241c8 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -229,8 +229,10 @@
/* 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_SIZE 1
+#define GEM_ENDIA_DESC_OFFSET 6 /* endian swap mode for management descriptor access */
+#define GEM_ENDIA_DESC_SIZE 1
+#define GEM_ENDIA_PKT_OFFSET 7 /* endian swap mode for packet data access */
+#define GEM_ENDIA_PKT_SIZE 1
#define GEM_RXBMS_OFFSET 8 /* RX packet buffer memory size select */
#define GEM_RXBMS_SIZE 2
#define GEM_TXPBMS_OFFSET 10 /* TX packet buffer memory size select */
@@ -423,17 +425,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
--
1.9.1

2015-02-26 10:47:44

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] net: macb: Add big endian CPU support

Hi Arun,

On 02/26/2015 11:44 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 <[email protected]>
> ---
> 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.
> Corrected GEM_* macros as per comments from Nicolas Ferre
> ---
> ---
> drivers/net/ethernet/cadence/macb.c | 32 ++++++++++++++++++++++++++------
> drivers/net/ethernet/cadence/macb.h | 18 ++++++++++--------
> 2 files changed, 36 insertions(+), 14 deletions(-)
>

Can you please send regular v3 and list all changes you have done?

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (198.00 B)
OpenPGP digital signature

2015-02-26 11:01:35

by Arun Chandran

[permalink] [raw]
Subject: [PATCH v3] net: macb: Add big endian CPU support

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 <[email protected]>
---
This patch is tested on xilinx ZC702 evaluation board with
CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs

changes since initial version:

* Added on the fly CPU endianness detection according to comments from Michal Simek.
* Corrected GEM_* defines as per comments from Nicolas Ferre.
---
---
drivers/net/ethernet/cadence/macb.c | 32 ++++++++++++++++++++++++++------
drivers/net/ethernet/cadence/macb.h | 18 ++++++++++--------
2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index ad76b8e..1fe8b94 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,24 @@ 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);
+
+ /* Find the CPU endianness by using the loopback bit of net_ctrl
+ * register. save it first. When the CPU is in big endian we
+ * need to program swaped mode for management descriptor access.
+ */
+ 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); /* CPU in big endian */
+
+ /* Restore net_ctrl */
+ macb_writel(bp, NCR, ncr);
+
if (bp->dev->features & NETIF_F_HW_CSUM)
dmacfg |= GEM_BIT(TXCOEN);
else
@@ -1832,14 +1850,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 +2209,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..83241c8 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -229,8 +229,10 @@
/* 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_SIZE 1
+#define GEM_ENDIA_DESC_OFFSET 6 /* endian swap mode for management descriptor access */
+#define GEM_ENDIA_DESC_SIZE 1
+#define GEM_ENDIA_PKT_OFFSET 7 /* endian swap mode for packet data access */
+#define GEM_ENDIA_PKT_SIZE 1
#define GEM_RXBMS_OFFSET 8 /* RX packet buffer memory size select */
#define GEM_RXBMS_SIZE 2
#define GEM_TXPBMS_OFFSET 10 /* TX packet buffer memory size select */
@@ -423,17 +425,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
--
1.9.1

2015-02-26 11:07:21

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH v3] net: macb: Add big endian CPU support

Le 26/02/2015 12:01, 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 <[email protected]>

It seems okay:
Acked-by: Nicolas Ferre <[email protected]>

> ---
> This patch is tested on xilinx ZC702 evaluation board with
> CONFIG_CPU_BIG_ENDIAN=y and booting NFS rootfs
>
> changes since initial version:
>
> * Added on the fly CPU endianness detection according to comments from Michal Simek.
> * Corrected GEM_* defines as per comments from Nicolas Ferre.
> ---
> ---
> drivers/net/ethernet/cadence/macb.c | 32 ++++++++++++++++++++++++++------
> drivers/net/ethernet/cadence/macb.h | 18 ++++++++++--------
> 2 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index ad76b8e..1fe8b94 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,24 @@ 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);
> +
> + /* Find the CPU endianness by using the loopback bit of net_ctrl
> + * register. save it first. When the CPU is in big endian we
> + * need to program swaped mode for management descriptor access.
> + */
> + 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); /* CPU in big endian */
> +
> + /* Restore net_ctrl */
> + macb_writel(bp, NCR, ncr);
> +
> if (bp->dev->features & NETIF_F_HW_CSUM)
> dmacfg |= GEM_BIT(TXCOEN);
> else
> @@ -1832,14 +1850,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 +2209,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..83241c8 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -229,8 +229,10 @@
> /* 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_SIZE 1
> +#define GEM_ENDIA_DESC_OFFSET 6 /* endian swap mode for management descriptor access */
> +#define GEM_ENDIA_DESC_SIZE 1
> +#define GEM_ENDIA_PKT_OFFSET 7 /* endian swap mode for packet data access */
> +#define GEM_ENDIA_PKT_SIZE 1
> #define GEM_RXBMS_OFFSET 8 /* RX packet buffer memory size select */
> #define GEM_RXBMS_SIZE 2
> #define GEM_TXPBMS_OFFSET 10 /* TX packet buffer memory size select */
> @@ -423,17 +425,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
>


--
Nicolas Ferre

2015-02-26 11:49:14

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH v3] net: macb: Add big endian CPU support

On 02/26/2015 12:06 PM, Nicolas Ferre wrote:
> Le 26/02/2015 12:01, 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 <[email protected]>
>
> It seems okay:
> Acked-by: Nicolas Ferre <[email protected]>

Tested on Zynq zc702 on LE and BE.

Tested-by: Michal Simek <[email protected]>

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (198.00 B)
OpenPGP digital signature

2015-02-27 22:24:35

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3] net: macb: Add big endian CPU support

From: Arun Chandran <[email protected]>
Date: Thu, 26 Feb 2015 16:31:14 +0530

> 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 <[email protected]>

This does not apply cleanly to net-next, please respin.

2015-02-28 10:43:18

by Arun Chandran

[permalink] [raw]
Subject: Re: [PATCH v3] net: macb: Add big endian CPU support

Hi David,

>> 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 <[email protected]>
>
> This does not apply cleanly to net-next, please respin.


The net-next tree already contains the initial version
of this change (commit: a50dad355a5314da64586da36804b86fbebb7c2a).

This applies cleanly if you revert that.

#~/work/open_source/net-next/net-next$ git log -2
commit 3cbef72505634b346dadb5f4ffd0fdb68c07710c
Author: Arun Chandran <[email protected]>
Date: Wed Feb 18 16:35:44 2015 +0530

net: macb: Add big endian CPU support

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 <[email protected]>

commit 693aba8f896f286d9ddd4445ced374d2c96b41a2
Author: Arun Chandran <[email protected]>
Date: Sat Feb 28 16:03:18 2015 +0530

Revert "net: macb: Add big endian CPU support"

This reverts commit a50dad355a5314da64586da36804b86fbebb7c2a.

--Arun

2015-02-28 18:02:52

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3] net: macb: Add big endian CPU support

From: Arun Chandran <[email protected]>
Date: Sat, 28 Feb 2015 16:10:47 +0530

> The net-next tree already contains the initial version
> of this change (commit: a50dad355a5314da64586da36804b86fbebb7c2a).
>
> This applies cleanly if you revert that.

You need to send me relative changes in this situation, rather
than expect me to revert existing changes in my tree.