[PATCH 0/2] net: sh_eth: add support for SH7786 and big-endian

[PATCH 0/2] net: sh_eth: add support for SH7786 and big-endian

[Thomas Petazzoni]

Hello,

I've recently been working on an SH7786 based platform, which uses the
sh_eth network controller. One peculiarity of my setup is that the CPU
is configured big-endian (even though little-endian is more
traditional in the Linux SuperH world), and the sh_eth driver was not
ready for this.

The first patch simply adds the sh_eth_cpu_data structure that
describes the SH7786 controller.

The second patch fixes the driver for big-endian operation. However,
I'd like this patch to be carefully reviewed by Sergei Shtylyov who
already did some endianness related changes in this driver. Indeed, my
change is based on the assumption that the DMA descriptors are in the
native endianness of the CPU.

Thanks,

Thomas

Thomas Petazzoni (2):
  net: sh_eth: add support for SH7786
  net: sh_eth: make work on big endian systems

 drivers/net/ethernet/renesas/sh_eth.c | 89 ++++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 34 deletions(-)

-- 
2.13.6

[PATCH 1/2] net: sh_eth: add support for SH7786

[Thomas Petazzoni]

This commit adds the sh_eth_cpu_data structure that describes the
SH7786 variant of the IP.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/renesas/sh_eth.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 0074c5998481..a3c48b2a713c 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -710,6 +710,30 @@ static struct sh_eth_cpu_data sh7724_data = {
 	.rpadir_value	= 0x00020000, /* NET_IP_ALIGN assumed to be 2 */
 };
 
+static struct sh_eth_cpu_data sh7786_data = {
+	.set_duplex	= sh_eth_set_duplex,
+
+	.register_type	= SH_ETH_REG_FAST_SH4,
+
+	.ecsr_value	= ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD,
+	.ecsipr_value	= ECSIPR_PSRTOIP | ECSIPR_LCHNGIP | ECSIPR_ICDIP,
+	.eesipr_value	= EESIPR_RFCOFIP | EESIPR_ADEIP | EESIPR_ECIIP |
+			  EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
+			  EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |
+			  EESIPR_RMAFIP | EESIPR_RRFIP |
+			  EESIPR_RTLFIP | EESIPR_RTSFIP |
+			  EESIPR_PREIP | EESIPR_CERFIP,
+
+	.tx_check	= EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
+	.eesr_err_check	= EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
+			  EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
+
+	.apr		= 1,
+	.mpr		= 1,
+	.tpauser	= 1,
+	.hw_swap	= 1,
+};
+
 static void sh_eth_set_rate_sh7757(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -3418,6 +3442,7 @@ static const 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 },
