diff mbox

[3/8] net: sh-eth: Remove duplicate sh_eth_set_duplex()

Message ID 1366268510-8121-3-git-send-email-nobuhiro.iwamatsu.yj@renesas.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Nobuhiro Iwamatsu April 18, 2013, 7:01 a.m. UTC 
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 drivers/net/ethernet/renesas/sh_eth.c |   65 ++++++---------------------------
 1 file changed, 12 insertions(+), 53 deletions(-)

Comments

Sergei Shtylyov April 18, 2013, 2:12 p.m. UTC | #1 
Hello.

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

> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nobuhiro Iwamatsu May 7, 2013, 5 a.m. UTC | #2 
Hi,

(2013/04/18 23:12), Sergei Shtylyov wrote:
> Hello.
>
> On 18-04-2013 11:01, Nobuhiro Iwamatsu wrote:
>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.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?

Yah, this is mybad.

>
>> .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.
>
I see.
I will remove sh_eth_set_duplex from SH7619, SH7710 and SH7712.

Thanks,
   Nobuhiro


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

Patch

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
+#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);
@@ -391,15 +397,6 @@  static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
 };
 #elif defined(CONFIG_CPU_SUBTYPE_SH7724)
 #define SH_ETH_RESET_DEFAULT	1
-static void sh_eth_set_duplex(struct net_device *ndev)
-{
-	struct sh_eth_private *mdp = netdev_priv(ndev);
-
-	if (mdp->duplex) /* Full */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_DM, ECMR);
-	else		/* Half */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_DM, ECMR);
-}
 
 static void sh_eth_set_rate(struct net_device *ndev)
 {
@@ -443,16 +440,6 @@  static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
 #define SH_ETH_HAS_TSU	1
 static int sh_eth_check_reset(struct net_device *ndev);
 
-static void sh_eth_set_duplex(struct net_device *ndev)
-{
-	struct sh_eth_private *mdp = netdev_priv(ndev);
-
-	if (mdp->duplex) /* Full */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_DM, ECMR);
-	else		/* Half */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_DM, ECMR);
-}
-
 static void sh_eth_set_rate(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -552,16 +539,6 @@  out:
 	return ret;
 }
 
-static void sh_eth_set_duplex_giga(struct net_device *ndev)
-{
-	struct sh_eth_private *mdp = netdev_priv(ndev);
-
-	if (mdp->duplex) /* Full */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_DM, ECMR);
-	else		/* Half */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_DM, ECMR);
-}
-
 static void sh_eth_set_rate_giga(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -584,7 +561,7 @@  static void sh_eth_set_rate_giga(struct net_device *ndev)
 /* SH7757(GETHERC) */
 static struct sh_eth_cpu_data sh_eth_my_cpu_data_giga = {
 	.chip_reset	= sh_eth_chip_reset_giga,
-	.set_duplex	= sh_eth_set_duplex_giga,
+	.set_duplex	= sh_eth_set_duplex,
 	.set_rate	= sh_eth_set_rate_giga,
 
 	.ecsr_value	= ECSR_ICD | ECSR_MPD,
@@ -635,16 +612,6 @@  static void sh_eth_chip_reset(struct net_device *ndev)
 	mdelay(1);
 }
 
-static void sh_eth_set_duplex(struct net_device *ndev)
-{
-	struct sh_eth_private *mdp = netdev_priv(ndev);
-
-	if (mdp->duplex) /* Full */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_DM, ECMR);
-	else		/* Half */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_DM, ECMR);
-}
-
 static void sh_eth_set_rate(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -774,16 +741,6 @@  out:
 	return ret;
 }
 
-static void sh_eth_set_duplex(struct net_device *ndev)
-{
-	struct sh_eth_private *mdp = netdev_priv(ndev);
-
-	if (mdp->duplex) /* Full */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) | ECMR_DM, ECMR);
-	else		/* Half */
-		sh_eth_write(ndev, sh_eth_read(ndev, ECMR) & ~ECMR_DM, ECMR);
-}
-
 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,
 	.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,
 	.eesipr_value	= DMAC_M_RFRMER | DMAC_M_ECI | 0x003fffff,
 	.tsu		= 1,
 	.tsu_multi_reg	= 1,