Re: [PATCH resubmit 2] net: fec: Fix FEC_ECR_EN1588 being cleared on link-down
From: Csókás Bence
Date: Fri Jun 14 2024 - 04:00:48 EST
On 6/13/24 17:12, Jakub Kicinski wrote:
On Tue, 11 Jun 2024 10:04:05 +0200 Csókás, Bence wrote:
+ if (fep->bufdesc_ex) {
+ val = readl(fep->hwp + FEC_ECNTRL);
+ val |= FEC_ECR_EN1588;
+ writel(val, fep->hwp + FEC_ECNTRL);
FEC_ECNTRL gets written multiple times in this function,
including with 0, and then you RMW it to add this flag.
Is this intentional? It really seems like you should be
adding this flag more consistently or making sure its
not cleared, rather than appending "add it back" at the
end of the function...
It only writes 0 if WOL is disabled AND the device has the MULTI_QUEUES
quirk. Otherwise, we either write FEC_ECR_RESET, which resets the device
(and the HW changes ECNTRL to its reset value), OR we RMW set the WOL
sleep bits. And then, if some more quirks are set, we set ETHEREN.
So I think RMW is the safest route here, instead of trying to keep track
of all these different branches, re-read ECNTRL after reset etc.
Bence