diff mbox series

[net-next,v1,04/12] net: dsa: microchip: ptp: Manipulating absolute time using ptp hw clock

Message ID 20221128103227.23171-5-arun.ramadoss@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: microchip: add PTP support for KSZ9563/KSZ8563 and LAN937x | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 115 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Arun Ramadoss Nov. 28, 2022, 10:32 a.m. UTC 
From: Christian Eggers <ceggers@arri.de>

This patch is used for reconstructing the absolute time from the 32bit
hardware time stamping value. The do_aux ioctl is used for reading the
ptp hardware clock and store it to global variable.
The timestamped value in tail tag during rx and register during tx are
32 bit value (2 bit seconds and 30 bit nanoseconds). The time taken to
read entire ptp clock will be time consuming. In order to speed up, the
software clock is maintained. This clock time will be added to 32 bit
timestamp to get the absolute time stamp.

Signed-off-by: Christian Eggers <ceggers@arri.de>
Co-developed-by: Arun Ramadoss <arun.ramadoss@microchip.com>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---

RFC v1
- This patch is based on Christian Eggers Initial hardware timestamping
support
---
 drivers/net/dsa/microchip/ksz_ptp.c | 58 ++++++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_ptp.h |  3 ++
 2 files changed, 59 insertions(+), 2 deletions(-)

Comments

