[RFC PATCH net-next v2 0/3] sh_eth: add support for r8a7790 SoC

[RFC PATCH net-next v2 0/3] sh_eth: add support for r8a7790 SoC

[Simon Horman]

Hi,

this short series enhances the sh_eth driver for use with the r8a7790 SoC.

Simon Horman (3):
  sh_eth: add support for gpio reset
  sh_eth: add support RMIIMODE register
  sh_eth: add support for r8a7790 SoC

 drivers/net/ethernet/renesas/Kconfig  |  2 +-
 drivers/net/ethernet/renesas/sh_eth.c | 14 ++++++++++++--
 drivers/net/ethernet/renesas/sh_eth.h |  2 ++
 include/linux/sh_eth.h                |  3 +++
 4 files changed, 18 insertions(+), 3 deletions(-)

-- 
1.8.2.1

[RFC PATCH net-next v2 0/3] sh_eth: add support for r8a7790 SoC

[Simon Horman]

Hi,

this short series enhances the sh_eth driver for use with the r8a7790 SoC.

Simon Horman (3):
  sh_eth: add support for gpio reset
  sh_eth: add support RMIIMODE register
  sh_eth: add support for r8a7790 SoC

 drivers/net/ethernet/renesas/Kconfig  |  2 +-
 drivers/net/ethernet/renesas/sh_eth.c | 14 ++++++++++++--
 drivers/net/ethernet/renesas/sh_eth.h |  2 ++
 include/linux/sh_eth.h                |  3 +++
 4 files changed, 18 insertions(+), 3 deletions(-)

-- 
1.8.2.1

[RFC PATCH net-next v2 1/3] sh_eth: add support for gpio reset

[Simon Horman]

Allow reset using a GPIO. In order to use this set the following
in the device's platform data:

	needs_gpio_reset = 1
	reset_gpio = GPIO pin to use

This patch is motivated by the lager board which uses the r8a7790 SoC
which has a GPIO pin to reset its sh_eth device.

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

--

v2
* First post
---
 drivers/net/ethernet/renesas/sh_eth.c | 4 ++++
 include/linux/sh_eth.h                | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 33dc6f2..016cf80 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -21,6 +21,7 @@
  *  the file called "COPYING".
  */
 
+#include <linux/gpio.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -2759,6 +2760,9 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 		ndev->features = NETIF_F_HW_VLAN_CTAG_FILTER;
 	}
 
+	if (pd->needs_gpio_reset)
+		gpio_request_one(pd->reset_gpio, GPIOF_OUT_INIT_HIGH, NULL);
+
 	/* initialize first or needed device */
 	if (!devno || pd->needs_init) {
 		if (mdp->cd->chip_reset)
diff --git a/include/linux/sh_eth.h b/include/linux/sh_eth.h
index fc30571..b3f1550 100644
--- a/include/linux/sh_eth.h
+++ b/include/linux/sh_eth.h
@@ -22,6 +22,9 @@ struct sh_eth_plat_data {
 	unsigned no_ether_link:1;
 	unsigned ether_link_active_low:1;
 	unsigned needs_init:1;
+	unsigned needs_gpio_reset:1;
+
+	unsigned reset_gpio;	/* Used if needs_gpio_reset is true */
 };
 
 #endif
-- 
1.8.2.1

[RFC PATCH net-next v2 1/3] sh_eth: add support for gpio reset

[Simon Horman]

Allow reset using a GPIO. In order to use this set the following
in the device's platform data:

	needs_gpio_reset = 1
	reset_gpio = GPIO pin to use

This patch is motivated by the lager board which uses the r8a7790 SoC
which has a GPIO pin to reset its sh_eth device.

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

--

v2
* First post
---
 drivers/net/ethernet/renesas/sh_eth.c | 4 ++++
 include/linux/sh_eth.h                | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 33dc6f2..016cf80 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -21,6 +21,7 @@
  *  the file called "COPYING".
  */
 
+#include <linux/gpio.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -2759,6 +2760,9 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 		ndev->features = NETIF_F_HW_VLAN_CTAG_FILTER;
 	}
 
