[PATCH RFC 0/3] Add ethernet support for r7s72100

[PATCH RFC 0/3] Add ethernet support for r7s72100

[Simon Horman]

Hi,

this is a first pass at adding ethernet support to sh-pfc for
the r7s72100 SoC.

It has been written with reference to the hardware documentation
but not well tested.

This series is based on the topic/r7s72100-v3.13-rc3-20131214v2
tag in my renesas tree.

The first patch, for the sh_eth driver, applies cleanly to net-next.

Simon Horman (3):
  sh_eth: Add support for r7s72100
  ARM: shmobile: r7s72100: Add clock for r7s72100-gether
  ARM: shmobile: genmai: Enable r7s72100-gether

 arch/arm/mach-shmobile/board-genmai.c   | 20 ++++++++++++++++++
 arch/arm/mach-shmobile/clock-r7s72100.c |  5 ++++-
 drivers/net/ethernet/renesas/sh_eth.c   | 37 ++++++++++++++++++++++++++++++---
 drivers/net/ethernet/renesas/sh_eth.h   |  2 +-
 4 files changed, 59 insertions(+), 5 deletions(-)

-- 
1.8.4

[PATCH RFC net-next 1/3] [RFC] sh_eth: Add support for r7s72100

[Simon Horman]

This appears to be quite similar to the r8a7740.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 drivers/net/ethernet/renesas/sh_eth.c | 37 ++++++++++++++++++++++++++++++++---
 drivers/net/ethernet/renesas/sh_eth.h |  2 +-
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index d256ce1..c7c1daf 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -705,6 +705,36 @@ 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_r8a7740,
+	.set_duplex	= sh_eth_set_duplex,
+	.set_rate	= sh_eth_set_rate_gether,
+
+	.register_type	= SH_ETH_REG_GIGABIT,
+
+	.ecsr_value	= ECSR_LCHNG | ECSR_ICD,
+	.ecsipr_value	= ECSIPR_LCHNGIP | 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,
 
@@ -1316,9 +1346,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 and R7S72100's
+		 * GETHER, 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;
@@ -2805,6 +2835,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-gether", (kernel_ulong_t)&r7s72100_data },
 	{ "r8a7740-gether", (kernel_ulong_t)&r8a7740_data },
 	{ "r8a777x-ether", (kernel_ulong_t)&r8a777x_data },
 	{ "r8a7790-ether", (kernel_ulong_t)&r8a7790_data },
diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
index f32c169..f2d3a9f 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -180,7 +180,7 @@ enum EDSR_BIT {
 };
 #define EDSR_ENALL (EDSR_ENT|EDSR_ENR)
 
-/* GECMR : sh7734, sh7763 and r8a7740 only */
+/* GECMR : sh7734, sh7763, r8a7740 and r7s72100 only */
 enum GECMR_BIT {
 	GECMR_10 = 0x0, GECMR_100 = 0x04, GECMR_1000 = 0x01,
 };
-- 
1.8.4

Re: [PATCH RFC net-next 1/3] [RFC] sh_eth: Add support for r7s72100

[Sergei Shtylyov]

Hello.

On 12/15/2013 11:24 AM, Simon Horman wrote:

> This appears to be quite similar to the r8a7740.

    Not quite, as it's only capable of 100/10 Mbit/s and lacks some registers.
It's good that you marjed the patch RFC as it doesn't seem correct in some 
places...

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 37 ++++++++++++++++++++++++++++++++---
>   drivers/net/ethernet/renesas/sh_eth.h |  2 +-
>   2 files changed, 35 insertions(+), 4 deletions(-)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index d256ce1..c7c1daf 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -705,6 +705,36 @@ 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_r8a7740,

    Doesn't seem right: it calls sh_eth_slect_mii() and RMII_MII register that 
function writes to is not documented in the v0.60 of the manual.

> +	.set_duplex	= sh_eth_set_duplex,
> +	.set_rate	= sh_eth_set_rate_gether,

    Last one does not seem right: it uses GECMR register which doesn't exist, 
according to the v0.60 of the manual. Looks like neither of the existing 
methods fit here and I don't see anything in the manual, so I guess this 
should be NULL instead...

> +
> +	.register_type	= SH_ETH_REG_GIGABIT,
> +
> +	.ecsr_value	= ECSR_LCHNG | ECSR_ICD,

    Hm, ECSR.LCHNG is reserved in my manual.

> +	.ecsipr_value	= ECSIPR_LCHNGIP | ECSIPR_ICDIP,

    Likewise ECSIPR.LCHNGIP. And I don't see the LINK signal in the signal 
list at all...

> +	.eesipr_value	= 0xff7f009f,
> +
> +	.tx_check	= EESR_TC1 | EESR_FTC,

    Note that both these bits mirror themselves.

> +	.eesr_err_check	= EESR_TWB1 | EESR_TWB

    These two mirror themselves too.