Pavan Chebbi Nov. 29, 2022, 8:43 a.m. UTC | #1 
On Mon, Nov 28, 2022 at 4:04 PM Arun Ramadoss
<arun.ramadoss@microchip.com> wrote:
> +/*  Function is pointer to the do_aux_work in the ptp_clock capability */
> +static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
> +{
> +       struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
> +       struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
> +       struct timespec64 ts;
> +
> +       mutex_lock(&ptp_data->lock);
> +       _ksz_ptp_gettime(dev, &ts);
> +       mutex_unlock(&ptp_data->lock);
> +
> +       spin_lock_bh(&ptp_data->clock_lock);
> +       ptp_data->clock_time = ts;
> +       spin_unlock_bh(&ptp_data->clock_lock);

If I understand this correctly, the software clock is updated with
full 64b every 1s. However only 32b timestamp registers are read while
processing packets and higher bits from this clock are used.
How do you ensure these higher order bits are in sync with the higher
order bits in the HW? IOW, what if lower 32b have wrapped around and
you are required to stamp a packet but you still don't have aux worker
updated.

> +
> +       return HZ;  /* reschedule in 1 second */
> +}
> +
>  static int ksz_ptp_start_clock(struct ksz_device *dev)
>  {
> -       return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
> +       struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +       int ret;
> +
> +       ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
> +       if (ret)
> +               return ret;
> +
> +       spin_lock_bh(&ptp_data->clock_lock);
> +       ptp_data->clock_time.tv_sec = 0;
> +       ptp_data->clock_time.tv_nsec = 0;
> +       spin_unlock_bh(&ptp_data->clock_lock);
> +
> +       return 0;
>  }
>
>  static const struct ptp_clock_info ksz_ptp_caps = {
> @@ -305,6 +357,7 @@ static const struct ptp_clock_info ksz_ptp_caps = {
>         .settime64      = ksz_ptp_settime,
>         .adjfine        = ksz_ptp_adjfine,
>         .adjtime        = ksz_ptp_adjtime,
> +       .do_aux_work    = ksz_ptp_do_aux_work,
>  };
>
>  int ksz_ptp_clock_register(struct dsa_switch *ds)
> @@ -315,6 +368,7 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
>
>         ptp_data = &dev->ptp_data;
>         mutex_init(&ptp_data->lock);
> +       spin_lock_init(&ptp_data->clock_lock);
>
>         ptp_data->caps = ksz_ptp_caps;
>
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
> index 17f455c3b2c5..81fa2e8b9cf4 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.h
> +++ b/drivers/net/dsa/microchip/ksz_ptp.h
> @@ -15,6 +15,9 @@ struct ksz_ptp_data {
>         struct ptp_clock *clock;
>         /* Serializes all operations on the PTP hardware clock */
>         struct mutex lock;
> +       /* lock for accessing the clock_time */
> +       spinlock_t clock_lock;
> +       struct timespec64 clock_time;
>  };
>
>  int ksz_ptp_clock_register(struct dsa_switch *ds);
> --
> 2.36.1
>
Arun Ramadoss Nov. 30, 2022, 4:22 a.m. UTC | #2 
Hi Pavan,
Thanks for the review comment.

On Tue, 2022-11-29 at 14:13 +0530, Pavan Chebbi wrote:
> On Mon, Nov 28, 2022 at 4:04 PM Arun Ramadoss
> <arun.ramadoss@microchip.com> wrote:
> > +/*  Function is pointer to the do_aux_work in the ptp_clock
> > capability */
> > +static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
> > +{
> > +       struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
> > +       struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
> > +       struct timespec64 ts;
> > +
> > +       mutex_lock(&ptp_data->lock);
> > +       _ksz_ptp_gettime(dev, &ts);
> > +       mutex_unlock(&ptp_data->lock);
> > +
> > +       spin_lock_bh(&ptp_data->clock_lock);
> > +       ptp_data->clock_time = ts;
> > +       spin_unlock_bh(&ptp_data->clock_lock);
> 
> If I understand this correctly, the software clock is updated with
> full 64b every 1s. However only 32b timestamp registers are read
> while
> processing packets and higher bits from this clock are used.
> How do you ensure these higher order bits are in sync with the higher
> order bits in the HW? IOW, what if lower 32b have wrapped around and
> you are required to stamp a packet but you still don't have aux
> worker
> updated.

The Ptp Hardware Clock (PHC) seconds register is 32 bit wide. To have
register overflow it takes 4,294,967,296 seconds which is approximately
around 136 Years. So, it is bigger value and assume that we don't need
to care of PHC second register overflow.
For the packet timestamping, value is read from 32 bit register. This
register is splited into 2 bits seconds + 30 bits nanoseconds register.
In the ksz_tstamp_reconstruct function, lower 2 bits in the ptp_data-
>clock_time is cleared and 2 bits from the timestamp register are
added. 

 spin_lock_bh(&ptp_data->clock_lock);
 ptp_clock_time = ptp_data->clock_time;
 spin_unlock_bh(&ptp_data->clock_lock);

/* calculate full time from partial time stamp */
 ts.tv_sec = (ptp_clock_time.tv_sec & ~3) | ts.tv_sec;

> 
> > +
> > +       return HZ;  /* reschedule in 1 second */
> > +}
> > +
> >  static int ksz_ptp_start_clock(struct ksz_device *dev)
> >  {
> > -       return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE,
> > PTP_CLK_ENABLE);
> > +       struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> > +       int ret;
> > +
> > +       ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE,
> > PTP_CLK_ENABLE);
> > +       if (ret)
> > +               return ret;
> > +
> > +       spin_lock_bh(&ptp_data->clock_lock);
> > +       ptp_data->clock_time.tv_sec = 0;
> > +       ptp_data->clock_time.tv_nsec = 0;
> > +       spin_unlock_bh(&ptp_data->clock_lock);
> > +
> > +       return 0;
> >  }
> > 
> >  
> > --
> > 2.36.1
> >
Pavan Chebbi Nov. 30, 2022, 6:11 a.m. UTC | #3 
On Wed, Nov 30, 2022 at 9:52 AM <Arun.Ramadoss@microchip.com> wrote:
>
> Hi Pavan,
> Thanks for the review comment.
>
> On Tue, 2022-11-29 at 14:13 +0530, Pavan Chebbi wrote:
> > On Mon, Nov 28, 2022 at 4:04 PM Arun Ramadoss
> > <arun.ramadoss@microchip.com> wrote:
> > > +/*  Function is pointer to the do_aux_work in the ptp_clock
> > > capability */
> > > +static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
> > > +{
> > > +       struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
> > > +       struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
> > > +       struct timespec64 ts;
> > > +
> > > +       mutex_lock(&ptp_data->lock);
> > > +       _ksz_ptp_gettime(dev, &ts);
> > > +       mutex_unlock(&ptp_data->lock);
> > > +
> > > +       spin_lock_bh(&ptp_data->clock_lock);
> > > +       ptp_data->clock_time = ts;
> > > +       spin_unlock_bh(&ptp_data->clock_lock);
> >
> > If I understand this correctly, the software clock is updated with
> > full 64b every 1s. However only 32b timestamp registers are read
> > while
> > processing packets and higher bits from this clock are used.
> > How do you ensure these higher order bits are in sync with the higher
> > order bits in the HW? IOW, what if lower 32b have wrapped around and
> > you are required to stamp a packet but you still don't have aux
> > worker
> > updated.
>
> The Ptp Hardware Clock (PHC) seconds register is 32 bit wide. To have
> register overflow it takes 4,294,967,296 seconds which is approximately
> around 136 Years. So, it is bigger value and assume that we don't need
> to care of PHC second register overflow.
> For the packet timestamping, value is read from 32 bit register. This
> register is splited into 2 bits seconds + 30 bits nanoseconds register.
> In the ksz_tstamp_reconstruct function, lower 2 bits in the ptp_data-
> >clock_time is cleared and 2 bits from the timestamp register are
> added.
>
>  spin_lock_bh(&ptp_data->clock_lock);
>  ptp_clock_time = ptp_data->clock_time;
>  spin_unlock_bh(&ptp_data->clock_lock);
>
> /* calculate full time from partial time stamp */
>  ts.tv_sec = (ptp_clock_time.tv_sec & ~3) | ts.tv_sec;
>
OK thanks. Looks like nano sec and seconds are not being stitched, if
that had happened the rollover would be very frequent. But stitching
the seconds with higher bits still has this problem.
But it should be OK since the window is so large.

> >
> > > +
> > > +       return HZ;  /* reschedule in 1 second */
> > > +}
> > > +
> > >  static int ksz_ptp_start_clock(struct ksz_device *dev)
> > >  {
> > > -       return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE,
> > > PTP_CLK_ENABLE);
> > > +       struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> > > +       int ret;
> > > +
> > > +       ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE,
> > > PTP_CLK_ENABLE);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       spin_lock_bh(&ptp_data->clock_lock);
> > > +       ptp_data->clock_time.tv_sec = 0;
> > > +       ptp_data->clock_time.tv_nsec = 0;
> > > +       spin_unlock_bh(&ptp_data->clock_lock);
> > > +
> > > +       return 0;
> > >  }
> > >
> > >
> > > --
> > > 2.36.1
> > >
Vladimir Oltean Dec. 1, 2022, 1:04 a.m. UTC | #4 
On Mon, Nov 28, 2022 at 04:02:19PM +0530, Arun Ramadoss wrote:
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 184aa57a8489..415522ef4ce9 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -200,6 +209,12 @@ static int ksz_ptp_settime(struct ptp_clock_info *ptp,
>  		goto error_return;
>  
>  	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_LOAD_TIME, PTP_LOAD_TIME);
> +	if (ret)
> +		goto error_return;
> +
> +	spin_lock_bh(&ptp_data->clock_lock);