+	if (pd->needs_gpio_reset)
+		gpio_request_one(pd->reset_gpio, GPIOF_OUT_INIT_HIGH, NULL);
+
 	/* initialize first or needed device */
 	if (!devno || pd->needs_init) {
 		if (mdp->cd->chip_reset)
diff --git a/include/linux/sh_eth.h b/include/linux/sh_eth.h
index fc30571..b3f1550 100644
--- a/include/linux/sh_eth.h
+++ b/include/linux/sh_eth.h
@@ -22,6 +22,9 @@ struct sh_eth_plat_data {
 	unsigned no_ether_link:1;
 	unsigned ether_link_active_low:1;
 	unsigned needs_init:1;
+	unsigned needs_gpio_reset:1;
+
+	unsigned reset_gpio;	/* Used if needs_gpio_reset is true */
 };
 
 #endif
-- 
1.8.2.1

[RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Simon Horman]

This change is motivated by the lager board which uses the r8a7790 SoC,
an R-Car SoC. For this board setting the RMIIMODE register is necessary.

This patch assumes this is valid for all R-Car SoCs.

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

--

v2
* Split from patch to add r8a7790 support to driver (ifdef nastiness)
* As suggested by Sergei Shtylyov
  - Add a field to struct sh_eth_cpu_data to indicate if the
    rmiimode register is present. This replaces an extremely ugly ifdef.
---
 drivers/net/ethernet/renesas/sh_eth.c | 5 +++++
 drivers/net/ethernet/renesas/sh_eth.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 016cf80..ff5613d 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -190,6 +190,7 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[RMCR]		= 0x0258,
 	[TFUCR]		= 0x0264,
 	[RFOCR]		= 0x0268,
+	[RMIIMODE]      = 0x026c,
 	[FCFTR]		= 0x0270,
 	[TRIMD]		= 0x027c,
 };
@@ -389,6 +390,7 @@ static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
 	.mpr		= 1,
 	.tpauser	= 1,
 	.hw_swap	= 1,
+	.rmiimode	= 1,
 };
 #elif defined(CONFIG_CPU_SUBTYPE_SH7724)
 #define SH_ETH_RESET_DEFAULT	1
@@ -1276,6 +1278,9 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
 	if (ret)
 		goto out;
 
+	if (mdp->cd->rmiimode)
+		sh_eth_write(ndev, 0x1, RMIIMODE);
+
 	/* Descriptor format */
 	sh_eth_ring_format(ndev);
 	if (mdp->cd->rpadir)
diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
index 1ddc9f2..1698bbd 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -60,6 +60,7 @@ enum {
 	EDOCR,
 	TFUCR,
 	RFOCR,
+	RMIIMODE,
 	FCFTR,
 	RPADIR,
 	TRIMD,
@@ -478,6 +479,7 @@ struct sh_eth_cpu_data {
 	unsigned no_ade:1;	/* E-DMAC DO NOT have ADE bit in EESR */
 	unsigned hw_crc:1;	/* E-DMAC have CSMR */
 	unsigned select_mii:1;	/* EtherC have RMII_MII (MII select register) */
+	unsigned rmiimode:1;		/* EtherC has RMIIMODE register */
 };
 
 struct sh_eth_private {
-- 
1.8.2.1

[RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Simon Horman]

This change is motivated by the lager board which uses the r8a7790 SoC,
an R-Car SoC. For this board setting the RMIIMODE register is necessary.

This patch assumes this is valid for all R-Car SoCs.

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

--

v2
* Split from patch to add r8a7790 support to driver (ifdef nastiness)
* As suggested by Sergei Shtylyov
  - Add a field to struct sh_eth_cpu_data to indicate if the
    rmiimode register is present. This replaces an extremely ugly ifdef.
---
 drivers/net/ethernet/renesas/sh_eth.c | 5 +++++
 drivers/net/ethernet/renesas/sh_eth.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 016cf80..ff5613d 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -190,6 +190,7 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
 	[RMCR]		= 0x0258,
 	[TFUCR]		= 0x0264,
 	[RFOCR]		= 0x0268,
+	[RMIIMODE]      = 0x026c,
 	[FCFTR]		= 0x0270,
 	[TRIMD]		= 0x027c,
 };
@@ -389,6 +390,7 @@ static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
 	.mpr		= 1,
 	.tpauser	= 1,
 	.hw_swap	= 1,
+	.rmiimode	= 1,
 };
 #elif defined(CONFIG_CPU_SUBTYPE_SH7724)
 #define SH_ETH_RESET_DEFAULT	1
