[PATCH v5 0/4] Add ethernet support for r7s72100

[PATCH v5 0/4] Add ethernet support for r7s72100

[Simon Horman]

Hi,

this series adds ethernet support to sh-pfc for the r7s72100 SoC.

This series is based on a merge of:
* The topic/r7s72100-v3.13-rc8-20140115 tag in my renesas tree
* net-next
  - Head revision: 08c93cd99b2f31ba9
    ("Merge branch 'for-davem' of git://gitorious.org/linux-can/linux-can-next")

The first two patches, targeted at net-next, also applies cleanly there.

Changes since v4
* Addressed feedback from Sergei Shtylyov as detailed in the changelog
  of "sh_eth: Add support for r7s72100"
* Rebase

Changes since v3
* Use bool as return type of sh_eth_is_gether()
  and sh_eth_is_rz_fast_ether()
* Correct coding style in sh_eth_get_stats()

Changes since v2
* Trivial rebase
* Dropped "RFC" from subject

Changes since v1 are noted in the changelog of each patch.

Simon Horman (4):
  sh_eth: Use bool as return type of sh_eth_is_gether()
  sh_eth: Add support for r7s72100
  ARM: shmobile: r7s72100: Add clock for r7s72100-ether
  ARM: shmobile: genmai: Enable r7s72100-ether

 arch/arm/mach-shmobile/board-genmai.c   |  21 +++++
 arch/arm/mach-shmobile/clock-r7s72100.c |   4 +
 drivers/net/ethernet/renesas/sh_eth.c   | 132 +++++++++++++++++++++++++++++---
 drivers/net/ethernet/renesas/sh_eth.h   |   3 +-
 4 files changed, 149 insertions(+), 11 deletions(-)

-- 
1.8.4

[PATCH v5 0/4] Add ethernet support for r7s72100

[Simon Horman]

Hi,

this series adds ethernet support to sh-pfc for the r7s72100 SoC.

This series is based on a merge of:
* The topic/r7s72100-v3.13-rc8-20140115 tag in my renesas tree
* net-next
  - Head revision: 08c93cd99b2f31ba9
    ("Merge branch 'for-davem' of git://gitorious.org/linux-can/linux-can-next")

The first two patches, targeted at net-next, also applies cleanly there.

Changes since v4
* Addressed feedback from Sergei Shtylyov as detailed in the changelog
  of "sh_eth: Add support for r7s72100"
* Rebase

Changes since v3
* Use bool as return type of sh_eth_is_gether()
  and sh_eth_is_rz_fast_ether()
* Correct coding style in sh_eth_get_stats()

Changes since v2
* Trivial rebase
* Dropped "RFC" from subject

Changes since v1 are noted in the changelog of each patch.

Simon Horman (4):
  sh_eth: Use bool as return type of sh_eth_is_gether()
  sh_eth: Add support for r7s72100
  ARM: shmobile: r7s72100: Add clock for r7s72100-ether
  ARM: shmobile: genmai: Enable r7s72100-ether

 arch/arm/mach-shmobile/board-genmai.c   |  21 +++++
 arch/arm/mach-shmobile/clock-r7s72100.c |   4 +
 drivers/net/ethernet/renesas/sh_eth.c   | 132 +++++++++++++++++++++++++++++---
 drivers/net/ethernet/renesas/sh_eth.h   |   3 +-
 4 files changed, 149 insertions(+), 11 deletions(-)

-- 
1.8.4

[PATCH v5 0/4] Add ethernet support for r7s72100

[Simon Horman]

Hi,

this series adds ethernet support to sh-pfc for the r7s72100 SoC.

This series is based on a merge of:
* The topic/r7s72100-v3.13-rc8-20140115 tag in my renesas tree
* net-next
  - Head revision: 08c93cd99b2f31ba9
    ("Merge branch 'for-davem' of git://gitorious.org/linux-can/linux-can-next")

The first two patches, targeted at net-next, also applies cleanly there.

Changes since v4
* Addressed feedback from Sergei Shtylyov as detailed in the changelog
  of "sh_eth: Add support for r7s72100"
* Rebase

Changes since v3
* Use bool as return type of sh_eth_is_gether()
  and sh_eth_is_rz_fast_ether()
* Correct coding style in sh_eth_get_stats()

Changes since v2
* Trivial rebase
* Dropped "RFC" from subject

Changes since v1 are noted in the changelog of each patch.

Simon Horman (4):
  sh_eth: Use bool as return type of sh_eth_is_gether()
  sh_eth: Add support for r7s72100
  ARM: shmobile: r7s72100: Add clock for r7s72100-ether
  ARM: shmobile: genmai: Enable r7s72100-ether

 arch/arm/mach-shmobile/board-genmai.c   |  21 +++++
 arch/arm/mach-shmobile/clock-r7s72100.c |   4 +
 drivers/net/ethernet/renesas/sh_eth.c   | 132 +++++++++++++++++++++++++++++---
 drivers/net/ethernet/renesas/sh_eth.h   |   3 +-
 4 files changed, 149 insertions(+), 11 deletions(-)

-- 
1.8.4

[PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Simon Horman]

Return a boolean and use true and false.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---
v5
* No change

v4
* First post
---
 drivers/net/ethernet/renesas/sh_eth.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index ba1f6c9..4f5cfad 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[TSU_ADRL31]	= 0x01fc,
 };
 
-static int sh_eth_is_gether(struct sh_eth_private *mdp)
+static bool sh_eth_is_gether(struct sh_eth_private *mdp)
 {
 	if (mdp->reg_offset = sh_eth_offset_gigabit)
-		return 1;
+		return true;
 	else
-		return 0;
+		return false;
 }
 
 static void sh_eth_select_mii(struct net_device *ndev)
-- 
1.8.4

[PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Simon Horman]

Return a boolean and use true and false.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---
v5
* No change

v4
* First post
---
 drivers/net/ethernet/renesas/sh_eth.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index ba1f6c9..4f5cfad 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[TSU_ADRL31]	= 0x01fc,
 };
 
-static int sh_eth_is_gether(struct sh_eth_private *mdp)
+static bool sh_eth_is_gether(struct sh_eth_private *mdp)
 {
 	if (mdp->reg_offset == sh_eth_offset_gigabit)
-		return 1;
+		return true;
 	else
-		return 0;
+		return false;
 }
 
 static void sh_eth_select_mii(struct net_device *ndev)
-- 
1.8.4

[PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Simon Horman]

Return a boolean and use true and false.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---
v5
* No change

v4
* First post
---
 drivers/net/ethernet/renesas/sh_eth.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index ba1f6c9..4f5cfad 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[TSU_ADRL31]	= 0x01fc,
 };
 
-static int sh_eth_is_gether(struct sh_eth_private *mdp)
+static bool sh_eth_is_gether(struct sh_eth_private *mdp)
 {
 	if (mdp->reg_offset == sh_eth_offset_gigabit)
-		return 1;
+		return true;
 	else
-		return 0;
+		return false;
 }
 
 static void sh_eth_select_mii(struct net_device *ndev)
-- 
1.8.4

[PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Simon Horman]

This is a fast ethernet controller.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---
v5
* As suggested by Sergei Shtylyov
  - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
    RMII registers. Instead use sh_eth_chip_reset.
  - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
  - Do not use reserved LCHNG bit of ECSR
  - Do not use reserved LCHNGIP bit of ECSIPR
  - Document that R8A779x also needs a 16 bit shift of the RFS bits
  - Do not document that the R7S72100 has GECMR, it does not

v4
* As requested by David Miller
  - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
  - Correct coding style in sh_eth_get_stats()

v3
* No change

v2
* As suggested by Magnus Damm and Sergei Shtylyov
  - r7s72100 ethernet is not gigabit so do not refer to it as such

* As suggested by Magnus Damm
  - As RZ specific register layout rather than using the gigabit layout
    which includes registers that do not exist on this chip.
---
 drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
 drivers/net/ethernet/renesas/sh_eth.h |   3 +-
 2 files changed, 121 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 4f5cfad..a7a0555 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[TRIMD]		= 0x027c,
 };
 
+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
+	[EDSR]		= 0x0000,
+	[EDMR]		= 0x0400,
+	[EDTRR]		= 0x0408,
+	[EDRRR]		= 0x0410,
+	[EESR]		= 0x0428,
+	[EESIPR]	= 0x0430,
+	[TDLAR]		= 0x0010,
+	[TDFAR]		= 0x0014,
+	[TDFXR]		= 0x0018,
+	[TDFFR]		= 0x001c,
+	[RDLAR]		= 0x0030,
+	[RDFAR]		= 0x0034,
+	[RDFXR]		= 0x0038,
+	[RDFFR]		= 0x003c,
+	[TRSCER]	= 0x0438,
+	[RMFCR]		= 0x0440,
+	[TFTR]		= 0x0448,
+	[FDR]		= 0x0450,
+	[RMCR]		= 0x0458,
+	[RPADIR]	= 0x0460,
+	[FCFTR]		= 0x0468,
+	[CSMR]		= 0x04E4,
+
+	[ECMR]		= 0x0500,
+	[RFLR]		= 0x0508,
+	[ECSR]		= 0x0510,
+	[ECSIPR]	= 0x0518,
+	[PIR]		= 0x0520,
+	[APR]		= 0x0554,
+	[MPR]		= 0x0558,
+	[PFTCR]		= 0x055c,
+	[PFRCR]		= 0x0560,
+	[TPAUSER]	= 0x0564,
+	[MAHR]		= 0x05c0,
+	[MALR]		= 0x05c8,
+	[CEFCR]		= 0x0740,
+	[FRECR]		= 0x0748,
+	[TSFRCR]	= 0x0750,
+	[TLFRCR]	= 0x0758,
+	[RFCR]		= 0x0760,
+	[MAFCR]		= 0x0778,
+
+	[ARSTR]		= 0x0000,
+	[TSU_CTRST]	= 0x0004,
+	[TSU_VTAG0]	= 0x0058,
+	[TSU_ADSBSY]	= 0x0060,
+	[TSU_TEN]	= 0x0064,
+	[TXNLCR0]	= 0x0080,
+	[TXALCR0]	= 0x0084,
+	[RXNLCR0]	= 0x0088,
+	[RXALCR0]	= 0x008C,
+	[TSU_ADRH0]	= 0x0100,
+	[TSU_ADRL0]	= 0x0104,
+	[TSU_ADRH31]	= 0x01f8,
+	[TSU_ADRL31]	= 0x01fc,
+};
+
 static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[ECMR]		= 0x0100,
 	[RFLR]		= 0x0108,
