| Message ID | 20201022012516.18720-5-chris.packham@alliedtelesis.co.nz |
|---|---|
| State | Deferred |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: dsa: mv88e6xxx: serdes link without phy | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Guessed tree name to be net-next |
| jkicinski/subject_prefix | warning | Target tree name not specified in the subject |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 90 lines checked |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
> +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
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.
On Tue, Oct 27, 2020 at 08:56:09PM +0000, 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
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 = &mv88e6097_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 = &mv88e6097_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, &val); + 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); int mv88e6352_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port); void mv88e6352_serdes_get_regs(struct mv88e6xxx_chip *chip, int port, void *_p); int mv88e6390_serdes_get_regs_len(struct mv88e6xxx_chip *chip, int port);
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 <chris.packham@alliedtelesis.co.nz> --- 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(+)