[PATCH net 0/3] net: dsa: mv88e6xxx: fix MV88E6393X PHC frequency on internal clock

[PATCH net 0/3] net: dsa: mv88e6xxx: fix MV88E6393X PHC frequency on internal clock

[Shenghao Yang]

The MV88E6393X family of switches can additionally run their cycle
counters using a 250MHz internal clock instead of the usual 125MHz
externa clock [1].

The driver currently assumes all designs utilize that external clock,
but MikroTik's RB5009 uses the internal source - causing the PHC to be
seen running at 2x real time in userspace, making synchronization
with ptp4l impossible.

This series adds support for reading off the cycle counter frequency
known to the hardware in the TAI_CLOCK_PERIOD register and picking an
appropriate set of scaling coefficients instead of using a fixed set
for each switch family.

Patch 1 groups those cycle counter coefficients into a new structure to
make it easier to pass those around.

Patch 2 modifies PTP initialization to probe TAI_CLOCK_PERIOD and
use an appropriate set of coefficients.

Patch 3 adds support for 4000ps cycle counter periods.

Thanks,

Shenghao

[1] https://lore.kernel.org/netdev/d6622575-bf1b-445a-b08f-2739e3642aae@lunn.ch/

Shenghao Yang (3):
  net: dsa: mv88e6xxx: group cycle counter coefficients
  net: dsa: mv88e6xxx: read cycle counter period from hardware
  net: dsa: mv88e6xxx: support 4000ps cycle counter periods

 drivers/net/dsa/mv88e6xxx/chip.h |   7 +--
 drivers/net/dsa/mv88e6xxx/ptp.c  | 105 +++++++++++++++++++++----------
 2 files changed, 75 insertions(+), 37 deletions(-)

-- 
2.46.1

[PATCH net 1/3] net: dsa: mv88e6xxx: group cycle counter coefficients

[Shenghao Yang]

Instead of having them as individual fields in ptp_ops, wrap the
coefficients in a separate struct so they can be referenced together.

Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
Signed-off-by: Shenghao Yang <me@shenghaoyang.info>
---
 drivers/net/dsa/mv88e6xxx/chip.h |  7 ++--
 drivers/net/dsa/mv88e6xxx/ptp.c  | 57 ++++++++++++++++----------------
 2 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index c34caf9815c5..bd66189a593f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -714,6 +714,8 @@ struct mv88e6xxx_avb_ops {
 	int (*tai_write)(struct mv88e6xxx_chip *chip, int addr, u16 data);
 };
 