@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
 		return false;
 }
 
+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
+{
+	if (mdp->reg_offset = sh_eth_offset_fast_rz)
+		return true;
+	else
+		return false;
+}
+
 static void sh_eth_select_mii(struct net_device *ndev)
 {
 	u32 value = 0x0;
@@ -701,6 +767,38 @@ static struct sh_eth_cpu_data r8a7740_data = {
 	.shift_rd0	= 1,
 };
 
+/* R7S72100 */
+static struct sh_eth_cpu_data r7s72100_data = {
+	.chip_reset	= sh_eth_chip_reset,
+	.set_duplex	= sh_eth_set_duplex,
+
+	.register_type	= SH_ETH_REG_FAST_RZ,
+
+	.ecsr_value	= ECSR_ICD,
+	.ecsipr_value	= ECSIPR_ICDIP,
+	.eesipr_value	= 0xff7f009f,
+
+	.tx_check	= EESR_TC1 | EESR_FTC,
+	.eesr_err_check	= EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
+			  EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
+			  EESR_TDE | EESR_ECI,
+	.fdr_value	= 0x0000070f,
+	.rmcr_value	= RMCR_RNC,
+
+	.no_psr		= 1,
+	.apr		= 1,
+	.mpr		= 1,
+	.tpauser	= 1,
+	.hw_swap	= 1,
+	.rpadir		= 1,
+	.rpadir_value   = 2 << 16,
+	.no_trimd	= 1,
+	.no_ade		= 1,
+	.hw_crc		= 1,
+	.tsu		= 1,
+	.shift_rd0	= 1,
+};
+
 static struct sh_eth_cpu_data sh7619_data = {
 	.register_type	= SH_ETH_REG_FAST_SH3_SH2,
 
@@ -767,7 +865,7 @@ static int sh_eth_reset(struct net_device *ndev)
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 	int ret = 0;
 
-	if (sh_eth_is_gether(mdp)) {
+	if (sh_eth_is_gether(mdp) || sh_eth_is_rz_fast_ether(mdp)) {
 		sh_eth_write(ndev, EDSR_ENALL, EDSR);
 		sh_eth_write(ndev, sh_eth_read(ndev, EDMR) | EDMR_SRST_GETHER,
 			     EDMR);
@@ -878,7 +976,7 @@ static void read_mac_address(struct net_device *ndev, unsigned char *mac)
 
 static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
 {
-	if (sh_eth_is_gether(mdp))
+	if (sh_eth_is_gether(mdp) || sh_eth_is_rz_fast_ether(mdp))
 		return EDTRR_TRNS_GETHER;
 	else
 		return EDTRR_TRNS_ETHER;
@@ -1041,7 +1139,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
 		/* Rx descriptor address set */
 		if (i = 0) {
 			sh_eth_write(ndev, mdp->rx_desc_dma, RDLAR);
-			if (sh_eth_is_gether(mdp))
+			if (sh_eth_is_gether(mdp) ||
+			    sh_eth_is_rz_fast_ether(mdp))
 				sh_eth_write(ndev, mdp->rx_desc_dma, RDFAR);
 		}
 	}
@@ -1062,7 +1161,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
 		if (i = 0) {
 			/* Tx descriptor address set */
 			sh_eth_write(ndev, mdp->tx_desc_dma, TDLAR);
-			if (sh_eth_is_gether(mdp))
+			if (sh_eth_is_gether(mdp) ||
+			    sh_eth_is_rz_fast_ether(mdp))
 				sh_eth_write(ndev, mdp->tx_desc_dma, TDFAR);
 		}
 	}
@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
 
 		/* In case of almost all GETHER/ETHERs, the Receive Frame State
 		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
-		 * bits are from bit 25 to bit 16. So, the driver needs right
-		 * shifting by 16.
+		 * bit 0. However, in case of the R8A7740, R8A779x and
+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
+		 * driver needs right shifting by 16.
 		 */
 		if (mdp->cd->shift_rd0)
 			desc_status >>= 16;
@@ -2061,6 +2161,9 @@ static struct net_device_stats *sh_eth_get_stats(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
+	if (sh_eth_is_rz_fast_ether(mdp))
+		return &ndev->stats;
+
 	pm_runtime_get_sync(&mdp->pdev->dev);
 
 	ndev->stats.tx_dropped += sh_eth_read(ndev, TROCR);
@@ -2442,6 +2545,11 @@ static int sh_eth_vlan_rx_kill_vid(struct net_device *ndev,
 /* SuperH's TSU register init function */
 static void sh_eth_tsu_init(struct sh_eth_private *mdp)
 {
+	if (sh_eth_is_rz_fast_ether(mdp)) {
+		sh_eth_tsu_write(mdp, 0, TSU_TEN); /* Disable all CAM entry */
+		return;
+	}
+
 	sh_eth_tsu_write(mdp, 0, TSU_FWEN0);	/* Disable forward(0->1) */
 	sh_eth_tsu_write(mdp, 0, TSU_FWEN1);	/* Disable forward(1->0) */
 	sh_eth_tsu_write(mdp, 0, TSU_FCM);	/* forward fifo 3k-3k */
@@ -2561,6 +2669,9 @@ static const u16 *sh_eth_get_register_offset(int register_type)
 	case SH_ETH_REG_GIGABIT:
 		reg_offset = sh_eth_offset_gigabit;
 		break;
+	case SH_ETH_REG_FAST_RZ:
+		reg_offset = sh_eth_offset_fast_rz;
+		break;
 	case SH_ETH_REG_FAST_RCAR:
 		reg_offset = sh_eth_offset_fast_rcar;
 		break;
@@ -2799,6 +2910,7 @@ static struct platform_device_id sh_eth_id_table[] = {
 	{ "sh7757-ether", (kernel_ulong_t)&sh7757_data },
 	{ "sh7757-gether", (kernel_ulong_t)&sh7757_data_giga },
 	{ "sh7763-gether", (kernel_ulong_t)&sh7763_data },
+	{ "r7s72100-ether", (kernel_ulong_t)&r7s72100_data },
 	{ "r8a7740-gether", (kernel_ulong_t)&r8a7740_data },
 	{ "r8a777x-ether", (kernel_ulong_t)&r8a777x_data },
 	{ "r8a7790-ether", (kernel_ulong_t)&r8a779x_data },
diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
index 0fe35b7..6075915 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -155,6 +155,7 @@ enum {
 
 enum {
 	SH_ETH_REG_GIGABIT,
+	SH_ETH_REG_FAST_RZ,
 	SH_ETH_REG_FAST_RCAR,
 	SH_ETH_REG_FAST_SH4,
 	SH_ETH_REG_FAST_SH3_SH2
@@ -169,7 +170,7 @@ enum {
 
 /* Register's bits
  */
-/* EDSR : sh7734, sh7757, sh7763, and r8a7740 only */
+/* EDSR : sh7734, sh7757, sh7763, r8a7740, and r7s72100 only */
 enum EDSR_BIT {
 	EDSR_ENT = 0x01, EDSR_ENR = 0x02,
 };
-- 
1.8.4

[PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Simon Horman]

This is a fast ethernet controller.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---
v5
* As suggested by Sergei Shtylyov
  - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
    RMII registers. Instead use sh_eth_chip_reset.
  - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
  - Do not use reserved LCHNG bit of ECSR
  - Do not use reserved LCHNGIP bit of ECSIPR
  - Document that R8A779x also needs a 16 bit shift of the RFS bits
  - Do not document that the R7S72100 has GECMR, it does not

v4
* As requested by David Miller
  - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
  - Correct coding style in sh_eth_get_stats()

v3
* No change

v2
* As suggested by Magnus Damm and Sergei Shtylyov
  - r7s72100 ethernet is not gigabit so do not refer to it as such

* As suggested by Magnus Damm
  - As RZ specific register layout rather than using the gigabit layout
    which includes registers that do not exist on this chip.
---
 drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
 drivers/net/ethernet/renesas/sh_eth.h |   3 +-
 2 files changed, 121 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 4f5cfad..a7a0555 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[TRIMD]		= 0x027c,
 };
 
+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
+	[EDSR]		= 0x0000,
+	[EDMR]		= 0x0400,
+	[EDTRR]		= 0x0408,
+	[EDRRR]		= 0x0410,
+	[EESR]		= 0x0428,
+	[EESIPR]	= 0x0430,
+	[TDLAR]		= 0x0010,
+	[TDFAR]		= 0x0014,
+	[TDFXR]		= 0x0018,
+	[TDFFR]		= 0x001c,
+	[RDLAR]		= 0x0030,
+	[RDFAR]		= 0x0034,
+	[RDFXR]		= 0x0038,
+	[RDFFR]		= 0x003c,
+	[TRSCER]	= 0x0438,
+	[RMFCR]		= 0x0440,
+	[TFTR]		= 0x0448,
+	[FDR]		= 0x0450,
+	[RMCR]		= 0x0458,
+	[RPADIR]	= 0x0460,
+	[FCFTR]		= 0x0468,
+	[CSMR]		= 0x04E4,
+
+	[ECMR]		= 0x0500,
+	[RFLR]		= 0x0508,
+	[ECSR]		= 0x0510,
+	[ECSIPR]	= 0x0518,
+	[PIR]		= 0x0520,
+	[APR]		= 0x0554,
+	[MPR]		= 0x0558,
+	[PFTCR]		= 0x055c,
+	[PFRCR]		= 0x0560,
+	[TPAUSER]	= 0x0564,
+	[MAHR]		= 0x05c0,
+	[MALR]		= 0x05c8,
+	[CEFCR]		= 0x0740,
+	[FRECR]		= 0x0748,
+	[TSFRCR]	= 0x0750,
+	[TLFRCR]	= 0x0758,
+	[RFCR]		= 0x0760,
+	[MAFCR]		= 0x0778,
+
+	[ARSTR]		= 0x0000,
+	[TSU_CTRST]	= 0x0004,
+	[TSU_VTAG0]	= 0x0058,
+	[TSU_ADSBSY]	= 0x0060,
+	[TSU_TEN]	= 0x0064,
+	[TXNLCR0]	= 0x0080,
+	[TXALCR0]	= 0x0084,
+	[RXNLCR0]	= 0x0088,
+	[RXALCR0]	= 0x008C,
+	[TSU_ADRH0]	= 0x0100,
+	[TSU_ADRL0]	= 0x0104,
+	[TSU_ADRH31]	= 0x01f8,
+	[TSU_ADRL31]	= 0x01fc,
+};
+
 static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[ECMR]		= 0x0100,
 	[RFLR]		= 0x0108,
@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
 		return false;
 }
 
+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
+{
+	if (mdp->reg_offset == sh_eth_offset_fast_rz)
+		return true;
+	else
+		return false;
+}
+
 static void sh_eth_select_mii(struct net_device *ndev)
 {
 	u32 value = 0x0;
@@ -701,6 +767,38 @@ static struct sh_eth_cpu_data r8a7740_data = {
 	.shift_rd0	= 1,
 };
 
+/* R7S72100 */
+static struct sh_eth_cpu_data r7s72100_data = {
+	.chip_reset	= sh_eth_chip_reset,
+	.set_duplex	= sh_eth_set_duplex,
+
+	.register_type	= SH_ETH_REG_FAST_RZ,
+
+	.ecsr_value	= ECSR_ICD,
+	.ecsipr_value	= ECSIPR_ICDIP,
+	.eesipr_value	= 0xff7f009f,
+
+	.tx_check	= EESR_TC1 | EESR_FTC,
+	.eesr_err_check	= EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
+			  EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
+			  EESR_TDE | EESR_ECI,
+	.fdr_value	= 0x0000070f,
+	.rmcr_value	= RMCR_RNC,
+
+	.no_psr		= 1,
+	.apr		= 1,
+	.mpr		= 1,
+	.tpauser	= 1,
+	.hw_swap	= 1,
+	.rpadir		= 1,
+	.rpadir_value   = 2 << 16,
+	.no_trimd	= 1,
+	.no_ade		= 1,
+	.hw_crc		= 1,
+	.tsu		= 1,
+	.shift_rd0	= 1,
+};
+
 static struct sh_eth_cpu_data sh7619_data = {
 	.register_type	= SH_ETH_REG_FAST_SH3_SH2,
 
@@ -767,7 +865,7 @@ static int sh_eth_reset(struct net_device *ndev)
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 	int ret = 0;
 
-	if (sh_eth_is_gether(mdp)) {
+	if (sh_eth_is_gether(mdp) || sh_eth_is_rz_fast_ether(mdp)) {
 		sh_eth_write(ndev, EDSR_ENALL, EDSR);
 		sh_eth_write(ndev, sh_eth_read(ndev, EDMR) | EDMR_SRST_GETHER,
 			     EDMR);
@@ -878,7 +976,7 @@ static void read_mac_address(struct net_device *ndev, unsigned char *mac)
 
 static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
 {
-	if (sh_eth_is_gether(mdp))
+	if (sh_eth_is_gether(mdp) || sh_eth_is_rz_fast_ether(mdp))
 		return EDTRR_TRNS_GETHER;
 	else
 		return EDTRR_TRNS_ETHER;
@@ -1041,7 +1139,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
 		/* Rx descriptor address set */
 		if (i == 0) {
 			sh_eth_write(ndev, mdp->rx_desc_dma, RDLAR);
-			if (sh_eth_is_gether(mdp))
+			if (sh_eth_is_gether(mdp) ||
+			    sh_eth_is_rz_fast_ether(mdp))
 				sh_eth_write(ndev, mdp->rx_desc_dma, RDFAR);
 		}
 	}
@@ -1062,7 +1161,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
 		if (i == 0) {
 			/* Tx descriptor address set */
 			sh_eth_write(ndev, mdp->tx_desc_dma, TDLAR);
-			if (sh_eth_is_gether(mdp))
+			if (sh_eth_is_gether(mdp) ||
+			    sh_eth_is_rz_fast_ether(mdp))
 				sh_eth_write(ndev, mdp->tx_desc_dma, TDFAR);
 		}
 	}
@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
 
 		/* In case of almost all GETHER/ETHERs, the Receive Frame State
 		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
-		 * bits are from bit 25 to bit 16. So, the driver needs right
-		 * shifting by 16.
+		 * bit 0. However, in case of the R8A7740, R8A779x and
+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
+		 * driver needs right shifting by 16.
 		 */
 		if (mdp->cd->shift_rd0)
 			desc_status >>= 16;
@@ -2061,6 +2161,9 @@ static struct net_device_stats *sh_eth_get_stats(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
+	if (sh_eth_is_rz_fast_ether(mdp))
+		return &ndev->stats;
+
 	pm_runtime_get_sync(&mdp->pdev->dev);
 
 	ndev->stats.tx_dropped += sh_eth_read(ndev, TROCR);
@@ -2442,6 +2545,11 @@ static int sh_eth_vlan_rx_kill_vid(struct net_device *ndev,
 /* SuperH's TSU register init function */
 static void sh_eth_tsu_init(struct sh_eth_private *mdp)
 {
+	if (sh_eth_is_rz_fast_ether(mdp)) {
+		sh_eth_tsu_write(mdp, 0, TSU_TEN); /* Disable all CAM entry */
+		return;
+	}
+
 	sh_eth_tsu_write(mdp, 0, TSU_FWEN0);	/* Disable forward(0->1) */
 	sh_eth_tsu_write(mdp, 0, TSU_FWEN1);	/* Disable forward(1->0) */
 	sh_eth_tsu_write(mdp, 0, TSU_FCM);	/* forward fifo 3k-3k */
@@ -2561,6 +2669,9 @@ static const u16 *sh_eth_get_register_offset(int register_type)
 	case SH_ETH_REG_GIGABIT:
 		reg_offset = sh_eth_offset_gigabit;
 		break;
+	case SH_ETH_REG_FAST_RZ:
+		reg_offset = sh_eth_offset_fast_rz;
+		break;
 	case SH_ETH_REG_FAST_RCAR:
 		reg_offset = sh_eth_offset_fast_rcar;
 		break;
@@ -2799,6 +2910,7 @@ static struct platform_device_id sh_eth_id_table[] = {
 	{ "sh7757-ether", (kernel_ulong_t)&sh7757_data },
 	{ "sh7757-gether", (kernel_ulong_t)&sh7757_data_giga },
 	{ "sh7763-gether", (kernel_ulong_t)&sh7763_data },
+	{ "r7s72100-ether", (kernel_ulong_t)&r7s72100_data },
 	{ "r8a7740-gether", (kernel_ulong_t)&r8a7740_data },
 	{ "r8a777x-ether", (kernel_ulong_t)&r8a777x_data },
 	{ "r8a7790-ether", (kernel_ulong_t)&r8a779x_data },
diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
index 0fe35b7..6075915 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -155,6 +155,7 @@ enum {
 
 enum {
 	SH_ETH_REG_GIGABIT,
+	SH_ETH_REG_FAST_RZ,
 	SH_ETH_REG_FAST_RCAR,
 	SH_ETH_REG_FAST_SH4,
 	SH_ETH_REG_FAST_SH3_SH2
@@ -169,7 +170,7 @@ enum {
 
 /* Register's bits
  */
-/* EDSR : sh7734, sh7757, sh7763, and r8a7740 only */
+/* EDSR : sh7734, sh7757, sh7763, r8a7740, and r7s72100 only */
 enum EDSR_BIT {
 	EDSR_ENT = 0x01, EDSR_ENR = 0x02,
 };
-- 
1.8.4

[PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Simon Horman]

This is a fast ethernet controller.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---
v5
* As suggested by Sergei Shtylyov
  - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
    RMII registers. Instead use sh_eth_chip_reset.
  - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
  - Do not use reserved LCHNG bit of ECSR
  - Do not use reserved LCHNGIP bit of ECSIPR
  - Document that R8A779x also needs a 16 bit shift of the RFS bits
  - Do not document that the R7S72100 has GECMR, it does not

v4
* As requested by David Miller
  - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
  - Correct coding style in sh_eth_get_stats()

v3
* No change

v2
* As suggested by Magnus Damm and Sergei Shtylyov
  - r7s72100 ethernet is not gigabit so do not refer to it as such

* As suggested by Magnus Damm
  - As RZ specific register layout rather than using the gigabit layout
    which includes registers that do not exist on this chip.
---
 drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
 drivers/net/ethernet/renesas/sh_eth.h |   3 +-
 2 files changed, 121 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 4f5cfad..a7a0555 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[TRIMD]		= 0x027c,
 };
 
+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
+	[EDSR]		= 0x0000,
+	[EDMR]		= 0x0400,
+	[EDTRR]		= 0x0408,
+	[EDRRR]		= 0x0410,
+	[EESR]		= 0x0428,
+	[EESIPR]	= 0x0430,
+	[TDLAR]		= 0x0010,
+	[TDFAR]		= 0x0014,
+	[TDFXR]		= 0x0018,
+	[TDFFR]		= 0x001c,
+	[RDLAR]		= 0x0030,
+	[RDFAR]		= 0x0034,
+	[RDFXR]		= 0x0038,
+	[RDFFR]		= 0x003c,
+	[TRSCER]	= 0x0438,
+	[RMFCR]		= 0x0440,
+	[TFTR]		= 0x0448,
+	[FDR]		= 0x0450,
+	[RMCR]		= 0x0458,
+	[RPADIR]	= 0x0460,
+	[FCFTR]		= 0x0468,
+	[CSMR]		= 0x04E4,
+
+	[ECMR]		= 0x0500,
+	[RFLR]		= 0x0508,
+	[ECSR]		= 0x0510,
+	[ECSIPR]	= 0x0518,
+	[PIR]		= 0x0520,
+	[APR]		= 0x0554,
+	[MPR]		= 0x0558,
+	[PFTCR]		= 0x055c,
+	[PFRCR]		= 0x0560,
+	[TPAUSER]	= 0x0564,
+	[MAHR]		= 0x05c0,
+	[MALR]		= 0x05c8,
+	[CEFCR]		= 0x0740,
+	[FRECR]		= 0x0748,
+	[TSFRCR]	= 0x0750,
+	[TLFRCR]	= 0x0758,
+	[RFCR]		= 0x0760,
+	[MAFCR]		= 0x0778,
+
+	[ARSTR]		= 0x0000,
+	[TSU_CTRST]	= 0x0004,
+	[TSU_VTAG0]	= 0x0058,
+	[TSU_ADSBSY]	= 0x0060,
+	[TSU_TEN]	= 0x0064,
+	[TXNLCR0]	= 0x0080,
+	[TXALCR0]	= 0x0084,
+	[RXNLCR0]	= 0x0088,
+	[RXALCR0]	= 0x008C,
+	[TSU_ADRH0]	= 0x0100,
+	[TSU_ADRL0]	= 0x0104,
+	[TSU_ADRH31]	= 0x01f8,
+	[TSU_ADRL31]	= 0x01fc,
+};
+
 static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[ECMR]		= 0x0100,
 	[RFLR]		= 0x0108,
@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
 		return false;
 }
 
+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
+{
+	if (mdp->reg_offset == sh_eth_offset_fast_rz)
+		return true;
+	else
+		return false;
+}
+
 static void sh_eth_select_mii(struct net_device *ndev)
 {
 	u32 value = 0x0;
@@ -701,6 +767,38 @@ static struct sh_eth_cpu_data r8a7740_data = {
 	.shift_rd0	= 1,
 };
 
+/* R7S72100 */
+static struct sh_eth_cpu_data r7s72100_data = {
+	.chip_reset	= sh_eth_chip_reset,
+	.set_duplex	= sh_eth_set_duplex,
+
+	.register_type	= SH_ETH_REG_FAST_RZ,
+
+	.ecsr_value	= ECSR_ICD,
+	.ecsipr_value	= ECSIPR_ICDIP,
+	.eesipr_value	= 0xff7f009f,
+
+	.tx_check	= EESR_TC1 | EESR_FTC,
+	.eesr_err_check	= EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
+			  EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
+			  EESR_TDE | EESR_ECI,
+	.fdr_value	= 0x0000070f,
+	.rmcr_value	= RMCR_RNC,
+
+	.no_psr		= 1,
+	.apr		= 1,
+	.mpr		= 1,
+	.tpauser	= 1,
+	.hw_swap	= 1,
+	.rpadir		= 1,
+	.rpadir_value   = 2 << 16,
+	.no_trimd	= 1,
+	.no_ade		= 1,
+	.hw_crc		= 1,
+	.tsu		= 1,
+	.shift_rd0	= 1,
+};
+
 static struct sh_eth_cpu_data sh7619_data = {
 	.register_type	= SH_ETH_REG_FAST_SH3_SH2,
 
@@ -767,7 +865,7 @@ static int sh_eth_reset(struct net_device *ndev)
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 	int ret = 0;
 
-	if (sh_eth_is_gether(mdp)) {
+	if (sh_eth_is_gether(mdp) || sh_eth_is_rz_fast_ether(mdp)) {
 		sh_eth_write(ndev, EDSR_ENALL, EDSR);
 		sh_eth_write(ndev, sh_eth_read(ndev, EDMR) | EDMR_SRST_GETHER,
 			     EDMR);
@@ -878,7 +976,7 @@ static void read_mac_address(struct net_device *ndev, unsigned char *mac)
 
 static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
 {
-	if (sh_eth_is_gether(mdp))
+	if (sh_eth_is_gether(mdp) || sh_eth_is_rz_fast_ether(mdp))
 		return EDTRR_TRNS_GETHER;
 	else
 		return EDTRR_TRNS_ETHER;
@@ -1041,7 +1139,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
 		/* Rx descriptor address set */
 		if (i == 0) {
 			sh_eth_write(ndev, mdp->rx_desc_dma, RDLAR);
-			if (sh_eth_is_gether(mdp))
+			if (sh_eth_is_gether(mdp) ||
+			    sh_eth_is_rz_fast_ether(mdp))
 				sh_eth_write(ndev, mdp->rx_desc_dma, RDFAR);
 		}
 	}
@@ -1062,7 +1161,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
 		if (i == 0) {
 			/* Tx descriptor address set */
 			sh_eth_write(ndev, mdp->tx_desc_dma, TDLAR);
-			if (sh_eth_is_gether(mdp))
+			if (sh_eth_is_gether(mdp) ||
+			    sh_eth_is_rz_fast_ether(mdp))
 				sh_eth_write(ndev, mdp->tx_desc_dma, TDFAR);
 		}
 	}
@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
 
 		/* In case of almost all GETHER/ETHERs, the Receive Frame State
 		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
-		 * bits are from bit 25 to bit 16. So, the driver needs right
-		 * shifting by 16.
+		 * bit 0. However, in case of the R8A7740, R8A779x and
+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
+		 * driver needs right shifting by 16.
 		 */
 		if (mdp->cd->shift_rd0)
 			desc_status >>= 16;
@@ -2061,6 +2161,9 @@ static struct net_device_stats *sh_eth_get_stats(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
+	if (sh_eth_is_rz_fast_ether(mdp))
+		return &ndev->stats;
+
 	pm_runtime_get_sync(&mdp->pdev->dev);
 
 	ndev->stats.tx_dropped += sh_eth_read(ndev, TROCR);
@@ -2442,6 +2545,11 @@ static int sh_eth_vlan_rx_kill_vid(struct net_device *ndev,
 /* SuperH's TSU register init function */
 static void sh_eth_tsu_init(struct sh_eth_private *mdp)
 {
+	if (sh_eth_is_rz_fast_ether(mdp)) {
+		sh_eth_tsu_write(mdp, 0, TSU_TEN); /* Disable all CAM entry */
+		return;
+	}
+
 	sh_eth_tsu_write(mdp, 0, TSU_FWEN0);	/* Disable forward(0->1) */
 	sh_eth_tsu_write(mdp, 0, TSU_FWEN1);	/* Disable forward(1->0) */
 	sh_eth_tsu_write(mdp, 0, TSU_FCM);	/* forward fifo 3k-3k */
@@ -2561,6 +2669,9 @@ static const u16 *sh_eth_get_register_offset(int register_type)
 	case SH_ETH_REG_GIGABIT:
 		reg_offset = sh_eth_offset_gigabit;
 		break;
+	case SH_ETH_REG_FAST_RZ:
+		reg_offset = sh_eth_offset_fast_rz;
+		break;
 	case SH_ETH_REG_FAST_RCAR:
 		reg_offset = sh_eth_offset_fast_rcar;
 		break;
@@ -2799,6 +2910,7 @@ static struct platform_device_id sh_eth_id_table[] = {
 	{ "sh7757-ether", (kernel_ulong_t)&sh7757_data },
 	{ "sh7757-gether", (kernel_ulong_t)&sh7757_data_giga },
 	{ "sh7763-gether", (kernel_ulong_t)&sh7763_data },
+	{ "r7s72100-ether", (kernel_ulong_t)&r7s72100_data },
 	{ "r8a7740-gether", (kernel_ulong_t)&r8a7740_data },
 	{ "r8a777x-ether", (kernel_ulong_t)&r8a777x_data },
 	{ "r8a7790-ether", (kernel_ulong_t)&r8a779x_data },
diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
index 0fe35b7..6075915 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -155,6 +155,7 @@ enum {
 
 enum {
 	SH_ETH_REG_GIGABIT,
+	SH_ETH_REG_FAST_RZ,
 	SH_ETH_REG_FAST_RCAR,
 	SH_ETH_REG_FAST_SH4,
 	SH_ETH_REG_FAST_SH3_SH2
@@ -169,7 +170,7 @@ enum {
 
 /* Register's bits
  */
-/* EDSR : sh7734, sh7757, sh7763, and r8a7740 only */
+/* EDSR : sh7734, sh7757, sh7763, r8a7740, and r7s72100 only */
 enum EDSR_BIT {
 	EDSR_ENT = 0x01, EDSR_ENR = 0x02,
 };
-- 
1.8.4

[PATCH v5 3/4] ARM: shmobile: r7s72100: Add clock for r7s72100-ether

[Simon Horman]

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Dave,

I plan to take this change through my tree.

v5
* Rebase

v3 - v4
* No change

v2
* As suggested by Sergei Shtylyov
  - Add MSTP74 to beginning of enum on a line by itself
* As suggested by Magnus Damm
  - r7s72100 ethernet is not gigabit so do not refer to it as such
---
 arch/arm/mach-shmobile/clock-r7s72100.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
index dd8ce87..0242ca5 100644
--- a/arch/arm/mach-shmobile/clock-r7s72100.c
+++ b/arch/arm/mach-shmobile/clock-r7s72100.c
@@ -27,6 +27,7 @@
 #define FRQCR2		0xfcfe0014
 #define STBCR3		0xfcfe0420
 #define STBCR4		0xfcfe0424
+#define STBCR7		0xfcfe0430
 #define STBCR9		0xfcfe0438
 
 #define PLL_RATE 30
@@ -146,6 +147,7 @@ struct clk div4_clks[DIV4_NR] = {
 };
 
 enum {	MSTP97, MSTP96, MSTP95, MSTP94,
+	MSTP74,
 	MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
 	MSTP33,	MSTP_NR };
 
@@ -154,6 +156,7 @@ static struct clk mstp_clks[MSTP_NR] = {
 	[MSTP96] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 6, 0), /* RIIC1 */
 	[MSTP95] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 5, 0), /* RIIC2 */
 	[MSTP94] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 4, 0), /* RIIC3 */
+	[MSTP74] = SH_CLK_MSTP8(&peripheral1_clk, STBCR7, 4, 0), /* Ether */
 	[MSTP47] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 7, 0), /* SCIF0 */
 	[MSTP46] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 6, 0), /* SCIF1 */
 	[MSTP45] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 5, 0), /* SCIF2 */
@@ -180,6 +183,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
 	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
 	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
+	CLKDEV_DEV_ID("r7s72100-ether", &mstp_clks[MSTP74]),
 	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
 
 	/* ICK */
-- 
1.8.4

[PATCH v5 3/4] ARM: shmobile: r7s72100: Add clock for r7s72100-ether

[Simon Horman]

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Dave,

I plan to take this change through my tree.

v5
* Rebase

v3 - v4
* No change

v2
* As suggested by Sergei Shtylyov
  - Add MSTP74 to beginning of enum on a line by itself
* As suggested by Magnus Damm
  - r7s72100 ethernet is not gigabit so do not refer to it as such
---
 arch/arm/mach-shmobile/clock-r7s72100.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
index dd8ce87..0242ca5 100644
--- a/arch/arm/mach-shmobile/clock-r7s72100.c
+++ b/arch/arm/mach-shmobile/clock-r7s72100.c
@@ -27,6 +27,7 @@
 #define FRQCR2		0xfcfe0014
 #define STBCR3		0xfcfe0420
 #define STBCR4		0xfcfe0424
+#define STBCR7		0xfcfe0430
 #define STBCR9		0xfcfe0438
 
 #define PLL_RATE 30
@@ -146,6 +147,7 @@ struct clk div4_clks[DIV4_NR] = {
 };
 
 enum {	MSTP97, MSTP96, MSTP95, MSTP94,
+	MSTP74,
 	MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
 	MSTP33,	MSTP_NR };
 
@@ -154,6 +156,7 @@ static struct clk mstp_clks[MSTP_NR] = {
 	[MSTP96] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 6, 0), /* RIIC1 */
 	[MSTP95] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 5, 0), /* RIIC2 */
 	[MSTP94] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 4, 0), /* RIIC3 */
+	[MSTP74] = SH_CLK_MSTP8(&peripheral1_clk, STBCR7, 4, 0), /* Ether */
 	[MSTP47] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 7, 0), /* SCIF0 */
 	[MSTP46] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 6, 0), /* SCIF1 */
 	[MSTP45] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 5, 0), /* SCIF2 */
@@ -180,6 +183,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
 	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
 	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
+	CLKDEV_DEV_ID("r7s72100-ether", &mstp_clks[MSTP74]),
 	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
 
 	/* ICK */
-- 
1.8.4

[PATCH v5 3/4] ARM: shmobile: r7s72100: Add clock for r7s72100-ether

[Simon Horman]

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Dave,

I plan to take this change through my tree.

v5
* Rebase

v3 - v4
* No change

v2
* As suggested by Sergei Shtylyov
  - Add MSTP74 to beginning of enum on a line by itself
* As suggested by Magnus Damm
  - r7s72100 ethernet is not gigabit so do not refer to it as such
---
 arch/arm/mach-shmobile/clock-r7s72100.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
index dd8ce87..0242ca5 100644
--- a/arch/arm/mach-shmobile/clock-r7s72100.c
+++ b/arch/arm/mach-shmobile/clock-r7s72100.c
@@ -27,6 +27,7 @@
 #define FRQCR2		0xfcfe0014
 #define STBCR3		0xfcfe0420
 #define STBCR4		0xfcfe0424
+#define STBCR7		0xfcfe0430
 #define STBCR9		0xfcfe0438
 
 #define PLL_RATE 30
@@ -146,6 +147,7 @@ struct clk div4_clks[DIV4_NR] = {
 };
 
 enum {	MSTP97, MSTP96, MSTP95, MSTP94,
+	MSTP74,
 	MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
 	MSTP33,	MSTP_NR };
 
@@ -154,6 +156,7 @@ static struct clk mstp_clks[MSTP_NR] = {
 	[MSTP96] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 6, 0), /* RIIC1 */
 	[MSTP95] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 5, 0), /* RIIC2 */
 	[MSTP94] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 4, 0), /* RIIC3 */
+	[MSTP74] = SH_CLK_MSTP8(&peripheral1_clk, STBCR7, 4, 0), /* Ether */
 	[MSTP47] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 7, 0), /* SCIF0 */
 	[MSTP46] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 6, 0), /* SCIF1 */
 	[MSTP45] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 5, 0), /* SCIF2 */
@@ -180,6 +183,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
 	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
 	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
+	CLKDEV_DEV_ID("r7s72100-ether", &mstp_clks[MSTP74]),
 	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
 
 	/* ICK */
-- 
1.8.4

[PATCH v5 4/4] ARM: shmobile: genmai: Enable r7s72100-ether

[Simon Horman]

Signed-off-by: Simon Horman <horms@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Dave,

I plan to take this change through my tree.

v3 - v5
* No change

v2
* As suggested by Magnus Damm and Sergei Shtylyov
  - r7s72100 ethernet is not gigabit so do not refer to it as such

* As suggested by Sergei Shtylyov
  - set no_ether_link as there is no LINK signal documented
    in the manual
---
 arch/arm/mach-shmobile/board-genmai.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
index 3e92e3c..a1f6fe1 100644
--- a/arch/arm/mach-shmobile/board-genmai.c
+++ b/arch/arm/mach-shmobile/board-genmai.c
@@ -20,15 +20,36 @@
 
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/sh_eth.h>
 #include <mach/common.h>
+#include <mach/irqs.h>
 #include <mach/r7s72100.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 
+/* Ether */
+static const struct sh_eth_plat_data ether_pdata __initconst = {
+	.phy			= 0x00, /* PD60610 */
+	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
+	.phy_interface		= PHY_INTERFACE_MODE_MII,
+	.no_ether_link		= 1
+};
+
+static const struct resource ether_resources[] __initconst = {
+	DEFINE_RES_MEM(0xe8203000, 0x800),
+	DEFINE_RES_MEM(0xe8204800, 0x200),
+	DEFINE_RES_IRQ(gic_iid(359)),
+};
+
 static void __init genmai_add_standard_devices(void)
 {
 	r7s72100_clock_init();
 	r7s72100_add_dt_devices();
+
+	platform_device_register_resndata(&platform_bus, "r7s72100-ether", -1,
+					  ether_resources,
+					  ARRAY_SIZE(ether_resources),
+					  &ether_pdata, sizeof(ether_pdata));
 }
 
 static const char * const genmai_boards_compat_dt[] __initconst = {
-- 
1.8.4

[PATCH v5 4/4] ARM: shmobile: genmai: Enable r7s72100-ether

[Simon Horman]

Signed-off-by: Simon Horman <horms@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Dave,

I plan to take this change through my tree.

v3 - v5
* No change

v2
* As suggested by Magnus Damm and Sergei Shtylyov
  - r7s72100 ethernet is not gigabit so do not refer to it as such

* As suggested by Sergei Shtylyov
  - set no_ether_link as there is no LINK signal documented
    in the manual
---
 arch/arm/mach-shmobile/board-genmai.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
index 3e92e3c..a1f6fe1 100644
--- a/arch/arm/mach-shmobile/board-genmai.c
+++ b/arch/arm/mach-shmobile/board-genmai.c
@@ -20,15 +20,36 @@
 
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/sh_eth.h>
 #include <mach/common.h>
+#include <mach/irqs.h>
 #include <mach/r7s72100.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 
+/* Ether */
+static const struct sh_eth_plat_data ether_pdata __initconst = {
+	.phy			= 0x00, /* PD60610 */
+	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
+	.phy_interface		= PHY_INTERFACE_MODE_MII,
+	.no_ether_link		= 1
+};
+
+static const struct resource ether_resources[] __initconst = {
+	DEFINE_RES_MEM(0xe8203000, 0x800),
+	DEFINE_RES_MEM(0xe8204800, 0x200),
+	DEFINE_RES_IRQ(gic_iid(359)),
+};
+
 static void __init genmai_add_standard_devices(void)
 {
 	r7s72100_clock_init();
 	r7s72100_add_dt_devices();
+
+	platform_device_register_resndata(&platform_bus, "r7s72100-ether", -1,
+					  ether_resources,
+					  ARRAY_SIZE(ether_resources),
+					  &ether_pdata, sizeof(ether_pdata));
 }
 
 static const char * const genmai_boards_compat_dt[] __initconst = {
-- 
1.8.4

[PATCH v5 4/4] ARM: shmobile: genmai: Enable r7s72100-ether

[Simon Horman]

Signed-off-by: Simon Horman <horms@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Dave,

I plan to take this change through my tree.

v3 - v5
* No change

v2
* As suggested by Magnus Damm and Sergei Shtylyov
  - r7s72100 ethernet is not gigabit so do not refer to it as such

* As suggested by Sergei Shtylyov
  - set no_ether_link as there is no LINK signal documented
    in the manual
---
 arch/arm/mach-shmobile/board-genmai.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
index 3e92e3c..a1f6fe1 100644
--- a/arch/arm/mach-shmobile/board-genmai.c
+++ b/arch/arm/mach-shmobile/board-genmai.c
@@ -20,15 +20,36 @@
 
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/sh_eth.h>
 #include <mach/common.h>
+#include <mach/irqs.h>
 #include <mach/r7s72100.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 
+/* Ether */
+static const struct sh_eth_plat_data ether_pdata __initconst = {
+	.phy			= 0x00, /* PD60610 */
+	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
+	.phy_interface		= PHY_INTERFACE_MODE_MII,
+	.no_ether_link		= 1
+};
+
+static const struct resource ether_resources[] __initconst = {
+	DEFINE_RES_MEM(0xe8203000, 0x800),
+	DEFINE_RES_MEM(0xe8204800, 0x200),
+	DEFINE_RES_IRQ(gic_iid(359)),
+};
+
 static void __init genmai_add_standard_devices(void)
 {
 	r7s72100_clock_init();
 	r7s72100_add_dt_devices();
+
+	platform_device_register_resndata(&platform_bus, "r7s72100-ether", -1,
+					  ether_resources,
+					  ARRAY_SIZE(ether_resources),
+					  &ether_pdata, sizeof(ether_pdata));
 }
 
 static const char * const genmai_boards_compat_dt[] __initconst = {
-- 
1.8.4

Re: [PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Joe Perches]

On Wed, 2014-01-15 at 15:12 +0900, Simon Horman wrote:
> Return a boolean and use true and false.
[]
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
[]
> @@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
>  	[TSU_ADRL31]	= 0x01fc,
>  };
>  
> -static int sh_eth_is_gether(struct sh_eth_private *mdp)
> +static bool sh_eth_is_gether(struct sh_eth_private *mdp)
>  {
>  	if (mdp->reg_offset = sh_eth_offset_gigabit)
> -		return 1;
> +		return true;
>  	else
> -		return 0;
> +		return false;
>  }

Or maybe:

static bool sh_eth_is_gether(struct sh_eth_private *mdp)
{
	return mdp->reg_offset = sh_eth_offset_gigabit;
}

Re: [PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Joe Perches]

On Wed, 2014-01-15 at 15:12 +0900, Simon Horman wrote:
> Return a boolean and use true and false.
[]
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
[]
> @@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
>  	[TSU_ADRL31]	= 0x01fc,
>  };
>  
> -static int sh_eth_is_gether(struct sh_eth_private *mdp)
> +static bool sh_eth_is_gether(struct sh_eth_private *mdp)
>  {
>  	if (mdp->reg_offset == sh_eth_offset_gigabit)
> -		return 1;
> +		return true;
>  	else
> -		return 0;
> +		return false;
>  }

Or maybe:

static bool sh_eth_is_gether(struct sh_eth_private *mdp)
{
	return mdp->reg_offset == sh_eth_offset_gigabit;
}

[PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Joe Perches]

On Wed, 2014-01-15 at 15:12 +0900, Simon Horman wrote:
> Return a boolean and use true and false.
[]
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
[]
> @@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
>  	[TSU_ADRL31]	= 0x01fc,
>  };
>  
> -static int sh_eth_is_gether(struct sh_eth_private *mdp)
> +static bool sh_eth_is_gether(struct sh_eth_private *mdp)
>  {
>  	if (mdp->reg_offset == sh_eth_offset_gigabit)
> -		return 1;
> +		return true;
>  	else
> -		return 0;
> +		return false;
>  }

Or maybe:

static bool sh_eth_is_gether(struct sh_eth_private *mdp)
{
	return mdp->reg_offset == sh_eth_offset_gigabit;
}

Re: [PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Sergei Shtylyov]

Hello.

On 01/15/2014 09:12 AM, Simon Horman wrote:

> This is a fast ethernet controller.

    I have to say it's not exact enough patch description: R7S72100 is not 
Ethernet controller itself, it's a SoC containing the Ethernet controller.

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

> ---
> v5
> * As suggested by Sergei Shtylyov
>    - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
>      RMII registers. Instead use sh_eth_chip_reset.
>    - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
>    - Do not use reserved LCHNG bit of ECSR
>    - Do not use reserved LCHNGIP bit of ECSIPR
>    - Document that R8A779x also needs a 16 bit shift of the RFS bits
>    - Do not document that the R7S72100 has GECMR, it does not

   The above change list was moved from v2 section and doesn't match the real 
changes done in v5. ;-)

> v4
> * As requested by David Miller
>    - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
>    - Correct coding style in sh_eth_get_stats()

> v3
> * No change

> v2
> * As suggested by Magnus Damm and Sergei Shtylyov
>    - r7s72100 ethernet is not gigabit so do not refer to it as such

> * As suggested by Magnus Damm
>    - As RZ specific register layout rather than using the gigabit layout
>      which includes registers that do not exist on this chip.
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
>   drivers/net/ethernet/renesas/sh_eth.h |   3 +-
>   2 files changed, 121 insertions(+), 8 deletions(-)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 4f5cfad..a7a0555 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[TRIMD]		= 0x027c,
>   };
>
> +static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {

    Shouldn't this map precede R-Car one since this SoC is newer the same way 
you've reordered *enum* values, etc.? Sorry for not noticing in the previous 
review...

[...]
> +	[ARSTR]		= 0x0000,
> +	[TSU_CTRST]	= 0x0004,
> +	[TSU_VTAG0]	= 0x0058,
> +	[TSU_ADSBSY]	= 0x0060,
> +	[TSU_TEN]	= 0x0064,
> +	[TXNLCR0]	= 0x0080,
> +	[TXALCR0]	= 0x0084,
> +	[RXNLCR0]	= 0x0088,
> +	[RXALCR0]	= 0x008C,

    Well, the above counter register subgroup stands out from the TSU_* 
registers in the Gigabit mapping, not sure if we should follow that. These 
registers are not currently used anyway...

> +	[TSU_ADRH0]	= 0x0100,
> +	[TSU_ADRL0]	= 0x0104,
> +	[TSU_ADRH31]	= 0x01f8,
> +	[TSU_ADRL31]	= 0x01fc,
> +};
> +
>   static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[ECMR]		= 0x0100,
>   	[RFLR]		= 0x0108,
> @@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
>   		return false;
>   }
>
> +static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> +{
> +	if (mdp->reg_offset = sh_eth_offset_fast_rz)
> +		return true;
> +	else
> +		return false;

    Perhaps you should compress the above functions to one-liners as Joe has 
suggested. Or I/you could do it in a separate patch...

> +}
> +
>   static void sh_eth_select_mii(struct net_device *ndev)
>   {
>   	u32 value = 0x0;
[...]
> @@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
>
>   		/* In case of almost all GETHER/ETHERs, the Receive Frame State
>   		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> -		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> -		 * bits are from bit 25 to bit 16. So, the driver needs right
> -		 * shifting by 16.
> +		 * bit 0. However, in case of the R8A7740, R8A779x and

    Small nit: comma needed before "and" as far as I know English grammar.

> +		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> +		 * driver needs right shifting by 16.
>   		 */
>   		if (mdp->cd->shift_rd0)
>   			desc_status >>= 16;

    Other than that, this looks fine now, you can add my:

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

WBR, Sergei

Re: [PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Sergei Shtylyov]

Hello.

On 01/15/2014 09:12 AM, Simon Horman wrote:

> This is a fast ethernet controller.

    I have to say it's not exact enough patch description: R7S72100 is not 
Ethernet controller itself, it's a SoC containing the Ethernet controller.

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

> ---
> v5
> * As suggested by Sergei Shtylyov
>    - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
>      RMII registers. Instead use sh_eth_chip_reset.
>    - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
>    - Do not use reserved LCHNG bit of ECSR
>    - Do not use reserved LCHNGIP bit of ECSIPR
>    - Document that R8A779x also needs a 16 bit shift of the RFS bits
>    - Do not document that the R7S72100 has GECMR, it does not

   The above change list was moved from v2 section and doesn't match the real 
changes done in v5. ;-)

> v4
> * As requested by David Miller
>    - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
>    - Correct coding style in sh_eth_get_stats()

> v3
> * No change

> v2
> * As suggested by Magnus Damm and Sergei Shtylyov
>    - r7s72100 ethernet is not gigabit so do not refer to it as such

> * As suggested by Magnus Damm
>    - As RZ specific register layout rather than using the gigabit layout
>      which includes registers that do not exist on this chip.
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
>   drivers/net/ethernet/renesas/sh_eth.h |   3 +-
>   2 files changed, 121 insertions(+), 8 deletions(-)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 4f5cfad..a7a0555 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[TRIMD]		= 0x027c,
>   };
>
> +static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {

    Shouldn't this map precede R-Car one since this SoC is newer the same way 
you've reordered *enum* values, etc.? Sorry for not noticing in the previous 
review...

[...]
> +	[ARSTR]		= 0x0000,
> +	[TSU_CTRST]	= 0x0004,
> +	[TSU_VTAG0]	= 0x0058,
> +	[TSU_ADSBSY]	= 0x0060,
> +	[TSU_TEN]	= 0x0064,
> +	[TXNLCR0]	= 0x0080,
> +	[TXALCR0]	= 0x0084,
> +	[RXNLCR0]	= 0x0088,
> +	[RXALCR0]	= 0x008C,

    Well, the above counter register subgroup stands out from the TSU_* 
registers in the Gigabit mapping, not sure if we should follow that. These 
registers are not currently used anyway...

> +	[TSU_ADRH0]	= 0x0100,
> +	[TSU_ADRL0]	= 0x0104,
> +	[TSU_ADRH31]	= 0x01f8,
> +	[TSU_ADRL31]	= 0x01fc,
> +};
> +
>   static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[ECMR]		= 0x0100,
>   	[RFLR]		= 0x0108,
> @@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
>   		return false;
>   }
>
> +static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> +{
> +	if (mdp->reg_offset == sh_eth_offset_fast_rz)
> +		return true;
> +	else
> +		return false;

    Perhaps you should compress the above functions to one-liners as Joe has 
suggested. Or I/you could do it in a separate patch...

> +}
> +
>   static void sh_eth_select_mii(struct net_device *ndev)
>   {
>   	u32 value = 0x0;
[...]
> @@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
>
>   		/* In case of almost all GETHER/ETHERs, the Receive Frame State
>   		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> -		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> -		 * bits are from bit 25 to bit 16. So, the driver needs right
> -		 * shifting by 16.
> +		 * bit 0. However, in case of the R8A7740, R8A779x and

    Small nit: comma needed before "and" as far as I know English grammar.

> +		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> +		 * driver needs right shifting by 16.
>   		 */
>   		if (mdp->cd->shift_rd0)
>   			desc_status >>= 16;

    Other than that, this looks fine now, you can add my:

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

WBR, Sergei

[PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Sergei Shtylyov]

Hello.

On 01/15/2014 09:12 AM, Simon Horman wrote:

> This is a fast ethernet controller.

    I have to say it's not exact enough patch description: R7S72100 is not 
Ethernet controller itself, it's a SoC containing the Ethernet controller.

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

> ---
> v5
> * As suggested by Sergei Shtylyov
>    - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
>      RMII registers. Instead use sh_eth_chip_reset.
>    - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
>    - Do not use reserved LCHNG bit of ECSR
>    - Do not use reserved LCHNGIP bit of ECSIPR
>    - Document that R8A779x also needs a 16 bit shift of the RFS bits
>    - Do not document that the R7S72100 has GECMR, it does not

   The above change list was moved from v2 section and doesn't match the real 
changes done in v5. ;-)

> v4
> * As requested by David Miller
>    - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
>    - Correct coding style in sh_eth_get_stats()

> v3
> * No change

> v2
> * As suggested by Magnus Damm and Sergei Shtylyov
>    - r7s72100 ethernet is not gigabit so do not refer to it as such

> * As suggested by Magnus Damm
>    - As RZ specific register layout rather than using the gigabit layout
>      which includes registers that do not exist on this chip.
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
>   drivers/net/ethernet/renesas/sh_eth.h |   3 +-
>   2 files changed, 121 insertions(+), 8 deletions(-)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 4f5cfad..a7a0555 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[TRIMD]		= 0x027c,
>   };
>
> +static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {

    Shouldn't this map precede R-Car one since this SoC is newer the same way 
you've reordered *enum* values, etc.? Sorry for not noticing in the previous 
review...

[...]
> +	[ARSTR]		= 0x0000,
> +	[TSU_CTRST]	= 0x0004,
> +	[TSU_VTAG0]	= 0x0058,
> +	[TSU_ADSBSY]	= 0x0060,
> +	[TSU_TEN]	= 0x0064,
> +	[TXNLCR0]	= 0x0080,
> +	[TXALCR0]	= 0x0084,
> +	[RXNLCR0]	= 0x0088,
> +	[RXALCR0]	= 0x008C,

    Well, the above counter register subgroup stands out from the TSU_* 
registers in the Gigabit mapping, not sure if we should follow that. These 
registers are not currently used anyway...

> +	[TSU_ADRH0]	= 0x0100,
> +	[TSU_ADRL0]	= 0x0104,
> +	[TSU_ADRH31]	= 0x01f8,
> +	[TSU_ADRL31]	= 0x01fc,
> +};
> +
>   static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[ECMR]		= 0x0100,
>   	[RFLR]		= 0x0108,
> @@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
>   		return false;
>   }
>
> +static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> +{
> +	if (mdp->reg_offset == sh_eth_offset_fast_rz)
> +		return true;
> +	else
> +		return false;

    Perhaps you should compress the above functions to one-liners as Joe has 
suggested. Or I/you could do it in a separate patch...

> +}
> +
>   static void sh_eth_select_mii(struct net_device *ndev)
>   {
>   	u32 value = 0x0;
[...]
> @@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
>
>   		/* In case of almost all GETHER/ETHERs, the Receive Frame State
>   		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> -		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> -		 * bits are from bit 25 to bit 16. So, the driver needs right
> -		 * shifting by 16.
> +		 * bit 0. However, in case of the R8A7740, R8A779x and

    Small nit: comma needed before "and" as far as I know English grammar.

> +		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> +		 * driver needs right shifting by 16.
>   		 */
>   		if (mdp->cd->shift_rd0)
>   			desc_status >>= 16;

    Other than that, this looks fine now, you can add my:

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

WBR, Sergei

Re: [PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Simon Horman]

On Thu, Jan 16, 2014 at 01:26:11AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/15/2014 09:12 AM, Simon Horman wrote:
> 
> >This is a fast ethernet controller.
> 
>    I have to say it's not exact enough patch description: R7S72100
> is not Ethernet controller itself, it's a SoC containing the
> Ethernet controller.

I will update the text to the following:

The r7s72100 SoC includes a fast ethernet controller.

> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >---
> >v5
> >* As suggested by Sergei Shtylyov
> >   - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
> >     RMII registers. Instead use sh_eth_chip_reset.
> >   - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
> >   - Do not use reserved LCHNG bit of ECSR
> >   - Do not use reserved LCHNGIP bit of ECSIPR
> >   - Document that R8A779x also needs a 16 bit shift of the RFS bits
> >   - Do not document that the R7S72100 has GECMR, it does not
> 
>   The above change list was moved from v2 section and doesn't match
> the real changes done in v5. ;-)

I'll fix that up so that when its automatically discarded by git
it can rest in peace in /dev/null.

> 
> >v4
> >* As requested by David Miller
> >   - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
> >   - Correct coding style in sh_eth_get_stats()
> 
> >v3
> >* No change
> 
> >v2
> >* As suggested by Magnus Damm and Sergei Shtylyov
> >   - r7s72100 ethernet is not gigabit so do not refer to it as such
> 
> >* As suggested by Magnus Damm
> >   - As RZ specific register layout rather than using the gigabit layout
> >     which includes registers that do not exist on this chip.
> >---
> >  drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
> >  drivers/net/ethernet/renesas/sh_eth.h |   3 +-
> >  2 files changed, 121 insertions(+), 8 deletions(-)
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 4f5cfad..a7a0555 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TRIMD]		= 0x027c,
> >  };
> >
> >+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
> 
>    Shouldn't this map precede R-Car one since this SoC is newer the
> same way you've reordered *enum* values, etc.? Sorry for not
> noticing in the previous review...

