On Mon, Jan 11, 2021 at 02:21:55AM +0100, Marek Behún wrote:
> From: Pavana Sharma <pavana.sha...@digi.com>
> 
> The Marvell 88E6393X device is a single-chip integration of a 11-port
> Ethernet switch with eight integrated Gigabit Ethernet (GbE)
> transceivers and three 10-Gigabit interfaces.
> 
> This patch adds functionalities specific to mv88e6393x family (88E6393X,
> 88E6193X and 88E6191X).
> 
> The main differences between previous devices and this one are:
> - port 0 can be a SERDES port
> - all SERDESes are one-lane, eg. no XAUI nor RXAUI
> - on the other hand the SERDESes can do USXGMII, 10GBASER and 5GBASER
>   (on 6191X only one SERDES is capable of more than 1g; USXGMII is not
>   yet supported with this change)
> - Port Policy CTL register is changed to Port Policy MGMT CTL register,
>   via which serveral more registers can be accessed indirectly
              ~~~~~~~~
              several
> - egress monitor port is configured differently
> - ingress monitor/CPU/mirror ports are configured differently and can be
>   configured per port (ie. each port can have different ingress monitor
>   port, for example)
> - port speed AltBit works differently than previously
> - PHY registers can be also accessed via MDIO address 0x18 and 0x19
>   (on previous devices they could be accessed only via Global 2 offsets
>    0x18 and 0x19, which means two indirections; this feature is not yet
>    leveraged with this patch)
> 
> Co-developed-by: Ashkan Boldaji <ashkan.bold...@digi.com>
> Signed-off-by: Ashkan Boldaji <ashkan.bold...@digi.com>
> Signed-off-by: Pavana Sharma <pavana.sha...@digi.com>
> Co-developed-by: Marek Behún <ka...@kernel.org>
> Signed-off-by: Marek Behún <ka...@kernel.org>
> ---
> +static void mv88e6393x_phylink_validate(struct mv88e6xxx_chip *chip, int 
> port,
> +                                     unsigned long *mask,
> +                                     struct phylink_link_state *state)
> +{
> +     if (port == 0 || port == 9 || port == 10) {
> +             phylink_set(mask, 10000baseT_Full);
> +             phylink_set(mask, 10000baseKR_Full);

I think I understand the reason for declaring 10GBase-KR support in
phylink_validate, in case the PHY supports that link mode on the media
side, but...

> +             phylink_set(mask, 10000baseCR_Full);
> +             phylink_set(mask, 10000baseSR_Full);
> +             phylink_set(mask, 10000baseLR_Full);
> +             phylink_set(mask, 10000baseLRM_Full);
> +             phylink_set(mask, 10000baseER_Full);
> +             phylink_set(mask, 5000baseT_Full);
> +             phylink_set(mask, 2500baseX_Full);
> +             phylink_set(mask, 2500baseT_Full);
> +     }
> +
> +     phylink_set(mask, 1000baseT_Full);
> +     phylink_set(mask, 1000baseX_Full);
> +
> +     mv88e6065_phylink_validate(chip, port, mask, state);
> +}
> +
> @@ -450,6 +559,9 @@ static int mv88e6xxx_port_set_cmode(struct mv88e6xxx_chip 
> *chip, int port,
>       case PHY_INTERFACE_MODE_2500BASEX:
>               cmode = MV88E6XXX_PORT_STS_CMODE_2500BASEX;
>               break;
> +     case PHY_INTERFACE_MODE_5GBASER:
> +             cmode = MV88E6393X_PORT_STS_CMODE_5GBASER;
> +             break;
>       case PHY_INTERFACE_MODE_XGMII:
>       case PHY_INTERFACE_MODE_XAUI:
>               cmode = MV88E6XXX_PORT_STS_CMODE_XAUI;
> @@ -457,6 +569,10 @@ static int mv88e6xxx_port_set_cmode(struct 
> mv88e6xxx_chip *chip, int port,
>       case PHY_INTERFACE_MODE_RXAUI:
>               cmode = MV88E6XXX_PORT_STS_CMODE_RXAUI;
>               break;
> +     case PHY_INTERFACE_MODE_10GBASER:
> +     case PHY_INTERFACE_MODE_10GKR:

Does the SERDES actually support 10GBase-KR (aka 10GBase-R for copper
backplanes)? It is different than plain 10GBase-R (abusingly called XFI)
by the need of a link training procedure to negotiate SERDES eye
parameters. There have been discussion in the past where it turned out
that drivers which didn't really support 10GBase-KR incorrectly reported
that they did.

> +             cmode = MV88E6393X_PORT_STS_CMODE_10GBASER;
> +             break;
>       default:
>               cmode = 0;
>       }
> @@ -541,6 +657,29 @@ int mv88e6390_port_set_cmode(struct mv88e6xxx_chip 
> *chip, int port,
>       return mv88e6xxx_port_set_cmode(chip, port, mode, false);
>  }
>  
> +int mv88e6393x_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
> +                           phy_interface_t mode)
> +{
> +     int err;
> +     u16 reg;
> +
> +     if (port != 0 && port != 9 && port != 10)
> +             return -EOPNOTSUPP;
> +
> +     /* mv88e6393x errata 4.5: EEE should be disabled on SERDES ports */
> +     err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_MAC_CTL, &reg);
> +     if (err)
> +             return err;
> +
> +     reg &= ~MV88E6XXX_PORT_MAC_CTL_EEE;
> +     reg |= MV88E6XXX_PORT_MAC_CTL_FORCE_EEE;
> +     err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_MAC_CTL, reg);
> +     if (err)
> +             return err;
> +
> +     return mv88e6xxx_port_set_cmode(chip, port, mode, false);
> +}
> +
>  static int mv88e6341_port_set_cmode_writable(struct mv88e6xxx_chip *chip,
>                                            int port)
>  {
> @@ -1164,6 +1303,135 @@ int mv88e6xxx_port_disable_pri_override(struct 
> mv88e6xxx_chip *chip, int port)
>       return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_PRI_OVERRIDE, 0);
>  }
>  
> +/* Offset 0x0E: Policy & MGMT Control Register for FAMILY 6191X 6193X 6393X 
> */
> +
> +static int mv88e6393x_port_policy_write(struct mv88e6xxx_chip *chip, int 
> port,
> +                                     u16 pointer, u8 data)
> +{
> +     u16 reg;
> +
> +     reg = MV88E6393X_PORT_POLICY_MGMT_CTL_UPDATE | pointer | data;

I think the assignment fits on the same line as the declaration?

> +
> +     return mv88e6xxx_port_write(chip, port, MV88E6393X_PORT_POLICY_MGMT_CTL,
> +                                 reg);
> +}
> +
> +int mv88e6393x_serdes_setup_errata(struct mv88e6xxx_chip *chip)
> +{
> +     int err;
> +
> +     err = mv88e6393x_serdes_port_errata(chip, MV88E6393X_PORT0_LANE);
> +     if (err)
> +             return err;
> +
> +     err = mv88e6393x_serdes_port_errata(chip, MV88E6393X_PORT9_LANE);
> +     if (err)
> +             return err;
> +
> +     return mv88e6393x_serdes_port_errata(chip, MV88E6393X_PORT10_LANE);
> +}
> +
> +static int mv88e6393x_serdes_port_config(struct mv88e6xxx_chip *chip, int 
> lane,
> +                                      bool on)
> +{
> +     u8 cmode = chip->ports[lane].cmode;
> +     u16 reg, pcs;
> +     int err;
> +
> +     if (on) {

And if "on" is false? Nothing? Why even pass it as an argument then? Why
even call mv88e6393x_serdes_port_config?

> +             switch (cmode) {
> +             case MV88E6XXX_PORT_STS_CMODE_SGMII:
> +                     pcs = MV88E6393X_PCS_SELECT_SGMII_MAC;
> +                     break;
> +             case MV88E6XXX_PORT_STS_CMODE_1000BASEX:
> +                     pcs = MV88E6393X_PCS_SELECT_1000BASEX;
> +                     break;
> +             case MV88E6XXX_PORT_STS_CMODE_2500BASEX:
> +                     pcs = MV88E6393X_PCS_SELECT_2500BASEX;
> +                     break;
> +             case MV88E6393X_PORT_STS_CMODE_5GBASER:
> +                     pcs = MV88E6393X_PCS_SELECT_5GBASER;
> +                     break;
> +             case MV88E6393X_PORT_STS_CMODE_10GBASER:
> +                     pcs = MV88E6393X_PCS_SELECT_10GBASER;
> +                     break;
> +             default:
> +                     pcs = MV88E6393X_PCS_SELECT_1000BASEX;
> +                     break;
> +             }
> +
> +             /* mv88e6393x family errata 3.6 :
> +              * When changing c_mode on Port 0 from [x]MII mode to any
> +              * SERDES mode SERDES will not be operational.
> +              * Workaround: Set Port0 SERDES register 4.F002.5=0
> +              */
> +             err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
> +                                         MV88E6393X_SERDES_POC, &reg);
> +             if (err)
> +                     return err;
> +
> +             reg &= ~(MV88E6393X_SERDES_POC_PCS_MODE_MASK |
> +                      MV88E6393X_SERDES_POC_PDOWN);
> +             reg |= pcs;
> +
> +             err = mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> +                                          MV88E6393X_SERDES_POC, reg);
> +             if (err)
> +                     return err;
> +
> +             reg |= MV88E6393X_SERDES_POC_RESET;
> +             err = mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> +                                          MV88E6393X_SERDES_POC, reg);
> +             if (err)
> +                     return err;
> +
> +             /* mv88e6393x family errata 3.7 :
> +              * When changing cmode on SERDES port from any other mode to
> +              * 1000BASE-X mode the link may not come up due to invalid
> +              * 1000BASE-X advertisement.
> +              * Workaround: Correct advertisement and reset PHY core.
> +              */
> +             if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASEX) {
> +                     reg = MV88E6390_SGMII_ANAR_1000BASEX_FD;
> +                     err = mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> +                                                  MV88E6390_SGMII_ANAR, reg);
> +                     if (err)
> +                             return err;
> +
> +                     /* soft reset the PCS/PMA */
> +                     err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
> +                                                 MV88E6390_SGMII_CONTROL,
> +                                                 &reg);
> +                     if (err)
> +                             return err;
> +
> +                     reg |= MV88E6390_SGMII_CONTROL_RESET;
> +                     err = mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> +                                                  MV88E6390_SGMII_CONTROL,
> +                                                  reg);
> +                     if (err)
> +                             return err;
> +             }
> +     }
> +
> +     return 0;
> +}
> +
> +int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
> +                         bool on)
> +{
> +     u8 cmode = chip->ports[port].cmode;
> +
> +     if (port != 0 && port != 9 && port != 10)
> +             return -EOPNOTSUPP;
> +
> +     mv88e6393x_serdes_port_config(chip, lane, on);
> +
> +     switch (cmode) {
> +     case MV88E6XXX_PORT_STS_CMODE_SGMII:
> +     case MV88E6XXX_PORT_STS_CMODE_1000BASEX:
> +     case MV88E6XXX_PORT_STS_CMODE_2500BASEX:
> +             return mv88e6390_serdes_power_sgmii(chip, lane, on);
> +     case MV88E6393X_PORT_STS_CMODE_5GBASER:
> +     case MV88E6393X_PORT_STS_CMODE_10GBASER:
> +             return mv88e6390_serdes_power_10g(chip, lane, on);
> +     }
> +
> +     return 0;
> +}

Reply via email to