+struct mv88e6xxx_cc_coeffs;
+
 struct mv88e6xxx_ptp_ops {
 	u64 (*clock_read)(const struct cyclecounter *cc);
 	int (*ptp_enable)(struct ptp_clock_info *ptp,
@@ -731,10 +733,7 @@ struct mv88e6xxx_ptp_ops {
 	int arr1_sts_reg;
 	int dep_sts_reg;
 	u32 rx_filters;
-	u32 cc_shift;
-	u32 cc_mult;
-	u32 cc_mult_num;
-	u32 cc_mult_dem;
+	const struct mv88e6xxx_cc_coeffs *cc_coeffs;
 };
 
 struct mv88e6xxx_pcs_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index 56391e09b325..89040c9af9f3 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -18,6 +18,13 @@
 
 #define MV88E6XXX_MAX_ADJ_PPB	1000000
 
+struct mv88e6xxx_cc_coeffs {
+	u32 cc_shift;
+	u32 cc_mult;
+	u32 cc_mult_num;
+	u32 cc_mult_dem;
+};
+
 /* Family MV88E6250:
  * Raw timestamps are in units of 10-ns clock periods.
  *
@@ -25,10 +32,12 @@
  * simplifies to
  * clkadj = scaled_ppm * 2^7 / 5^5
  */
-#define MV88E6250_CC_SHIFT	28
-#define MV88E6250_CC_MULT	(10 << MV88E6250_CC_SHIFT)
-#define MV88E6250_CC_MULT_NUM	(1 << 7)
-#define MV88E6250_CC_MULT_DEM	3125ULL
+const struct mv88e6xxx_cc_coeffs mv88e6250_cc_coeffs = {
+	.cc_shift = 28,
+	.cc_mult = 10 << 28,
+	.cc_mult_num = 1 << 7,
+	.cc_mult_dem = 3125ULL,
+};
 
 /* Other families:
  * Raw timestamps are in units of 8-ns clock periods.
@@ -37,10 +46,12 @@
  * simplifies to
  * clkadj = scaled_ppm * 2^9 / 5^6
  */
-#define MV88E6XXX_CC_SHIFT	28
-#define MV88E6XXX_CC_MULT	(8 << MV88E6XXX_CC_SHIFT)
-#define MV88E6XXX_CC_MULT_NUM	(1 << 9)
-#define MV88E6XXX_CC_MULT_DEM	15625ULL
+const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_coeffs = {
+	.cc_shift = 28,
+	.cc_mult = 8 << 28,
+	.cc_mult_num = 1 << 9,
+	.cc_mult_dem = 15625ULL
+};
 
 #define TAI_EVENT_WORK_INTERVAL msecs_to_jiffies(100)
 
@@ -214,10 +225,10 @@ static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 		scaled_ppm = -scaled_ppm;
 	}
 
-	mult = ptp_ops->cc_mult;
-	adj = ptp_ops->cc_mult_num;
+	mult = ptp_ops->cc_coeffs->cc_mult;
+	adj = ptp_ops->cc_coeffs->cc_mult_num;
 	adj *= scaled_ppm;
-	diff = div_u64(adj, ptp_ops->cc_mult_dem);
+	diff = div_u64(adj, ptp_ops->cc_coeffs->cc_mult_dem);
 
 	mv88e6xxx_reg_lock(chip);
 
@@ -364,10 +375,7 @@ const struct mv88e6xxx_ptp_ops mv88e6165_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_shift = MV88E6XXX_CC_SHIFT,
-	.cc_mult = MV88E6XXX_CC_MULT,
-	.cc_mult_num = MV88E6XXX_CC_MULT_NUM,
-	.cc_mult_dem = MV88E6XXX_CC_MULT_DEM,
+	.cc_coeffs = &mv88e6xxx_cc_coeffs
 };
 
 const struct mv88e6xxx_ptp_ops mv88e6250_ptp_ops = {
@@ -391,10 +399,7 @@ const struct mv88e6xxx_ptp_ops mv88e6250_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_shift = MV88E6250_CC_SHIFT,
-	.cc_mult = MV88E6250_CC_MULT,
-	.cc_mult_num = MV88E6250_CC_MULT_NUM,
-	.cc_mult_dem = MV88E6250_CC_MULT_DEM,
+	.cc_coeffs = &mv88e6250_cc_coeffs,
 };
 
 const struct mv88e6xxx_ptp_ops mv88e6352_ptp_ops = {
@@ -418,10 +423,7 @@ const struct mv88e6xxx_ptp_ops mv88e6352_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_shift = MV88E6XXX_CC_SHIFT,
-	.cc_mult = MV88E6XXX_CC_MULT,
-	.cc_mult_num = MV88E6XXX_CC_MULT_NUM,
-	.cc_mult_dem = MV88E6XXX_CC_MULT_DEM,
+	.cc_coeffs = &mv88e6xxx_cc_coeffs,
 };
 
 const struct mv88e6xxx_ptp_ops mv88e6390_ptp_ops = {
@@ -446,10 +448,7 @@ const struct mv88e6xxx_ptp_ops mv88e6390_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_shift = MV88E6XXX_CC_SHIFT,
-	.cc_mult = MV88E6XXX_CC_MULT,
-	.cc_mult_num = MV88E6XXX_CC_MULT_NUM,
-	.cc_mult_dem = MV88E6XXX_CC_MULT_DEM,
+	.cc_coeffs = &mv88e6xxx_cc_coeffs,
 };
 
 static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)