I will move it, thanks.

> [...]
> >+	[ARSTR]		= 0x0000,
> >+	[TSU_CTRST]	= 0x0004,
> >+	[TSU_VTAG0]	= 0x0058,
> >+	[TSU_ADSBSY]	= 0x0060,
> >+	[TSU_TEN]	= 0x0064,
> >+	[TXNLCR0]	= 0x0080,
> >+	[TXALCR0]	= 0x0084,
> >+	[RXNLCR0]	= 0x0088,
> >+	[RXALCR0]	= 0x008C,
> 
>    Well, the above counter register subgroup stands out from the
> TSU_* registers in the Gigabit mapping, not sure if we should follow
> that. These registers are not currently used anyway...

I will add a blank line.

> 
> >+	[TSU_ADRH0]	= 0x0100,
> >+	[TSU_ADRL0]	= 0x0104,
> >+	[TSU_ADRH31]	= 0x01f8,
> >+	[TSU_ADRL31]	= 0x01fc,
> >+};
> >+
> >  static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[ECMR]		= 0x0100,
> >  	[RFLR]		= 0x0108,
> >@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  		return false;
> >  }
> >
> >+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> >+{
> >+	if (mdp->reg_offset = sh_eth_offset_fast_rz)
> >+		return true;
> >+	else
> >+		return false;
> 
>    Perhaps you should compress the above functions to one-liners as
> Joe has suggested. Or I/you could do it in a separate patch...

Will do, Joe's suggestion is a good one.

> 
> >+}
> >+
> >  static void sh_eth_select_mii(struct net_device *ndev)
> >  {
> >  	u32 value = 0x0;
> [...]
> >@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> >
> >  		/* In case of almost all GETHER/ETHERs, the Receive Frame State
> >  		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> >-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> >-		 * bits are from bit 25 to bit 16. So, the driver needs right
> >-		 * shifting by 16.
> >+		 * bit 0. However, in case of the R8A7740, R8A779x and
> 
>    Small nit: comma needed before "and" as far as I know English grammar.

To be honest I don't think it is. But I'll add one for you.

> 
> >+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> >+		 * driver needs right shifting by 16.
> >  		 */
> >  		if (mdp->cd->shift_rd0)
> >  			desc_status >>= 16;
> 
>    Other than that, this looks fine now, you can add my:
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks.

Re: [PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Simon Horman]

On Thu, Jan 16, 2014 at 01:26:11AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/15/2014 09:12 AM, Simon Horman wrote:
> 
> >This is a fast ethernet controller.
> 
>    I have to say it's not exact enough patch description: R7S72100
> is not Ethernet controller itself, it's a SoC containing the
> Ethernet controller.

I will update the text to the following:

The r7s72100 SoC includes a fast ethernet controller.

> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >---
> >v5
> >* As suggested by Sergei Shtylyov
> >   - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
> >     RMII registers. Instead use sh_eth_chip_reset.
> >   - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
> >   - Do not use reserved LCHNG bit of ECSR
> >   - Do not use reserved LCHNGIP bit of ECSIPR
> >   - Document that R8A779x also needs a 16 bit shift of the RFS bits
> >   - Do not document that the R7S72100 has GECMR, it does not
> 
>   The above change list was moved from v2 section and doesn't match
> the real changes done in v5. ;-)

