Re: [EXT] Re: [PATCH net] net: freescale: fec: Fix ethtool -d runtime PM

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Jan 03, 2020 at 02:02:50AM +0000, Andy Duan wrote:
> From: Andrew Lunn <andrew@lunn.ch> Sent: Friday, January 3, 2020 9:02 AM
> > > This fix will do, but you should consider implementing
> > > ethtool_ops::begin and ethtool_ops::end to make sure this condition is
> > > resolved for all ethtool operations.
> > >
> > > For instance the following looks possibly problematic too:
> > > fec_enet_set_coalesce -> fec_enet_itr_coal_set
> > 
> > Hi Florian
> > 
> > I did a quick test of all the ethtool operations which the driver supports,
> > including setting coalescing. I did not exhaustively try all possible coalescing
> > settings, but the ones i did try did not provoke a data abort.
>

> The original design is that driver power off clocks when net interface is down,
> use ethtool to dump registers are not allowed. Only .get_regs/ .get/set_coalesce
> are allowed when the net interface is up by checking netif_running(ndev).

Hi Andy

The function fec_enet_get_regs() does not perform a
netif_running(ndev) check. So it seems like it was intended to work
with the interface down. It could be a difference between variants of
the FEC. I was seeing the data abort on the vf610. Maybe its clocks
are different to other FEC implementations?

I've no strong preference about seeing the registers when the
interface is down, or return maybe -ENETDOWN.  What i don't want is a
data abort, and locks left in a bad state.

    Andrew