Why disable bottom halves? Where is the bottom half that this races with?

> +	ptp_data->clock_time = *ts;
> +	spin_unlock_bh(&ptp_data->clock_lock);
>  
>  error_return:
>  	mutex_unlock(&ptp_data->lock);
> @@ -254,6 +269,7 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>  {
>  	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
>  	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
> +	struct timespec64 delta64 = ns_to_timespec64(delta);
>  	s32 sec, nsec;
>  	u16 data16;
>  	int ret;
> @@ -286,15 +302,51 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>  		data16 |= PTP_STEP_DIR;
>  
>  	ret = ksz_write16(dev, REG_PTP_CLK_CTRL, data16);
> +	if (ret)
> +		goto error_return;
> +
> +	spin_lock_bh(&ptp_data->clock_lock);
> +	ptp_data->clock_time = timespec64_add(ptp_data->clock_time, delta64);
> +	spin_unlock_bh(&ptp_data->clock_lock);
>  
>  error_return:
>  	mutex_unlock(&ptp_data->lock);
>  	return ret;
>  }
>  
> +/*  Function is pointer to the do_aux_work in the ptp_clock capability */
> +static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
> +{
> +	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
> +	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
> +	struct timespec64 ts;
> +
> +	mutex_lock(&ptp_data->lock);
> +	_ksz_ptp_gettime(dev, &ts);
> +	mutex_unlock(&ptp_data->lock);

Why don't you call ksz_ptp_gettime(ptp, &ts) directly?

> +
> +	spin_lock_bh(&ptp_data->clock_lock);
> +	ptp_data->clock_time = ts;
> +	spin_unlock_bh(&ptp_data->clock_lock);
> +
> +	return HZ;  /* reschedule in 1 second */
> +}
> +
>  static int ksz_ptp_start_clock(struct ksz_device *dev)
>  {
> -	return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +	int ret;
> +
> +	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
> +	if (ret)
> +		return ret;
> +
> +	spin_lock_bh(&ptp_data->clock_lock);
> +	ptp_data->clock_time.tv_sec = 0;
> +	ptp_data->clock_time.tv_nsec = 0;
> +	spin_unlock_bh(&ptp_data->clock_lock);

Does ksz_ptp_start_clock() race with anything? The PTP clock has not
even been registered by the time this has been called. This is literally
an example of the "spin_lock_init(); spin_lock();" antipattern.

> +
> +	return 0;
>  }
Arun Ramadoss Dec. 2, 2022, 9:40 a.m. UTC | #5 
Hi Vladimir,
Thanks for the review comment.