@@ -1276,6 +1278,9 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
 	if (ret)
 		goto out;
 
+	if (mdp->cd->rmiimode)
+		sh_eth_write(ndev, 0x1, RMIIMODE);
+
 	/* Descriptor format */
 	sh_eth_ring_format(ndev);
 	if (mdp->cd->rpadir)
diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
index 1ddc9f2..1698bbd 100644
--- a/drivers/net/ethernet/renesas/sh_eth.h
+++ b/drivers/net/ethernet/renesas/sh_eth.h
@@ -60,6 +60,7 @@ enum {
 	EDOCR,
 	TFUCR,
 	RFOCR,
+	RMIIMODE,
 	FCFTR,
 	RPADIR,
 	TRIMD,
@@ -478,6 +479,7 @@ struct sh_eth_cpu_data {
 	unsigned no_ade:1;	/* E-DMAC DO NOT have ADE bit in EESR */
 	unsigned hw_crc:1;	/* E-DMAC have CSMR */
 	unsigned select_mii:1;	/* EtherC have RMII_MII (MII select register) */
+	unsigned rmiimode:1;		/* EtherC has RMIIMODE register */
 };
 
 struct sh_eth_private {
-- 
1.8.2.1

[RFC PATCH net-next v2 3/3] sh_eth: add support for r8a7790 SoC

[Simon Horman]

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

--

v2
* Move RMIIMODE portion into a separate patch
---
 drivers/net/ethernet/renesas/Kconfig  | 2 +-
 drivers/net/ethernet/renesas/sh_eth.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/Kconfig b/drivers/net/ethernet/renesas/Kconfig
index bed9841..d664c0f 100644
--- a/drivers/net/ethernet/renesas/Kconfig
+++ b/drivers/net/ethernet/renesas/Kconfig
@@ -9,7 +9,7 @@ config SH_ETH
 		 CPU_SUBTYPE_SH7763 || CPU_SUBTYPE_SH7619 || \
 		 CPU_SUBTYPE_SH7724 || CPU_SUBTYPE_SH7734 || \
 		 CPU_SUBTYPE_SH7757 || ARCH_R8A7740 || \
-		 ARCH_R8A7778 || ARCH_R8A7779)
+		 ARCH_R8A7778 || ARCH_R8A7779 || ARCH_R8A7790)
 	select CRC32
 	select NET_CORE
 	select MII
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index ff5613d..e832d00 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -344,7 +344,8 @@ static void sh_eth_select_mii(struct net_device *ndev)
 #endif
 
 /* There is CPU dependent code */
-#if defined(CONFIG_ARCH_R8A7778) || defined(CONFIG_ARCH_R8A7779)
+#if defined(CONFIG_ARCH_R8A7778) || defined(CONFIG_ARCH_R8A7779) || \
+	defined(CONFIG_ARCH_R8A7790)
 #define SH_ETH_RESET_DEFAULT	1
 static void sh_eth_set_duplex(struct net_device *ndev)
 {
@@ -372,7 +373,7 @@ static void sh_eth_set_rate(struct net_device *ndev)
 	}
 }
 
-/* R8A7778/9 */
+/* R8A7778, R8A7779, R8A7790 */
 static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
 	.set_duplex	= sh_eth_set_duplex,
 	.set_rate	= sh_eth_set_rate,
-- 
1.8.2.1

[RFC PATCH net-next v2 3/3] sh_eth: add support for r8a7790 SoC

[Simon Horman]

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

--

v2
* Move RMIIMODE portion into a separate patch
---
 drivers/net/ethernet/renesas/Kconfig  | 2 +-
 drivers/net/ethernet/renesas/sh_eth.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/renesas/Kconfig b/drivers/net/ethernet/renesas/Kconfig
index bed9841..d664c0f 100644
--- a/drivers/net/ethernet/renesas/Kconfig
+++ b/drivers/net/ethernet/renesas/Kconfig
@@ -9,7 +9,7 @@ config SH_ETH
 		 CPU_SUBTYPE_SH7763 || CPU_SUBTYPE_SH7619 || \
 		 CPU_SUBTYPE_SH7724 || CPU_SUBTYPE_SH7734 || \
 		 CPU_SUBTYPE_SH7757 || ARCH_R8A7740 || \
