Re: [PATCH 4/4] net: dsa: mv88e6xxx: Support serdes ports on MV88E6123/6131

2020-10-27 Thread Andrew Lunn 
On Tue, Oct 27, 2020 at 08:56:09PM +, Chris Packham wrote:
> 
> On 24/10/20 11:42 am, Andrew Lunn wrote:
> >> +int mv88e6123_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port)
> >> +{
> >> +  if (mv88e6xxx_serdes_get_lane(chip, port) == 0)
> >> +  return 0;
> >> +
> >> +  return 26 * sizeof(u16);
> >> +}
> > Hi Chris
> >
> > Where did 26 come from?

> In the 88E6123 Serdes Register Description the highest register address 
> was 26 so that's what I used.

> Technically there are 32 possible 
> addresses in that space so I could go up to 32. Equally registers 9-14, 
> 20, 22-23 are "reserved" so I could remove them from the total and have 
> mv88e6123_serdes_get_regs() skip over them. I'm guessing skipping some 
> (27-32) and not others is probably less than ideal.

Hi Chris

I would dump all 32 and let userspace figure out if they mean
anything. The current register dump for the 6390 SEDES is horrible,
and i missed a register, and it is not easy to put in its correct
place because of ABI reasons. If you can do KISS, all the better.

  Andrew

Re: [PATCH 4/4] net: dsa: mv88e6xxx: Support serdes ports on MV88E6123/6131

2020-10-27 Thread Chris Packham 

On 24/10/20 11:42 am, Andrew Lunn wrote:
>> +int mv88e6123_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port)
>> +{
>> +if (mv88e6xxx_serdes_get_lane(chip, port) == 0)
>> +return 0;
>> +
>> +return 26 * sizeof(u16);
>> +}
> Hi Chris
>
> Where did 26 come from?
In the 88E6123 Serdes Register Description the highest register address 
was 26 so that's what I used. Technically there are 32 possible 
addresses in that space so I could go up to 32. Equally registers 9-14, 
20, 22-23 are "reserved" so I could remove them from the total and have 
mv88e6123_serdes_get_regs() skip over them. I'm guessing skipping some 
(27-32) and not others is probably less than ideal.

Re: [PATCH 4/4] net: dsa: mv88e6xxx: Support serdes ports on MV88E6123/6131

2020-10-23 Thread Andrew Lunn 
> +int mv88e6123_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port)
> +{
> + if (mv88e6xxx_serdes_get_lane(chip, port) == 0)
> + return 0;
> +
> + return 26 * sizeof(u16);
> +}

Hi Chris

Where did 26 come from?

  Andrew

[PATCH 4/4] net: dsa: mv88e6xxx: Support serdes ports on MV88E6123/6131

2020-10-21 Thread Chris Packham 
Implement serdes_power, serdes_get_lane and serdes_pcs_get_state ops for
the MV88E6123 so that the ports without a built-in PHY supported as
serdes ports and directly connected to other network interfaces or to
SFPs. Also implement serdes_get_regs_len and serdes_get_regs to aid
future debugging.

Signed-off-by: Chris Packham 
---
This is untested (apart from compilation) it assumes the SERDES "phy"
address corresponds to the port number but I'm not confident that is a
valid assumption.

Changes in v4:
- Error handling in mv88e6123_serdes_get_regs
Changes in v3:
- None
Changes in v2:
- new

 drivers/net/dsa/mv88e6xxx/chip.c   | 10 +++
 drivers/net/dsa/mv88e6xxx/serdes.c | 46 ++
 drivers/net/dsa/mv88e6xxx/serdes.h |  4 +++
 3 files changed, 60 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b582d98ca437..737416c666c1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3575,6 +3575,11 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
