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  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:	Thu, 18 Apr 2013 18:12:35 +0400
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@...esas.com>
CC:	netdev@...r.kernel.org, yoshihiro.shimoda.uh@...esas.com
Subject: Re: [PATCH 3/8] net: sh-eth: Remove duplicate sh_eth_set_duplex()

Hello.

On 18-04-2013 11:01, Nobuhiro Iwamatsu wrote:

> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@...esas.com>

    Step in the right direction.

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 313d6ef..5c4e82c 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -341,9 +341,11 @@ static void sh_eth_select_mii(struct net_device *ndev)
>   }
>   #endif
>
> -/* There is CPU dependent code */
> -#if defined(CONFIG_ARCH_R8A7779)
> -#define SH_ETH_RESET_DEFAULT	1
> +#if defined(CONFIG_CPU_SUBTYPE_SH7619) || \
> +	defined(CONFIG_CPU_SUBTYPE_SH7710) || \
> +	defined(CONFIG_CPU_SUBTYPE_SH7712)
> +#  define sh_eth_set_duplex NULL

    I don't think this is neeeded at all.

> +#else
>   static void sh_eth_set_duplex(struct net_device *ndev)
>   {
>   	struct sh_eth_private *mdp = netdev_priv(ndev);
> @@ -353,7 +355,11 @@ static void sh_eth_set_duplex(struct net_device *ndev)
>   	else		/* Half */
>   		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_DM, ECMR);
>   }
> +#endif
>
> +/* There is CPU dependent code */
> +#if defined(CONFIG_ARCH_R8A7779)
> +#define SH_ETH_RESET_DEFAULT	1
>   static void sh_eth_set_rate(struct net_device *ndev)
>   {
>   	struct sh_eth_private *mdp = netdev_priv(ndev);
[...]
> @@ -834,6 +791,7 @@ static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
>   #elif defined(CONFIG_CPU_SUBTYPE_SH7619)
>   #define SH_ETH_RESET_DEFAULT	1
>   static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
> +	.set_duplex = sh_eth_set_duplex,

    Please, align = like in other initializers.
    Wait, is this change legitimate at all? You mean NULL will be set here?
But why bother doing this, when 0s will be set to all fields without
explicit initializer?

>   	.eesipr_value	= DMAC_M_RFRMER | DMAC_M_ECI | 0x003fffff,
>
>   	.apr		= 1,
> @@ -845,6 +803,7 @@ static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
>   #define SH_ETH_RESET_DEFAULT	1
>   #define SH_ETH_HAS_TSU	1
>   static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
> +	.set_duplex = sh_eth_set_duplex,

    Same coments here.

WBR, Sergei


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

Powered by blists - more mailing lists