netdev - Re: [PATCH 2/2] sh_eth: remove 'register_type' field from 'struct sh_eth_plat

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]

Date:	Wed, 21 Aug 2013 00:50:39 +0200
From:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net, lethal@...ux-sh.org,
	linux-sh@...r.kernel.org
Subject: Re: [PATCH 2/2] sh_eth: remove 'register_type' field from 'struct sh_eth_plat_data'

Hi Sergei,

On Wednesday 21 August 2013 02:09:49 Sergei Shtylyov wrote:
> On 08/20/2013 06:27 PM, Sergei Shtylyov wrote:
> >>> Now that the 'register_type' field of the 'sh_eth' driver's platform
> >>> data is not used by the driver anymore, it's time to remove it and  its
> >>> initializers from the SH platform code. Also  move *enum* declaring
> >>> values for this  field from <linux/sh_eth.h>  to  the  local driver's 
> >>> header file as they're only needed by the driver itself  now...
> >>> 
> >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
> 
> [...]
> 
> >>>   /* Driver's parameters */
> >>>   #if defined(CONFIG_CPU_SH4) || defined(CONFIG_ARCH_SHMOBILE)
> >>>   #define SH4_SKB_RX_ALIGN    32
> >>> 
> >>> Index: net-next/include/linux/sh_eth.h
> >>> ===================================================================
> >>> --- net-next.orig/include/linux/sh_eth.h
> >>> +++ net-next/include/linux/sh_eth.h
> >>> @@ -5,17 +5,10 @@
> >>> 
> >>>   #include <linux/if_ether.h>
> >>>   
> >>>   enum {EDMAC_LITTLE_ENDIAN, EDMAC_BIG_ENDIAN};
> >>> 
> >>> -enum {
> >>> -    SH_ETH_REG_GIGABIT,
> >>> -    SH_ETH_REG_FAST_RCAR,
> >>> -    SH_ETH_REG_FAST_SH4,
> >>> -    SH_ETH_REG_FAST_SH3_SH2
> >>> -};
> >>> 
> >>>   struct sh_eth_plat_data {
> >>>   
> >>>       int phy;
> >>>       int edmac_endian;
> >> 
> >> Wouldn't it make sense to move the edmac_endian field to sh_eth_cpu_data
> >> as well ?
> >> 
> > No, it depends on the SoC endianness which is determined by power-on pin
> > strapping -- which is board specific.

Does SoC endianness affect the ARM core endianness, the ethernet registers 
endianness, or both ? If it affects the ARM core endianness only, the kernel 
needs to be compiled in little-endian or big-endian mode anyway, and the 
sh_eth driver should use cpu_to_le32() unconditionally. If it affects both the 
ARM core and the ethernet controller there's not need to care about the 
endianness, as it will always be good. We only need to care about it if it 
affects the ethernet controller registers only, which would seem weird to me.

> BTW, I don't think the driver works correctly in the BE case since it uses
> io{read|write}32() to access the registers and those functions assume LE
> ordering on MMIO.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html