Re: [RFC Patch net-next v2 2/8] net: dsa: microchip: adding the posix clock support

From: Vladimir Oltean
Date: Mon Nov 21 2022 - 16:33:26 EST


On Mon, Nov 21, 2022 at 09:11:44PM +0530, Arun Ramadoss wrote:
> +int ksz_ptp_clock_register(struct dsa_switch *ds)
> +{
> + /* Register the PTP Clock */
> + ptp_data->clock = ptp_clock_register(&ptp_data->caps, dev->dev);
> + if (IS_ERR_OR_NULL(ptp_data->clock))
> + return PTR_ERR(ptp_data->clock);
> +}
> +
> +void ksz_ptp_clock_unregister(struct dsa_switch *ds)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +
> + if (IS_ERR_OR_NULL(ptp_data->clock))
> + return;
> +
> + ptp_clock_unregister(ptp_data->clock);
> +}

API usage seems to be incorrect here (probably copied from sja1105 which
is written by me and also incorrect, yay).

The intention with IS_ERR_OR_NULL() is for the caller to return 0
(success) when ptp_clock_register() returns NULL (when PTP support
is compiled out), and this will not make the driver fail to probe.

There isn't a reason to use IS_ERR_OR_NULL() in the normal unregister
code path, because the code won't get there in the IS_ERR() case.
So a simple "if (ptp_data->clock) ptp_clock_unregister(ptp_data->clock)"
would do.