Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 21 Nov 2022 23:33:04 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Arun Ramadoss <arun.ramadoss@microchip.com>
Cc:     linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        woojung.huh@microchip.com, UNGLinuxDriver@microchip.com,
        andrew@lunn.ch, vivien.didelot@gmail.com, f.fainelli@gmail.com,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, linux@armlinux.org.uk,
        Tristram.Ha@microchip.com, richardcochran@gmail.com
Subject: Re: [RFC Patch net-next v2 2/8] net: dsa: microchip: adding the
 posix clock support
Message-ID: <20221121213304.vytvbfvikuwcw3oi@skbuf>
References: <20221121154150.9573-1-arun.ramadoss@microchip.com>
 <20221121154150.9573-1-arun.ramadoss@microchip.com>
 <20221121154150.9573-3-arun.ramadoss@microchip.com>
 <20221121154150.9573-3-arun.ramadoss@microchip.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221121154150.9573-3-arun.ramadoss@microchip.com>
 <20221121154150.9573-3-arun.ramadoss@microchip.com>
Precedence: bulk

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.