To initialize the taprio block in lan966x, it is required to configure
the register REVISIT_DLY. The purpose of this register is to set the
delay before revisit the next gate and the value of this register depends
on the system clock. The problem is that the we calculated wrong the value
of the system clock period in picoseconds. The actual system clock is
~165.617754MHZ and this correspond to a period of 6038 pico seconds and
not 15125 as currently set.

Fixes: e462b2717380b4 ("net: lan966x: Add offload support for taprio")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
index b4377b8613c3a..905a967002def 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
@@ -1126,5 +1126,5 @@ void lan966x_ptp_rxtstamp(struct lan966x *lan966x, struct sk_buff *skb,
 u32 lan966x_ptp_get_period_ps(void)
 {
 	/* This represents the system clock period in picoseconds */
-	return 15125;
+	return 6038;
 }
-- 
2.34.1

On Mon, Nov 17, 2025 at 03:43:09PM +0100, Horatiu Vultur wrote:
> To initialize the taprio block in lan966x, it is required to configure
> the register REVISIT_DLY. The purpose of this register is to set the
> delay before revisit the next gate and the value of this register depends
> on the system clock. The problem is that the we calculated wrong the value
> of the system clock period in picoseconds. The actual system clock is
> ~165.617754MHZ and this correspond to a period of 6038 pico seconds and
> not 15125 as currently set.

Is the system clock available as a linux clock? Can you do a
clk_get_rate() on it?

	Andrew

The 11/18/2025 20:20, Andrew Lunn wrote:

Hi Andrew,

> 
> On Mon, Nov 17, 2025 at 03:43:09PM +0100, Horatiu Vultur wrote:
> > To initialize the taprio block in lan966x, it is required to configure
> > the register REVISIT_DLY. The purpose of this register is to set the
> > delay before revisit the next gate and the value of this register depends
> > on the system clock. The problem is that the we calculated wrong the value
> > of the system clock period in picoseconds. The actual system clock is
> > ~165.617754MHZ and this correspond to a period of 6038 pico seconds and
> > not 15125 as currently set.
> 
> Is the system clock available as a linux clock? Can you do a
> clk_get_rate() on it?

Unfortunetly, I can not do clk_get_rate because in the device tree for the
switch node I don't have any clocks property. And maybe that is the
problem because I have the system clock (sys_clk) in the lan966x.dtsi
file. But if I go this way then I need add a bigger changeset and add
it to multiple kernel trees which complicate the things.
So maybe I should not change this patch and then create another one
targeting net-next where I can start using clk_get_rate()

> 
>         Andrew

-- 
/Horatiu

On Wed, Nov 19, 2025 at 09:26:46AM +0100, Horatiu Vultur wrote:
> The 11/18/2025 20:20, Andrew Lunn wrote:
> 
> Hi Andrew,
> 
> > 
> > On Mon, Nov 17, 2025 at 03:43:09PM +0100, Horatiu Vultur wrote:
> > > To initialize the taprio block in lan966x, it is required to configure
> > > the register REVISIT_DLY. The purpose of this register is to set the
> > > delay before revisit the next gate and the value of this register depends
> > > on the system clock. The problem is that the we calculated wrong the value
> > > of the system clock period in picoseconds. The actual system clock is
> > > ~165.617754MHZ and this correspond to a period of 6038 pico seconds and
> > > not 15125 as currently set.
> > 
> > Is the system clock available as a linux clock? Can you do a
> > clk_get_rate() on it?
> 
> Unfortunetly, I can not do clk_get_rate because in the device tree for the
> switch node I don't have any clocks property. And maybe that is the
> problem because I have the system clock (sys_clk) in the lan966x.dtsi
> file. But if I go this way then I need add a bigger changeset and add
> it to multiple kernel trees which complicate the things.
> So maybe I should not change this patch and then create another one
> targeting net-next where I can start using clk_get_rate()

This is fine as is. But maybe rather than use the magic number, add a
#defines for the system clock rate, and convert to pico seconds in the
code, and let the compiler do it at compile time. The code then
documents what is going on.

	Andrew

The 11/19/2025 15:26, Andrew Lunn wrote:
> 
> On Wed, Nov 19, 2025 at 09:26:46AM +0100, Horatiu Vultur wrote:
> > The 11/18/2025 20:20, Andrew Lunn wrote:
> >
> > Hi Andrew,
> >
> > >
> > > On Mon, Nov 17, 2025 at 03:43:09PM +0100, Horatiu Vultur wrote:
> > > > To initialize the taprio block in lan966x, it is required to configure
> > > > the register REVISIT_DLY. The purpose of this register is to set the
> > > > delay before revisit the next gate and the value of this register depends
> > > > on the system clock. The problem is that the we calculated wrong the value
> > > > of the system clock period in picoseconds. The actual system clock is
> > > > ~165.617754MHZ and this correspond to a period of 6038 pico seconds and
> > > > not 15125 as currently set.
> > >
> > > Is the system clock available as a linux clock? Can you do a
> > > clk_get_rate() on it?
> >
> > Unfortunetly, I can not do clk_get_rate because in the device tree for the
> > switch node I don't have any clocks property. And maybe that is the
> > problem because I have the system clock (sys_clk) in the lan966x.dtsi
> > file. But if I go this way then I need add a bigger changeset and add
> > it to multiple kernel trees which complicate the things.
> > So maybe I should not change this patch and then create another one
> > targeting net-next where I can start using clk_get_rate()
> 
> This is fine as is. But maybe rather than use the magic number, add a
> #defines for the system clock rate, and convert to pico seconds in the
> code, and let the compiler do it at compile time. The code then
> documents what is going on.

Yes, I will add a define for system clock.
I will do it as suggested by Eric.

> 
>         Andrew

-- 
/Horatiu

On Wed, Nov 19, 2025 at 12:26 AM Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> The 11/18/2025 20:20, Andrew Lunn wrote:
>
> Hi Andrew,
>
> >
> > On Mon, Nov 17, 2025 at 03:43:09PM +0100, Horatiu Vultur wrote:
> > > To initialize the taprio block in lan966x, it is required to configure
> > > the register REVISIT_DLY. The purpose of this register is to set the
> > > delay before revisit the next gate and the value of this register depends
> > > on the system clock. The problem is that the we calculated wrong the value
> > > of the system clock period in picoseconds. The actual system clock is
> > > ~165.617754MHZ and this correspond to a period of 6038 pico seconds and
> > > not 15125 as currently set.
> >
> > Is the system clock available as a linux clock? Can you do a
> > clk_get_rate() on it?
>
> Unfortunetly, I can not do clk_get_rate because in the device tree for the
> switch node I don't have any clocks property. And maybe that is the
> problem because I have the system clock (sys_clk) in the lan966x.dtsi
> file. But if I go this way then I need add a bigger changeset and add
> it to multiple kernel trees which complicate the things.
> So maybe I should not change this patch and then create another one
> targeting net-next where I can start using clk_get_rate()

Or define a clock rate

#define LAN9X66_CLOCK_RATE 165617754

then use

return PICO /  LAN9X66_CLOCK_RATE;

This would look less random....