> @@ -1316,9 +1346,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 and R7S72100's
> +		 * GETHER, the RFS bits are from bit 25 to bit 16.

    Not only these SoCs, R8A779x also, if you want to list them all. And 
R7S72100 is not GETHER, it's explicitly called ETHER in the manual.
    Fortunately, this deviation is documented for this SoC. :-)

> +		 * So, the driver needs right shifting by 16.
>   		 */
>   		if (mdp->cd->shift_rd0)
>   			desc_status >>= 16;
> @@ -2805,6 +2835,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-gether", (kernel_ulong_t)&r7s72100_data },

    "-ether" as it doesn't support 1000 Mbit/s.

>   	{ "r8a7740-gether", (kernel_ulong_t)&r8a7740_data },
>   	{ "r8a777x-ether", (kernel_ulong_t)&r8a777x_data },
>   	{ "r8a7790-ether", (kernel_ulong_t)&r8a7790_data },
> diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> index f32c169..f2d3a9f 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.h
> +++ b/drivers/net/ethernet/renesas/sh_eth.h
> @@ -180,7 +180,7 @@ enum EDSR_BIT {
>   };
>   #define EDSR_ENALL (EDSR_ENT|EDSR_ENR)
>
> -/* GECMR : sh7734, sh7763 and r8a7740 only */
> +/* GECMR : sh7734, sh7763, r8a7740 and r7s72100 only */

    Where have you seen this register on R7S72100?

>   enum GECMR_BIT {
>   	GECMR_10 = 0x0, GECMR_100 = 0x04, GECMR_1000 = 0x01,
>   };

WBR, Sergei

[PATCH RFC 2/3] [RFC] ARM: shmobile: r7s72100: Add clock for r7s72100-gether

[Simon Horman]

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/clock-r7s72100.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
index 7b457ae..7bce0c1 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 PLL_RATE 30
 
@@ -145,9 +146,10 @@ struct clk div4_clks[DIV4_NR] = {
 };
 
 enum { MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
-	MSTP33,	MSTP_NR };
+	MSTP33,	MSTP74, MSTP_NR };
 
 static struct clk mstp_clks[MSTP_NR] = {
+	[MSTP74] = SH_CLK_MSTP8(&peripheral1_clk, STBCR7, 4, 0), /* GEther */
 	[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 */
@@ -170,6 +172,7 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_CON_ID("cpu_clk", &div4_clks[DIV4_I]),
 
 	/* MSTP clocks */
+	CLKDEV_DEV_ID("r7s72100-gether", &mstp_clks[MSTP74]),
 	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
 
 	/* ICK */
-- 
1.8.4

Re: [PATCH RFC 2/3] [RFC] ARM: shmobile: r7s72100: Add clock for r7s72100-gether

[Sergei Shtylyov]

Hello.

On 12/15/2013 11:24 AM, Simon Horman wrote:

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>   arch/arm/mach-shmobile/clock-r7s72100.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)

> diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
> index 7b457ae..7bce0c1 100644
> --- a/arch/arm/mach-shmobile/clock-r7s72100.c
> +++ b/arch/arm/mach-shmobile/clock-r7s72100.c
[...]
> @@ -145,9 +146,10 @@ struct clk div4_clks[DIV4_NR] = {
>   };
>
>   enum { MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
> -	MSTP33,	MSTP_NR };
> +	MSTP33,	MSTP74, MSTP_NR };

    Shouldn't MSTP74 be on a line of its own at the start of the *enum*?

WBR, Sergei

Re: [PATCH RFC 2/3] [RFC] ARM: shmobile: r7s72100: Add clock for r7s72100-gether

[Simon Horman]

On Sun, Dec 15, 2013 at 11:18:51PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/15/2013 11:24 AM, Simon Horman wrote:
> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >---
> >  arch/arm/mach-shmobile/clock-r7s72100.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> >diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
> >index 7b457ae..7bce0c1 100644
> >--- a/arch/arm/mach-shmobile/clock-r7s72100.c
> >+++ b/arch/arm/mach-shmobile/clock-r7s72100.c
> [...]
> >@@ -145,9 +146,10 @@ struct clk div4_clks[DIV4_NR] = {
> >  };
> >
> >  enum { MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
> >-	MSTP33,	MSTP_NR };
> >+	MSTP33,	MSTP74, MSTP_NR };
> 
>    Shouldn't MSTP74 be on a line of its own at the start of the *enum*?

Yes, thanks, I will fix that.

[PATCH RFC 3/3] [RFC] ARM: shmobile: genmai: Enable r7s72100-gether

[Simon Horman]

---
 arch/arm/mach-shmobile/board-genmai.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
index 3e92e3c..838325b 100644
--- a/arch/arm/mach-shmobile/board-genmai.c
+++ b/arch/arm/mach-shmobile/board-genmai.c
@@ -20,15 +20,35 @@
 
 #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>
 