+	{ "sh7786-ether", (kernel_ulong_t)&sh7786_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(platform, sh_eth_id_table);
-- 
2.13.6

[PATCH 2/2] net: sh_eth: make work on big endian systems

[Thomas Petazzoni]

The sh_eth driver uses cpu_to_le32() and le32_to_cpu() to manipulate
the fields of the DMA descriptors, making the assumption that the DMA
descriptors are little-endian.

However, testing on the Renesas SH7786 running in big-endian mode
reveals that the DMA descriptors are also big-endian when running
big-endian. Therefore, all the endianness conversion needs to be
removed for the sh_eth driver to work.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Note: I see that Sergei Shtylyov has done some work around endianness
on this driver back in 2015. I am not sure if it's used on other
non-SuperH platforms in platforms where the CPU might run big-endian
but the IP still be little-endian. If this is the case, then we would
need a different approach and more discussion.

Therefore, it would be useful to wait for an ACK from Sergei before
applying this patch.
---
 drivers/net/ethernet/renesas/sh_eth.c | 64 ++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index a3c48b2a713c..0de8fae143ab 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1163,31 +1163,29 @@ static int sh_eth_tx_free(struct net_device *ndev, bool sent_only)
 	for (; mdp->cur_tx - mdp->dirty_tx > 0; mdp->dirty_tx++) {
 		entry = mdp->dirty_tx % mdp->num_tx_ring;
 		txdesc = &mdp->tx_ring[entry];
-		sent = !(txdesc->status & cpu_to_le32(TD_TACT));
+		sent = !(txdesc->status & TD_TACT);
 		if (sent_only && !sent)
 			break;
 		/* TACT bit must be checked before all the following reads */
 		dma_rmb();
 		netif_info(mdp, tx_done, ndev,
 			   "tx entry %d status 0x%08x\n",
-			   entry, le32_to_cpu(txdesc->status));
+			   entry, txdesc->status);
 		/* Free the original skb. */
 		if (mdp->tx_skbuff[entry]) {
-			dma_unmap_single(&mdp->pdev->dev,
-					 le32_to_cpu(txdesc->addr),
-					 le32_to_cpu(txdesc->len) >> 16,
-					 DMA_TO_DEVICE);
+			dma_unmap_single(&mdp->pdev->dev, txdesc->addr,
+					 txdesc->len >> 16, DMA_TO_DEVICE);
 			dev_kfree_skb_irq(mdp->tx_skbuff[entry]);
 			mdp->tx_skbuff[entry] = NULL;
 			free_num++;
 		}
-		txdesc->status = cpu_to_le32(TD_TFP);
+		txdesc->status = TD_TFP;
 		if (entry >= mdp->num_tx_ring - 1)
-			txdesc->status |= cpu_to_le32(TD_TDLE);
+			txdesc->status |= TD_TDLE;
 
 		if (sent) {
 			ndev->stats.tx_packets++;
-			ndev->stats.tx_bytes += le32_to_cpu(txdesc->len) >> 16;
+			ndev->stats.tx_bytes += txdesc->len >> 16;
 		}
 	}
 	return free_num;
