Re: [PATCH net-next v4 09/10] dt-bindings: net: renesas,etheravb: Add optional gPTP phandle for Gen4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Fatal error propagation from ptp_clock_register() breaks Ethernet TX timeout recovery and ring parameter updates.
--

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

dt-bindings: net: renesas,etheravb: Add optional gPTP phandle for Gen4

This commit adds an optional ptp-timer property for the RAVB module on Gen4,
allowing the driver to find and use an external system-wide gPTP clock.
The clock is shared with RTSN on V4H and RSWITCH on S4. The module can
function without it, but will not provide PTP functionality in such cases.

> diff --git a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
> index 1e00ef5b3acda..fad002492421a 100644
> --- a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
> +++ b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
> @@ -97,6 +97,8 @@ properties:
>  
>    phy-handle: true
>  
> +  ptp-timer: true
> +

[Severity: High]
If this PTP timer is optional, could this introduce a regression where its
absence causes the driver to fail during initialization or timeout recovery?

Looking at ravb_ptp_init() in drivers/net/ethernet/renesas/ravb_ptp.c,
it propagates a fatal error if PTP registration fails:

drivers/net/ethernet/renesas/ravb_ptp.c:ravb_ptp_init() {
    priv->ptp.clock = ptp_clock_register(&priv->ptp.info, &priv->pdev->dev);
    if (IS_ERR(priv->ptp.clock)) {
        ret = PTR_ERR(priv->ptp.clock);
        priv->ptp.clock = NULL;
        ravb_ptp_stop(ndev);
    }
    return ret;
}

This error returns up the call chain through ravb_dmac_init() to
ravb_tx_timeout_work() in drivers/net/ethernet/renesas/ravb_main.c:

drivers/net/ethernet/renesas/ravb_main.c:ravb_tx_timeout_work() {
    error = ravb_dmac_init(ndev);
    if (error) {
        netdev_err(ndev, "%s: ravb_dmac_init() failed, error %d\n",
                   __func__, error);
        goto out_unlock;
    }
}

Could this cause the main Ethernet interface to become permanently disabled
on a TX timeout if the optional PTP clock fails to register?

Similarly, in ravb_set_ringparam(), an error from ravb_dmac_init() interrupts
the ring parameter update:

drivers/net/ethernet/renesas/ravb_main.c:ravb_set_ringparam() {
    error = ravb_dmac_init(ndev);
    if (error) {
        netdev_err(ndev,
                   "%s: ravb_dmac_init() failed, error %d\n",
                   __func__, error);
        return error;
    }
}

Would this leave the network device detached when applying ring parameters
if the PTP clock is absent?

>    '#address-cells':
>      description: Number of address cells for the MDIO bus.
>      const: 1

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