On Thu, 2022-12-01 at 03:04 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Mon, Nov 28, 2022 at 04:02:19PM +0530, Arun Ramadoss wrote:
> > diff --git a/drivers/net/dsa/microchip/ksz_ptp.c
> > b/drivers/net/dsa/microchip/ksz_ptp.c
> > index 184aa57a8489..415522ef4ce9 100644
> > --- a/drivers/net/dsa/microchip/ksz_ptp.c
> > +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> > @@ -200,6 +209,12 @@ static int ksz_ptp_settime(struct
> > ptp_clock_info *ptp,
> >               goto error_return;
> > 
> >       ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_LOAD_TIME,
> > PTP_LOAD_TIME);
> > +     if (ret)
> > +             goto error_return;
> > +
> > +     spin_lock_bh(&ptp_data->clock_lock);
> 
> Why disable bottom halves? Where is the bottom half that this races
> with?

The interrupts are added in the following patches in the series. During
the deferred packet timestamping, partial timestamps are reconstructed
to absolute time using the ptp_data->clock_time in the bottom halves.
So we need this spin_lock_bh.

> 
> > +     ptp_data->clock_time = *ts;
> > +     spin_unlock_bh(&ptp_data->clock_lock);
> > 
> >  error_return:
> >       mutex_unlock(&ptp_data->lock);
> >  }
> > 
> > +/*  Function is pointer to the do_aux_work in the ptp_clock
> > capability */
> > +static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
> > +{
> > +     struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
> > +     struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
> > +     struct timespec64 ts;
> > +
> > +     mutex_lock(&ptp_data->lock);
> > +     _ksz_ptp_gettime(dev, &ts);
> > +     mutex_unlock(&ptp_data->lock);
> 
> Why don't you call ksz_ptp_gettime(ptp, &ts) directly?

I will use ksz_ptp_gettime directly.

> 
> > +
> > +     spin_lock_bh(&ptp_data->clock_lock);
> > +     ptp_data->clock_time = ts;
> > +     spin_unlock_bh(&ptp_data->clock_lock);
> > +
> > +     return HZ;  /* reschedule in 1 second */
> > +}
> > +
> >  static int ksz_ptp_start_clock(struct ksz_device *dev)
> >  {
> > -     return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE,
> > PTP_CLK_ENABLE);
> > +     struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> > +     int ret;
> > +
> > +     ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE,
> > PTP_CLK_ENABLE);
> > +     if (ret)
> > +             return ret;
> > +
> > +     spin_lock_bh(&ptp_data->clock_lock);
> > +     ptp_data->clock_time.tv_sec = 0;
> > +     ptp_data->clock_time.tv_nsec = 0;
> > +     spin_unlock_bh(&ptp_data->clock_lock);
> 
> Does ksz_ptp_start_clock() race with anything? The PTP clock has not
> even been registered by the time this has been called. This is
> literally
> an example of the "spin_lock_init(); spin_lock();" antipattern.

