RE: [PATCH net-next v10 08/11] onsemi: s2500: Add driver support for S2500 MAC-PHY

[Selvamani Rajagopal]

> -----Original Message-----
> From: netdev-bot+sashiko@xxxxxxxxxx <netdev-bot+sashiko@xxxxxxxxxx>
> Subject: Re: [PATCH net-next v10 08/11] onsemi: s2500: Add driver support for S2500
> MAC-PHY

> 
> > + if (end_valid && frame_drop) {
> > + oa_tc6_cleanup_ongoing_rx_skb(tc6);
> 
> [Severity: Medium]
> Nothing here checks tc6->quirk_flags & OA_TC6_CONFIG_RXCTE or
> tc6->ctc_capable. Doesn't this change RX behaviour for every OA TC6
> driver?

Yes. It does. But dropping frames based on FD bit is in the specification.
I don't see any issue enabling this. In fact, ignoring this bit would cause
driver to receive malformed or invalid frame.

I fixed the commit message to make it clear

> 
> [Severity: Medium]
> This goto was added earlier in the series, not here, but the new
> cut-through code sits right after it. Doesn't this oops if the STDCAP
> read fails?

Yes. Fixing it.

> 
> 
> 
> 
> [Severity: Medium]
> Does the S2500 latch PTPNSEC when PTP_SEC is read? The octet statistics
> registers have a comment describing this kind of latching, but the PTP
> time registers have nothing similar.
> 
> Without such a latch, a read near a seconds rollover could pair the old
> second with a wrapped nanosecond value, and be off by almost a second.


We are doing the best we could do. That is reading both registers in a single read.
Nothing to fix here.


> > + }
> 
> [ ... ]
> 
> > +
> > +static const char s2500_mac_stat_strings[][ETH_GSTRING_LEN] = {
> > + "rx_mismatch_drops",
> > +};
> > +
> 
> [Severity: Medium]
> Isn't this counter already covered by the standard netdev stats?


I did consider this. To use the existing mechanism, I have to add a polling worker
just to accommodate one counter. Because API doesn't allow blocking type SPI calls 
to gather stats.So decided to keep this way.

> 
> 
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106-s2500-
> mac-phy-support-v10-0-31b5a69b85b7%40onsemi.com
> <https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106-s2500-mac-phy-support-v10-0-31b5a69b85b7%40onsemi.com
> =netdev-ai.bots.linux.dev>