[RFC,net-next,v1,1/1] net: fec: Fix RMON registers on imx6

commit 38ae92d "fec: Add support for reading
RMON registers" causes the imx6Q to crash.

This fixes it and clears it using the MIB Control
Register's MIB_CLEAR bit.

Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
---
 drivers/net/ethernet/freescale/fec_main.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

On 06/28/13 23:08, Jim Baxter wrote:

> commit 38ae92d "fec: Add support for reading RMON registers" causes the imx6Q to crash.
>
> This fixes it and clears it using the MIB Control Register's MIB_CLEAR bit.
>
> Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index ed6180e..a1f0413 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -607,8 +607,12 @@ fec_restart(struct net_device *ndev, int duplex)  #ifndef CONFIG_M5272
>  	/* Disable, clear, and enable the MIB */
>  	writel(1 << 31, fep->hwp + FEC_MIB_CTRLSTAT);
> -	for (i = RMON_T_DROP; i < IEEE_R_OCTETS_OK; i++)
> -		writel(0, fep->hwp + i);
> +	if (strcmp(id_entry->name, "imx6q-fec") == 0) {
> +		writel(1 << 29, fep->hwp + FEC_MIB_CTRLSTAT);
> +	} else {
> +		for (i = RMON_T_DROP; i < IEEE_R_OCTETS_OK; i++)
> +			writel(0, fep->hwp + i);
> +	}
>  	writel(0, fep->hwp + FEC_MIB_CTRLSTAT);  #endif
>  
> --
> 1.7.10.4

It is not necessary to clear MIB since fec_restart() will reset MAC which cause all MAC register auto reset to  zero.
 
 
Thanks,
Andy

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 30/06/13 04:55, Chris Healy wrote:
> On the i.MX28, I tested just setting bit 31 to 0 and not doing any of
> the clearing work and it performed correctly.  I'd suggest we just turn
> on the MIB logic in fec_restart regardless of which i.MX platform it
> is.  (Still need to not do anything if it is one of the coldfire
> platforms though.  Jim, would you like to do this our would you like me to?
> 
> Chris
> 
> 
> On Sat, Jun 29, 2013 at 3:00 PM, Chris Healy <cphealy@gmail.com
> <mailto:cphealy@gmail.com>> wrote:
> 
>     I took a look at what was going on in the various use cases with and
>     without the i.MX6x.
> 
>     The i.MX6x AND i.MX28 both have bit 29 which clears all the RMON
>     regs to zero when written to.  I was not aware of bit 29 with the
>     i.MX28 as I originally did this function for the i.MX51 and i.MX53
>     which do not have bit 29.
> 
>     Regardless, according to Andy, clearing the RMON registers in
>     fec_restart is moot as fec_restart restarts the MAC.  This being
>     true, we could probably just enable the MIB by setting bit 31 of
>     FEC_MIB_CTRLSTAT and be done.
> 
>     I have not tested this yet though.
> 
>     Chris
> 
> 
>     On Fri, Jun 28, 2013 at 10:58 PM, Duan Fugang-B38611
>     <B38611@freescale.com <mailto:B38611@freescale.com>> wrote:
> 
>         On 06/28/13 23:08, Jim Baxter wrote:
> 
>         > commit 38ae92d "fec: Add support for reading RMON registers"
>         causes the imx6Q to crash.
>         >
>         > This fixes it and clears it using the MIB Control Register's
>         MIB_CLEAR bit.
>         >
>         > Signed-off-by: Jim Baxter <jim_baxter@mentor.com
>         <mailto:jim_baxter@mentor.com>>
>         > ---
>         >  drivers/net/ethernet/freescale/fec_main.c |    8 ++++++--
>         >  1 file changed, 6 insertions(+), 2 deletions(-)
>         >
>         > diff --git a/drivers/net/ethernet/freescale/fec_main.c
>         b/drivers/net/ethernet/freescale/fec_main.c
>         > index ed6180e..a1f0413 100644
>         > --- a/drivers/net/ethernet/freescale/fec_main.c
>         > +++ b/drivers/net/ethernet/freescale/fec_main.c
>         > @@ -607,8 +607,12 @@ fec_restart(struct net_device *ndev, int
>         duplex)  #ifndef CONFIG_M5272
>         >       /* Disable, clear, and enable the MIB */
>         >       writel(1 << 31, fep->hwp + FEC_MIB_CTRLSTAT);
>         > -     for (i = RMON_T_DROP; i < IEEE_R_OCTETS_OK; i++)
>         > -             writel(0, fep->hwp + i);
>         > +     if (strcmp(id_entry->name, "imx6q-fec") == 0) {
>         > +             writel(1 << 29, fep->hwp + FEC_MIB_CTRLSTAT);
>         > +     } else {
>         > +             for (i = RMON_T_DROP; i < IEEE_R_OCTETS_OK; i++)
>         > +                     writel(0, fep->hwp + i);
>         > +     }
>         >       writel(0, fep->hwp + FEC_MIB_CTRLSTAT);  #endif
>         >
>         > --
>         > 1.7.10.4
> 
>         It is not necessary to clear MIB since fec_restart() will reset
>         MAC which cause all MAC register auto reset to  zero.
> 
> 
>         Thanks,
>         Andy
> 
> 
> 
Hi Chris,

I have submitted another patch, please check if that is what you meant?

Thank you,
Jim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html