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

[David Miller]

From: Chris Brandt 
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 0x which can cause a memory
> corruption or panic.
> 
> This patch fixes the panic and properly enables CAM.
> 
> Reported-by: Daniel Palmer 
> Signed-off-by: Chris Brandt 

Applied, thanks.

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

[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 0x which can cause a memory
corruption or panic.

This patch fixes the panic and properly enables CAM.

Reported-by: Daniel Palmer 
Signed-off-by: Chris Brandt 
---
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

[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 0x 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 
Signed-off-by: Chris Brandt 
---
v2:
* POST registers really do exist, so just add them


Acked-by: Sergei Shtylyov 

MBR, Sergei