@@ -1204,8 +1202,7 @@ static void sh_eth_ring_free(struct net_device *ndev)
 			if (mdp->rx_skbuff[i]) {
 				struct sh_eth_rxdesc *rxdesc = &mdp->rx_ring[i];
 
-				dma_unmap_single(&mdp->pdev->dev,
-						 le32_to_cpu(rxdesc->addr),
+				dma_unmap_single(&mdp->pdev->dev, rxdesc->addr,
 						 ALIGN(mdp->rx_buf_sz, 32),
 						 DMA_FROM_DEVICE);
 			}
@@ -1280,9 +1277,9 @@ static void sh_eth_ring_format(struct net_device *ndev)
 
 		/* RX descriptor */
 		rxdesc = &mdp->rx_ring[i];
-		rxdesc->len = cpu_to_le32(buf_len << 16);
-		rxdesc->addr = cpu_to_le32(dma_addr);
-		rxdesc->status = cpu_to_le32(RD_RACT | RD_RFP);
+		rxdesc->len = buf_len << 16;
+		rxdesc->addr = dma_addr;
+		rxdesc->status = RD_RACT | RD_RFP;
 
 		/* Rx descriptor address set */
 		if (i == 0) {
@@ -1297,7 +1294,7 @@ static void sh_eth_ring_format(struct net_device *ndev)
 
 	/* Mark the last entry as wrapping the ring. */
 	if (rxdesc)
-		rxdesc->status |= cpu_to_le32(RD_RDLE);
+		rxdesc->status |= RD_RDLE;
 
 	memset(mdp->tx_ring, 0, tx_ringsize);
 
@@ -1305,8 +1302,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
 	for (i = 0; i < mdp->num_tx_ring; i++) {
 		mdp->tx_skbuff[i] = NULL;
 		txdesc = &mdp->tx_ring[i];
-		txdesc->status = cpu_to_le32(TD_TFP);
-		txdesc->len = cpu_to_le32(0);
+		txdesc->status = TD_TFP;
+		txdesc->len = 0;
 		if (i == 0) {
 			/* Tx descriptor address set */
 			sh_eth_write(ndev, mdp->tx_desc_dma, TDLAR);
@@ -1316,7 +1313,7 @@ static void sh_eth_ring_format(struct net_device *ndev)
 		}
 	}
 
-	txdesc->status |= cpu_to_le32(TD_TDLE);
+	txdesc->status |= TD_TDLE;
 }
 
 /* Get skb and descriptor buffer */
@@ -1462,7 +1459,7 @@ static void sh_eth_dev_exit(struct net_device *ndev)
 	 * packet boundary if it's currently running
 	 */
 	for (i = 0; i < mdp->num_tx_ring; i++)
-		mdp->tx_ring[i].status &= ~cpu_to_le32(TD_TACT);
+		mdp->tx_ring[i].status &= ~TD_TACT;
 
 	/* Disable TX FIFO egress to MAC */
 	sh_eth_rcv_snd_disable(ndev);
@@ -1502,11 +1499,11 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
 	boguscnt = min(boguscnt, *quota);
 	limit = boguscnt;
 	rxdesc = &mdp->rx_ring[entry];
-	while (!(rxdesc->status & cpu_to_le32(RD_RACT))) {
+	while (!(rxdesc->status & RD_RACT)) {
 		/* RACT bit must be checked before all the following reads */
 		dma_rmb();
-		desc_status = le32_to_cpu(rxdesc->status);
-		pkt_len = le32_to_cpu(rxdesc->len) & RD_RFL;
+		desc_status = rxdesc->status;
+		pkt_len = rxdesc->len & RD_RFL;
 
 		if (--boguscnt < 0)
 			break;
@@ -1544,7 +1541,7 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
 			if (desc_status & RD_RFS10)
 				ndev->stats.rx_over_errors++;
 		} else	if (skb) {
-			dma_addr = le32_to_cpu(rxdesc->addr);
+			dma_addr = rxdesc->addr;
 			if (!mdp->cd->hw_swap)
 				sh_eth_soft_swap(
 					phys_to_virt(ALIGN(dma_addr, 4)),
@@ -1573,7 +1570,7 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
 		rxdesc = &mdp->rx_ring[entry];
 		/* The size of the buffer is 32 byte boundary. */
 		buf_len = ALIGN(mdp->rx_buf_sz, 32);
-		rxdesc->len = cpu_to_le32(buf_len << 16);
+		rxdesc->len = buf_len << 16;
 
 		if (mdp->rx_skbuff[entry] == NULL) {
 			skb = netdev_alloc_skb(ndev, skbuff_size);
@@ -1589,14 +1586,13 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
 			mdp->rx_skbuff[entry] = skb;
 
 			skb_checksum_none_assert(skb);
-			rxdesc->addr = cpu_to_le32(dma_addr);
+			rxdesc->addr = dma_addr;
 		}
 		dma_wmb(); /* RACT bit must be set after all the above writes */
 		if (entry >= mdp->num_rx_ring - 1)
-			rxdesc->status |=
-				cpu_to_le32(RD_RACT | RD_RFP | RD_RDLE);
+			rxdesc->status |= RD_RACT | RD_RFP | RD_RDLE;
 		else
-			rxdesc->status |= cpu_to_le32(RD_RACT | RD_RFP);
+			rxdesc->status |= RD_RACT | RD_RFP;
 	}
 
 	/* Restart Rx engine if stopped. */
@@ -2426,8 +2422,8 @@ static void sh_eth_tx_timeout(struct net_device *ndev)
 	/* Free all the skbuffs in the Rx queue. */
 	for (i = 0; i < mdp->num_rx_ring; i++) {
 		rxdesc = &mdp->rx_ring[i];
-		rxdesc->status = cpu_to_le32(0);
-		rxdesc->addr = cpu_to_le32(0xBADF00D0);
+		rxdesc->status = 0;
+		rxdesc->addr = 0xBADF00D0;
 		dev_kfree_skb(mdp->rx_skbuff[i]);
 		mdp->rx_skbuff[i] = NULL;
 	}
@@ -2477,14 +2473,14 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		kfree_skb(skb);
 		return NETDEV_TX_OK;
 	}
-	txdesc->addr = cpu_to_le32(dma_addr);
-	txdesc->len  = cpu_to_le32(skb->len << 16);
+	txdesc->addr = dma_addr;
+	txdesc->len  = skb->len << 16;
 
 	dma_wmb(); /* TACT bit must be set after all the above writes */
 	if (entry >= mdp->num_tx_ring - 1)
-		txdesc->status |= cpu_to_le32(TD_TACT | TD_TDLE);
+		txdesc->status |= TD_TACT | TD_TDLE;
 	else
-		txdesc->status |= cpu_to_le32(TD_TACT);
+		txdesc->status |= TD_TACT;
 
 	mdp->cur_tx++;
 
-- 
2.13.6

Re: [PATCH 2/2] net: sh_eth: make work on big endian systems

[Sergei Shtylyov]

On 12/04/2017 05:17 PM, Thomas Petazzoni wrote:

> The sh_eth driver uses cpu_to_le32() and le32_to_cpu() to manipulate the
> fields of the DMA descriptors, making the assumption that the DMA 
> descriptors are little-endian.
> 
> However, testing on the Renesas SH7786 running in big-endian mode reveals
> that the DMA descriptors are also big-endian when running big-endian.
> Therefore, all the endianness conversion needs to be removed for the sh_eth
> driver to work.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- 
> Note: I see that Sergei Shtylyov has done some work around endianness on
> this driver back in 2015. I am not sure if it's used on other non-SuperH
> platforms in platforms where the CPU might run big-endian but the IP still
> be little-endian.

   The original Renesas' code assumed that... but the big-endian descriptors
(marked as such) were never used by the arch/sh/ code (and the code wouldn't
work right even if they did!). Therefore I did remove EDMAC_BIG_ENDIAN (and
now EDMAC_LITTLE_ENDIAN can be removed as well).

> If this is the case, then we would need a different approach and more
> discussion.

    No, the ARM platforms sh_eth is used on (mostly R-Car) seem to be
little-endian only.

> Therefore, it would be useful to wait for an ACK from Sergei before 
> applying this patch.
    You have it (-:

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

MBR, Sergei

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

On 12/04/2017 05:17 PM, Thomas Petazzoni wrote:

> This commit adds the sh_eth_cpu_data structure that describes the
> SH7786 variant of the IP.

    The manual seems to be unavailable, so I have to trust you. :-)

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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

[...]

MBR, Sergei

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

On 12/04/2017 07:56 PM, Sergei Shtylyov wrote:

>> This commit adds the sh_eth_cpu_data structure that describes the
>> SH7786 variant of the IP.
> 
>     The manual seems to be unavailable, so I have to trust you. :-)
> 
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

    I actually meant:

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

> [...]

MBR, Sergei

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Thomas Petazzoni]

