All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
Cc: wsa+renesas@sang-engineering.com, devicetree@vger.kernel.org,
	conor+dt@kernel.org, robh@kernel.org
Subject: Re: [net-next 1/9] net: ethernet: ravb: Remove gPTP control from WoL setup and restore
Date: Thu, 11 Jun 2026 10:26:00 +0000	[thread overview]
Message-ID: <20260611102601.2B1BC1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260610102432.3538432-2-niklas.soderlund+renesas@ragnatech.se>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked return value of ptp_clock_register() leads to a kernel panic in ravb_ptp_stop() on failure.
--

commit 4d5ada4dabf46fceeae07f219f72c4daaba09e17
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

net: ethernet: ravb: Remove gPTP control from WoL setup and restore

This commit removes the explicit stopping and starting of the gPTP clock
during Wake-on-LAN (WoL) setup and restore. This is because the ndev is
always closed and re-opened during system suspend and resume, which handles
the clock state automatically.

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 5f88733094d0f..77c0645a1c4d3 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[ ... ]
> @@ -3201,9 +3198,6 @@ static int ravb_wol_restore(struct net_device *ndev)
>  	if (error)
>  		return error;
>  
> -	if (priv->info->ccc_gac)
> -		ravb_ptp_init(ndev, priv->pdev);
> -

[Severity: High]
This is a pre-existing issue, but since this patch involves the usage of
ravb_ptp_init() and ravb_ptp_stop(), does ravb_ptp_init() need to check
the return value of ptp_clock_register()?

In drivers/net/ethernet/renesas/ravb_ptp.c:ravb_ptp_init(), the return value
is assigned directly without using IS_ERR():

    priv->ptp.clock = ptp_clock_register(&priv->ptp.info, &pdev->dev);

If ptp_clock_register() fails and returns an error pointer like
ERR_PTR(-ENOMEM), priv->ptp.clock will hold this error pointer instead of
a valid clock or NULL.

Later, when ravb_ptp_stop() is unconditionally called during teardown or
driver removal, it executes:

    ptp_clock_unregister(priv->ptp.clock);

Could this cause a kernel panic when ptp_clock_unregister() dereferences
the error pointer (e.g., when locking the internal mutex via
ptp_vclock_in_use)?

>  	if (info->nc_queues)
>  		napi_enable(&priv->napi[RAVB_NC]);
>  	napi_enable(&priv->napi[RAVB_BE]);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610102432.3538432-2-niklas.soderlund+renesas@ragnatech.se?part=1

  reply	other threads:[~2026-06-11 10:26 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10 10:24 [net-next 0/9] ravb: Add gPTP support for Gen4 Niklas Söderlund
2026-06-10 10:24 ` [net-next 1/9] net: ethernet: ravb: Remove gPTP control from WoL setup and restore Niklas Söderlund
2026-06-11 10:26   ` sashiko-bot [this message]
2026-06-10 10:24 ` [net-next 2/9] net: ethernet: ravb: Move programming of gPTP timer interval Niklas Söderlund
2026-06-11 10:26   ` sashiko-bot
2026-06-10 10:24 ` [net-next 3/9] net: ethernet: ravb: Simplify gPTP start and stop Niklas Söderlund
2026-06-10 10:24 ` [net-next 4/9] net: ethernet: ravb: Remove redundant argument to ravb_ptp_init() Niklas Söderlund
2026-06-10 10:24 ` [net-next 5/9] net: ethernet: ravb: Replace gPTP flags with callbacks Niklas Söderlund
2026-06-10 10:24 ` [net-next 6/9] net: ethernet: ravb: Add callback for gPTP probe Niklas Söderlund
2026-06-10 10:24 ` [net-next 7/9] net: ethernet: ravb: Add callback for gPTP clock index Niklas Söderlund
2026-06-10 10:24 ` [net-next 8/9] dt-bindings: net: renesas,etheravb: Add optional gPTP phandle for Gen4 Niklas Söderlund
2026-06-11 10:26   ` sashiko-bot
2026-06-10 10:24 ` [net-next 9/9] net: ethernet: ravb: Add gPTP support " Niklas Söderlund
2026-06-10 10:27 ` [net-next 0/9] " Krzysztof Kozlowski
2026-06-10 10:38   ` Niklas Söderlund
2026-06-10 10:47     ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260611102601.2B1BC1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.