I'll fix that up so that when its automatically discarded by git
it can rest in peace in /dev/null.

> 
> >v4
> >* As requested by David Miller
> >   - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
> >   - Correct coding style in sh_eth_get_stats()
> 
> >v3
> >* No change
> 
> >v2
> >* As suggested by Magnus Damm and Sergei Shtylyov
> >   - r7s72100 ethernet is not gigabit so do not refer to it as such
> 
> >* As suggested by Magnus Damm
> >   - As RZ specific register layout rather than using the gigabit layout
> >     which includes registers that do not exist on this chip.
> >---
> >  drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
> >  drivers/net/ethernet/renesas/sh_eth.h |   3 +-
> >  2 files changed, 121 insertions(+), 8 deletions(-)
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 4f5cfad..a7a0555 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TRIMD]		= 0x027c,
> >  };
> >
> >+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
> 
>    Shouldn't this map precede R-Car one since this SoC is newer the
> same way you've reordered *enum* values, etc.? Sorry for not
> noticing in the previous review...

I will move it, thanks.

> [...]
> >+	[ARSTR]		= 0x0000,
> >+	[TSU_CTRST]	= 0x0004,
> >+	[TSU_VTAG0]	= 0x0058,
> >+	[TSU_ADSBSY]	= 0x0060,
> >+	[TSU_TEN]	= 0x0064,
> >+	[TXNLCR0]	= 0x0080,
> >+	[TXALCR0]	= 0x0084,
> >+	[RXNLCR0]	= 0x0088,
> >+	[RXALCR0]	= 0x008C,
> 
>    Well, the above counter register subgroup stands out from the
> TSU_* registers in the Gigabit mapping, not sure if we should follow
> that. These registers are not currently used anyway...

