[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20251108051451.GC4126953@ragnatech.se>
Date: Sat, 8 Nov 2025 06:14:51 +0100
From: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
To: Simon Horman <horms@...nel.org>, Paul Barker <paul@...rker.dev>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Magnus Damm <magnus.damm@...il.com>,
Richard Cochran <richardcochran@...il.com>, netdev@...r.kernel.org,
linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH] net: ravb: Correct bad check of timestamp control flags
Hi,
Obviously this should have been tagged to target net-next. Will wait a
few days and resend.
On 2025-11-07 21:01:00 +0100, Niklas Söderlund wrote:
> When converting the Renesas network drivers to use flags from enum
> hwtstamp_rx_filters to control when to timestamp packages instead of a
> driver specific schema with bit-wise flags an error was made.
>
> The bit-wise driver specific flags correct logic to set get_ts was:
>
> q: RAVB_BE + tstamp_rx_ctrl: 0 => 0
> q: RAVB_NC + tstamp_rx_ctrl: 0 => 0
> q: RAVB_BE + tstamp_rx_ctrl: RAVB_RXTSTAMP_TYPE_V2_L2_EVENT => 0
> q: RAVB_NC + tstamp_rx_ctrl: RAVB_RXTSTAMP_TYPE_V2_L2_EVENT => 1
> q: RAVB_BE + tstamp_rx_ctrl: RAVB_RXTSTAMP_TYPE_ALL => 1
> q: RAVB_NC + tstamp_rx_ctrl: RAVB_RXTSTAMP_TYPE_ALL => 1
>
> The converted logic to use enum flags mapped tstamp_rx_ctrl as
>
> 0 to HWTSTAMP_FILTER_NONE
> RAVB_RXTSTAMP_TYPE_V2_L2_EVENT to HWTSTAMP_FILTER_PTP_V2_L2_EVENT
> RAVB_RXTSTAMP_TYPE_ALL to HWTSTAMP_FILTER_ALL
>
> But the logic was incorrectly changed to:
>
> q: RAVB_BE + tstamp_rx_ctrl: HWTSTAMP_FILTER_NONE => 1 (error)
> q: RAVB_NC + tstamp_rx_ctrl: HWTSTAMP_FILTER_NONE => 0
> q: RAVB_BE + tstamp_rx_ctrl: HWTSTAMP_FILTER_PTP_V2_L2_EVENT => 0
> q: RAVB_NC + tstamp_rx_ctrl: HWTSTAMP_FILTER_PTP_V2_L2_EVENT => 1
> q: RAVB_BE + tstamp_rx_ctrl: HWTSTAMP_FILTER_ALL => 1
> q: RAVB_NC + tstamp_rx_ctrl: HWTSTAMP_FILTER_ALL => 0 (error)
>
> This change restores the converted flag check to the correct logic of
> the bit-wise driver specific flags.
>
> Reported-by: Simon Horman <horms@...nel.org>
> Closes: https://lore.kernel.org/linux-renesas-soc/aQ4xSv9629XF-Bt3@horms.kernel.org/
> Fixes: 16e2e6cf75e6 ("net: ravb: Use common defines for time stamping control")
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
> ---
> Hi,
>
> See link in the closes tag for details on how this was found.
>
> I added a fixes tag to this patch, however the patch that introduces the
> error is in net-next, so there is no stable tree to port this fix to.
>
> I'm sorry I made such a clumsy mistake. I'm happy Simon happened to try
> out a new tool on this one commit so this issue could be fixed quickly.
>
> Thanks Simon!
> ---
> drivers/net/ethernet/renesas/ravb_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 1680e94b9242..57b0db314fb5 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -955,9 +955,9 @@ static void ravb_rx_rcar_hwstamp(struct ravb_private *priv, int q,
> bool get_ts;
>
> if (q == RAVB_NC)
> - get_ts = priv->tstamp_rx_ctrl == HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
> + get_ts = priv->tstamp_rx_ctrl != HWTSTAMP_FILTER_NONE;
> else
> - get_ts = priv->tstamp_rx_ctrl != HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
> + get_ts = priv->tstamp_rx_ctrl == HWTSTAMP_FILTER_ALL;
>
> if (!get_ts)
> return;
> --
> 2.51.1
>
--
Kind Regards,
Niklas Söderlund
Powered by blists - more mailing lists