-		 ARCH_R8A7778 || ARCH_R8A7779)
+		 ARCH_R8A7778 || ARCH_R8A7779 || ARCH_R8A7790)
 	select CRC32
 	select NET_CORE
 	select MII
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index ff5613d..e832d00 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -344,7 +344,8 @@ static void sh_eth_select_mii(struct net_device *ndev)
 #endif
 
 /* There is CPU dependent code */
-#if defined(CONFIG_ARCH_R8A7778) || defined(CONFIG_ARCH_R8A7779)
+#if defined(CONFIG_ARCH_R8A7778) || defined(CONFIG_ARCH_R8A7779) || \
+	defined(CONFIG_ARCH_R8A7790)
 #define SH_ETH_RESET_DEFAULT	1
 static void sh_eth_set_duplex(struct net_device *ndev)
 {
@@ -372,7 +373,7 @@ static void sh_eth_set_rate(struct net_device *ndev)
 	}
 }
 
-/* R8A7778/9 */
+/* R8A7778, R8A7779, R8A7790 */
 static struct sh_eth_cpu_data sh_eth_my_cpu_data = {
 	.set_duplex	= sh_eth_set_duplex,
 	.set_rate	= sh_eth_set_rate,
-- 
1.8.2.1

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 14-05-2013 6:37, Simon Horman wrote:

> This change is motivated by the lager board which uses the r8a7790 SoC,
> an R-Car SoC. For this board setting the RMIIMODE register is necessary.

> This patch assumes this is valid for all R-Car SoCs.

    No, RMIIMODE register is not documented on R8A7778/9 SoCs and this 
location is described as reserved. How's it called in R8A7790 manual, CXR15?

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

> --

    Missed a dash here.

> v2
> * Split from patch to add r8a7790 support to driver (ifdef nastiness)
> * As suggested by Sergei Shtylyov
>    - Add a field to struct sh_eth_cpu_data to indicate if the
>      rmiimode register is present. This replaces an extremely ugly ifdef.
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 5 +++++
>   drivers/net/ethernet/renesas/sh_eth.h | 2 ++
>   2 files changed, 7 insertions(+)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 016cf80..ff5613d 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -190,6 +190,7 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[RMCR]		= 0x0258,
>   	[TFUCR]		= 0x0264,
>   	[RFOCR]		= 0x0268,
> +	[RMIIMODE]      = 0x026c,
>   	[FCFTR]		= 0x0270,
>   	[TRIMD]		= 0x027c,
>   };

WBR, Sergei

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 14-05-2013 6:37, Simon Horman wrote:

> This change is motivated by the lager board which uses the r8a7790 SoC,
> an R-Car SoC. For this board setting the RMIIMODE register is necessary.

> This patch assumes this is valid for all R-Car SoCs.

    No, RMIIMODE register is not documented on R8A7778/9 SoCs and this 
location is described as reserved. How's it called in R8A7790 manual, CXR15?

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

> --

    Missed a dash here.

> v2
> * Split from patch to add r8a7790 support to driver (ifdef nastiness)
> * As suggested by Sergei Shtylyov
>    - Add a field to struct sh_eth_cpu_data to indicate if the
>      rmiimode register is present. This replaces an extremely ugly ifdef.
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 5 +++++
>   drivers/net/ethernet/renesas/sh_eth.h | 2 ++
>   2 files changed, 7 insertions(+)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 016cf80..ff5613d 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -190,6 +190,7 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
>   	[RMCR]		= 0x0258,
>   	[TFUCR]		= 0x0264,
>   	[RFOCR]		= 0x0268,
> +	[RMIIMODE]      = 0x026c,
>   	[FCFTR]		= 0x0270,
>   	[TRIMD]		= 0x027c,
>   };

WBR, Sergei

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Simon Horman]

On Tue, May 14, 2013 at 05:48:55PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 14-05-2013 6:37, Simon Horman wrote:
> 
> >This change is motivated by the lager board which uses the r8a7790 SoC,
> >an R-Car SoC. For this board setting the RMIIMODE register is necessary.
> 
> >This patch assumes this is valid for all R-Car SoCs.
> 
>    No, RMIIMODE register is not documented on R8A7778/9 SoCs and
> this location is described as reserved. How's it called in R8A7790
> manual, CXR15?