I will add a blank line.

> 
> >+	[TSU_ADRH0]	= 0x0100,
> >+	[TSU_ADRL0]	= 0x0104,
> >+	[TSU_ADRH31]	= 0x01f8,
> >+	[TSU_ADRL31]	= 0x01fc,
> >+};
> >+
> >  static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[ECMR]		= 0x0100,
> >  	[RFLR]		= 0x0108,
> >@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  		return false;
> >  }
> >
> >+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> >+{
> >+	if (mdp->reg_offset == sh_eth_offset_fast_rz)
> >+		return true;
> >+	else
> >+		return false;
> 
>    Perhaps you should compress the above functions to one-liners as
> Joe has suggested. Or I/you could do it in a separate patch...

Will do, Joe's suggestion is a good one.

> 
> >+}
> >+
> >  static void sh_eth_select_mii(struct net_device *ndev)
> >  {
> >  	u32 value = 0x0;
> [...]
> >@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> >
> >  		/* In case of almost all GETHER/ETHERs, the Receive Frame State
> >  		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> >-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> >-		 * bits are from bit 25 to bit 16. So, the driver needs right
> >-		 * shifting by 16.
> >+		 * bit 0. However, in case of the R8A7740, R8A779x and
> 
>    Small nit: comma needed before "and" as far as I know English grammar.

To be honest I don't think it is. But I'll add one for you.

> 
> >+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> >+		 * driver needs right shifting by 16.
> >  		 */
> >  		if (mdp->cd->shift_rd0)
> >  			desc_status >>= 16;
> 
>    Other than that, this looks fine now, you can add my:
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks.

[PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100

[Simon Horman]

On Thu, Jan 16, 2014 at 01:26:11AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/15/2014 09:12 AM, Simon Horman wrote:
> 
> >This is a fast ethernet controller.
> 
>    I have to say it's not exact enough patch description: R7S72100
> is not Ethernet controller itself, it's a SoC containing the
> Ethernet controller.

I will update the text to the following:

The r7s72100 SoC includes a fast ethernet controller.

> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >---
> >v5
> >* As suggested by Sergei Shtylyov
> >   - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
> >     RMII registers. Instead use sh_eth_chip_reset.
> >   - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
> >   - Do not use reserved LCHNG bit of ECSR
> >   - Do not use reserved LCHNGIP bit of ECSIPR
> >   - Document that R8A779x also needs a 16 bit shift of the RFS bits
> >   - Do not document that the R7S72100 has GECMR, it does not
> 
>   The above change list was moved from v2 section and doesn't match
> the real changes done in v5. ;-)

I'll fix that up so that when its automatically discarded by git
it can rest in peace in /dev/null.

> 
> >v4
> >* As requested by David Miller
> >   - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
> >   - Correct coding style in sh_eth_get_stats()
> 
> >v3
> >* No change
> 
> >v2
> >* As suggested by Magnus Damm and Sergei Shtylyov
> >   - r7s72100 ethernet is not gigabit so do not refer to it as such
> 
> >* As suggested by Magnus Damm
> >   - As RZ specific register layout rather than using the gigabit layout
> >     which includes registers that do not exist on this chip.
> >---
> >  drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
> >  drivers/net/ethernet/renesas/sh_eth.h |   3 +-
> >  2 files changed, 121 insertions(+), 8 deletions(-)
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 4f5cfad..a7a0555 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TRIMD]		= 0x027c,
> >  };
> >
> >+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
> 
>    Shouldn't this map precede R-Car one since this SoC is newer the
> same way you've reordered *enum* values, etc.? Sorry for not
> noticing in the previous review...

