[PATCH 0/2] sh_eth: RPADIR related clean-ups

[PATCH 0/2] sh_eth: RPADIR related clean-ups

[Sergei Shtylyov]

Hello!

Here's a set of 2 patches against DaveM's 'net-next.git' repo. They are
clean-ups related to RPADIR (DMA padding to NET_IP_ALIGN)...

[1/2] sh_eth: fix *enum* RPADIR_BIT
[2/2] sh_eth: remove sh_eth_cpu_data::rpadir_value

MBR, Sergei

[PATCH 1/2] sh_eth: fix *enum* RPADIR_BIT

[Sergei Shtylyov]

The *enum*  RPADIR_BIT  was declared in the commit 86a74ff21a7a ("net:
sh_eth: add support for Renesas SuperH Ethernet") adding SH771x support,
however the SH771x manual doesn't have the RPADIR register described and,
moreover, tells why the padding insertion must not be used. The newer SoC
manuals do have RPADIR documented, though with somewhat different layout --
update the *enum* according to these manuals...

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

---
 drivers/net/ethernet/renesas/sh_eth.h |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: net-next/drivers/net/ethernet/renesas/sh_eth.h
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
+++ net-next/drivers/net/ethernet/renesas/sh_eth.h
@@ -403,8 +403,7 @@ enum DESC_I_BIT {
 
 /* RPADIR */
 enum RPADIR_BIT {
-	RPADIR_PADS1 = 0x20000, RPADIR_PADS0 = 0x10000,
-	RPADIR_PADR = 0x0003f,
+	RPADIR_PADS = 0x1f0000, RPADIR_PADR = 0xffff,
 };
 
 /* FDR */

Re: [PATCH 1/2] sh_eth: fix *enum* RPADIR_BIT

[Geert Uytterhoeven]

Hi Sergei,

On Mon, Jun 25, 2018 at 10:37 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> The *enum*  RPADIR_BIT  was declared in the commit 86a74ff21a7a ("net:
> sh_eth: add support for Renesas SuperH Ethernet") adding SH771x support,
> however the SH771x manual doesn't have the RPADIR register described and,
> moreover, tells why the padding insertion must not be used. The newer SoC
> manuals do have RPADIR documented, though with somewhat different layout --
> update the *enum* according to these manuals...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
> +++ net-next/drivers/net/ethernet/renesas/sh_eth.h
> @@ -403,8 +403,7 @@ enum DESC_I_BIT {
>
>  /* RPADIR */
>  enum RPADIR_BIT {
> -       RPADIR_PADS1 = 0x20000, RPADIR_PADS0 = 0x10000,
> -       RPADIR_PADR = 0x0003f,
> +       RPADIR_PADS = 0x1f0000, RPADIR_PADR = 0xffff,

Perhaps add some comments?

        RPADIR_PADS = 0x1f0000; /* Padding Size (insert N bytes of padding) */
        RPADIR_PADR = 0xffff;   /* Padding Slot (insert padding at byte N) */
>  };

Note that none of the RPADIR enums are actually used.

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] sh_eth: fix *enum* RPADIR_BIT

[Sergei Shtylyov]

On 6/26/2018 10:25 AM, Geert Uytterhoeven wrote:

>> The *enum*  RPADIR_BIT  was declared in the commit 86a74ff21a7a ("net:
>> sh_eth: add support for Renesas SuperH Ethernet") adding SH771x support,
>> however the SH771x manual doesn't have the RPADIR register described and,
>> moreover, tells why the padding insertion must not be used. The newer SoC
>> manuals do have RPADIR documented, though with somewhat different layout --
>> update the *enum* according to these manuals...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
>> --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
>> +++ net-next/drivers/net/ethernet/renesas/sh_eth.h
>> @@ -403,8 +403,7 @@ enum DESC_I_BIT {
>>
>>   /* RPADIR */
>>   enum RPADIR_BIT {
>> -       RPADIR_PADS1 = 0x20000, RPADIR_PADS0 = 0x10000,
>> -       RPADIR_PADR = 0x0003f,
>> +       RPADIR_PADS = 0x1f0000, RPADIR_PADR = 0xffff,
> 
> Perhaps add some comments?
> 
>          RPADIR_PADS = 0x1f0000; /* Padding Size (insert N bytes of padding) */
>          RPADIR_PADR = 0xffff;   /* Padding Slot (insert padding at byte N) */

    It would be nice but inconsistent with what we do for the other registers...

>>   };
> 
> Note that none of the RPADIR enums are actually used.

    I'd surely noted that. :-)

> Gr{oetje,eeting}s,
> 
>                          Geert
> 

MBR, Sergei

[PATCH 2/2] sh_eth: remove sh_eth_cpu_data::rpadir_value

[Sergei Shtylyov]

If RPADIR exists, the value written to it is always the same for all SoCs
(and derived from NET_IP_ALIGN), so there has not  been any need to store
it in the *struct* sh_eth_cpu_data...

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

---
 drivers/net/ethernet/renesas/sh_eth.c |    8 +-------
 drivers/net/ethernet/renesas/sh_eth.h |    1 -
 2 files changed, 1 insertion(+), 8 deletions(-)

Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net-next/drivers/net/ethernet/renesas/sh_eth.c
@@ -622,7 +622,6 @@ static struct sh_eth_cpu_data r7s72100_d
 	.tpauser	= 1,
 	.hw_swap	= 1,
 	.rpadir		= 1,
-	.rpadir_value   = 2 << 16,
 	.no_trimd	= 1,
 	.no_ade		= 1,
 	.xdfar_rw	= 1,
@@ -672,7 +671,6 @@ static struct sh_eth_cpu_data r8a7740_da
 	.bculr		= 1,
 	.hw_swap	= 1,
 	.rpadir		= 1,
-	.rpadir_value   = 2 << 16,
 	.no_trimd	= 1,
 	.no_ade		= 1,
 	.xdfar_rw	= 1,
@@ -798,7 +796,6 @@ static struct sh_eth_cpu_data r8a77980_d
 	.hw_swap	= 1,
 	.nbst		= 1,
 	.rpadir		= 1,
-	.rpadir_value   = 2 << 16,
 	.no_trimd	= 1,
 	.no_ade		= 1,
 	.xdfar_rw	= 1,
@@ -851,7 +848,6 @@ static struct sh_eth_cpu_data sh7724_dat
 	.tpauser	= 1,
 	.hw_swap	= 1,
 	.rpadir		= 1,
-	.rpadir_value	= 0x00020000, /* NET_IP_ALIGN assumed to be 2 */
 };
 
 static void sh_eth_set_rate_sh7757(struct net_device *ndev)
@@ -898,7 +894,6 @@ static struct sh_eth_cpu_data sh7757_dat
 	.hw_swap	= 1,
 	.no_ade		= 1,
 	.rpadir		= 1,
-	.rpadir_value   = 2 << 16,
 	.rtrate		= 1,
 	.dual_port	= 1,
 };
@@ -978,7 +973,6 @@ static struct sh_eth_cpu_data sh7757_dat
 	.bculr		= 1,
 	.hw_swap	= 1,
 	.rpadir		= 1,
-	.rpadir_value   = 2 << 16,
 	.no_trimd	= 1,
 	.no_ade		= 1,
 	.xdfar_rw	= 1,
@@ -1467,7 +1461,7 @@ static int sh_eth_dev_init(struct net_de
 	/* Descriptor format */
 	sh_eth_ring_format(ndev);
 	if (mdp->cd->rpadir)
-		sh_eth_write(ndev, mdp->cd->rpadir_value, RPADIR);
+		sh_eth_write(ndev, NET_IP_ALIGN << 16, RPADIR);
 
 	/* all sh_eth int mask */
 	sh_eth_write(ndev, 0, EESIPR);
Index: net-next/drivers/net/ethernet/renesas/sh_eth.h
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
+++ net-next/drivers/net/ethernet/renesas/sh_eth.h
@@ -487,7 +487,6 @@ struct sh_eth_cpu_data {
 	u32 ecsipr_value;
 	u32 fdr_value;
 	u32 fcftr_value;
-	u32 rpadir_value;
 
 	/* interrupt checking mask */
 	u32 tx_check;

Re: [PATCH 2/2] sh_eth: remove sh_eth_cpu_data::rpadir_value

[Geert Uytterhoeven]

On Mon, Jun 25, 2018 at 10:38 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> If RPADIR exists, the value written to it is always the same for all SoCs
> (and derived from NET_IP_ALIGN), so there has not  been any need to store
> it in the *struct* sh_eth_cpu_data...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 0/2] sh_eth: RPADIR related clean-ups

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Mon, 25 Jun 2018 23:34:52 +0300

> Here's a set of 2 patches against DaveM's 'net-next.git' repo. They are
> clean-ups related to RPADIR (DMA padding to NET_IP_ALIGN)...

Series applied.

[PATCH net-next] sh_eth: fix *enum* {A|M}PR_BIT

[Sergei Shtylyov]

The *enum* {A|M}PR_BIT were declared in the commit 86a74ff21a7a ("net:
sh_eth: add support for  Renesas SuperH Ethernet") adding SH771x support,
however the SH771x manual  doesn't have the APR/MPR registers described
and the code writing to them for SH7710 was later removed by the commit
380af9e390ec ("net: sh_eth: CPU dependency code collect to "struct
sh_eth_cpu_data""). All the newer SoC manuals have these registers
documented as having a 16-bit TIME parameter of the PAUSE frame, not
1-bit -- update the *enum* accordingly, fixing up the APR/MPR writes...

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

---
This patch is against DaveM's 'net-next.git' repo.

 drivers/net/ethernet/renesas/sh_eth.c |    4 ++--
 drivers/net/ethernet/renesas/sh_eth.h |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net-next/drivers/net/ethernet/renesas/sh_eth.c
@@ -1521,9 +1521,9 @@ static int sh_eth_dev_init(struct net_de
 
 	/* mask reset */
 	if (mdp->cd->apr)
-		sh_eth_write(ndev, APR_AP, APR);
+		sh_eth_write(ndev, 1, APR);
 	if (mdp->cd->mpr)
-		sh_eth_write(ndev, MPR_MP, MPR);
+		sh_eth_write(ndev, 1, MPR);
 	if (mdp->cd->tpauser)
 		sh_eth_write(ndev, TPAUSER_UNLIMITED, TPAUSER);
 
Index: net-next/drivers/net/ethernet/renesas/sh_eth.h
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
+++ net-next/drivers/net/ethernet/renesas/sh_eth.h
@@ -383,12 +383,12 @@ enum ECSIPR_STATUS_MASK_BIT {
 
 /* APR */
 enum APR_BIT {
-	APR_AP = 0x00000001,
+	APR_AP = 0x0000ffff,
 };
 
 /* MPR */
 enum MPR_BIT {
-	MPR_MP = 0x00000001,
+	MPR_MP = 0x0000ffff,
 };
 
 /* TRSCER */

Re: [PATCH net-next] sh_eth: fix *enum* {A|M}PR_BIT

[Geert Uytterhoeven]

On Tue, Jun 26, 2018 at 5:43 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> The *enum* {A|M}PR_BIT were declared in the commit 86a74ff21a7a ("net:
> sh_eth: add support for  Renesas SuperH Ethernet") adding SH771x support,
> however the SH771x manual  doesn't have the APR/MPR registers described
> and the code writing to them for SH7710 was later removed by the commit
> 380af9e390ec ("net: sh_eth: CPU dependency code collect to "struct
> sh_eth_cpu_data""). All the newer SoC manuals have these registers
> documented as having a 16-bit TIME parameter of the PAUSE frame, not
> 1-bit -- update the *enum* accordingly, fixing up the APR/MPR writes...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 net-next] sh_eth: fix *enum* {A|M}PR_BIT

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Tue, 26 Jun 2018 18:42:33 +0300

> The *enum* {A|M}PR_BIT were declared in the commit 86a74ff21a7a ("net:
> sh_eth: add support for  Renesas SuperH Ethernet") adding SH771x support,
> however the SH771x manual  doesn't have the APR/MPR registers described
> and the code writing to them for SH7710 was later removed by the commit
> 380af9e390ec ("net: sh_eth: CPU dependency code collect to "struct
> sh_eth_cpu_data""). All the newer SoC manuals have these registers
> documented as having a 16-bit TIME parameter of the PAUSE frame, not
> 1-bit -- update the *enum* accordingly, fixing up the APR/MPR writes...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Applied.