.set_egress_port = mv88e6095_g1_set_egress_port,
.watchdog_ops = _watchdog_ops,
.mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
+   .serdes_power = mv88e6123_serdes_power,
+   .serdes_get_lane = mv88e6185_serdes_get_lane,
+   .serdes_pcs_get_state = mv88e6185_serdes_pcs_get_state,
+   .serdes_get_regs_len = mv88e6123_serdes_get_regs_len,
+   .serdes_get_regs = mv88e6123_serdes_get_regs,
.pot_clear = mv88e6xxx_g2_pot_clear,
.reset = mv88e6352_g1_reset,
.atu_get_hash = mv88e6165_g1_atu_get_hash,
@@ -3615,6 +3620,11 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
.set_egress_port = mv88e6095_g1_set_egress_port,
.watchdog_ops = _watchdog_ops,
.mgmt_rsvd2cpu = mv88e6185_g2_mgmt_rsvd2cpu,
+   .serdes_power = mv88e6123_serdes_power,
+   .serdes_get_lane = mv88e6185_serdes_get_lane,
+   .serdes_pcs_get_state = mv88e6185_serdes_pcs_get_state,
+   .serdes_get_regs_len = mv88e6123_serdes_get_regs_len,
+   .serdes_get_regs = mv88e6123_serdes_get_regs,
.ppu_enable = mv88e6185_g1_ppu_enable,
.set_cascade_port = mv88e6185_g1_set_cascade_port,
.ppu_disable = mv88e6185_g1_ppu_disable,
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c 
b/drivers/net/dsa/mv88e6xxx/serdes.c
index ec9ca7210bb0..0e84d5c7be61 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -430,6 +430,52 @@ u8 mv88e6341_serdes_get_lane(struct mv88e6xxx_chip *chip, 
int port)
return lane;
 }
 
+int mv88e6123_serdes_power(struct mv88e6xxx_chip *chip, int port, u8 lane,
+  bool up)
+{
+   u16 val, new_val;
+   int err;
+
+   err = mv88e6xxx_phy_read(chip, port, MII_BMCR, );
+   if (err)
+   return err;
+
+   if (up)
+   new_val = val & ~BMCR_PDOWN;
+   else
+   new_val = val | BMCR_PDOWN;
+
+   if (val != new_val)
+   err = mv88e6xxx_phy_write(chip, port, MII_BMCR, val);
+
+   return err;
+}
+
+int mv88e6123_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port)
+{
+   if (mv88e6xxx_serdes_get_lane(chip, port) == 0)
+   return 0;
+
+   return 26 * sizeof(u16);
+}
+
+void mv88e6123_serdes_get_regs(struct mv88e6xxx_chip *chip, int port, void *_p)
+{
+   u16 *p = _p;
+   u16 reg;
+   int err;
+   int i;
+
+   if (mv88e6xxx_serdes_get_lane(chip, port) == 0)
+   return;
+
+   for (i = 0; i < 26; i++) {
+   err = mv88e6xxx_phy_read(chip, port, i, );
+   if (!err)
+   p[i] = reg;
+   }
+}
+
 int mv88e6185_serdes_power(struct mv88e6xxx_chip *chip, int port, u8 lane,
   bool up)
 {
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h 
b/drivers/net/dsa/mv88e6xxx/serdes.h
index c24ec4122c9e..b573139928c4 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -104,6 +104,8 @@ unsigned int mv88e6352_serdes_irq_mapping(struct 
mv88e6xxx_chip *chip,
  int port);
 unsigned int mv88e6390_serdes_irq_mapping(struct mv88e6xxx_chip *chip,
  int port);
+int mv88e6123_serdes_power(struct mv88e6xxx_chip *chip, int port, u8 lane,
+  bool up);
 int mv88e6185_serdes_power(struct mv88e6xxx_chip *chip, int port, u8 lane,
   bool up);
 int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, u8 lane,
@@ -129,6 +131,8 @@ int mv88e6390_serdes_get_strings(struct mv88e6xxx_chip 
*chip,
 int mv88e6390_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
   uint64_t *data);
 
+int mv88e6123_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port);
+void mv88e6123_serdes_get_regs(struct mv88e6xxx_chip *chip, int port, void 
*_p);