Re: [PATCH net-next v8 03/11] r8152: add rtl8157_change_mtu() for RTL8157 and RTL8159

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch carries two Fixes: tags, so it will go to stable, but
  the commit message gives no user-visible symptom for the old behaviour.

--

r8152: add rtl8157_change_mtu() for RTL8157 and RTL8159

This adds rtl8157_change_mtu() for RTL8157 and RTL8159. It sets
PLA_MTPS from the configured MTU (max_pkt_size / 64, at least 10K,
capped at 0xff) instead of the fixed MTPS_JUMBO that rtl8156_change_mtu()
uses. rtl8156_up() now calls tp->rtl_ops.change_mtu() so each chip gets
its own helper.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index bb4106f238a3a..1517753f843ef 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -6800,6 +6800,28 @@ static void rtl8156_change_mtu(struct r8152 *tp)
>  		       ALIGN(rx_max_size + tp->tx_desc.size, 1024) / 16);
>  }
>  
> +static void rtl8157_change_mtu(struct r8152 *tp)
> +{
> +	u32 max_pkt_size = mtu_to_size(tp->netdev->mtu);
> +	u32 ocp_data;
> +
> +	ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, max_pkt_size);
> +
> +	/* Use at least 10K for MTPS */
> +	ocp_data = max_t(u32, max_pkt_size, 10 * 1024) / 64;
> +
> +	/* 16 * 1024 / 64 = 0x100, so the max is 0xff for 8 bits data */
> +	ocp_data = min_t(u32, ocp_data, 0xff);
> +
> +	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, ocp_data);

[Severity: Low]
Could the commit message say what user-visible problem the old fixed
value caused?

The patch has two Fixes: tags, so it will probably be picked up for
stable. The only reason the message gives is:

  Per the datasheet they need an MTPS scaled from the configured MTU
  instead: max_pkt_size / 64, at least 10K and capped at 0xff.

Before this patch, RTL8157 and RTL8159 always wrote MTPS_JUMBO to
PLA_MTPS through rtl8156_change_mtu():

  #define MTPS_JUMBO		(12 * 1024 / 64)

That value is 192. At the default MTU of 1500, mtu_to_size() returns
1522. So rtl8157_change_mtu() now writes max(1522, 10240) / 64 = 160,
which is 10K instead of 12K.

For any MTU below about 12266, MTPS is now lower than it was before.

Did the 12K setting cause a failure, such as stalled or dropped TX for
jumbo frames above 12K? Or is this only a change to match the datasheet
recommendation?

Could the message also say plainly that every RTL8157/RTL8159 at the
default MTU now gets a smaller MTPS?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110032416.16968-1-nic_swsd%40realtek.com