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

[PATCH v4 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-rc7-20140107 tag in my renesas tree
* net-next
  - Head revision: 80077935cad223b29
    ("Merge branch 'master' of
      git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next")

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

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   | 124 +++++++++++++++++++++++++++++---
 drivers/net/ethernet/renesas/sh_eth.h   |   4 +-
 4 files changed, 143 insertions(+), 10 deletions(-)

-- 
1.8.4

[PATCH v4 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>

---

Dave,

please consider this for net-next.

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 8884107..4b38533 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 v4 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>

---

Dave,

please consider this for net-next.

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.

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
---
 drivers/net/ethernet/renesas/sh_eth.c | 118 ++++++++++++++++++++++++++++++++--
 drivers/net/ethernet/renesas/sh_eth.h |   4 +-
 2 files changed, 115 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 4b38533..cc6d4af 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -190,6 +190,59 @@ 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,
+	[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,
+	[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 +371,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 +762,35 @@ 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,
+
+	.apr		= 1,
+	.mpr		= 1,
+	.tpauser	= 1,
+	.hw_swap	= 1,
+	.rpadir		= 1,
+	.rpadir_value   = 2 << 16,
+	.no_trimd	= 1,
+	.tsu		= 1,
+	.shift_rd0	= 1,
+};
+
 static struct sh_eth_cpu_data sh7619_data = {
 	.register_type	= SH_ETH_REG_FAST_SH3_SH2,
 
@@ -767,7 +857,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);
@@ -880,6 +970,8 @@ static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
 {
 	if (sh_eth_is_gether(mdp))
 		return EDTRR_TRNS_GETHER;
+	else if (sh_eth_is_rz_fast_ether(mdp))
+		return EDTRR_TRNS_RZ_ETHER;
 	else
 		return EDTRR_TRNS_ETHER;
 }
@@ -1041,7 +1133,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 +1155,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 +1403,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 +2155,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 +2539,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 */
@@ -2564,6 +2666,9 @@ static const u16 *sh_eth_get_register_offset(int register_type)
 	case SH_ETH_REG_FAST_RCAR:
 		reg_offset = sh_eth_offset_fast_rcar;
 		break;
+	case SH_ETH_REG_FAST_RZ:
+		reg_offset = sh_eth_offset_fast_rz;
+		break;
 	case SH_ETH_REG_FAST_SH4:
 		reg_offset = sh_eth_offset_fast_sh4;
 		break;
@@ -2799,6 +2904,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..0bcde90 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -156,6 +156,7 @@ enum {
 enum {
 	SH_ETH_REG_GIGABIT,
 	SH_ETH_REG_FAST_RCAR,
+	SH_ETH_REG_FAST_RZ,
 	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,
 };
@@ -191,6 +192,7 @@ enum DMAC_M_BIT {
 /* EDTRR */
 enum DMAC_T_BIT {
 	EDTRR_TRNS_GETHER = 0x03,
+	EDTRR_TRNS_RZ_ETHER = 0x03,
 	EDTRR_TRNS_ETHER = 0x01,
 };
 
-- 
1.8.4

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

[Sergei Shtylyov]

Hello.

On 01/08/2014 11:02 AM, Simon Horman wrote:

> This is a fast ethernet controller.

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

> ---

> Dave,

> please consider this for net-next.

    Dave, I'm currently reviewing this patch and there are errors still, so 
please don't apply it yet.

WBR, Sergei

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

[Sergei Shtylyov]

On 01/08/2014 11:02 AM, Simon Horman wrote:

> This is a fast ethernet controller.

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

[...]

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 4b38533..cc6d4af 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -190,6 +190,59 @@ 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] = {
[...]
> +	[ECMR]		= 0x0500,
> +	[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,

    You've missed RFLR @ 0x0508. It's a vital register which the driver 
requires to be always mapped.

> +
> +	[ARSTR]		= 0x0000,
> +	[TSU_CTRST]	= 0x0004,
> +	[TSU_VTAG0]	= 0x0058,
> +	[TSU_ADSBSY]	= 0x0060,
> +	[TSU_TEN]	= 0x0064,
> +	[TSU_ADRH0]	= 0x0100,
> +	[TSU_ADRL0]	= 0x0104,
> +	[TSU_ADRH31]	= 0x01f8,
> +	[TSU_ADRL31]	= 0x01fc,

    Looking at the manual, you've missed [TR]X[NA]LCR regs starting at offset 
0x0080 from TSU block.

    I see that both E-MAC and TSU blocks turned out to be different from the 
Gigabit version upon further scrutiny...

> +};
> +
>   static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[ECMR]		= 0x0100,
>   	[RFLR]		= 0x0108,
[...]
> @@ -701,6 +762,35 @@ 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,
> +
> +	.apr		= 1,
> +	.mpr		= 1,
> +	.tpauser	= 1,
> +	.hw_swap	= 1,
> +	.rpadir		= 1,
> +	.rpadir_value   = 2 << 16,
> +	.no_trimd	= 1,
> +	.tsu		= 1,
> +	.shift_rd0	= 1,

    Perhaps this field should be renamed to something talking about check 
summing support (since bits 0..15 of RD0 contain a frame check sum for those 
SoCs). Or maybe it should be just merged with the 'hw_crc' field...

    Well, now the comments about your initializer: you've missed to set the 
'no_psr' field -- this SoC doesn't have PSR (which usually holds the LINK 
signal status). It's not fatal since you're setting 'no_ether_link' in the 
platform data but should be fixed anyway. You've also missed to set 'no_ade' 
field, though 'eesipr_value' correctly has EESIPR.ADEIP cleared. And it looks 
like you've also missed to set 'hw_crc' field since this SoC has CSMR...

[...]
> @@ -880,6 +970,8 @@ static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
>   {
>   	if (sh_eth_is_gether(mdp))
>   		return EDTRR_TRNS_GETHER;
> +	else if (sh_eth_is_rz_fast_ether(mdp))
> +		return EDTRR_TRNS_RZ_ETHER;

    I'd just merge this with the GEther case.

>   	else
>   		return EDTRR_TRNS_ETHER;
>   }
[...]
> @@ -1062,7 +1155,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);

    Hmm, TDFAR exists also on SH4 Ethers...

[...]
> @@ -2564,6 +2666,9 @@ static const u16 *sh_eth_get_register_offset(int register_type)
>   	case SH_ETH_REG_FAST_RCAR:
>   		reg_offset = sh_eth_offset_fast_rcar;
>   		break;
> +	case SH_ETH_REG_FAST_RZ:
> +		reg_offset = sh_eth_offset_fast_rz;
> +		break;

    I think it should precede the R-Car case as this chip is newer than R-Car 
and the SoC families appear here in the reverse order.

>   	case SH_ETH_REG_FAST_SH4:
>   		reg_offset = sh_eth_offset_fast_sh4;
>   		break;
[...]
> diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> index 0fe35b7..0bcde90 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.h
> +++ b/drivers/net/ethernet/renesas/sh_eth.h
> @@ -156,6 +156,7 @@ enum {
>   enum {
>   	SH_ETH_REG_GIGABIT,
>   	SH_ETH_REG_FAST_RCAR,
> +	SH_ETH_REG_FAST_RZ,

    I think it should precede the R-Car value.

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

    Need comma before "and". Sorry for the grammar nitpicking. :-)

>   enum EDSR_BIT {
>   	EDSR_ENT = 0x01, EDSR_ENR = 0x02,
>   };
> @@ -191,6 +192,7 @@ enum DMAC_M_BIT {
>   /* EDTRR */
>   enum DMAC_T_BIT {
>   	EDTRR_TRNS_GETHER = 0x03,
> +	EDTRR_TRNS_RZ_ETHER = 0x03,

    I doubt we need a special case here. You didn't introduce one for the 
software reset bits.

>   	EDTRR_TRNS_ETHER = 0x01,
>   };

WBR, Sergei

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

[Simon Horman]

On Wed, Jan 08, 2014 at 11:58:09PM +0300, Sergei Shtylyov wrote:
> On 01/08/2014 11:02 AM, Simon Horman wrote:
> 
> >This is a fast ethernet controller.
> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> [...]
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 4b38533..cc6d4af 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,59 @@ 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] = {
> [...]
> >+	[ECMR]		= 0x0500,
> >+	[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,
> 
>    You've missed RFLR @ 0x0508. It's a vital register which the
> driver requires to be always mapped.

Thanks, I will fix that.

> >+
> >+	[ARSTR]		= 0x0000,
> >+	[TSU_CTRST]	= 0x0004,
> >+	[TSU_VTAG0]	= 0x0058,
> >+	[TSU_ADSBSY]	= 0x0060,
> >+	[TSU_TEN]	= 0x0064,
> >+	[TSU_ADRH0]	= 0x0100,
> >+	[TSU_ADRL0]	= 0x0104,
> >+	[TSU_ADRH31]	= 0x01f8,
> >+	[TSU_ADRL31]	= 0x01fc,
> 
>    Looking at the manual, you've missed [TR]X[NA]LCR regs starting
> at offset 0x0080 from TSU block.

Thanks, I will add them.

>    I see that both E-MAC and TSU blocks turned out to be different
> from the Gigabit version upon further scrutiny...
> 
> >+};
> >+
> >  static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[ECMR]		= 0x0100,
> >  	[RFLR]		= 0x0108,
> [...]
> >@@ -701,6 +762,35 @@ 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,
> >+
> >+	.apr		= 1,
> >+	.mpr		= 1,
> >+	.tpauser	= 1,
> >+	.hw_swap	= 1,
> >+	.rpadir		= 1,
> >+	.rpadir_value   = 2 << 16,
> >+	.no_trimd	= 1,
> >+	.tsu		= 1,
> >+	.shift_rd0	= 1,
> 
>    Perhaps this field should be renamed to something talking about
> check summing support (since bits 0..15 of RD0 contain a frame check
> sum for those SoCs). Or maybe it should be just merged with the
> 'hw_crc' field...

I have no feelings about that one way or another.
But it seems to be orthogonal to this patch.

>    Well, now the comments about your initializer: you've missed to
> set the 'no_psr' field -- this SoC doesn't have PSR (which usually
> holds the LINK signal status). It's not fatal since you're setting
> 'no_ether_link' in the platform data but should be fixed anyway.
> You've also missed to set 'no_ade' field, though 'eesipr_value'
> correctly has EESIPR.ADEIP cleared. And it looks like you've also
> missed to set 'hw_crc' field since this SoC has CSMR...

Thanks, I will add those fields.

> [...]
> >@@ -880,6 +970,8 @@ static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
> >  {
> >  	if (sh_eth_is_gether(mdp))
> >  		return EDTRR_TRNS_GETHER;
> >+	else if (sh_eth_is_rz_fast_ether(mdp))
> >+		return EDTRR_TRNS_RZ_ETHER;
> 
>    I'd just merge this with the GEther case.

Sure, but in that case should we change the name.
As both you and Magnus pointed out to me, the rz is not gigabit.

> 
> >  	else
> >  		return EDTRR_TRNS_ETHER;
> >  }
> [...]
> >@@ -1062,7 +1155,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);
> 
>    Hmm, TDFAR exists also on SH4 Ethers...

Lets fix that separately.

> 
> [...]
> >@@ -2564,6 +2666,9 @@ static const u16 *sh_eth_get_register_offset(int register_type)
> >  	case SH_ETH_REG_FAST_RCAR:
> >  		reg_offset = sh_eth_offset_fast_rcar;
> >  		break;
> >+	case SH_ETH_REG_FAST_RZ:
> >+		reg_offset = sh_eth_offset_fast_rz;
> >+		break;
> 
>    I think it should precede the R-Car case as this chip is newer
> than R-Car and the SoC families appear here in the reverse order.

Sure, will do.

> >  	case SH_ETH_REG_FAST_SH4:
> >  		reg_offset = sh_eth_offset_fast_sh4;
> >  		break;
> [...]
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> >index 0fe35b7..0bcde90 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.h
> >+++ b/drivers/net/ethernet/renesas/sh_eth.h
> >@@ -156,6 +156,7 @@ enum {
> >  enum {
> >  	SH_ETH_REG_GIGABIT,
> >  	SH_ETH_REG_FAST_RCAR,
> >+	SH_ETH_REG_FAST_RZ,
> 
>    I think it should precede the R-Car value.

Sure, will do.

> >  	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 */
> 
>    Need comma before "and". Sorry for the grammar nitpicking. :-)

Will do.

> >  enum EDSR_BIT {
> >  	EDSR_ENT = 0x01, EDSR_ENR = 0x02,
> >  };
> >@@ -191,6 +192,7 @@ enum DMAC_M_BIT {
> >  /* EDTRR */
> >  enum DMAC_T_BIT {
> >  	EDTRR_TRNS_GETHER = 0x03,
> >+	EDTRR_TRNS_RZ_ETHER = 0x03,
> 
>    I doubt we need a special case here. You didn't introduce one for
> the software reset bits.

True, but RZ is not Gigabit. So I think we either need two names
or to choose a more generic name.


> 
> >  	EDTRR_TRNS_ETHER = 0x01,
> >  };
> 
> WBR, Sergei
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Sergei Shtylyov]

Hello.

On 01/09/2014 08:03 AM, Simon Horman wrote:

>>> This is a fast ethernet controller.

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

>> [...]

>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
>>> index 4b38533..cc6d4af 100644
>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>>> @@ -190,6 +190,59 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
[...]
>>> @@ -701,6 +762,35 @@ 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,
>>> +
>>> +	.apr		= 1,
>>> +	.mpr		= 1,
>>> +	.tpauser	= 1,
>>> +	.hw_swap	= 1,
>>> +	.rpadir		= 1,
>>> +	.rpadir_value   = 2 << 16,
>>> +	.no_trimd	= 1,
>>> +	.tsu		= 1,
>>> +	.shift_rd0	= 1,

>>     Perhaps this field should be renamed to something talking about
>> check summing support (since bits 0..15 of RD0 contain a frame check
>> sum for those SoCs). Or maybe it should be just merged with the
>> 'hw_crc' field...

> I have no feelings about that one way or another.

    Do you happen to have R8A7740 manual by chance? If so, does it talk about 
RX check summing support and using RD0 for that?

> But it seems to be orthogonal to this patch.

    Of course, was a note to self. :-)

[...]
>>> @@ -880,6 +970,8 @@ static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
>>>   {
>>>   	if (sh_eth_is_gether(mdp))
>>>   		return EDTRR_TRNS_GETHER;
>>> +	else if (sh_eth_is_rz_fast_ether(mdp))
>>> +		return EDTRR_TRNS_RZ_ETHER;

>>     I'd just merge this with the GEther case.

> Sure, but in that case should we change the name.
> As both you and Magnus pointed out to me, the rz is not gigabit.

    See below.

>>>   	else
>>>   		return EDTRR_TRNS_ETHER;
>>>   }
>> [...]
>>> @@ -1062,7 +1155,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);

>>     Hmm, TDFAR exists also on SH4 Ethers...

> Lets fix that separately.

    Of course, was just another not to self.

[...]
>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
>>> index 0fe35b7..0bcde90 100644
>>> --- a/drivers/net/ethernet/renesas/sh_eth.h
>>> +++ b/drivers/net/ethernet/renesas/sh_eth.h
[...]
>>> @@ -191,6 +192,7 @@ enum DMAC_M_BIT {
>>>   /* EDTRR */
>>>   enum DMAC_T_BIT {
>>>   	EDTRR_TRNS_GETHER = 0x03,
>>> +	EDTRR_TRNS_RZ_ETHER = 0x03,

>>     I doubt we need a special case here. You didn't introduce one for
>> the software reset bits.

> True, but RZ is not Gigabit. So I think we either need two names
> or to choose a more generic name.

    Well, R7S72100 manual calls these bits just TR[1:0]. Don't know what SoCs 
having Gigabit call it in the manuals...

>>>   	EDTRR_TRNS_ETHER = 0x01,

    R-Car manuals seem to call the bit TRNS (as well as the prehistoric SH 
manuals probably). Perhaps we could use that difference, TRNS vs TR, don't know...

>>>   };

WBR, Sergei

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

[Simon Horman]

On Thu, Jan 16, 2014 at 01:43:30AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/09/2014 08:03 AM, Simon Horman wrote:
> 
> >>>This is a fast ethernet controller.
> 
> >>>Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >>[...]
> 
> >>>diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >>>index 4b38533..cc6d4af 100644
> >>>--- a/drivers/net/ethernet/renesas/sh_eth.c
> >>>+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >>>@@ -190,6 +190,59 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> [...]
> >>>@@ -701,6 +762,35 @@ 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,
> >>>+
> >>>+	.apr		= 1,
> >>>+	.mpr		= 1,
> >>>+	.tpauser	= 1,
> >>>+	.hw_swap	= 1,
> >>>+	.rpadir		= 1,
> >>>+	.rpadir_value   = 2 << 16,
> >>>+	.no_trimd	= 1,
> >>>+	.tsu		= 1,
> >>>+	.shift_rd0	= 1,
> 
> >>    Perhaps this field should be renamed to something talking about
> >>check summing support (since bits 0..15 of RD0 contain a frame check
> >>sum for those SoCs). Or maybe it should be just merged with the
> >>'hw_crc' field...
> 
> >I have no feelings about that one way or another.
> 
>    Do you happen to have R8A7740 manual by chance? If so, does it
> talk about RX check summing support and using RD0 for that?

Yes and yes.

I have taken a quick look and the documentation for RX checksumming on the
R8A7740 appears to be very similar if not the same as that of the R7S72100.

In particular both refer to using the bottom 16 bits of RD0 as
containing the packet checksum.

> >But it seems to be orthogonal to this patch.
> 
>    Of course, was a note to self. :-)
> 
> [...]
> >>>@@ -880,6 +970,8 @@ static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
> >>>  {
> >>>  	if (sh_eth_is_gether(mdp))
> >>>  		return EDTRR_TRNS_GETHER;
> >>>+	else if (sh_eth_is_rz_fast_ether(mdp))
> >>>+		return EDTRR_TRNS_RZ_ETHER;
> 
> >>    I'd just merge this with the GEther case.
> 
> >Sure, but in that case should we change the name.
> >As both you and Magnus pointed out to me, the rz is not gigabit.
> 
>    See below.
> 
> >>>  	else
> >>>  		return EDTRR_TRNS_ETHER;
> >>>  }
> >>[...]
> >>>@@ -1062,7 +1155,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);
> 
> >>    Hmm, TDFAR exists also on SH4 Ethers...
> 
> >Lets fix that separately.
> 
>    Of course, was just another not to self.
> 
> [...]
> >>>diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> >>>index 0fe35b7..0bcde90 100644
> >>>--- a/drivers/net/ethernet/renesas/sh_eth.h
> >>>+++ b/drivers/net/ethernet/renesas/sh_eth.h
> [...]
> >>>@@ -191,6 +192,7 @@ enum DMAC_M_BIT {
> >>>  /* EDTRR */
> >>>  enum DMAC_T_BIT {
> >>>  	EDTRR_TRNS_GETHER = 0x03,
> >>>+	EDTRR_TRNS_RZ_ETHER = 0x03,
> 
> >>    I doubt we need a special case here. You didn't introduce one for
> >>the software reset bits.
> 
> >True, but RZ is not Gigabit. So I think we either need two names
> >or to choose a more generic name.
> 
>    Well, R7S72100 manual calls these bits just TR[1:0]. Don't know
> what SoCs having Gigabit call it in the manuals...
> 
> >>>  	EDTRR_TRNS_ETHER = 0x01,
> 
>    R-Car manuals seem to call the bit TRNS (as well as the
> prehistoric SH manuals probably). Perhaps we could use that
> difference, TRNS vs TR, don't know...

Perhaps we should just leave it as-is, using EDTRR_TRNS_GETHER and
EDTRR_TRNS_RZ_ETHER, after all.

At least until we can think of a better names :)

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

[Sergei Shtylyov]

Hello.

On 16-01-2014 4:49, Simon Horman wrote:

>>>>> This is a fast ethernet controller.

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

>>>> [...]

>>>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
>>>>> index 4b38533..cc6d4af 100644
>>>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>>>>> @@ -190,6 +190,59 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
>> [...]
>>>>> @@ -701,6 +762,35 @@ 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,
>>>>> +
>>>>> +	.apr		= 1,
>>>>> +	.mpr		= 1,
>>>>> +	.tpauser	= 1,
>>>>> +	.hw_swap	= 1,
>>>>> +	.rpadir		= 1,
>>>>> +	.rpadir_value   = 2 << 16,
>>>>> +	.no_trimd	= 1,
>>>>> +	.tsu		= 1,
>>>>> +	.shift_rd0	= 1,
>>
>>>>     Perhaps this field should be renamed to something talking about
>>>> check summing support (since bits 0..15 of RD0 contain a frame check
>>>> sum for those SoCs). Or maybe it should be just merged with the
>>>> 'hw_crc' field...

>>> I have no feelings about that one way or another.

>>     Do you happen to have R8A7740 manual by chance? If so, does it
>> talk about RX check summing support and using RD0 for that?

> Yes and yes.

> I have taken a quick look and the documentation for RX checksumming on the
> R8A7740 appears to be very similar if not the same as that of the R7S72100.

> In particular both refer to using the bottom 16 bits of RD0 as
> containing the packet checksum.

    OK, now if you had SH7734 manual to completely confirm that check sum is 
stored in the same place there... most probably it is, of course, and we 
should merge 'hw_crc' and 'shift_rd0' into a single field.

[...]
>>>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
>>>>> index 0fe35b7..0bcde90 100644
>>>>> --- a/drivers/net/ethernet/renesas/sh_eth.h
>>>>> +++ b/drivers/net/ethernet/renesas/sh_eth.h
>> [...]
>>>>> @@ -191,6 +192,7 @@ enum DMAC_M_BIT {
>>>>>   /* EDTRR */
>>>>>   enum DMAC_T_BIT {
>>>>>   	EDTRR_TRNS_GETHER = 0x03,
>>>>> +	EDTRR_TRNS_RZ_ETHER = 0x03,

>>>>     I doubt we need a special case here. You didn't introduce one for
>>>> the software reset bits.

>>> True, but RZ is not Gigabit. So I think we either need two names
>>> or to choose a more generic name.

>>     Well, R7S72100 manual calls these bits just TR[1:0]. Don't know
>> what SoCs having Gigabit call it in the manuals...

>>>>>   	EDTRR_TRNS_ETHER = 0x01,

>>     R-Car manuals seem to call the bit TRNS (as well as the
>> prehistoric SH manuals probably). Perhaps we could use that
>> difference, TRNS vs TR, don't know...

> Perhaps we should just leave it as-is, using EDTRR_TRNS_GETHER and
> EDTRR_TRNS_RZ_ETHER, after all.

    No, I liked your last version more. At least it's more consistent, not 
adding separate values for either TR[1:0] or soft reset bits.

> At least until we can think of a better names :)

    I doubt we can come up with something better.

WBR, Sergei

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

[Simon Horman]

On Thu, Jan 16, 2014 at 07:36:45PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 16-01-2014 4:49, Simon Horman wrote:
> 
> >>>>>This is a fast ethernet controller.
> 
> >>>>>Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >>>>[...]
> 
> >>>>>diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >>>>>index 4b38533..cc6d4af 100644
> >>>>>--- a/drivers/net/ethernet/renesas/sh_eth.c
> >>>>>+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >>>>>@@ -190,6 +190,59 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >>[...]
> >>>>>@@ -701,6 +762,35 @@ 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,
> >>>>>+
> >>>>>+	.apr		= 1,
> >>>>>+	.mpr		= 1,
> >>>>>+	.tpauser	= 1,
> >>>>>+	.hw_swap	= 1,
> >>>>>+	.rpadir		= 1,
> >>>>>+	.rpadir_value   = 2 << 16,
> >>>>>+	.no_trimd	= 1,
> >>>>>+	.tsu		= 1,
> >>>>>+	.shift_rd0	= 1,
> >>
> >>>>    Perhaps this field should be renamed to something talking about
> >>>>check summing support (since bits 0..15 of RD0 contain a frame check
> >>>>sum for those SoCs). Or maybe it should be just merged with the
> >>>>'hw_crc' field...
> 
> >>>I have no feelings about that one way or another.
> 
> >>    Do you happen to have R8A7740 manual by chance? If so, does it
> >>talk about RX check summing support and using RD0 for that?
> 
> >Yes and yes.
> 
> >I have taken a quick look and the documentation for RX checksumming on the
> >R8A7740 appears to be very similar if not the same as that of the R7S72100.
> 
> >In particular both refer to using the bottom 16 bits of RD0 as
> >containing the packet checksum.
> 
>    OK, now if you had SH7734 manual to completely confirm that check
> sum is stored in the same place there... most probably it is, of
> course, and we should merge 'hw_crc' and 'shift_rd0' into a single
> field.

Unfortunately I don't have access to that manual.

> 
> [...]
> >>>>>diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> >>>>>index 0fe35b7..0bcde90 100644
> >>>>>--- a/drivers/net/ethernet/renesas/sh_eth.h
> >>>>>+++ b/drivers/net/ethernet/renesas/sh_eth.h
> >>[...]
> >>>>>@@ -191,6 +192,7 @@ enum DMAC_M_BIT {
> >>>>>  /* EDTRR */
> >>>>>  enum DMAC_T_BIT {
> >>>>>  	EDTRR_TRNS_GETHER = 0x03,
> >>>>>+	EDTRR_TRNS_RZ_ETHER = 0x03,
> 
> >>>>    I doubt we need a special case here. You didn't introduce one for
> >>>>the software reset bits.
> 
> >>>True, but RZ is not Gigabit. So I think we either need two names
> >>>or to choose a more generic name.
> 
> >>    Well, R7S72100 manual calls these bits just TR[1:0]. Don't know
> >>what SoCs having Gigabit call it in the manuals...
> 
> >>>>>  	EDTRR_TRNS_ETHER = 0x01,
> 
> >>    R-Car manuals seem to call the bit TRNS (as well as the
> >>prehistoric SH manuals probably). Perhaps we could use that
> >>difference, TRNS vs TR, don't know...
> 
> >Perhaps we should just leave it as-is, using EDTRR_TRNS_GETHER and
> >EDTRR_TRNS_RZ_ETHER, after all.
> 
>    No, I liked your last version more. At least it's more
> consistent, not adding separate values for either TR[1:0] or soft
> reset bits.
> 
> >At least until we can think of a better names :)
> 
>    I doubt we can come up with something better.
> 
> WBR, Sergei
> 

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

[Sergei Shtylyov]

Hello.

On 17-01-2014 10:13, Simon Horman wrote:

>>>>>>> This is a fast ethernet controller.

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

>>>>>> [...]

>>>>>>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
>>>>>>> index 4b38533..cc6d4af 100644
>>>>>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>>>>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>>>>>>> @@ -190,6 +190,59 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
>>>> [...]
>>>>>>> @@ -701,6 +762,35 @@ 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,
>>>>>>> +
>>>>>>> +	.apr		= 1,
>>>>>>> +	.mpr		= 1,
>>>>>>> +	.tpauser	= 1,
>>>>>>> +	.hw_swap	= 1,
>>>>>>> +	.rpadir		= 1,
>>>>>>> +	.rpadir_value   = 2 << 16,
>>>>>>> +	.no_trimd	= 1,
>>>>>>> +	.tsu		= 1,
>>>>>>> +	.shift_rd0	= 1,

>>>>>>     Perhaps this field should be renamed to something talking about
>>>>>> check summing support (since bits 0..15 of RD0 contain a frame check
>>>>>> sum for those SoCs). Or maybe it should be just merged with the
>>>>>> 'hw_crc' field...

>>>>> I have no feelings about that one way or another.

>>>>     Do you happen to have R8A7740 manual by chance? If so, does it
>>>> talk about RX check summing support and using RD0 for that?

>>> Yes and yes.

>>> I have taken a quick look and the documentation for RX checksumming on the
>>> R8A7740 appears to be very similar if not the same as that of the R7S72100.

>>> In particular both refer to using the bottom 16 bits of RD0 as
>>> containing the packet checksum.

>>     OK, now if you had SH7734 manual to completely confirm that check
>> sum is stored in the same place there... most probably it is, of
>> course, and we should merge 'hw_crc' and 'shift_rd0' into a single
>> field.

> Unfortunately I don't have access to that manual.

    Anyway, we also need Gen2 manuals accepting the fact that checksumming is 
also supported (they also set 'shift_rd0' field) and giving the mapping of CSMR...

WBR, Sergei

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

[Simon Horman]

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

---

Dave,

I plan to take this change through my tree.

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 e6ab0cd..c4ba651 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 */
@@ -176,6 +179,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_CON_ID("cpu_clk", &div4_clks[DIV4_I]),
 
 	/* MSTP clocks */
+	CLKDEV_DEV_ID("r7s72100-ether", &mstp_clks[MSTP74]),
 	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
 
 	/* ICK */
-- 
1.8.4

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

[Sergei Shtylyov]

Hello.

On 01/08/2014 11:02 AM, Simon Horman wrote:

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

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

WBR, Sergei

[PATCH v4 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 - v4
* 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