Hello,

On Mon, 4 Dec 2017 19:56:35 +0300, Sergei Shtylyov wrote:
> On 12/04/2017 05:17 PM, Thomas Petazzoni wrote:
> 
> > This commit adds the sh_eth_cpu_data structure that describes the
> > SH7786 variant of the IP.  
> 
>     The manual seems to be unavailable, so I have to trust you. :-)

Yes, sadly. However, if you tell me what to double check, I'd be happy
to do so.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Geert Uytterhoeven]

On Mon, Dec 4, 2017 at 5:56 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 12/04/2017 05:17 PM, Thomas Petazzoni wrote:
>
>> This commit adds the sh_eth_cpu_data structure that describes the
>> SH7786 variant of the IP.
>
>
>    The manual seems to be unavailable, so I have to trust you. :-)

Google rej09b0501_7786hm.pdf

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

Hello!

On 12/05/2017 10:49 AM, Thomas Petazzoni wrote:

>>> This commit adds the sh_eth_cpu_data structure that describes the
>>> SH7786 variant of the IP.
>>
>>      The manual seems to be unavailable, so I have to trust you. :-)
> 
> Yes, sadly. However, if you tell me what to double check, I'd be happy
> to do so.

    I have the manual now, will check against it...
    DaveM, I'm retracting my ACK for the time being.

> Thanks!
> 
> Thomas

MBR, Sergei

Re: [PATCH 0/2] net: sh_eth: add support for SH7786 and big-endian

[David Miller]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Mon,  4 Dec 2017 15:17:42 +0100