It appears to be undocumented in the R8A7790 I have.
Empirically it seems necessary for the lager/R8A7790.
To be honest I am entirely unsure of its relevance to other SoCs.

I wonder if a better approach would be to add a flag to platform
data rather than struct sh_eth_cpu_data. This would allow the register
to only be accessed on boards where it is known to be useful.

> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >--
> 
>    Missed a dash here.
> 
> >v2
> >* Split from patch to add r8a7790 support to driver (ifdef nastiness)
> >* As suggested by Sergei Shtylyov
> >   - Add a field to struct sh_eth_cpu_data to indicate if the
> >     rmiimode register is present. This replaces an extremely ugly ifdef.
> >---
> >  drivers/net/ethernet/renesas/sh_eth.c | 5 +++++
> >  drivers/net/ethernet/renesas/sh_eth.h | 2 ++
> >  2 files changed, 7 insertions(+)
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 016cf80..ff5613d 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,7 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[RMCR]		= 0x0258,
> >  	[TFUCR]		= 0x0264,
> >  	[RFOCR]		= 0x0268,
> >+	[RMIIMODE]      = 0x026c,
> >  	[FCFTR]		= 0x0270,
> >  	[TRIMD]		= 0x027c,
> >  };
> 
> WBR, Sergei
> 

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Simon Horman]

On Tue, May 14, 2013 at 05:48:55PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 14-05-2013 6:37, Simon Horman wrote:
> 
> >This change is motivated by the lager board which uses the r8a7790 SoC,
> >an R-Car SoC. For this board setting the RMIIMODE register is necessary.
> 
> >This patch assumes this is valid for all R-Car SoCs.
> 
>    No, RMIIMODE register is not documented on R8A7778/9 SoCs and
> this location is described as reserved. How's it called in R8A7790
> manual, CXR15?

It appears to be undocumented in the R8A7790 I have.
Empirically it seems necessary for the lager/R8A7790.
To be honest I am entirely unsure of its relevance to other SoCs.

I wonder if a better approach would be to add a flag to platform
data rather than struct sh_eth_cpu_data. This would allow the register
to only be accessed on boards where it is known to be useful.

> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >--
> 
>    Missed a dash here.
> 
> >v2
> >* Split from patch to add r8a7790 support to driver (ifdef nastiness)
> >* As suggested by Sergei Shtylyov
> >   - Add a field to struct sh_eth_cpu_data to indicate if the
> >     rmiimode register is present. This replaces an extremely ugly ifdef.
> >---
> >  drivers/net/ethernet/renesas/sh_eth.c | 5 +++++
> >  drivers/net/ethernet/renesas/sh_eth.h | 2 ++
> >  2 files changed, 7 insertions(+)
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 016cf80..ff5613d 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,7 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[RMCR]		= 0x0258,
> >  	[TFUCR]		= 0x0264,
> >  	[RFOCR]		= 0x0268,
> >+	[RMIIMODE]      = 0x026c,
> >  	[FCFTR]		= 0x0270,
> >  	[TRIMD]		= 0x027c,
> >  };
> 
> WBR, Sergei
> 

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 15-05-2013 4:51, Simon Horman wrote:

>>> This change is motivated by the lager board which uses the r8a7790 SoC,
>>> an R-Car SoC. For this board setting the RMIIMODE register is necessary.

>>> This patch assumes this is valid for all R-Car SoCs.

>>     No, RMIIMODE register is not documented on R8A7778/9 SoCs and
>> this location is described as reserved. How's it called in R8A7790
>> manual, CXR15?

> It appears to be undocumented in the R8A7790 I have.
> Empirically it seems necessary for the lager/R8A7790.

    I wonder where you got the info then (and what writing 1 to this 
register means)...

> To be honest I am entirely unsure of its relevance to other SoCs.

> I wonder if a better approach would be to add a flag to platform
> data rather than struct sh_eth_cpu_data. This would allow the register
> to only be accessed on boards where it is known to be useful.

    Maybe...

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

WBR, Sergei

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 15-05-2013 4:51, Simon Horman wrote:

>>> This change is motivated by the lager board which uses the r8a7790 SoC,
>>> an R-Car SoC. For this board setting the RMIIMODE register is necessary.

>>> This patch assumes this is valid for all R-Car SoCs.

>>     No, RMIIMODE register is not documented on R8A7778/9 SoCs and
>> this location is described as reserved. How's it called in R8A7790
>> manual, CXR15?

> It appears to be undocumented in the R8A7790 I have.
> Empirically it seems necessary for the lager/R8A7790.

    I wonder where you got the info then (and what writing 1 to this 
register means)...

> To be honest I am entirely unsure of its relevance to other SoCs.

> I wonder if a better approach would be to add a flag to platform
> data rather than struct sh_eth_cpu_data. This would allow the register
> to only be accessed on boards where it is known to be useful.

    Maybe...

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

WBR, Sergei

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 05/15/2013 04:51 AM, Simon Horman wrote:

>>> This change is motivated by the lager board which uses the r8a7790 SoC,
>>> an R-Car SoC. For this board setting the RMIIMODE register is necessary.
>>> This patch assumes this is valid for all R-Car SoCs.
>>     No, RMIIMODE register is not documented on R8A7778/9 SoCs and
>> this location is described as reserved. How's it called in R8A7790
>> manual, CXR15?
> It appears to be undocumented in the R8A7790 I have.

     Of the manuals I have now this register is only documented in R-Car 
H2 user's manual 0.50,

> Empirically it seems necessary for the lager/R8A7790.

    The manual confirms that.

> To be honest I am entirely unsure of its relevance to other SoCs.
>
> I wonder if a better approach would be to add a flag to platform
> data rather than struct sh_eth_cpu_data. This would allow the register
> to only be accessed on boards where it is known to be useful.

    No, this doesn't depend on the board, judging on the H2 manual.
You just must write it to 1 on R-Car H2 in order for Ether to operate.

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

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 05/15/2013 04:51 AM, Simon Horman wrote:

>>> This change is motivated by the lager board which uses the r8a7790 SoC,
>>> an R-Car SoC. For this board setting the RMIIMODE register is necessary.
>>> This patch assumes this is valid for all R-Car SoCs.
>>     No, RMIIMODE register is not documented on R8A7778/9 SoCs and
>> this location is described as reserved. How's it called in R8A7790
>> manual, CXR15?
> It appears to be undocumented in the R8A7790 I have.

     Of the manuals I have now this register is only documented in R-Car 
H2 user's manual 0.50,

> Empirically it seems necessary for the lager/R8A7790.

    The manual confirms that.

> To be honest I am entirely unsure of its relevance to other SoCs.
>
> I wonder if a better approach would be to add a flag to platform
> data rather than struct sh_eth_cpu_data. This would allow the register
> to only be accessed on boards where it is known to be useful.

    No, this doesn't depend on the board, judging on the H2 manual.
You just must write it to 1 on R-Car H2 in order for Ether to operate.

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

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Simon Horman]

On Thu, Jun 06, 2013 at 12:43:55AM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 05/15/2013 04:51 AM, Simon Horman wrote:
> 
> >>>This change is motivated by the lager board which uses the r8a7790 SoC,
> >>>an R-Car SoC. For this board setting the RMIIMODE register is necessary.
> >>>This patch assumes this is valid for all R-Car SoCs.
> >>    No, RMIIMODE register is not documented on R8A7778/9 SoCs and
> >>this location is described as reserved. How's it called in R8A7790
> >>manual, CXR15?
> >It appears to be undocumented in the R8A7790 I have.
> 
>     Of the manuals I have now this register is only documented in
> R-Car H2 user's manual 0.50,
> 
> >Empirically it seems necessary for the lager/R8A7790.
> 
>    The manual confirms that.
> 
> >To be honest I am entirely unsure of its relevance to other SoCs.
> >
> >I wonder if a better approach would be to add a flag to platform
> >data rather than struct sh_eth_cpu_data. This would allow the register
> >to only be accessed on boards where it is known to be useful.
> 
>    No, this doesn't depend on the board, judging on the H2 manual.
> You just must write it to 1 on R-Car H2 in order for Ether to operate.

