[RFC PATCH 1/1] mv88e6xxx: Cache scratch config of 6352 in setup

[RFC PATCH 1/1] mv88e6xxx: Cache scratch config of 6352 in setup

[Fidan Aliyeva]

In mv88e6352 scratch register in Global Control 2 set of registers
returns which port is attached to SERDES. This value is set and
stays constant after the switch is released from reset; thus, it
can be cached during chip setup instead of reading the register
everytime when SERDES config is needed.

1. Add setup_chip_specific member to mv88e6xxx_ops and
g2_scratch_config3 member to mv88e6xxx_chip.
2. Add mv88e6352_g2_cache_global_scratch_config3 function as
.setup_chip_specific which reads the CONFIG3 value from the scratch
register and caches it.
3. Call .setup_chip_specific during chip setup.
4. Refactor mv88e6352_g2_scratch_port_has_serdes to use the cached
value.
5. Remove the locks surrounding mv88e6352_g2_scratch_port_has_serdes.
Not needed anymore since no register access in the function.

Co-developed-by: Thomas Eckerman <thomas.eckerman.ext@ericsson.com>
Signed-off-by: Thomas Eckerman <thomas.eckerman.ext@ericsson.com>
Signed-off-by: Fidan Aliyeva <fidan.aliyeva.ext@ericsson.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c            | 13 ++++++--
 drivers/net/dsa/mv88e6xxx/chip.h            |  6 ++++
 drivers/net/dsa/mv88e6xxx/global2.h         |  1 +
 drivers/net/dsa/mv88e6xxx/global2_scratch.c | 36 +++++++++++++++------
 drivers/net/dsa/mv88e6xxx/pcs-6352.c        |  2 --
 drivers/net/dsa/mv88e6xxx/serdes.c          |  2 --
 6 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8ca5fd40df92..ab354bc2e2f1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -685,9 +685,6 @@ static void mv88e6352_phylink_get_caps(struct mv88e6xxx_chip *chip, int port,
 	/* Port 4 supports automedia if the serdes is associated with it. */
 	if (port == 4) {
 		err = mv88e6352_g2_scratch_port_has_serdes(chip, port);
-		if (err < 0)
-			dev_err(chip->dev, "p%d: failed to read scratch\n",
-				port);
 		if (err <= 0)
 			return;

@@ -3997,6 +3994,12 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 			goto unlock;
 	}

+	if (chip->info->ops->setup_chip_specific) {
+		err = chip->info->ops->setup_chip_specific(chip);
+		if (err)
+			goto unlock;
+	}
+
 	/* Cache the cmode of each port. */
 	for (i = 0; i < mv88e6xxx_num_ports(chip); i++) {
 		if (chip->info->ops->port_get_cmode) {
@@ -4662,6 +4665,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.stu_getnext = mv88e6352_g1_stu_getnext,
 	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
+	.setup_chip_specific = mv88e6352_g2_cache_global_scratch_config3,
 	.serdes_get_regs_len = mv88e6352_serdes_get_regs_len,
 	.serdes_get_regs = mv88e6352_serdes_get_regs,
 	.gpio_ops = &mv88e6352_gpio_ops,
@@ -4765,6 +4769,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.stu_getnext = mv88e6352_g1_stu_getnext,
 	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
+	.setup_chip_specific = mv88e6352_g2_cache_global_scratch_config3,
 	.serdes_irq_mapping = mv88e6352_serdes_irq_mapping,
 	.serdes_get_regs_len = mv88e6352_serdes_get_regs_len,
 	.serdes_get_regs = mv88e6352_serdes_get_regs,
@@ -5040,6 +5045,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.stu_getnext = mv88e6352_g1_stu_getnext,
 	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
+	.setup_chip_specific = mv88e6352_g2_cache_global_scratch_config3,
 	.serdes_irq_mapping = mv88e6352_serdes_irq_mapping,
 	.serdes_get_regs_len = mv88e6352_serdes_get_regs_len,
 	.serdes_get_regs = mv88e6352_serdes_get_regs,
@@ -5475,6 +5481,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.avb_ops = &mv88e6352_avb_ops,
 	.ptp_ops = &mv88e6352_ptp_ops,
+	.setup_chip_specific = mv88e6352_g2_cache_global_scratch_config3,
 	.serdes_get_sset_count = mv88e6352_serdes_get_sset_count,
 	.serdes_get_strings = mv88e6352_serdes_get_strings,
 	.serdes_get_stats = mv88e6352_serdes_get_stats,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 2b235ac2c5df..683f5a626f34 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -454,6 +454,9 @@ struct mv88e6xxx_chip {

 	/* TCAM entries */
 	struct mv88e6xxx_tcam tcam;
+
+	/* Global2 scratch register config data3 */
+	u8 g2_scratch_config3;
 };

 #define TCAM_MATCH_SIZE 96
@@ -719,6 +722,9 @@ struct mv88e6xxx_ops {

 	/* Ternary Content Addressable Memory operations */
 	const struct mv88e6xxx_tcam_ops *tcam_ops;
+
+	/* Apply chip specific setup steps */
+	int (*setup_chip_specific)(struct mv88e6xxx_chip *chip);
 };

 struct mv88e6xxx_irq_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 82f9b410de0b..ff5adf7c9bc3 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -382,6 +382,7 @@ int mv88e6390_g2_scratch_gpio_set_smi(struct mv88e6xxx_chip *chip,
 				      bool external);
 int mv88e6393x_g2_scratch_gpio_set_smi(struct mv88e6xxx_chip *chip,
 				       bool external);
+int mv88e6352_g2_cache_global_scratch_config3(struct mv88e6xxx_chip *chip);
 int mv88e6352_g2_scratch_port_has_serdes(struct mv88e6xxx_chip *chip, int port);
 int mv88e6xxx_g2_atu_stats_set(struct mv88e6xxx_chip *chip, u16 kind, u16 bin);
 int mv88e6xxx_g2_atu_stats_get(struct mv88e6xxx_chip *chip, u16 *stats);
diff --git a/drivers/net/dsa/mv88e6xxx/global2_scratch.c b/drivers/net/dsa/mv88e6xxx/global2_scratch.c
index 53a6d3ed63b3..e57b25aa2a4c 100644
--- a/drivers/net/dsa/mv88e6xxx/global2_scratch.c
+++ b/drivers/net/dsa/mv88e6xxx/global2_scratch.c
@@ -322,18 +322,18 @@ int mv88e6393x_g2_scratch_gpio_set_smi(struct mv88e6xxx_chip *chip,
 }

 /**
- * mv88e6352_g2_scratch_port_has_serdes - indicate if a port can have a serdes
+ * mv88e6352_g2_cache_global_scratch_config3 - caches G2 CONFIG3 value
  * @chip: chip private data
- * @port: port number to check for serdes
  *
- * Indicates whether the port may have a serdes attached according to the
- * pin strapping. Returns negative error number, 0 if the port is not
- * configured to have a serdes, and 1 if the port is configured to have a
- * serdes attached.
+ * Reads and stores config3 value of global2 scratch registers, which
+ * can be used to determine if the port is attached to a serdes. The
+ * value does not change once the switch is released from reset. Returns
+ * negative error number if the register read fails; otherwise, 0
  */
-int mv88e6352_g2_scratch_port_has_serdes(struct mv88e6xxx_chip *chip, int port)
+
+int mv88e6352_g2_cache_global_scratch_config3(struct mv88e6xxx_chip *chip)
 {
-	u8 config3, p;
+	u8 config3;
 	int err;

 	err = mv88e6xxx_g2_scratch_read(chip, MV88E6352_G2_SCRATCH_CONFIG_DATA3,
@@ -341,7 +341,25 @@ int mv88e6352_g2_scratch_port_has_serdes(struct mv88e6xxx_chip *chip, int port)
 	if (err)
 		return err;

-	if (config3 & MV88E6352_G2_SCRATCH_CONFIG_DATA3_S_SEL)
+	chip->g2_scratch_config3 = config3;
+
+	return 0;
+}
+
+/**
+ * mv88e6352_g2_scratch_port_has_serdes - indicate if a port has serdes
+ * @chip: chip private data
+ * @port: port number to check for serdes
+ *
+ * Indicates whether the port may have a serdes attached according to the
+ * pin strapping. Returns 0 if the port is not configured to have a serdes, and
+ * 1 if the port is configured to have a serdes attached.
+ */
+int mv88e6352_g2_scratch_port_has_serdes(struct mv88e6xxx_chip *chip, int port)
+{
+	u8 p;
+
+	if (chip->g2_scratch_config3 & MV88E6352_G2_SCRATCH_CONFIG_DATA3_S_SEL)
 		p = 5;
 	else
 		p = 4;
diff --git a/drivers/net/dsa/mv88e6xxx/pcs-6352.c b/drivers/net/dsa/mv88e6xxx/pcs-6352.c
index 9ebf0f89f817..308655d72d52 100644
--- a/drivers/net/dsa/mv88e6xxx/pcs-6352.c
+++ b/drivers/net/dsa/mv88e6xxx/pcs-6352.c
@@ -326,9 +326,7 @@ static int mv88e6352_pcs_init(struct mv88e6xxx_chip *chip, int port)
 	unsigned int irq;
 	int err;

-	mv88e6xxx_reg_lock(chip);
 	err = mv88e6352_g2_scratch_port_has_serdes(chip, port);
-	mv88e6xxx_reg_unlock(chip);
 	if (err <= 0)
 		return err;

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index a936ee80ce00..d3d3c121a32d 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -202,9 +202,7 @@ int mv88e6352_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port)
 {
 	int err;

-	mv88e6xxx_reg_lock(chip);
 	err = mv88e6352_g2_scratch_port_has_serdes(chip, port);
-	mv88e6xxx_reg_unlock(chip);
 	if (err <= 0)
 		return err;

--
2.36.0

Re: [RFC PATCH 1/1] mv88e6xxx: Cache scratch config of 6352 in setup

[Fidan Aliyeva]

>
> In mv88e6352 scratch register in Global Control 2 set of registers
> returns which port is attached to SERDES. This value is set and
> stays constant after the switch is released from reset; thus, it
> can be cached during chip setup instead of reading the register
> everytime when SERDES config is needed.
> 
> 1. Add setup_chip_specific member to mv88e6xxx_ops and
> g2_scratch_config3 member to mv88e6xxx_chip.
> 2. Add mv88e6352_g2_cache_global_scratch_config3 function as
> .setup_chip_specific which reads the CONFIG3 value from the scratch
> register and caches it.
> 3. Call .setup_chip_specific during chip setup.
> 

Can we actually use setup_errata for this instead of adding a new member
to mv88e6xxx_ops?

Thank you for your reviews in advance.

Best,
Fidan

Re: [RFC PATCH 1/1] mv88e6xxx: Cache scratch config of 6352 in setup

[Andrew Lunn]

On Sun, May 10, 2026 at 11:45:18PM +0200, Fidan Aliyeva wrote:
> >
> > In mv88e6352 scratch register in Global Control 2 set of registers
> > returns which port is attached to SERDES. This value is set and
> > stays constant after the switch is released from reset; thus, it
> > can be cached during chip setup instead of reading the register
> > everytime when SERDES config is needed.
> > 
> > 1. Add setup_chip_specific member to mv88e6xxx_ops and
> > g2_scratch_config3 member to mv88e6xxx_chip.
> > 2. Add mv88e6352_g2_cache_global_scratch_config3 function as
> > .setup_chip_specific which reads the CONFIG3 value from the scratch
> > register and caches it.
> > 3. Call .setup_chip_specific during chip setup.
> > 
> 
> Can we actually use setup_errata for this instead of adding a new member
> to mv88e6xxx_ops?

I was also thinking if there was another ops which could be used.

How about...

CONFIG3 holds the pin strapping values at reset. So how about
ops.reset?

Add a int mv88e6352_reset(struct mv88e6xxx_chip *chip) which first
calls mv88e6352_g1_reset() and then reads the scratch register?

    Andrew

Re: [RFC PATCH 1/1] mv88e6xxx: Cache scratch config of 6352 in setup

[Andrew Lunn]

On Sun, May 10, 2026 at 11:34:29PM +0200, Fidan Aliyeva wrote:
> In mv88e6352 scratch register in Global Control 2 set of registers
> returns which port is attached to SERDES. This value is set and
> stays constant after the switch is released from reset; thus, it
> can be cached during chip setup instead of reading the register
> everytime when SERDES config is needed.
> 
> 1. Add setup_chip_specific member to mv88e6xxx_ops and
> g2_scratch_config3 member to mv88e6xxx_chip.
> 2. Add mv88e6352_g2_cache_global_scratch_config3 function as
> .setup_chip_specific which reads the CONFIG3 value from the scratch
> register and caches it.
> 3. Call .setup_chip_specific during chip setup.
> 4. Refactor mv88e6352_g2_scratch_port_has_serdes to use the cached
> value.
> 5. Remove the locks surrounding mv88e6352_g2_scratch_port_has_serdes.
> Not needed anymore since no register access in the function.

We ask for lots of small patches which are obviously correct and have
good commit messages.

It seems like you could break this up into smaller patches. Taking to
account my other comment, how about 4 Patches:

1) Add mv88e6532_reset() which simply called mv88e6532_g1_reset(), and
make use of it for the 6352 family ops structure.

2) add g2_scratch_config3 member to mv88e6xxx_chip, add a helper to
read the register, and call it from mv88e6532_reset().

3) Rework mv88e6352_g2_scratch_port_has_serdes() to use the cached value.

4) Remove the locks.

   Andrew