> I've recently been working on an SH7786 based platform, which uses the
> sh_eth network controller. One peculiarity of my setup is that the CPU
> is configured big-endian (even though little-endian is more
> traditional in the Linux SuperH world), and the sh_eth driver was not
> ready for this.
> 
> The first patch simply adds the sh_eth_cpu_data structure that
> describes the SH7786 controller.
> 
> The second patch fixes the driver for big-endian operation. However,
> I'd like this patch to be carefully reviewed by Sergei Shtylyov who
> already did some endianness related changes in this driver. Indeed, my
> change is based on the assumption that the DMA descriptors are in the
> native endianness of the CPU.

Sergei, please let me know when you've re-reviewed this series now
that you have documentation in hand...

Thanks.

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

On 12/05/2017 10:04 PM, Sergei Shtylyov wrote:

>>>> This commit adds the sh_eth_cpu_data structure that describes the
>>>> SH7786 variant of the IP.
>>>
>>>      The manual seems to be unavailable, so I have to trust you. :-)
>>
>> Yes, sadly. However, if you tell me what to double check, I'd be happy
>> to do so.
> 
>     I have the manual now, will check against it...
>     DaveM, I'm retracting my ACK for the time being.

    Starting to look into the manual, the current patch is wrong. SH7786 SoC 
was probably the 1st one to use what we thought was R-Car specific register 
layout. Definite NAK on this version.

>> Thanks!
>>
>> Thomas

MBR, Sergei

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Tue, 5 Dec 2017 22:49:10 +0300

> On 12/05/2017 10:04 PM, Sergei Shtylyov wrote:
> 
>>>>> This commit adds the sh_eth_cpu_data structure that describes the
>>>>> SH7786 variant of the IP.
>>>>
>>>>      The manual seems to be unavailable, so I have to trust you. :-)
>>>
>>> Yes, sadly. However, if you tell me what to double check, I'd be happy
>>> to do so.
>>     I have the manual now, will check against it...
>>     DaveM, I'm retracting my ACK for the time being.
> 
>    Starting to look into the manual, the current patch is wrong. SH7786
>    SoC was probably the 1st one to use what we thought was R-Car specific
>    register layout. Definite NAK on this version.

Ok.

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Thomas Petazzoni]

Hello,

On Tue, 5 Dec 2017 22:49:10 +0300, Sergei Shtylyov wrote:

> >>>> This commit adds the sh_eth_cpu_data structure that describes the
> >>>> SH7786 variant of the IP.  
> >>>
> >>>      The manual seems to be unavailable, so I have to trust you. :-)  
> >>
> >> Yes, sadly. However, if you tell me what to double check, I'd be happy
> >> to do so.  
> > 
> >     I have the manual now, will check against it...
> >     DaveM, I'm retracting my ACK for the time being.  
> 
>     Starting to look into the manual, the current patch is wrong. SH7786 SoC 
> was probably the 1st one to use what we thought was R-Car specific register 
> layout. Definite NAK on this version.

Thanks for the feedback. How do we proceed from there ? I don't have
access to a lot of datasheets of the different Renesas SoCs, so it's
not easy to figure out which IP variant the SH7786 is using compared to
other Renesas SoCs.

Just out of curiosity, which specific aspect makes you think the
proposed patch is wrong ? Have you noticed a specific register or field
that isn't compatible with SH_ETH_REG_FAST_SH4 layout ?

Note that my patch makes Ethernet work in practice on SH7784, I have
root over NFS working as we speak. This certainly doesn't mean that the
patch is entirely correct, but it definitely means that the
SH_ETH_REG_FAST_SH4 is close enough to what the SH7786 is using :-)

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

Hello!

On 12/08/2017 06:40 PM, Thomas Petazzoni wrote:

>>>>>> This commit adds the sh_eth_cpu_data structure that describes the
>>>>>> SH7786 variant of the IP.
>>>>>
>>>>>       The manual seems to be unavailable, so I have to trust you. :-)
>>>>
>>>> Yes, sadly. However, if you tell me what to double check, I'd be happy
>>>> to do so.
>>>
>>>      I have the manual now, will check against it...
>>>      DaveM, I'm retracting my ACK for the time being.
>>
>>      Starting to look into the manual, the current patch is wrong. SH7786 SoC
>> was probably the 1st one to use what we thought was R-Car specific register
>> layout. Definite NAK on this version.
> 
> Thanks for the feedback. How do we proceed from there ? I don't have

    Please use SH_ETH_REG_FAST_RCAR for the register layout.