What is your preferred way to add support for this to the driver?

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Simon Horman]

On Thu, Jun 06, 2013 at 12:43:55AM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 05/15/2013 04:51 AM, Simon Horman wrote:
> 
> >>>This change is motivated by the lager board which uses the r8a7790 SoC,
> >>>an R-Car SoC. For this board setting the RMIIMODE register is necessary.
> >>>This patch assumes this is valid for all R-Car SoCs.
> >>    No, RMIIMODE register is not documented on R8A7778/9 SoCs and
> >>this location is described as reserved. How's it called in R8A7790
> >>manual, CXR15?
> >It appears to be undocumented in the R8A7790 I have.
> 
>     Of the manuals I have now this register is only documented in
> R-Car H2 user's manual 0.50,
> 
> >Empirically it seems necessary for the lager/R8A7790.
> 
>    The manual confirms that.
> 
> >To be honest I am entirely unsure of its relevance to other SoCs.
> >
> >I wonder if a better approach would be to add a flag to platform
> >data rather than struct sh_eth_cpu_data. This would allow the register
> >to only be accessed on boards where it is known to be useful.
> 
>    No, this doesn't depend on the board, judging on the H2 manual.
> You just must write it to 1 on R-Car H2 in order for Ether to operate.

What is your preferred way to add support for this to the driver?

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 28-06-2013 5:41, Simon Horman wrote:

>>>>> This change is motivated by the lager board which uses the r8a7790 SoC,
>>>>> an R-Car SoC. For this board setting the RMIIMODE register is necessary.
>>>>> This patch assumes this is valid for all R-Car SoCs.
>>>>     No, RMIIMODE register is not documented on R8A7778/9 SoCs and
>>>> this location is described as reserved. How's it called in R8A7790
>>>> manual, CXR15?

>>> It appears to be undocumented in the R8A7790 I have.

>>      Of the manuals I have now this register is only documented in
>> R-Car H2 user's manual 0.50,

>>> Empirically it seems necessary for the lager/R8A7790.

>>     The manual confirms that.

>>> To be honest I am entirely unsure of its relevance to other SoCs.

>>> I wonder if a better approach would be to add a flag to platform
>>> data rather than struct sh_eth_cpu_data. This would allow the register
>>> to only be accessed on boards where it is known to be useful.

>>     No, this doesn't depend on the board, judging on the H2 manual.
>> You just must write it to 1 on R-Car H2 in order for Ether to operate.

> What is your preferred way to add support for this to the driver?

    Like you did, add a field to 'struct sh_eth_cpu_data' that's 1 if 
RMIIMODE register is present but don't set it in 'r8a777x_data' and 
create separate 'r8a7790_data' instead (where to set this field).

WBR, Sergei

Re: [RFC PATCH net-next v2 2/3] sh_eth: add support RMIIMODE register

[Sergei Shtylyov]

Hello.

On 28-06-2013 5:41, Simon Horman wrote:

>>>>> This change is motivated by the lager board which uses the r8a7790 SoC,
>>>>> an R-Car SoC. For this board setting the RMIIMODE register is necessary.
>>>>> This patch assumes this is valid for all R-Car SoCs.
>>>>     No, RMIIMODE register is not documented on R8A7778/9 SoCs and
>>>> this location is described as reserved. How's it called in R8A7790
>>>> manual, CXR15?

>>> It appears to be undocumented in the R8A7790 I have.

>>      Of the manuals I have now this register is only documented in
>> R-Car H2 user's manual 0.50,

>>> Empirically it seems necessary for the lager/R8A7790.

>>     The manual confirms that.

>>> To be honest I am entirely unsure of its relevance to other SoCs.

>>> I wonder if a better approach would be to add a flag to platform
>>> data rather than struct sh_eth_cpu_data. This would allow the register
>>> to only be accessed on boards where it is known to be useful.

>>     No, this doesn't depend on the board, judging on the H2 manual.
>> You just must write it to 1 on R-Car H2 in order for Ether to operate.

> What is your preferred way to add support for this to the driver?

    Like you did, add a field to 'struct sh_eth_cpu_data' that's 1 if 
RMIIMODE register is present but don't set it in 'r8a777x_data' and 
create separate 'r8a7790_data' instead (where to set this field).

WBR, Sergei