I will move it, thanks.

> [...]
> >+	[ARSTR]		= 0x0000,
> >+	[TSU_CTRST]	= 0x0004,
> >+	[TSU_VTAG0]	= 0x0058,
> >+	[TSU_ADSBSY]	= 0x0060,
> >+	[TSU_TEN]	= 0x0064,
> >+	[TXNLCR0]	= 0x0080,
> >+	[TXALCR0]	= 0x0084,
> >+	[RXNLCR0]	= 0x0088,
> >+	[RXALCR0]	= 0x008C,
> 
>    Well, the above counter register subgroup stands out from the
> TSU_* registers in the Gigabit mapping, not sure if we should follow
> that. These registers are not currently used anyway...

I will add a blank line.

> 
> >+	[TSU_ADRH0]	= 0x0100,
> >+	[TSU_ADRL0]	= 0x0104,
> >+	[TSU_ADRH31]	= 0x01f8,
> >+	[TSU_ADRL31]	= 0x01fc,
> >+};
> >+
> >  static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[ECMR]		= 0x0100,
> >  	[RFLR]		= 0x0108,
> >@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  		return false;
> >  }
> >
> >+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> >+{
> >+	if (mdp->reg_offset == sh_eth_offset_fast_rz)
> >+		return true;
> >+	else
> >+		return false;
> 
>    Perhaps you should compress the above functions to one-liners as
> Joe has suggested. Or I/you could do it in a separate patch...

Will do, Joe's suggestion is a good one.

> 
> >+}
> >+
> >  static void sh_eth_select_mii(struct net_device *ndev)
> >  {
> >  	u32 value = 0x0;
> [...]
> >@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> >
> >  		/* In case of almost all GETHER/ETHERs, the Receive Frame State
> >  		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> >-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> >-		 * bits are from bit 25 to bit 16. So, the driver needs right
> >-		 * shifting by 16.
> >+		 * bit 0. However, in case of the R8A7740, R8A779x and
> 
>    Small nit: comma needed before "and" as far as I know English grammar.

To be honest I don't think it is. But I'll add one for you.

> 
> >+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> >+		 * driver needs right shifting by 16.
> >  		 */
> >  		if (mdp->cd->shift_rd0)
> >  			desc_status >>= 16;
> 
>    Other than that, this looks fine now, you can add my:
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks.

Re: [PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Simon Horman]

On Wed, Jan 15, 2014 at 01:35:20AM -0800, Joe Perches wrote:
> On Wed, 2014-01-15 at 15:12 +0900, Simon Horman wrote:
> > Return a boolean and use true and false.
> []
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> []
> > @@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TSU_ADRL31]	= 0x01fc,
> >  };
> >  
> > -static int sh_eth_is_gether(struct sh_eth_private *mdp)
> > +static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  {
> >  	if (mdp->reg_offset = sh_eth_offset_gigabit)
> > -		return 1;
> > +		return true;
> >  	else
> > -		return 0;
> > +		return false;
> >  }
> 
> Or maybe:
> 
> static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> {
> 	return mdp->reg_offset = sh_eth_offset_gigabit;
> }

