On 8/5/26 18:40, David Laight wrote:
> 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.

Sure, but that feels like a change for a separate patch.
The changes to this file are a global search and replace.
The final produced object is identical before and after.

Regards
Greg


> 	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");
>>>   	}
>>> 
>>
>>
>