[PATCH v2] net: ethernet: renesas: sh_eth: add POST registers for rz

[PATCH v2] net: ethernet: renesas: sh_eth: add POST registers for rz

[Chris Brandt]

Due to a mistake in the hardware manual, the FWSLC and POST1-4 registers
were not documented and left out of the driver for RZ/A making the CAM
feature non-operational.
Additionally, when the offset values for POST1-4 are left blank, the driver
attempts to set them using an offset of 0xFFFF which can cause a memory
corruption or panic.

This patch fixes the panic and properly enables CAM.

Reported-by: Daniel Palmer <daniel@0x0f.com>
Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
* POST registers really do exist, so just add them
---
 drivers/net/ethernet/renesas/sh_eth.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 1f8240a..440ae27 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -201,9 +201,14 @@ static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
 
 	[ARSTR]		= 0x0000,
 	[TSU_CTRST]	= 0x0004,
+	[TSU_FWSLC]	= 0x0038,
 	[TSU_VTAG0]	= 0x0058,
 	[TSU_ADSBSY]	= 0x0060,
 	[TSU_TEN]	= 0x0064,
+	[TSU_POST1]	= 0x0070,
+	[TSU_POST2]	= 0x0074,
+	[TSU_POST3]	= 0x0078,
+	[TSU_POST4]	= 0x007c,
 	[TSU_ADRH0]	= 0x0100,
 
 	[TXNLCR0]	= 0x0080,
@@ -2781,6 +2786,8 @@ 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 */
+		sh_eth_tsu_write(mdp, TSU_FWSLC_POSTENU | TSU_FWSLC_POSTENL,
+				 TSU_FWSLC);	/* Enable POST registers */
 		return;
 	}
 
-- 
2.9.2

Re: [PATCH v2] net: ethernet: renesas: sh_eth: add POST registers for rz

[Sergei Shtylyov]

Hello.

On 09/07/2016 09:57 PM, Chris Brandt wrote:

> Due to a mistake in the hardware manual, the FWSLC and POST1-4 registers
> were not documented and left out of the driver for RZ/A making the CAM
> feature non-operational.
> Additionally, when the offset values for POST1-4 are left blank, the driver
> attempts to set them using an offset of 0xFFFF which can cause a memory
> corruption or panic.

    You didn't really fix the root cause here...

> This patch fixes the panic and properly enables CAM.
>
> Reported-by: Daniel Palmer <daniel@0x0f.com>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2:
> * POST registers really do exist, so just add them

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

MBR, Sergei

Re: [PATCH v2] net: ethernet: renesas: sh_eth: add POST registers for rz

[Sergei Shtylyov]

On 09/07/2016 09:57 PM, Chris Brandt wrote:

> Due to a mistake in the hardware manual, the FWSLC and POST1-4 registers
> were not documented and left out of the driver for RZ/A making the CAM
> feature non-operational.
> Additionally, when the offset values for POST1-4 are left blank, the driver
> attempts to set them using an offset of 0xFFFF which can cause a memory
> corruption or panic.
>
> This patch fixes the panic and properly enables CAM.
>
> Reported-by: Daniel Palmer <daniel@0x0f.com>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2:
> * POST registers really do exist, so just add them
> ---
>  drivers/net/ethernet/renesas/sh_eth.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 1f8240a..440ae27 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
[...]
> @@ -2781,6 +2786,8 @@ 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 */
> +		sh_eth_tsu_write(mdp, TSU_FWSLC_POSTENU | TSU_FWSLC_POSTENL,
> +				 TSU_FWSLC);	/* Enable POST registers */
>  		return;
>  	}

    Wait, don't you also need to write 0s to the POST registers like done at 
the end of this function?

MBR, Sergei

RE: [PATCH v2] net: ethernet: renesas: sh_eth: add POST registers for rz

[Chris Brandt]

On 9/9/2016, Sergei Shtylyov wrote:
> > sh_eth_private *mdp)  {
> >  	if (sh_eth_is_rz_fast_ether(mdp)) {
> >  		sh_eth_tsu_write(mdp, 0, TSU_TEN); /* Disable all CAM entry */
> > +		sh_eth_tsu_write(mdp, TSU_FWSLC_POSTENU | TSU_FWSLC_POSTENL,
> > +				 TSU_FWSLC);	/* Enable POST registers */
> >  		return;
> >  	}
> 
>     Wait, don't you also need to write 0s to the POST registers like done
> at the end of this function?

Nope.

The sh_eth_chip_reset() function will write to register ARSTR which will do a HW reset on the block and clear all the registers, including all the POST registers.

static struct sh_eth_cpu_data r7s72100_data = {
	.chip_reset	= sh_eth_chip_reset,


So, before sh_eth_tsu_init() is ever called, the hardware will always be reset.


	/* initialize first or needed device */
	if (!devno || pd->needs_init) {
		if (mdp->cd->chip_reset)
			mdp->cd->chip_reset(ndev);

		if (mdp->cd->tsu) {
			/* TSU init (Init only)*/
			sh_eth_tsu_init(mdp);
		}
	}


Therefore there is no reason to set the POST registers back to 0 because they are already at 0 from the reset.


Chris

Re: [PATCH v2] net: ethernet: renesas: sh_eth: add POST registers for rz

[David Miller]

From: Chris Brandt <chris.brandt@renesas.com>
Date: Wed,  7 Sep 2016 14:57:09 -0400

> Due to a mistake in the hardware manual, the FWSLC and POST1-4 registers
> were not documented and left out of the driver for RZ/A making the CAM
> feature non-operational.
> Additionally, when the offset values for POST1-4 are left blank, the driver
> attempts to set them using an offset of 0xFFFF which can cause a memory
> corruption or panic.
> 
> This patch fixes the panic and properly enables CAM.
> 
> Reported-by: Daniel Palmer <daniel@0x0f.com>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Applied, thanks.