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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <54E5D5A0.90005@monstr.eu>
Date:	Thu, 19 Feb 2015 13:22:56 +0100
From:	Michal Simek <monstr@...str.eu>
To:	Arun Chandran <achandran@...sta.com>,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
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 <achandran@...sta.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



Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)