[PATCH] net: macb: Add big endian CPU support

[PATCH] net: macb: Add big endian CPU support

[Arun Chandran]

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

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

[Nicolas Ferre]

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 <achandran@mvista.com>
> ---
> 	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 <nicolas.ferre@atmel.com>

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

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

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]

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 <achandran@mvista.com>
> ---
> 	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: 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Arun Chandran]

On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <monstr@monstr.eu> 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 <achandran@mvista.com>
> > ---
> >       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

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

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 4419 bytes --]

Hi Arun,

On 02/20/2015 12:45 PM, Arun Chandran wrote:
> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <monstr@monstr.eu> 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 <achandran@mvista.com>
>>> ---
>>>       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: 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Nicolas Ferre]

Le 20/02/2015 12:45, Arun Chandran a écrit :
> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <monstr@monstr.eu> 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 <achandran@mvista.com>
>>> ---
>>>       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

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

[Arun Chandran]

On Mon, Feb 23, 2015 at 8:08 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> Le 20/02/2015 12:45, Arun Chandran a écrit :
>> On Thu, Feb 19, 2015 at 5:52 PM, Michal Simek <monstr@monstr.eu> 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 <achandran@mvista.com>
>>>> ---
>>>>       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

[PATCH] net: macb: Add big endian CPU support

[Arun Chandran]

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);
+
+		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

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

[Nicolas Ferre]

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
> 


-- 
Nicolas Ferre

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

[Arun Chandran]

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

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

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 3249 bytes --]

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
-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Arun Chandran]

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

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

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] net: macb: Add big endian CPU support

[Arun Chandran]

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

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

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Arun Chandran]

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

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

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

[Nicolas Ferre]

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 <achandran@mvista.com>

It seems okay:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

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

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

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

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 <achandran@mvista.com>
> 
> It seems okay:
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Tested on Zynq zc702 on LE and BE.

Tested-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[David Miller]

From: Arun Chandran <achandran@mvista.com>
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 <achandran@mvista.com>

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

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

[Arun Chandran]

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 <achandran@mvista.com>
>
> 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 <achandran@mvista.com>
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 <achandran@mvista.com>

commit 693aba8f896f286d9ddd4445ced374d2c96b41a2
Author: Arun Chandran <achandran@mvista.com>
Date:   Sat Feb 28 16:03:18 2015 +0530

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

    This reverts commit a50dad355a5314da64586da36804b86fbebb7c2a.

--Arun

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

[David Miller]

From: Arun Chandran <achandran@mvista.com>
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.

[PATCH 1/2] net: macb: Add on the fly CPU endianness detection

[Arun Chandran]

Program management descriptor's access mode according to the
dynamically detected CPU endianness.

Signed-off-by: Arun Chandran <achandran@mvista.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Tested-by: Michal Simek <michal.simek@xilinx.com>
---
* Adding CPU endianness detection according to Michal Simek
* Added Acked-by: Nicolas Ferre
* Added Tested-by: Michal Simek
---
---
 drivers/net/ethernet/cadence/macb.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 05fb36d..1fe8b94 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -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);
@@ -1586,10 +1587,23 @@ static void macb_configure_dma(struct macb *bp)
 			dmacfg = GEM_BFINS(FBLDO, bp->dma_burst_length, dmacfg);
 		dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
 		dmacfg &= ~GEM_BIT(ENDIA_PKT);
-		/* Tell the chip to byteswap descriptors on big-endian hosts */
-#ifdef __BIG_ENDIAN
-		dmacfg |= GEM_BIT(ENDIA_DESC);
-#endif
+
+		/* 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
-- 
1.9.1

[PATCH 2/2] net: macb: Properly add DMACFG bit definitions

[Arun Chandran]

Add *_SIZE macros for the bits ENDIA_DESC and
ENDIA_PKT

Signed-off-by: Arun Chandran <achandran@mvista.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
* Correct these macros according to comments from Nicolas Ferre. Also add his Acked-by
---
---
 drivers/net/ethernet/cadence/macb.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 57f0a1a..83241c8 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -230,8 +230,9 @@
 #define GEM_FBLDO_OFFSET	0 /* fixed burst length for DMA */
 #define GEM_FBLDO_SIZE		5
 #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_SIZE		1
+#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 */
-- 
1.9.1

Re: [PATCH 1/2] net: macb: Add on the fly CPU endianness detection

[David Miller]

From: Arun Chandran <achandran@mvista.com>
Date: Sun,  1 Mar 2015 11:38:02 +0530

> Program management descriptor's access mode according to the
> dynamically detected CPU endianness.
> 
> Signed-off-by: Arun Chandran <achandran@mvista.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Tested-by: Michal Simek <michal.simek@xilinx.com>

Applied to net-next.

Re: [PATCH 2/2] net: macb: Properly add DMACFG bit definitions

[David Miller]

From: Arun Chandran <achandran@mvista.com>
Date: Sun,  1 Mar 2015 11:38:03 +0530

> Add *_SIZE macros for the bits ENDIA_DESC and
> ENDIA_PKT
> 
> Signed-off-by: Arun Chandran <achandran@mvista.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Also applied to net-next.