Re: [PATCH v2 net-next 01/12] net: aquantia: PTP skeleton declarations and callbacks

[Richard Cochran <richardcochran@gmail.com>]

On Tue, Oct 08, 2019 at 10:56:36AM +0000, Igor Russkikh wrote:

> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ptp.c b/drivers/net/ethernet/aquantia/atlantic/aq_ptp.c
> new file mode 100644
> index 000000000000..d5a28904f708
> --- /dev/null
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ptp.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Aquantia Corporation Network Driver
> + * Copyright (C) 2014-2019 Aquantia Corporation. All rights reserved
> + */
> +
> +/* File aq_ptp.c:
> + * Definition of functions for Linux PTP support.
> + */
> +
> +#include <linux/ptp_clock_kernel.h>
> +#include <linux/clocksource.h>
> +
> +#include "aq_nic.h"
> +#include "aq_ptp.h"
> +
> +struct aq_ptp_s {
> +	struct aq_nic_s *aq_nic;
> +

Nit: no blank line here.

> +	struct ptp_clock *ptp_clock;
> +	struct ptp_clock_info ptp_info;
> +};
> +
> +static struct ptp_clock_info aq_ptp_clock = {
> +	.owner		= THIS_MODULE,
> +	.name		= "atlantic ptp",
> +	.n_ext_ts	= 0,
> +	.pps		= 0,
> +	.n_per_out     = 0,
> +	.n_pins        = 0,
> +	.pin_config    = NULL,

Nit: use tabs to align columns (here some are spaces).

> +};
> +
> +int aq_ptp_init(struct aq_nic_s *aq_nic, unsigned int idx_vec)
> +{
> +	struct hw_atl_utils_mbox mbox;
> +	struct ptp_clock *clock;
> +	struct aq_ptp_s *aq_ptp;
> +	int err = 0;
> +
> +	hw_atl_utils_mpi_read_stats(aq_nic->aq_hw, &mbox);
> +
> +	if (!(mbox.info.caps_ex & BIT(CAPS_EX_PHY_PTP_EN))) {
> +		aq_nic->aq_ptp = NULL;
> +		return 0;
> +	}
> +
> +	aq_ptp = kzalloc(sizeof(*aq_ptp), GFP_KERNEL);
> +	if (!aq_ptp) {
> +		err = -ENOMEM;
> +		goto err_exit;
> +	}
> +
> +	aq_ptp->aq_nic = aq_nic;
> +
> +	aq_ptp->ptp_info = aq_ptp_clock;
> +	clock = ptp_clock_register(&aq_ptp->ptp_info, &aq_nic->ndev->dev);
> +	if (!clock) {
> +		netdev_err(aq_nic->ndev, "ptp_clock_register failed\n");
> +		err = 0;
> +		goto err_exit;
> +	}

You need to test for PTR_ERR and NULL:

 * ptp_clock_register() - register a PTP hardware clock driver
 *
 * @info:   Structure describing the new clock.
 * @parent: Pointer to the parent device of the new clock.
 *
 * Returns a valid pointer on success or PTR_ERR on failure.  If PHC
 * support is missing at the configuration level, this function
 * returns NULL, and drivers are expected to gracefully handle that
 * case separately.

> +	aq_ptp->ptp_clock = clock;
> +
> +	aq_nic->aq_ptp = aq_ptp;
> +
> +	return 0;
> +
> +err_exit:
> +	kfree(aq_ptp);
> +	aq_nic->aq_ptp = NULL;
> +	return err;
> +}

Thanks,
Richard