From: "Marek Behún" <kabel@kernel.org>
To: Pavana Sharma <pavana.sharma@digi.com>
Cc: andrew@lunn.ch, ashkan.boldaji@digi.com, davem@davemloft.net,
f.fainelli@gmail.com, kuba@kernel.org, lkp@intel.com,
netdev@vger.kernel.org, vivien.didelot@gmail.com
Subject: Re: [net-next PATCH v13 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell
Date: Fri, 8 Jan 2021 14:51:09 +0100 [thread overview]
Message-ID: <20210108145109.71264cbe@kernel.org> (raw)
In-Reply-To: <0044dda2a5d1d03494ff753ee14ed4268f653e9c.1610071984.git.pavana.sharma@digi.com>
Pavana, some last nitpicks, sorry I didn't check this before.
I will send another patch to you which shall fix all this things.
On Fri, 8 Jan 2021 19:50:56 +1000
Pavana Sharma <pavana.sharma@digi.com> wrote:
> +/* Support 10, 100, 200, 1000, 2500, 5000, 10000 Mbps (e.g. 88E6393X)
> + * This function adds new speed 5000 supported by Amethyst family.
> + * Function mv88e6xxx_port_set_speed_duplex() can't be used as the register
> + * values for speeds 2500 & 5000 conflict.
> + */
> +
> +int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
> + int speed, int duplex)
There are basically 2 empty lines between the comment and the function
signature, one containting only comment end token " */" and the other
truly empty. I think you can remove the empty line, since the comment
is already visibly separated from the function body.
I think you can also remove the second line of the comment:
"This function adds new speed 5000 supported by Amethyst family."
The alignment of the speed argument is wrong. It should start at the
same column as the first argument.
So:
/* Support 10, 100, 200, 1000, 2500, 5000, 10000 Mbps (e.g. 88E6393X)
* Function mv88e6xxx_port_set_speed_duplex() can't be used as the register
* values for speeds 2500 & 5000 conflict.
*/
int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
int speed, int duplex)
> + reg &= ~(MV88E6XXX_PORT_MAC_CTL_SPEED_MASK |
> + MV88E6390_PORT_MAC_CTL_ALTSPEED |
> + MV88E6390_PORT_MAC_CTL_FORCE_SPEED);
alignment:
reg &= ~(MV88E6XXX_PORT_MAC_CTL_SPEED_MASK |
MV88E6390_PORT_MAC_CTL_ALTSPEED |
MV88E6390_PORT_MAC_CTL_FORCE_SPEED);
> +static int mv88e6393x_port_policy_write(struct mv88e6xxx_chip *chip, u16 pointer,
> + u8 data)
> +{
> +
> + int err = 0;
Unneeded empty line before int err = 0;
Also alignment of the data argument:
static int mv88e6393x_port_policy_write(struct mv88e6xxx_chip *chip, u16 pointer,
u8 data)
(It seems to me that you only align with tabs. If you need to align for
example to 20th column, use 2 tabs and 4 spaces (2*8 + 4 = 20).)
> +int mv88e6393x_port_set_ether_type(struct mv88e6xxx_chip *chip, int port,
> + u16 etype)
Alignment:
int mv88e6393x_port_set_ether_type(struct mv88e6xxx_chip *chip, int port,
u16 etype)
> +#define MV88E6XXX_PORT_STS_CMODE_5GBASER 0x000c
> +#define MV88E6XXX_PORT_STS_CMODE_10GBASER 0x000d
> +#define MV88E6XXX_PORT_STS_CMODE_USXGMII 0x000e
These macros should have prefix
MV88E6393X_
instead of
MV88E6XXX_
since these values apply for 6393X family and they conflict with the
values for other switches.
> +int mv88e6xxx_port_wait_bit(struct mv88e6xxx_chip *chip, int port, int reg,
> + int bit, int val);
Alignment.
> +int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
> + int speed, int duplex);
Alignment.
> +int mv88e6393x_port_set_ether_type(struct mv88e6xxx_chip *chip, int port,
> + u16 etype);
Alignment.
> +/* 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)
> + return -EOPNOTSUPP;
> +
> + 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)
Alignment.
> + lane = port;
> + return lane;
> +}
> +int mv88e6393x_serdes_irq_enable(struct mv88e6xxx_chip *chip, int port,
> + int lane, bool enable)
Alignment.
> +irqreturn_t mv88e6393x_serdes_irq_status(struct mv88e6xxx_chip *chip, int port,
> + int lane)
Alignment.
> +static int mv88e6393x_serdes_port_config(struct mv88e6xxx_chip *chip, int lane,
> + bool on)
Alignment.
> + mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
> + MV88E6393X_SERDES_POC, &config);
> + config &= ~(MV88E6393X_SERDES_POC_PCS_MODE_MASK |
> + MV88E6393X_SERDES_POC_PDOWN);
> + config |= pcs;
> + mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> + MV88E6393X_SERDES_POC, config);
> + config |= MV88E6393X_SERDES_POC_RESET;
> + mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> + MV88E6393X_SERDES_POC, config);
> +
> + /* 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.
> + */
> + config = MV88E6390_SGMII_ANAR_1000BASEX_FD;
> + mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> + MV88E6390_SGMII_ANAR, config);
> +
> + /* soft reset the PCS/PMA */
> + mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
> + MV88E6390_SGMII_CONTROL, &config);
> + config |= MV88E6390_SGMII_CONTROL_RESET;
> + mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
> + MV88E6390_SGMII_CONTROL, config);
Alignment in all calls to mv88e6390_serdes_write.
> +int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
> + bool on)
Alignment.
> +int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
> + bool on);
Alignment.
> +int mv88e6393x_serdes_irq_enable(struct mv88e6xxx_chip *chip, int port,
> + int lane, bool enable);
Alignment.
> +irqreturn_t mv88e6393x_serdes_irq_status(struct mv88e6xxx_chip *chip, int port,
> + int lane);
Alignment.
next prev parent reply other threads:[~2021-01-08 13:52 UTC|newest]
Thread overview: 101+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <djc@djc.id.au; danc86@gmail.com[PATCH v2] Add support for mv88e6393x family of Marvell.>
2020-10-16 2:09 ` [PATCH v3] Add support for mv88e6393x family of Marvell Pavana Sharma
2020-10-16 2:37 ` Florian Fainelli
2020-10-17 19:30 ` Andrew Lunn
2020-10-26 5:52 ` [PATCH v4 0/3] " Pavana Sharma
2020-10-26 5:54 ` [PATCH v4 1/3] " Pavana Sharma
2020-10-26 8:58 ` kernel test robot
2020-10-27 19:10 ` kernel test robot
2020-10-26 5:58 ` [PATCH v4 2/3] Add phy interface for 5GBASER mode Pavana Sharma
2020-10-26 13:38 ` Andrew Lunn
2020-10-26 13:42 ` Florian Fainelli
2020-10-26 5:58 ` [PATCH v4 3/3] Change serdes lane parameter from u8 type to int Pavana Sharma
2020-10-26 13:43 ` [PATCH v4 0/3] Add support for mv88e6393x family of Marvell Florian Fainelli
2020-10-28 0:07 ` [PATCH v5 " Pavana Sharma
2020-10-28 0:08 ` [PATCH v5 1/3] net: phy: Add 5GBASER interface mode Pavana Sharma
2020-10-28 12:03 ` Andrew Lunn
2020-10-28 0:09 ` [PATCH v5 2/3] dt-bindings: net: Add 5GBASER phy " Pavana Sharma
2020-10-28 12:03 ` Andrew Lunn
2020-10-28 0:09 ` [PATCH v5 3/3] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2020-10-28 2:03 ` Marek Behun
2020-10-28 12:21 ` Andrew Lunn
2020-10-29 5:40 ` [PATCH v6 0/4] " Pavana Sharma
2020-10-29 5:41 ` [PATCH v6 1/4] dt-bindings: net: Add 5GBASER phy interface mode Pavana Sharma
2020-10-29 5:42 ` [PATCH v6 2/4] net: phy: Add 5GBASER " Pavana Sharma
2020-10-29 6:11 ` Marek Behun
2020-10-29 12:42 ` Andrew Lunn
2020-10-29 5:42 ` [PATCH v6 3/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2020-10-29 6:31 ` Marek Behun
2020-11-02 6:40 ` [PATCH v7 0/4] " Pavana Sharma
2020-11-02 6:41 ` [PATCH v7 1/4] dt-bindings: net: Add 5GBASER phy interface mode Pavana Sharma
2020-11-02 6:42 ` [PATCH v7 2/4] net: phy: Add 5GBASER " Pavana Sharma
2020-11-02 13:09 ` Andrew Lunn
2020-11-03 1:34 ` Pavana Sharma
2020-11-03 2:12 ` Florian Fainelli
2020-11-03 3:16 ` Andrew Lunn
2020-11-03 8:48 ` [PATCH v8 0/4] Add support for mv88e6393x family of Marvell Pavana Sharma
2020-11-03 8:49 ` [PATCH v8 1/4] dt-bindings: net: Add 5GBASER phy interface mode Pavana Sharma
2020-11-06 1:42 ` Jakub Kicinski
2020-11-03 8:49 ` [PATCH v8 2/4] net: phy: Add 5GBASER " Pavana Sharma
2020-11-03 8:50 ` [PATCH v8 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter from u8 type to int Pavana Sharma
2020-11-06 1:40 ` Jakub Kicinski
2020-11-03 8:50 ` [PATCH v8 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2020-11-06 1:52 ` Jakub Kicinski
2020-11-19 8:01 ` [PATCH v9 0/4] " Pavana Sharma
2020-11-19 8:02 ` [PATCH v9 1/4] dt-bindings: net: Add 5GBASER phy interface mode Pavana Sharma
2020-11-19 8:03 ` [PATCH v9 2/4] net: phy: Add 5GBASER " Pavana Sharma
2020-11-19 8:03 ` [PATCH v9 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter from u8 type to int Pavana Sharma
2020-11-19 8:04 ` [PATCH v9 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2020-11-19 15:33 ` kernel test robot
2020-11-19 19:12 ` kernel test robot
2020-11-20 0:24 ` [PATCH v10 0/4] " Pavana Sharma
2020-11-20 0:25 ` [PATCH v10 1/4] dt-bindings: net: Add 5GBASER phy interface mode Pavana Sharma
2020-11-20 0:52 ` Andrew Lunn
2020-11-20 0:25 ` [PATCH v10 2/4] net: phy: Add 5GBASER " Pavana Sharma
2020-11-20 0:55 ` Andrew Lunn
2020-11-20 0:26 ` [PATCH v10 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter from u8 type to int Pavana Sharma
2020-11-20 0:59 ` Andrew Lunn
2020-11-20 0:26 ` [PATCH v10 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2020-11-20 1:29 ` Andrew Lunn
2020-11-20 1:43 ` Marek Behun
2020-11-20 1:54 ` Andrew Lunn
2020-12-09 5:02 ` [PATCH v11 0/4] " Pavana Sharma
2020-12-09 5:03 ` [PATCH v11 1/4] dt-bindings: net: Add 5GBASER phy interface mode Pavana Sharma
2020-12-09 23:15 ` Andrew Lunn
2020-12-10 13:43 ` Pavana Sharma
2020-12-09 5:04 ` [PATCH v11 2/4] net: phy: Add 5GBASER " Pavana Sharma
2020-12-09 23:18 ` Andrew Lunn
2020-12-09 5:05 ` [PATCH v11 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter type from u8 type to int Pavana Sharma
2020-12-09 23:24 ` Andrew Lunn
2020-12-09 5:05 ` [PATCH v11 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2020-12-09 23:40 ` Andrew Lunn
2020-12-09 19:37 ` [PATCH v11 0/4] " Jakub Kicinski
2020-12-11 12:44 ` [net-next PATCH v12 " Pavana Sharma
2020-12-11 12:46 ` [net-next PATCH v12 1/4] dt-bindings: net: Add 5GBASER phy interface mode Pavana Sharma
2020-12-14 22:56 ` Rob Herring
2020-12-11 12:46 ` [net-next PATCH v12 2/4] net: phy: Add 5GBASER " Pavana Sharma
2020-12-11 12:49 ` [net-next PATCH v12 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter type from u8 type to int Pavana Sharma
2020-12-11 12:51 ` [net-next PATCH v12 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2021-01-05 12:15 ` Marek Behún
2021-01-06 0:45 ` Pavana Sharma
2021-01-06 12:20 ` Marek Behún
2021-01-05 12:37 ` patch fixing mv88e6393x SERDES IRQ for Pavana's series Marek Behún
2021-01-08 9:47 ` [net-next PATCH v13 0/4] Add support for mv88e6393x family of Marvell Pavana Sharma
2021-01-08 9:48 ` [net-next PATCH v13 1/4] dt-bindings: net: Add 5GBASER phy interface Pavana Sharma
2021-01-08 13:49 ` Andrew Lunn
2021-01-08 9:49 ` [net-next PATCH v13 2/4] net: phy: Add 5GBASER interface mode Pavana Sharma
2021-01-08 13:50 ` Andrew Lunn
2021-01-08 9:50 ` [net-next PATCH v13 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter type from u8 type to int Pavana Sharma
2021-01-08 9:50 ` [net-next PATCH v13 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2021-01-08 13:51 ` Marek Behún [this message]
2021-01-08 14:02 ` Marek Behún
2021-01-08 14:36 ` [PATCH] changes for Pavana Marek Behún
2021-01-09 21:31 ` Marek Behún
2020-11-02 6:43 ` [PATCH v7 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter from u8 type to int Pavana Sharma
2020-11-02 13:34 ` Andrew Lunn
2020-11-02 13:40 ` Andrew Lunn
2020-11-02 6:43 ` [PATCH v7 4/4] net: dsa: mv88e6xxx: Add support for mv88e6393x family of Marvell Pavana Sharma
2020-11-02 13:12 ` [PATCH v7 0/4] " Andrew Lunn
2020-10-29 5:43 ` [PATCH v6 4/4] net: dsa: mv88e6xxx: Change serdes lane parameter from u8 to int Pavana Sharma
2020-10-29 6:07 ` [PATCH v6 0/4] Add support for mv88e6393x family of Marvell Marek Behun
2020-10-28 23:12 ` [PATCH v5 3/3] net: dsa: mv88e6xxx: " Jakub Kicinski
2020-10-29 4:25 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210108145109.71264cbe@kernel.org \
--to=kabel@kernel.org \
--cc=andrew@lunn.ch \
--cc=ashkan.boldaji@digi.com \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=lkp@intel.com \
--cc=netdev@vger.kernel.org \
--cc=pavana.sharma@digi.com \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).