On Tue, 3 Nov 2020 18:50:38 +1000 Pavana Sharma wrote:
> 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)
Please fix all checkpatch --strict --min-conf-desc-length=80 warnings
and what I point out below
> 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>
> + reg &= ~(MV88E6XXX_PORT_MAC_CTL_SPEED_MASK |
> + MV88E6390_PORT_MAC_CTL_ALTSPEED |
> + MV88E6390_PORT_MAC_CTL_FORCE_SPEED);
Align the continuation lines under the opening bracket, like the kernel
coding style require, please.
> +
> + if (speed != SPEED_UNFORCED)
> + reg |= MV88E6390_PORT_MAC_CTL_FORCE_SPEED;
> +
> + reg |= ctrl;
> +
> + err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_MAC_CTL, reg);
> + if (err)
> + return err;
> +
> + return 0;
no need to set err, just directly do:
return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_MAC_CTL,
reg);
> +}
> +
> static int mv88e6xxx_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
> phy_interface_t mode, bool force)
> {
> +/* Offset 0x0E: Policy & MGMT Control Register for FAMILY 6191X 6193X 6393X*/
Missing space at the end of that comment.
> +static int mv88e6393x_port_policy_write(struct mv88e6xxx_chip *chip, u16
> pointer,
> + u8 data)
> +{
> +
> + int err = 0;
> + int port;
> + u16 reg;
> +
> + /* Setup per Port policy register */
> + for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
> + if (dsa_is_unused_port(chip->ds, port))
> + continue;
> +
> + /* Prevent the use of an invalid port. */
> + if (mv88e6xxx_is_invalid_port(chip, port)) {
> + dev_err(chip->dev, "port %d is invalid\n", port);
> + err = -EINVAL;
did you mean to exit here? this assignment looks pointless
> + }
> + reg = MV88E6393X_PORT_POLICY_MGMT_CTL_UPDATE | pointer | data;
> + err = mv88e6xxx_port_write(chip, port,
> MV88E6393X_PORT_POLICY_MGMT_CTL, reg);
> + }
> + return err;
> +}
> +int mv88e6393x_port_mgmt_rsvd2cpu(struct mv88e6xxx_chip *chip)
> +{
> + u16 ptr;
> + int err;
> +
> + /* Consider the frames with reserved multicast destination
> + * addresses matching 01:80:c2:00:00:00 and
> + * 01:80:c2:00:00:02 as MGMT.
> + */
> + ptr = MV88E6393X_PORT_POLICY_MGMT_CTL_PTR_01C280000000XLO;
> + err = mv88e6393x_port_policy_write(chip, ptr, 0xff);
> + if (err)
> + return err;
> +
> + ptr = MV88E6393X_PORT_POLICY_MGMT_CTL_PTR_01C280000000XHI;
> + err = mv88e6393x_port_policy_write(chip, ptr, 0xff);
> + if (err)
> + return err;
> +
> + ptr = MV88E6393X_PORT_POLICY_MGMT_CTL_PTR_01C280000002XLO;
> + err = mv88e6393x_port_policy_write(chip, ptr, 0xff);
> + if (err)
> + return err;
> +
> + ptr = MV88E6393X_PORT_POLICY_MGMT_CTL_PTR_01C280000002XHI;
> + err = mv88e6393x_port_policy_write(chip, ptr, 0xff);
> + if (err)
> + return err;
> +
> + return 0;
return mv...
> +}
> +
> + err = mv88e6xxx_port_write(chip, port, MV88E6393X_PORT_EPC_CMD, val);
> + if (err)
> + return err;
> +
> + return 0;
ditto
> +}
> +
> +int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
> + bool on)
> +{
> + if (port == 0 || port == 9 || port == 10) {
Flip the condition, return early. Entire body of a function should not
have to be indented.
> + u8 cmode = chip->ports[port].cmode;
> +
> + mv88e6393x_serdes_port_config(chip, lane, on);
> +
> + switch (cmode) {
> + case MV88E6XXX_PORT_STS_CMODE_1000BASEX:
> + case MV88E6XXX_PORT_STS_CMODE_2500BASEX:
> + return mv88e6390_serdes_power_sgmii(chip, lane, on);
> + case MV88E6XXX_PORT_STS_CMODE_10GBASER:
> + return mv88e6390_serdes_power_10g(chip, lane, on);
> + }
> + }
> +
> + return 0;
> +}
> @@ -130,7 +169,7 @@ int mv88e6390_serdes_get_regs_len(struct mv88e6xxx_chip
> *chip, int port);
> void mv88e6390_serdes_get_regs(struct mv88e6xxx_chip *chip, int port, void
> *_p);
>
> /* Return the (first) SERDES lane address a port is using, ERROR otherwise.
> */
> -static inline u8 mv88e6xxx_serdes_get_lane(struct mv88e6xxx_chip *chip,
> +static inline int mv88e6xxx_serdes_get_lane(struct mv88e6xxx_chip *chip,
> int port)
Looks like this should be in patch 3?
> {
> if (!chip->info->ops->serdes_get_lane)
