On Fri, 8 May 2026 02:46:38 +0000
Wei Fang <wei.fang@nxp.com> wrote:

> >  static void
> >  fec_stop(struct net_device *ndev)
> >  {
> >  	struct fec_enet_private *fep = netdev_priv(ndev);
> > -	u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & FEC_RCR_RMII;
> > +	u32 rmii_mode = fec_readl(fep->hwp + FEC_R_CNTRL) & FEC_RCR_RMII;  
> 
> This is not an issue, but since you changed this line, the new code should
> follow the "reverse xmas tree" style.

Looking rmii_mode isn't even used until much later in the function.
(and then not very often)
Much better to read it just before it is needed.

	David


> 
> See: https://elixir.bootlin.com/linux/v7.0.1/source/Documentation/process/maintainer-netdev.rst#L380
> 
> >  	u32 val;
> > 
> >  	/* We cannot expect a graceful transmit stop without link !!! */
> >  	if (fep->link) {
> > -		writel(1, fep->hwp + FEC_X_CNTRL); /* Graceful transmit stop */
> > +		fec_writel(1, fep->hwp + FEC_X_CNTRL); /* Graceful transmit stop */
> >  		udelay(10);
> > -		if (!(readl(fep->hwp + FEC_IEVENT) & FEC_ENET_GRA))
> > +		if (!(fec_readl(fep->hwp + FEC_IEVENT) & FEC_ENET_GRA))
> >  			netdev_err(ndev, "Graceful transmit stop did not complete!\n");
> >  	}
> >   
> 
>