Re: [PATCHv3 net-next] net: fec: Ensure clocks are enabled while using mdio bus

[Andrew Lunn <andrew@lunn.ch>]

> > int mii_id, int regnum)  {
> >  	struct fec_enet_private *fep = bus->priv;
> >  	unsigned long time_left;
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(fep->clk_ipg);
> > +	if (ret)
> > +		return ret;
> > 
> >  	fep->mii_timeout = 0;
> >  	init_completion(&fep->mdio_done);
> > @@ -1779,11 +1785,14 @@ static int fec_enet_mdio_read(struct mii_bus *bus,
> > int mii_id, int regnum)
> >  	if (time_left == 0) {
> >  		fep->mii_timeout = 1;
> >  		netdev_err(fep->netdev, "MDIO read timeout\n");
> > +		clk_disable_unprepare(fep->clk_ipg);
> >  		return -ETIMEDOUT;
> >  	}
> > 
> > -	/* return value */
> > -	return FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA));
> > +	ret = FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA));
> > +	clk_disable_unprepare(fep->clk_ipg);
> > +
> > +	return ret;
> >  }
> > 
> 

> I suggest you use runtime pm to enable/disable clock for performance
> consideration. Not every time for mdio bus access needs to
> enable/disable clock.

Can you explain that some more. When are you suggesting doing a
runtime enable/disable? Given the current DSA architecture, i would
probably do a runtime enable as soon as i lookup the mdio bus, and
never do a runtime disable. Also, how would you include this runtime
pm into the phy state machine which is going to be polling your mdio
bus around once per second for normal phys?

Florian, as Maintainer of the phy state machine, what do you think
about adding runtime PM to it?

  Thanks
	Andrew