> access to a lot of datasheets of the different Renesas SoCs, so it's
> not easy to figure out which IP variant the SH7786 is using compared to
> other Renesas SoCs.

    I've already done that for you. :-)

> Just out of curiosity, which specific aspect makes you think the
> proposed patch is wrong ?

    Total Ether register/bit documentation rehaul done for SH7786/R-Car -- 
including the register (and bit) rename and moving the registers to different 
offsets...

> Have you noticed a specific register or field
> that isn't compatible with SH_ETH_REG_FAST_SH4 layout ?

    There are surely SH4 registers that don't exist on SH7786 -- like BCFRR, 
RTRATE, RPADIR, RBWAR, RDFAR, TBRAR, TDFAR...

> Note that my patch makes Ethernet work in practice on SH7784, I have
> root over NFS working as we speak.

   I don't doubt it...

> This certainly doesn't mean that the
> patch is entirely correct, but it definitely means that the
> SH_ETH_REG_FAST_SH4 is close enough to what the SH7786 is using :-)

    SH_ETH_REG_FAST_RCAR is definitely closer. :-)

> Thanks!
> 
> Thomas

MBR, Sergei

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

On 12/04/2017 05:17 PM, Thomas Petazzoni wrote:

> This commit adds the sh_eth_cpu_data structure that describes the
> SH7786 variant of the IP.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 0074c5998481..a3c48b2a713c 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -710,6 +710,30 @@ static struct sh_eth_cpu_data sh7724_data = {
>   	.rpadir_value	= 0x00020000, /* NET_IP_ALIGN assumed to be 2 */
>   };
>   
> +static struct sh_eth_cpu_data sh7786_data = {
> +	.set_duplex	= sh_eth_set_duplex,

    Hm, no bitrate switching?
    (ECMR.OLB is said to be unused indeed in the manual...)

> +
> +	.register_type	= SH_ETH_REG_FAST_SH4,

    SH_ETH_REG_FAST_RCAR.

> +
> +	.ecsr_value	= ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD,
> +	.ecsipr_value	= ECSIPR_PSRTOIP | ECSIPR_LCHNGIP | ECSIPR_ICDIP,

    Good enough (though magic packet interrupt should work too)...

> +	.eesipr_value	= EESIPR_RFCOFIP | EESIPR_ADEIP | EESIPR_ECIIP |
> +			  EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
> +			  EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |
> +			  EESIPR_RMAFIP | EESIPR_RRFIP |
> +			  EESIPR_RTLFIP | EESIPR_RTSFIP |
> +			  EESIPR_PREIP | EESIPR_CERFIP,
> +
> +	.tx_check	= EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
> +	.eesr_err_check	= EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
> +			  EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,

    I think you also need:

	.fdr_value	= 0x00000f0f,

like on R-Car gen1 SoCs...

[...]

    The reset looks good...

MBR, Sergei

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

On 12/10/2017 03:20 PM, Sergei Shtylyov wrote:

[...]

>     The reset looks good...

   Sorry, I meant to type "rest". :-)

MBR, Sergei

Re: [PATCH 1/2] net: sh_eth: add support for SH7786

[Sergei Shtylyov]

On 12/10/2017 03:20 PM, Sergei Shtylyov wrote:

>> This commit adds the sh_eth_cpu_data structure that describes the
>> SH7786 variant of the IP.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> ---
>>   drivers/net/ethernet/renesas/sh_eth.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c 
>> b/drivers/net/ethernet/renesas/sh_eth.c
>> index 0074c5998481..a3c48b2a713c 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -710,6 +710,30 @@ static struct sh_eth_cpu_data sh7724_data = {
>>       .rpadir_value    = 0x00020000, /* NET_IP_ALIGN assumed to be 2 */
>>   };
>> +static struct sh_eth_cpu_data sh7786_data = {
>> +    .set_duplex    = sh_eth_set_duplex,
> 
>     Hm, no bitrate switching?
>     (ECMR.OLB is said to be unused indeed in the manual...)

    I meant CXR20.OLB -- as it's called in the SH7786 and R-Car manuals.

MBR, Sergei