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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] net: ptp: max_adj exceeds 32-bit scaled_ppm limit
- [Low] net: ethtool: duplication of standard rx_dropped stat
--

commit 00c8e30757f8b1220a6ef1275c224d8bbca5148c
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

    onsemi: s2500: Add driver support for S2500 MAC-PHY
    
    Support for onsemi's S2500, 802.3 cg compliant Ethernet transceiver with
    integrated MAC-PHY.

diff --git a/drivers/net/ethernet/onsemi/s2500/s2500_ethtool.c b/drivers/net/ethernet/onsemi/s2500/s2500_ethtool.c
--- /dev/null
+++ b/drivers/net/ethernet/onsemi/s2500/s2500_ethtool.c
[ ... ]
> +static const char s2500_mac_stat_strings[][ETH_GSTRING_LEN] = {
> +	"rx_mismatch_drops",
> +};
> +
> +static const u8 s2500_mac_stat_map[S2500_NUM_MAC_STATS] = {
> +	S2500_MSTRXDROPPED_IDX,
> +};

[Severity: Low]
Does this regression duplicate a standard uAPI statistic? The network core
guidelines indicate that counters with standard representations must not be
exposed as private ethtool -S strings. Since there is a standard representation
for dropped frames, could this private string be removed to avoid duplication?

[ ... ]
diff --git a/drivers/net/ethernet/onsemi/s2500/s2500_hw_def.h b/drivers/net/ethernet/onsemi/s2500/s2500_hw_def.h
--- /dev/null
+++ b/drivers/net/ethernet/onsemi/s2500/s2500_hw_def.h
[ ... ]
> +/* Frequency adjust magnitude, 1 LSB = 0.000125 ppm = 1/8 ppb */
> +#define S2500_PTP_ADJ_MAX		GENMASK(27, 0)
> +#define S2500_PTP_MAX_ADJ_PPB		(S2500_PTP_ADJ_MAX / 8)
[ ... ]
diff --git a/drivers/net/ethernet/onsemi/s2500/s2500_ptp.c b/drivers/net/ethernet/onsemi/s2500/s2500_ptp.c
--- /dev/null
+++ b/drivers/net/ethernet/onsemi/s2500/s2500_ptp.c
[ ... ]
> +int s2500_ptp_register(struct s2500_info *priv)
> +{
> +	struct ptp_clock_info *info = &priv->ptp_clock_info;
> +	int ret;
> +
> +	info->max_adj = S2500_PTP_MAX_ADJ_PPB;

[Severity: Medium]
Will this regression cause the max_adj value to overflow on 32-bit
architectures? The driver advertises a max_adj of 268435455 / 8 = 33,554,431
PPB. The PTP subsystem converts PPB to scaled_ppm by multiplying by 65.536 and
passes the result as a signed long to the driver's adjfine callback.

On 32-bit architectures, 33554431 * 65.536 = 2199023243, which exceeds the
32-bit signed long maximum of 2147483647.

Could this mathematical wrap-around directly cause large positive frequency
adjustments to be passed as negative values into s2500_ptp_adjfine(), driving
severe clock drift in the opposite direction?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-s2500-mac-phy-support-v9-0-dcefe1d0bf0d@onsemi.com?part=8