netdev - Re: [PATCH v10 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201120012906.GA1804098@lunn.ch>
Date:   Fri, 20 Nov 2020 02:29:06 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Pavana Sharma <pavana.sharma@...i.com>
Cc:     lkp@...el.com, ashkan.boldaji@...i.com,
        clang-built-linux@...glegroups.com, davem@...emloft.net,
        f.fainelli@...il.com, gregkh@...uxfoundation.org,
        kbuild-all@...ts.01.org, kuba@...nel.org,
        linux-kernel@...r.kernel.org, marek.behun@....cz,
        netdev@...r.kernel.org, robh+dt@...nel.org,
        devicetree@...r.kernel.org, vivien.didelot@...il.com
Subject: Re: [PATCH v10 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x
 family  of Marvell

> @@ -222,8 +231,8 @@ static int mv88e6xxx_port_set_speed_duplex(struct mv88e6xxx_chip *chip,
>  		return err;
>  
>  	reg &= ~(MV88E6XXX_PORT_MAC_CTL_SPEED_MASK |
> -		 MV88E6XXX_PORT_MAC_CTL_FORCE_DUPLEX |
> -		 MV88E6XXX_PORT_MAC_CTL_DUPLEX_FULL);
> +		MV88E6XXX_PORT_MAC_CTL_FORCE_DUPLEX |
> +		MV88E6XXX_PORT_MAC_CTL_DUPLEX_FULL);

This looks like a white space change?


>  	if (alt_bit)
>  		reg &= ~MV88E6390_PORT_MAC_CTL_ALTSPEED;
> @@ -390,6 +399,84 @@ phy_interface_t mv88e6390x_port_max_speed_mode(int port)
>  	return PHY_INTERFACE_MODE_NA;
>  }
>  
> +/* Support 10, 100, 200, 1000, 2500, 5000, 10000 Mbps (e.g. 88E6393X) */
> +int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
> +		int speed, int duplex)
> +{
> +	u16 reg, ctrl;
> +	int err;
> +
> +	if (speed == SPEED_MAX)
> +		speed = (port > 0 && port < 9) ? 1000 : 10000;
> +
> +	if (speed == 200 && port != 0)
> +		return -EOPNOTSUPP;
> +
> +	if (speed >= 2500 && port > 0 && port < 9)
> +		return -EOPNOTSUPP;

Maybe i'm missing something, but it looks like at this point you can
call

	return mv88e6xxx_port_set_speed_duplex(chip, port, speed, true, true, duplex);


> +/* Offset 0x0E: Policy & MGMT Control Register for FAMILY 6191X 6193X 6393X */
> +
> +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);
> +			return -EINVAL;
> +		}

        /* Mark certain ports as invalid. This is required for example for the
         * MV88E6220 (which is in general a MV88E6250 with 7 ports) but the
         * ports 2-4 are not routed to pins.
         */
        unsigned int invalid_port_mask;

You have not set this in the info structure of the 6393x devices, so
you can skip this check.


> +/* Only Ports 0, 9 and 10 have SERDES lanes. Return the SERDES lane address
> + * a port is using else Returns -ENODEV.
> + */
> +int mv88e6393x_serdes_get_lane(struct mv88e6xxx_chip *chip, int port)
> +{
> +	u8 cmode = chip->ports[port].cmode;
> +	int lane = -ENODEV;
> +
> +	if (port == 0 || port == 9 || port == 10) {

Maybe 

	if (port != 0 && port != 9 && port == 10)
		return -ENODEV

> +		if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASEX ||
> +			cmode == MV88E6XXX_PORT_STS_CMODE_SGMII ||
> +		    cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX ||
> +			cmode == MV88E6XXX_PORT_STS_CMODE_5GBASER ||
> +		    cmode == MV88E6XXX_PORT_STS_CMODE_10GBASER ||
> +		    cmode == MV88E6XXX_PORT_STS_CMODE_USXGMII)

Indentation is messed up.

> +			lane = port;

	return port;

	Andrew