+/* GEther */
+static const struct sh_eth_plat_data gether_pdata __initconst = {
+	.phy			= 0x00, /* PD60610 */
+	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
+	.phy_interface		= PHY_INTERFACE_MODE_MII,
+};
+
+static const struct resource gether_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-gether", -1,
+					  gether_resources,
+					  ARRAY_SIZE(gether_resources),
+					  &gether_pdata, sizeof(gether_pdata));
 }
 
 static const char * const genmai_boards_compat_dt[] __initconst = {
-- 
1.8.4

Re: [PATCH RFC 3/3] [RFC] ARM: shmobile: genmai: Enable r7s72100-gether

[Sergei Shtylyov]

Hello.

On 12/15/2013 11:24 AM, Simon Horman wrote:

    You forgot to sign off.

[...]

> diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
> index 3e92e3c..838325b 100644
> --- a/arch/arm/mach-shmobile/board-genmai.c
> +++ b/arch/arm/mach-shmobile/board-genmai.c
> @@ -20,15 +20,35 @@
>
>   #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>
>
> +/* GEther */
> +static const struct sh_eth_plat_data gether_pdata __initconst = {
> +	.phy			= 0x00, /* PD60610 */
> +	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
> +	.phy_interface		= PHY_INTERFACE_MODE_MII,

    Does MII work for Gigabit Ethernet?

WBR, Sergei

Re: [PATCH RFC 3/3] [RFC] ARM: shmobile: genmai: Enable r7s72100-gether

[Simon Horman]

On Sun, Dec 15, 2013 at 11:31:35PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/15/2013 11:24 AM, Simon Horman wrote:
> 
>    You forgot to sign off.
> 
> [...]
> 
> >diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
> >index 3e92e3c..838325b 100644
> >--- a/arch/arm/mach-shmobile/board-genmai.c
> >+++ b/arch/arm/mach-shmobile/board-genmai.c
> >@@ -20,15 +20,35 @@
> >
> >  #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>
> >
> >+/* GEther */
> >+static const struct sh_eth_plat_data gether_pdata __initconst = {
> >+	.phy			= 0x00, /* PD60610 */
> >+	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
> >+	.phy_interface		= PHY_INTERFACE_MODE_MII,
> 
>    Does MII work for Gigabit Ethernet?

My understanding is that although the SH ethernet block on
the SoC supports Gigabit it is only wired up to allow 100Mbit
access on the board and that MII is the interface used.

I believe this is the same situation as the Armadillo 800 EVA board.

Re: [PATCH RFC 3/3] [RFC] ARM: shmobile: genmai: Enable r7s72100-gether

[Sergei Shtylyov]

On 12/16/2013 04:30 AM, Simon Horman wrote:

>>     You forgot to sign off.

>> [...]

>>> diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
>>> index 3e92e3c..838325b 100644
>>> --- a/arch/arm/mach-shmobile/board-genmai.c
>>> +++ b/arch/arm/mach-shmobile/board-genmai.c
>>> @@ -20,15 +20,35 @@
>>>
>>>   #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>
>>>
>>> +/* GEther */
>>> +static const struct sh_eth_plat_data gether_pdata __initconst = {
>>> +	.phy			= 0x00, /* PD60610 */
>>> +	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
>>> +	.phy_interface		= PHY_INTERFACE_MODE_MII,

>>     Does MII work for Gigabit Ethernet?

> My understanding is that although the SH ethernet block on
> the SoC supports Gigabit it is only wired up to allow 100Mbit
> access on the board and that MII is the interface used.

    No, it's the SoC manual that limits the Ethernet speed to 100 Mbps.

WBR, Sergei

Re: [PATCH RFC 3/3] [RFC] ARM: shmobile: genmai: Enable r7s72100-gether

[Sergei Shtylyov]

Hello.

On 12/15/2013 11:24 AM, Simon Horman wrote:

> ---
>   arch/arm/mach-shmobile/board-genmai.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)

> diff --git a/arch/arm/mach-shmobile/board-genmai.c b/arch/arm/mach-shmobile/board-genmai.c
> index 3e92e3c..838325b 100644
> --- a/arch/arm/mach-shmobile/board-genmai.c
> +++ b/arch/arm/mach-shmobile/board-genmai.c
> @@ -20,15 +20,35 @@
>
>   #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>
>
> +/* GEther */

    Just Ether.

> +static const struct sh_eth_plat_data gether_pdata __initconst = {
> +	.phy			= 0x00, /* PD60610 */
> +	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
> +	.phy_interface		= PHY_INTERFACE_MODE_MII,

    I think .no_ether_link should be set to 1 -- I don't see the LINK signal 
in the manual.

> +};
> +
> +static const struct resource gether_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-gether", -1,

    We can't call it "-gether" if it lacks 1000 Mbps support. And the manual 
explicitly calls it ETHER.

WBR, Sergei