@@ -487,8 +486,8 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
 	memset(&chip->tstamp_cc, 0, sizeof(chip->tstamp_cc));
 	chip->tstamp_cc.read	= mv88e6xxx_ptp_clock_read;
 	chip->tstamp_cc.mask	= CYCLECOUNTER_MASK(32);
-	chip->tstamp_cc.mult	= ptp_ops->cc_mult;
-	chip->tstamp_cc.shift	= ptp_ops->cc_shift;
+	chip->tstamp_cc.mult	= ptp_ops->cc_coeffs->cc_mult;
+	chip->tstamp_cc.shift	= ptp_ops->cc_coeffs->cc_shift;
 
 	timecounter_init(&chip->tstamp_tc, &chip->tstamp_cc,
 			 ktime_to_ns(ktime_get_real()));
-- 
2.46.1

Re: [PATCH net 1/3] net: dsa: mv88e6xxx: group cycle counter coefficients

[Andrew Lunn]

> -#define MV88E6250_CC_SHIFT	28
> -#define MV88E6250_CC_MULT	(10 << MV88E6250_CC_SHIFT)
> -#define MV88E6250_CC_MULT_NUM	(1 << 7)
> -#define MV88E6250_CC_MULT_DEM	3125ULL
> +const struct mv88e6xxx_cc_coeffs mv88e6250_cc_coeffs = {
> +	.cc_shift = 28,
> +	.cc_mult = 10 << 28,

This is slightly less readable, you loose that the 28 is the same as
cc_shift. I would suggest you keep the MV88E6250_CC_SHIFT and use it
for these two lines. Please do the same for other instances of this
structure.

	Andrew

[PATCH net 2/3] net: dsa: mv88e6xxx: read cycle counter period from hardware

[Shenghao Yang]

Instead of relying on a fixed mapping of hardware family to cycle
counter frequency, pull this information from the
MV88E6XXX_TAI_CLOCK_PERIOD register.

This lets us support switches with whose cycle counter frequencies
depend on board design.

Hardware with inaccessible clock period registers or unsupported periods
will fall back to the fixed mapping.

Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Shenghao Yang <me@shenghaoyang.info>
---
 drivers/net/dsa/mv88e6xxx/chip.h |  6 ++--
 drivers/net/dsa/mv88e6xxx/ptp.c  | 48 ++++++++++++++++++++++++--------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index bd66189a593f..8ff3f15e0d01 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -206,6 +206,7 @@ struct mv88e6xxx_gpio_ops;
 struct mv88e6xxx_avb_ops;
 struct mv88e6xxx_ptp_ops;
 struct mv88e6xxx_pcs_ops;
+struct mv88e6xxx_cc_coeffs;
 
 struct mv88e6xxx_irq {
 	u16 masked;
@@ -408,6 +409,7 @@ struct mv88e6xxx_chip {
 	struct cyclecounter	tstamp_cc;
 	struct timecounter	tstamp_tc;
 	struct delayed_work	overflow_work;
+	const struct mv88e6xxx_cc_coeffs *cc_coeffs;
 
 	struct ptp_clock	*ptp_clock;
 	struct ptp_clock_info	ptp_clock_info;
@@ -714,8 +716,6 @@ struct mv88e6xxx_avb_ops {
 	int (*tai_write)(struct mv88e6xxx_chip *chip, int addr, u16 data);
 };
 
-struct mv88e6xxx_cc_coeffs;
-
 struct mv88e6xxx_ptp_ops {
 	u64 (*clock_read)(const struct cyclecounter *cc);
 	int (*ptp_enable)(struct ptp_clock_info *ptp,
@@ -733,7 +733,7 @@ struct mv88e6xxx_ptp_ops {
 	int arr1_sts_reg;
 	int dep_sts_reg;
 	u32 rx_filters;
-	const struct mv88e6xxx_cc_coeffs *cc_coeffs;
+	const struct mv88e6xxx_cc_coeffs *default_cc_coeffs;
 };
 
 struct mv88e6xxx_pcs_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index 89040c9af9f3..be1fcbf75440 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -32,7 +32,7 @@ struct mv88e6xxx_cc_coeffs {
  * simplifies to
  * clkadj = scaled_ppm * 2^7 / 5^5
  */
-const struct mv88e6xxx_cc_coeffs mv88e6250_cc_coeffs = {
+const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_10ns_coeffs = {
 	.cc_shift = 28,
 	.cc_mult = 10 << 28,
 	.cc_mult_num = 1 << 7,
@@ -46,7 +46,7 @@ const struct mv88e6xxx_cc_coeffs mv88e6250_cc_coeffs = {
  * simplifies to
  * clkadj = scaled_ppm * 2^9 / 5^6
  */
-const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_coeffs = {
+const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_8ns_coeffs = {
 	.cc_shift = 28,
 	.cc_mult = 8 << 28,
 	.cc_mult_num = 1 << 9,
@@ -94,6 +94,30 @@ static int mv88e6352_set_gpio_func(struct mv88e6xxx_chip *chip, int pin,
 	return chip->info->ops->gpio_ops->set_pctl(chip, pin, func);
 }
 
+static const struct mv88e6xxx_cc_coeffs *
+mv88e6xxx_cc_coeff_get(struct mv88e6xxx_chip *chip)
+{
+	u16 period_ps;
+	int err;
+
+	err = mv88e6xxx_tai_read(chip, MV88E6XXX_TAI_CLOCK_PERIOD, &period_ps, 1);
+	if (err) {
+		dev_warn(chip->dev, "failed to read cycle counter period");
+		return chip->info->ops->ptp_ops->default_cc_coeffs;
+	}
+
+	switch (period_ps) {
+	case 8000:
+		return &mv88e6xxx_cc_8ns_coeffs;
+	case 10000:
+		return &mv88e6xxx_cc_10ns_coeffs;
+	default:
+		dev_warn(chip->dev, "unexpected cycle counter period of %u ps",
+			 period_ps);
+		return chip->info->ops->ptp_ops->default_cc_coeffs;
+	}
+}
+
 static u64 mv88e6352_ptp_clock_read(const struct cyclecounter *cc)
 {
 	struct mv88e6xxx_chip *chip = cc_to_chip(cc);
@@ -215,7 +239,6 @@ static void mv88e6352_tai_event_work(struct work_struct *ugly)
 static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	struct mv88e6xxx_chip *chip = ptp_to_chip(ptp);
-	const struct mv88e6xxx_ptp_ops *ptp_ops = chip->info->ops->ptp_ops;
 	int neg_adj = 0;
 	u32 diff, mult;
 	u64 adj;
@@ -225,10 +248,10 @@ static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 		scaled_ppm = -scaled_ppm;
 	}
 
-	mult = ptp_ops->cc_coeffs->cc_mult;
-	adj = ptp_ops->cc_coeffs->cc_mult_num;
+	mult = chip->cc_coeffs->cc_mult;
+	adj = chip->cc_coeffs->cc_mult_num;
 	adj *= scaled_ppm;
-	diff = div_u64(adj, ptp_ops->cc_coeffs->cc_mult_dem);
+	diff = div_u64(adj, chip->cc_coeffs->cc_mult_dem);
 
 	mv88e6xxx_reg_lock(chip);
 
@@ -375,7 +398,7 @@ const struct mv88e6xxx_ptp_ops mv88e6165_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_coeffs = &mv88e6xxx_cc_coeffs
+	.default_cc_coeffs = &mv88e6xxx_cc_8ns_coeffs
 };
 
 const struct mv88e6xxx_ptp_ops mv88e6250_ptp_ops = {
@@ -399,7 +422,7 @@ const struct mv88e6xxx_ptp_ops mv88e6250_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_coeffs = &mv88e6250_cc_coeffs,
+	.default_cc_coeffs = &mv88e6xxx_cc_10ns_coeffs,
 };
 
 const struct mv88e6xxx_ptp_ops mv88e6352_ptp_ops = {
@@ -423,7 +446,7 @@ const struct mv88e6xxx_ptp_ops mv88e6352_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_coeffs = &mv88e6xxx_cc_coeffs,
+	.default_cc_coeffs = &mv88e6xxx_cc_8ns_coeffs,
 };
 
 const struct mv88e6xxx_ptp_ops mv88e6390_ptp_ops = {
@@ -448,7 +471,7 @@ const struct mv88e6xxx_ptp_ops mv88e6390_ptp_ops = {
 		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
 		(1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
-	.cc_coeffs = &mv88e6xxx_cc_coeffs,
+	.default_cc_coeffs = &mv88e6xxx_cc_8ns_coeffs,
 };
 
 static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)
@@ -483,11 +506,12 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
 	int i;
 
 	/* Set up the cycle counter */
+	chip->cc_coeffs = mv88e6xxx_cc_coeff_get(chip);
 	memset(&chip->tstamp_cc, 0, sizeof(chip->tstamp_cc));
 	chip->tstamp_cc.read	= mv88e6xxx_ptp_clock_read;
 	chip->tstamp_cc.mask	= CYCLECOUNTER_MASK(32);
-	chip->tstamp_cc.mult	= ptp_ops->cc_coeffs->cc_mult;
-	chip->tstamp_cc.shift	= ptp_ops->cc_coeffs->cc_shift;
+	chip->tstamp_cc.mult	= chip->cc_coeffs->cc_mult;
+	chip->tstamp_cc.shift	= chip->cc_coeffs->cc_shift;
 
 	timecounter_init(&chip->tstamp_tc, &chip->tstamp_cc,
 			 ktime_to_ns(ktime_get_real()));
-- 
2.46.1

Re: [PATCH net 2/3] net: dsa: mv88e6xxx: read cycle counter period from hardware

[Andrew Lunn]

> +static const struct mv88e6xxx_cc_coeffs *
> +mv88e6xxx_cc_coeff_get(struct mv88e6xxx_chip *chip)
> +{
> +	u16 period_ps;
> +	int err;
> +
> +	err = mv88e6xxx_tai_read(chip, MV88E6XXX_TAI_CLOCK_PERIOD, &period_ps, 1);
> +	if (err) {
> +		dev_warn(chip->dev, "failed to read cycle counter period");
> +		return chip->info->ops->ptp_ops->default_cc_coeffs;
> +	}
> +
> +	switch (period_ps) {
> +	case 8000:
> +		return &mv88e6xxx_cc_8ns_coeffs;
> +	case 10000:
> +		return &mv88e6xxx_cc_10ns_coeffs;
> +	default:
> +		dev_warn(chip->dev, "unexpected cycle counter period of %u ps",
> +			 period_ps);
> +		return chip->info->ops->ptp_ops->default_cc_coeffs;

This chip mv88e6xxx_cc_coeffs vs ptp_ops mv88e6xxx_cc_coeffs all seems
a bit messy.

The mv88e6xxx_tai_read() MV88E6XXX_TAI_CLOCK_PERIOD is not going to
fail, except for the hardware is dead. There is nothing you can do
about that, so return the error code and let the probe fail.

What you are more worried about is if the value you get back is not
what you expect. It is not 8000 or 10000. I would do a dev_err() and
return -ENODEV, and let the probe fail. The datasheets suggests this
should not happen. But if it does, we should get reports from users
that PTP is issuing an error and the switch is not probing. We can
then fix the problem.

You can then drop mv88e6xxx_cc_coeffs from ptp_ops.

    Andrew

---
pw-bot: cr

Re: [PATCH net 2/3] net: dsa: mv88e6xxx: read cycle counter period from hardware

[Shenghao Yang]

On 30/9/24 23:31, Andrew Lunn wrote:
> The mv88e6xxx_tai_read() MV88E6XXX_TAI_CLOCK_PERIOD is not going to
> fail, except for the hardware is dead. There is nothing you can do
> about that, so return the error code and let the probe fail.

> What you are more worried about is if the value you get back is not
> what you expect. It is not 8000 or 10000. I would do a dev_err() and
> return -ENODEV, and let the probe fail.

Hi Andrew,

Thanks! I'll sort those and the naming in v2.

Shenghao

Re: [PATCH net 2/3] net: dsa: mv88e6xxx: read cycle counter period from hardware

[kernel test robot]

Hi Shenghao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Shenghao-Yang/net-dsa-mv88e6xxx-group-cycle-counter-coefficients/20240929-182245
base:   net/main
patch link:    https://lore.kernel.org/r/20240929101949.723658-3-me%40shenghaoyang.info
patch subject: [PATCH net 2/3] net: dsa: mv88e6xxx: read cycle counter period from hardware
config: sparc64-randconfig-r133-20240930 (https://download.01.org/0day-ci/archive/20241001/202410011105.XXvEbI6k-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20241001/202410011105.XXvEbI6k-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410011105.XXvEbI6k-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/dsa/mv88e6xxx/ptp.c:35:34: sparse: sparse: symbol 'mv88e6xxx_cc_10ns_coeffs' was not declared. Should it be static?
>> drivers/net/dsa/mv88e6xxx/ptp.c:49:34: sparse: sparse: symbol 'mv88e6xxx_cc_8ns_coeffs' was not declared. Should it be static?

vim +/mv88e6xxx_cc_10ns_coeffs +35 drivers/net/dsa/mv88e6xxx/ptp.c

    27	
    28	/* Family MV88E6250:
    29	 * Raw timestamps are in units of 10-ns clock periods.
    30	 *
    31	 * clkadj = scaled_ppm * 10*2^28 / (10^6 * 2^16)
    32	 * simplifies to
    33	 * clkadj = scaled_ppm * 2^7 / 5^5
    34	 */
  > 35	const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_10ns_coeffs = {
    36		.cc_shift = 28,
    37		.cc_mult = 10 << 28,
    38		.cc_mult_num = 1 << 7,
    39		.cc_mult_dem = 3125ULL,
    40	};
    41	
    42	/* Other families:
    43	 * Raw timestamps are in units of 8-ns clock periods.
    44	 *
    45	 * clkadj = scaled_ppm * 8*2^28 / (10^6 * 2^16)
    46	 * simplifies to
    47	 * clkadj = scaled_ppm * 2^9 / 5^6
    48	 */
  > 49	const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_8ns_coeffs = {
    50		.cc_shift = 28,
    51		.cc_mult = 8 << 28,
    52		.cc_mult_num = 1 << 9,
    53		.cc_mult_dem = 15625ULL
    54	};
    55	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH net 3/3] net: dsa: mv88e6xxx: support 4000ps cycle counter period

[Shenghao Yang]

The MV88E6393X family of devices can run its cycle counter off
an internal 250MHz clock instead of an external 125MHz one.

Add support for this cycle counter period by adding another set
of coefficients and lowering the periodic cycle counter read interval
to compensate for faster overflows at the increased frequency.

Otherwise, the PHC runs at 2x real time in userspace and cannot be
synchronized.

Fixes: de776d0d316f ("net: dsa: mv88e6xxx: add support for mv88e6393x family")
Signed-off-by: Shenghao Yang <me@shenghaoyang.info>
---
 drivers/net/dsa/mv88e6xxx/ptp.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index be1fcbf75440..6a88895845ea 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -39,7 +39,7 @@ const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_10ns_coeffs = {
 	.cc_mult_dem = 3125ULL,
 };
 
-/* Other families:
+/* Other families except MV88E6393X in internal clock mode:
  * Raw timestamps are in units of 8-ns clock periods.
  *
  * clkadj = scaled_ppm * 8*2^28 / (10^6 * 2^16)
@@ -53,6 +53,20 @@ const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_8ns_coeffs = {
 	.cc_mult_dem = 15625ULL
 };
 
+/* Family MV88E6393X using internal clock:
+ * Raw timestamps are in units of 4-ns clock periods.
+ *
+ * clkadj = scaled_ppm * 4*2^28 / (10^6 * 2^16)
+ * simplifies to
+ * clkadj = scaled_ppm * 2^8 / 5^6
+ */
+const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_4ns_coeffs = {
+	.cc_shift = 28,
+	.cc_mult = 4 << 28,
+	.cc_mult_num = 1 << 8,
+	.cc_mult_dem = 15625ULL
+};
+
 #define TAI_EVENT_WORK_INTERVAL msecs_to_jiffies(100)
 
 #define cc_to_chip(cc) container_of(cc, struct mv88e6xxx_chip, tstamp_cc)
@@ -107,6 +121,8 @@ mv88e6xxx_cc_coeff_get(struct mv88e6xxx_chip *chip)
 	}
 
 	switch (period_ps) {
+	case 4000:
+		return &mv88e6xxx_cc_4ns_coeffs;
 	case 8000:
 		return &mv88e6xxx_cc_8ns_coeffs;
 	case 10000:
@@ -484,10 +500,10 @@ static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)
 	return 0;
 }
 
-/* With a 125MHz input clock, the 32-bit timestamp counter overflows in ~34.3
+/* With a 250MHz input clock, the 32-bit timestamp counter overflows in ~17.2
  * seconds; this task forces periodic reads so that we don't miss any.
  */
-#define MV88E6XXX_TAI_OVERFLOW_PERIOD (HZ * 16)
+#define MV88E6XXX_TAI_OVERFLOW_PERIOD (HZ * 8)
 static void mv88e6xxx_ptp_overflow_check(struct work_struct *work)
 {
 	struct delayed_work *dw = to_delayed_work(work);
-- 
2.46.1

Re: [PATCH net 3/3] net: dsa: mv88e6xxx: support 4000ps cycle counter period

[kernel test robot]

Hi Shenghao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Shenghao-Yang/net-dsa-mv88e6xxx-group-cycle-counter-coefficients/20240929-182245
base:   net/main
patch link:    https://lore.kernel.org/r/20240929101949.723658-5-me%40shenghaoyang.info
patch subject: [PATCH net 3/3] net: dsa: mv88e6xxx: support 4000ps cycle counter period
config: sparc64-randconfig-r133-20240930 (https://download.01.org/0day-ci/archive/20241001/202410011454.X1kiOgOz-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20241001/202410011454.X1kiOgOz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410011454.X1kiOgOz-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/net/dsa/mv88e6xxx/ptp.c:35:34: sparse: sparse: symbol 'mv88e6xxx_cc_10ns_coeffs' was not declared. Should it be static?
   drivers/net/dsa/mv88e6xxx/ptp.c:49:34: sparse: sparse: symbol 'mv88e6xxx_cc_8ns_coeffs' was not declared. Should it be static?
>> drivers/net/dsa/mv88e6xxx/ptp.c:63:34: sparse: sparse: symbol 'mv88e6xxx_cc_4ns_coeffs' was not declared. Should it be static?

vim +/mv88e6xxx_cc_4ns_coeffs +63 drivers/net/dsa/mv88e6xxx/ptp.c

    55	
    56	/* Family MV88E6393X using internal clock:
    57	 * Raw timestamps are in units of 4-ns clock periods.
    58	 *
    59	 * clkadj = scaled_ppm * 4*2^28 / (10^6 * 2^16)
    60	 * simplifies to
    61	 * clkadj = scaled_ppm * 2^8 / 5^6
    62	 */
  > 63	const struct mv88e6xxx_cc_coeffs mv88e6xxx_cc_4ns_coeffs = {
    64		.cc_shift = 28,
    65		.cc_mult = 4 << 28,
    66		.cc_mult_num = 1 << 8,
    67		.cc_mult_dem = 15625ULL
    68	};
    69	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net 0/3] net: dsa: mv88e6xxx: fix MV88E6393X PHC frequency on internal clock

[Andrew Lunn]

On Sun, Sep 29, 2024 at 06:19:44PM +0800, Shenghao Yang wrote:
> The MV88E6393X family of switches can additionally run their cycle
> counters using a 250MHz internal clock instead of the usual 125MHz
> externa clock [1].

external

	Andrew