Yes, this function is called before PTP clock registeration. I will
remove the spin_lock for here.

> 
> > +
> > +     return 0;
> >  }
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 184aa57a8489..415522ef4ce9 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -22,11 +22,20 @@ 
 
 static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable)
 {
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
 	u16 data = 0;
+	int ret;
 
-	if (enable)
+	if (enable) {
 		data = PTP_ENABLE;
 
+		ret = ptp_schedule_worker(ptp_data->clock, 0);
+		if (ret)
+			return ret;
+	} else {
+		ptp_cancel_worker_sync(ptp_data->clock);
+	}
+
 	return ksz_rmw16(dev, REG_PTP_MSG_CONF1, PTP_ENABLE, data);
 }
 
@@ -200,6 +209,12 @@  static int ksz_ptp_settime(struct ptp_clock_info *ptp,
 		goto error_return;
 
 	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_LOAD_TIME, PTP_LOAD_TIME);
+	if (ret)
+		goto error_return;
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = *ts;
+	spin_unlock_bh(&ptp_data->clock_lock);
 
 error_return:
 	mutex_unlock(&ptp_data->lock);
@@ -254,6 +269,7 @@  static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
 	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 delta64 = ns_to_timespec64(delta);
 	s32 sec, nsec;
 	u16 data16;
 	int ret;
@@ -286,15 +302,51 @@  static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 		data16 |= PTP_STEP_DIR;
 
 	ret = ksz_write16(dev, REG_PTP_CLK_CTRL, data16);
+	if (ret)
+		goto error_return;
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = timespec64_add(ptp_data->clock_time, delta64);
+	spin_unlock_bh(&ptp_data->clock_lock);
 
 error_return:
 	mutex_unlock(&ptp_data->lock);
 	return ret;
 }
 
+/*  Function is pointer to the do_aux_work in the ptp_clock capability */
+static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 ts;
+
+	mutex_lock(&ptp_data->lock);
+	_ksz_ptp_gettime(dev, &ts);
+	mutex_unlock(&ptp_data->lock);
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = ts;
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+	return HZ;  /* reschedule in 1 second */
+}
+
 static int ksz_ptp_start_clock(struct ksz_device *dev)
 {
-	return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	int ret;
+
+	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
+	if (ret)
+		return ret;
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time.tv_sec = 0;
+	ptp_data->clock_time.tv_nsec = 0;
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+	return 0;
 }
 
 static const struct ptp_clock_info ksz_ptp_caps = {
@@ -305,6 +357,7 @@  static const struct ptp_clock_info ksz_ptp_caps = {
 	.settime64	= ksz_ptp_settime,
 	.adjfine	= ksz_ptp_adjfine,
 	.adjtime	= ksz_ptp_adjtime,
+	.do_aux_work	= ksz_ptp_do_aux_work,
 };
 
 int ksz_ptp_clock_register(struct dsa_switch *ds)
@@ -315,6 +368,7 @@  int ksz_ptp_clock_register(struct dsa_switch *ds)
 
 	ptp_data = &dev->ptp_data;
 	mutex_init(&ptp_data->lock);
+	spin_lock_init(&ptp_data->clock_lock);
 
 	ptp_data->caps = ksz_ptp_caps;
 
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 17f455c3b2c5..81fa2e8b9cf4 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -15,6 +15,9 @@  struct ksz_ptp_data {
 	struct ptp_clock *clock;
 	/* Serializes all operations on the PTP hardware clock */
 	struct mutex lock;
+	/* lock for accessing the clock_time */
+	spinlock_t clock_lock;
+	struct timespec64 clock_time;
 };
 
 int ksz_ptp_clock_register(struct dsa_switch *ds);