mbox series

[net-next,0/7] bnxt_en: Add hardware PTP timestamping support on 575XX devices.

Message ID 1622249601-7106-1-git-send-email-michael.chan@broadcom.com
Headers show
Series bnxt_en: Add hardware PTP timestamping support on 575XX devices. | expand

Message

Michael Chan May 29, 2021, 12:53 a.m. UTC 
Add PTP RX and TX hardware timestamp support on 575XX devices.  These
devices use the two-step method to implement the IEEE-1588 timestamping
support.

Michael Chan (4):
  bnxt_en: Update firmware interface to 1.10.2.34.
  bnxt_en: Get PTP hardware capability from firmware.
  bnxt_en: Add PTP clock APIs, ioctls, and ethtool methods.
  bnxt_en: Enable hardware PTP support.

Pavan Chebbi (3):
  bnxt_en: Get the full 48-bit hardware timestamp periodically.
  bnxt_en: Get the RX packet timestamp.
  bnxt_en: Transmit and retrieve packet timestamps.

 drivers/net/ethernet/broadcom/Kconfig         |   1 +
 drivers/net/ethernet/broadcom/bnxt/Makefile   |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 125 ++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   8 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  34 ++
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 434 +++++++++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 406 ++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h |  62 +++
 8 files changed, 1046 insertions(+), 26 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h

Comments

Jakub Kicinski May 29, 2021, 1:39 a.m. UTC | #1 
On Fri, 28 May 2021 20:53:19 -0400 Michael Chan wrote:
> +	struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
> +	u64 time;
> +
> +	if (!ptp)
> +		return -ENODEV;
> +
> +	time = READ_ONCE(ptp->old_time);

READ_ONCE() on a u64? That's not gonna prevent tearing the read on 32
bit architectures, right?

> +	*ts = (time & BNXT_HI_TIMER_MASK) | pkt_ts;
> +	if (pkt_ts < (time & BNXT_LO_TIMER_MASK))
> +		*ts += BNXT_LO_TIMER_MASK + 1;

The stamp is from the MAC, I hope, or otherwise packet which could have
been sitting on the ring for some approximation of eternity. You can
easily see a packet stamp older than the value stashed in ptp->old_time
if you run soon after the refresh.
Michael Chan May 29, 2021, 5:06 a.m. UTC | #2 
On Fri, May 28, 2021 at 6:42 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 28 May 2021 20:53:17 -0400 Michael Chan wrote:
> > +int bnxt_ptp_init(struct bnxt *bp)
>
> This function never fails.
>
> > +     struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
> > +
> > +     if (!ptp)
> > +             return 0;
> > +
> > +     atomic_set(&ptp->tx_avail, BNXT_MAX_TX_TS);
> > +
> > +     memset(&ptp->cc, 0, sizeof(ptp->cc));
> > +     ptp->cc.read = bnxt_cc_read;
> > +     ptp->cc.mask = CYCLECOUNTER_MASK(64);
> > +     ptp->cc.shift = 0;
> > +     ptp->cc.mult = 1;
> > +
> > +     timecounter_init(&ptp->tc, &ptp->cc, ktime_to_ns(ktime_get_real()));
> > +
> > +     ptp->ptp_info = bnxt_ptp_caps;
> > +     ptp->ptp_clock = ptp_clock_register(&ptp->ptp_info, &bp->pdev->dev);
> > +     if (IS_ERR(ptp->ptp_clock))
> > +             ptp->ptp_clock = NULL;
>
> Why not propagate the error? I thought only NULL should be silently
> ignored? I could be confused about the rules, tho :)

Yeah, we should propagate the error so the caller can print a warning.

>
> > +     return 0;