Thanks, will do.

Re: [PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Simon Horman]

On Wed, Jan 15, 2014 at 01:35:20AM -0800, Joe Perches wrote:
> On Wed, 2014-01-15 at 15:12 +0900, Simon Horman wrote:
> > Return a boolean and use true and false.
> []
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> []
> > @@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TSU_ADRL31]	= 0x01fc,
> >  };
> >  
> > -static int sh_eth_is_gether(struct sh_eth_private *mdp)
> > +static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  {
> >  	if (mdp->reg_offset == sh_eth_offset_gigabit)
> > -		return 1;
> > +		return true;
> >  	else
> > -		return 0;
> > +		return false;
> >  }
> 
> Or maybe:
> 
> static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> {
> 	return mdp->reg_offset == sh_eth_offset_gigabit;
> }

Thanks, will do.

[PATCH v5 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()

[Simon Horman]

On Wed, Jan 15, 2014 at 01:35:20AM -0800, Joe Perches wrote:
> On Wed, 2014-01-15 at 15:12 +0900, Simon Horman wrote:
> > Return a boolean and use true and false.
> []
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> []
> > @@ -310,12 +310,12 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TSU_ADRL31]	= 0x01fc,
> >  };
> >  
> > -static int sh_eth_is_gether(struct sh_eth_private *mdp)
> > +static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  {
> >  	if (mdp->reg_offset == sh_eth_offset_gigabit)
> > -		return 1;
> > +		return true;
> >  	else
> > -		return 0;
> > +		return false;
> >  }
> 
> Or maybe:
> 
> static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> {
> 	return mdp->reg_offset == sh_eth_offset_gigabit;
> }

Thanks, will do.