* Re: [PATCH net-next 1/8] net: phy: Add initial support for Microsemi Ocelot internal PHYs.
From: Florian Fainelli @ 2018-03-23 21:08 UTC (permalink / raw)
To: Alexandre Belloni, David S . Miller
Cc: Allan Nielsen, razvan.stefanescu, po.liu, Thomas Petazzoni,
Andrew Lunn, netdev, devicetree, linux-kernel, linux-mips,
Raju Lakkaraju
In-Reply-To: <20180323201117.8416-2-alexandre.belloni@bootlin.com>
On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> Add Microsemi Ocelot internal PHY ids. For now, simply use the genphy
> functions but more features are available.
>
> Cc: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/net/phy/mscc.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
> index 650c2667d523..e1ab3acd1cdb 100644
> --- a/drivers/net/phy/mscc.c
> +++ b/drivers/net/phy/mscc.c
> @@ -91,6 +91,7 @@ enum rgmii_rx_clock_delay {
> #define SECURE_ON_PASSWD_LEN_4 0x4000
>
> /* Microsemi PHY ID's */
> +#define PHY_ID_OCELOT 0x00070540
> #define PHY_ID_VSC8530 0x00070560
> #define PHY_ID_VSC8531 0x00070570
> #define PHY_ID_VSC8540 0x00070760
> @@ -658,6 +659,19 @@ static int vsc85xx_probe(struct phy_device *phydev)
>
> /* Microsemi VSC85xx PHYs */
> static struct phy_driver vsc85xx_driver[] = {
> +{
> + .phy_id = PHY_ID_OCELOT,
> + .name = "Microsemi OCELOT",
> + .phy_id_mask = 0xfffffff0,
> + .features = PHY_GBIT_FEATURES,
> + .soft_reset = &genphy_soft_reset,
> + .config_init = &genphy_config_init,
> + .config_aneg = &genphy_config_aneg,
> + .aneg_done = &genphy_aneg_done,
> + .read_status = &genphy_read_status,
> + .suspend = &genphy_suspend,
> + .resume = &genphy_resume,
With the exception of config_init(), suspend and resume, everything else
is already the default when you don't provide a callback. To echo to
what Andrew wrote already, if the purpose is just to show a nice name,
and do nothing else, consider using the Generic PHY driver (default).
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Florian Fainelli @ 2018-03-23 21:11 UTC (permalink / raw)
To: Alexandre Belloni, David S . Miller
Cc: Allan Nielsen, razvan.stefanescu, po.liu, Thomas Petazzoni,
Andrew Lunn, netdev, devicetree, linux-kernel, linux-mips,
Rob Herring
In-Reply-To: <20180323201117.8416-5-alexandre.belloni@bootlin.com>
On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> .../devicetree/bindings/net/mscc-ocelot.txt | 62 ++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
>
> diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> new file mode 100644
> index 000000000000..ee092a85b5a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> @@ -0,0 +1,62 @@
> +Microsemi Ocelot network Switch
> +===============================
> +
> +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> +VSC7514)
> +
> +Required properties:
> +- compatible: Should be "mscc,ocelot-switch"
> +- reg: Must contain an (offset, length) pair of the register set for each
> + entry in reg-names.
> +- reg-names: Must include the following entries:
> + - "sys"
> + - "rew"
> + - "qs"
> + - "hsio"
> + - "qsys"
> + - "ana"
> + - "portX" with X from 0 to the number of last port index available on that
> + switch
> +- interrupts: Should contain the switch interrupts for frame extraction and
> + frame injection
> +- interrupt-names: should contain the interrupt names: "xtr", "inj"
You are not documenting the "ports" subnode(s).Please move the
individual ports definition under a ports subnode, mainly for two reasons:
- it makes it easy at the .dtsi level to have all ports disabled by default
- this makes you strictly conforming to the DSA binding for Ethernet
switches and this is good for consistency (both parsing code and just
representation).
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 6/8] MIPS: mscc: Add switch to ocelot
From: Florian Fainelli @ 2018-03-23 21:17 UTC (permalink / raw)
To: Alexandre Belloni, David S . Miller
Cc: Allan Nielsen, razvan.stefanescu, po.liu, Thomas Petazzoni,
Andrew Lunn, netdev, devicetree, linux-kernel, linux-mips,
James Hogan
In-Reply-To: <20180323201117.8416-7-alexandre.belloni@bootlin.com>
On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> Ocelot has an integrated switch, add support for it.
>
> Cc: James Hogan <jhogan@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> arch/mips/boot/dts/mscc/ocelot.dtsi | 84 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
> diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi
> index dd239cab2f9d..22a86373b1c9 100644
> --- a/arch/mips/boot/dts/mscc/ocelot.dtsi
> +++ b/arch/mips/boot/dts/mscc/ocelot.dtsi
> @@ -91,6 +91,69 @@
> status = "disabled";
> };
>
> + switch@1010000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "mscc,ocelot-switch";
> + reg = <0x1010000 0x10000>,
> + <0x1030000 0x10000>,
> + <0x1080000 0x100>,
> + <0x10d0000 0x10000>,
> + <0x11e0000 0x100>,
> + <0x11f0000 0x100>,
> + <0x1200000 0x100>,
> + <0x1210000 0x100>,
> + <0x1220000 0x100>,
> + <0x1230000 0x100>,
> + <0x1240000 0x100>,
> + <0x1250000 0x100>,
> + <0x1260000 0x100>,
> + <0x1270000 0x100>,
> + <0x1280000 0x100>,
> + <0x1800000 0x80000>,
> + <0x1880000 0x10000>;
> + reg-names = "sys", "rew", "qs", "hsio", "port0",
> + "port1", "port2", "port3", "port4", "port5",
> + "port6", "port7", "port8", "port9", "port10",
> + "qsys", "ana";
> + interrupts = <21 22>;
> + interrupt-names = "xtr", "inj";
See my comment about the binding patch, this should be moved to a ports
subnode so it is conforming to the existing DSA binding and makes it a
lot easier to have all ports disabled by default at the .dsti level by
not defini
> +
> + port0: port@0 {
> + reg = <0>;
> + };
> + port1: port@1 {
> + reg = <1>;
> + };
> + port2: port@2 {
> + reg = <2>;
> + };
> + port3: port@3 {
> + reg = <3>;
> + };
> + port4: port@4 {
> + reg = <4>;
> + };
> + port5: port@5 {
> + reg = <5>;
> + };
> + port6: port@6 {
> + reg = <6>;
> + };
> + port7: port@7 {
> + reg = <7>;
> + };
> + port8: port@8 {
> + reg = <8>;
> + };
> + port9: port@9 {
> + reg = <9>;
> + };
> + port10: port@10 {
> + reg = <10>;
> + };
> + };
> +
> reset@1070008 {
> compatible = "mscc,ocelot-chip-reset";
> reg = <0x1070008 0x4>;
> @@ -113,5 +176,26 @@
> function = "uart2";
> };
> };
> +
> + mdio0: mdio@107009c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "mscc,ocelot-miim";
> + reg = <0x107009c 0x36>, <0x10700f0 0x8>;
> + interrupts = <14>;
status = "disabled" by default?
> +
> + phy0: ethernet-phy@0 {
> + reg = <0>;
> + };
> + phy1: ethernet-phy@1 {
> + reg = <1>;
> + };
> + phy2: ethernet-phy@2 {
> + reg = <2>;
> + };
> + phy3: ethernet-phy@3 {
> + reg = <3>;
> + };
These PHYs should be defined at the board DTS level.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 6/8] MIPS: mscc: Add switch to ocelot
From: Alexandre Belloni @ 2018-03-23 21:22 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S . Miller, Allan Nielsen, razvan.stefanescu, po.liu,
Thomas Petazzoni, Andrew Lunn, netdev, devicetree, linux-kernel,
linux-mips, James Hogan
In-Reply-To: <e488fd29-0094-d005-a078-873f6f5add13@gmail.com>
On 23/03/2018 at 14:17:48 -0700, Florian Fainelli wrote:
> On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> > +
> > + phy0: ethernet-phy@0 {
> > + reg = <0>;
> > + };
> > + phy1: ethernet-phy@1 {
> > + reg = <1>;
> > + };
> > + phy2: ethernet-phy@2 {
> > + reg = <2>;
> > + };
> > + phy3: ethernet-phy@3 {
> > + reg = <3>;
> > + };
>
> These PHYs should be defined at the board DTS level.
Those are internal PHYs, present on the SoC, I doubt anyone will have
anything different while using the same SoC.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next 5/8] net: mscc: Add initial Ocelot switch support
From: Andrew Lunn @ 2018-03-23 21:25 UTC (permalink / raw)
To: Alexandre Belloni
Cc: David S . Miller, Allan Nielsen, razvan.stefanescu, po.liu,
Thomas Petazzoni, Florian Fainelli, netdev, devicetree,
linux-kernel, linux-mips
In-Reply-To: <20180323201117.8416-6-alexandre.belloni@bootlin.com>
Hi Alexandre
> +static int ocelot_port_bridge_join(struct ocelot_port *ocelot_port,
> + struct net_device *bridge)
> +{
> + struct ocelot *ocelot = ocelot_port->ocelot;
> +
> + if (!ocelot->bridge_mask) {
> + ocelot->hw_bridge_dev = bridge;
> + } else {
> + if (ocelot->hw_bridge_dev != bridge)
> + return -ENODEV; //TODO: do something clever here
> + }
> +
> + ocelot->bridge_mask |= BIT(ocelot_port->chip_port);
> +
> + return 0;
> +}
I could be missing something here, but this looks way too simple.
By default, you have individual ports. They can just forward frames
between the host CPU and the port. There is no port-to-port traffic
allowed.
You then create a bridge, and add ports to the bridge. When you add a
port, you need to program the hardware that it can now forward frames
from this port to other parts in the same bridge. You also need to
tell other parts in the same bridge it can forward frames to it. You
might also need to tell the port which forwarding database it should
use, since each bridge might have its own.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 6/8] MIPS: mscc: Add switch to ocelot
From: Andrew Lunn @ 2018-03-23 21:33 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Florian Fainelli, David S . Miller, Allan Nielsen,
razvan.stefanescu, po.liu, Thomas Petazzoni, netdev, devicetree,
linux-kernel, linux-mips, James Hogan
In-Reply-To: <20180323212230.GA12808@piout.net>
On Fri, Mar 23, 2018 at 10:22:30PM +0100, Alexandre Belloni wrote:
> On 23/03/2018 at 14:17:48 -0700, Florian Fainelli wrote:
> > On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> > > +
> > > + phy0: ethernet-phy@0 {
> > > + reg = <0>;
> > > + };
> > > + phy1: ethernet-phy@1 {
> > > + reg = <1>;
> > > + };
> > > + phy2: ethernet-phy@2 {
> > > + reg = <2>;
> > > + };
> > > + phy3: ethernet-phy@3 {
> > > + reg = <3>;
> > > + };
> >
> > These PHYs should be defined at the board DTS level.
>
> Those are internal PHYs, present on the SoC, I doubt anyone will have
> anything different while using the same SoC.
With DSA, there is no need to list internal PHYs.
That is the trade off of having a standalone MDIO bus driver. Maybe
add a phandle to the internal MDIO bus? The switch driver could then
follow the phandle, and direct connect the internal PHYs?
Andrew
^ permalink raw reply
* Re: [PATCH net-next 5/8] net: mscc: Add initial Ocelot switch support
From: Florian Fainelli @ 2018-03-23 21:41 UTC (permalink / raw)
To: Alexandre Belloni, David S . Miller
Cc: Allan Nielsen, razvan.stefanescu, po.liu, Thomas Petazzoni,
Andrew Lunn, netdev, devicetree, linux-kernel, linux-mips
In-Reply-To: <20180323201117.8416-6-alexandre.belloni@bootlin.com>
On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> Add a driver for Microsemi Ocelot Ethernet switch support.
>
> This makes two modules:
> mscc_ocelot_common handles all the common features that doesn't depend on
> how the switch is integrated in the SoC. Currently, it handles offloading
> bridging to the hardware. ocelot_io.c handles register accesses. This is
> unfortunately needed because the register layout is packed and then depends
> on the number of ports available on the switch. The register definition
> files are automatically generated.
>
> ocelot_board handles the switch integration on the SoC and on the board.
>
> Frame injection and extraction to/from the CPU port is currently done using
> register accesses which is quite slow. DMA is possible but the port is not
> able to absorb the whole switch bandwidth.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Random drive by comments because this is quite a number of lines to review!
Overall, looks quite good for a first version. Out of curiosity, is
there a particular switch test you ran this driver against? LNST?
> +static int ocelot_mact_learn(struct ocelot *ocelot, int port,
> + const unsigned char mac[ETH_ALEN],
> + unsigned int vid,
> + enum macaccess_entry_type type)
> +{
> + u32 macl = 0, mach = 0;
> +
> + /* Set the MAC address to learn and the vlan associated in a format
> + * understood by the hardware.
> + */
> + mach |= vid << 16;
> + mach |= mac[0] << 8;
> + mach |= mac[1] << 0;
> + macl |= mac[2] << 24;
> + macl |= mac[3] << 16;
> + macl |= mac[4] << 8;
> + macl |= mac[5] << 0;
> +
> + ocelot_write(ocelot, macl, ANA_TABLES_MACLDATA);
> + ocelot_write(ocelot, mach, ANA_TABLES_MACHDATA);
You are repeating this in the function right below, can you factor it
somehow into a common function that this one, and the one right below
could call?
[snip]
> +static void ocelot_port_adjust_link(struct net_device *dev)
> +{
This is fine for now, but I would suggest implementing PHYLINK to be
future proof.
[snip]
> +static int ocelot_port_stop(struct net_device *dev)
> +{
> + struct ocelot_port *port = netdev_priv(dev);
> +
> + phy_disconnect(port->phy);
> +
> + dev->phydev = NULL;
You don't have anything else to do, like disabling the port so it
possibly saves power or anything, aside from the PHY which will be
suspended here.
[snip]
> +static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct ocelot_port *port = netdev_priv(dev);
> + struct ocelot *ocelot = port->ocelot;
> + u32 val, ifh[IFH_LEN];
> + struct frame_info info = {};
> + u8 grp = 0; /* Send everything on CPU group 0 */
> + int i, count, last;
unsigned int for these types.
> +
> + val = ocelot_read(ocelot, QS_INJ_STATUS);
> + if (!(val & QS_INJ_STATUS_FIFO_RDY(BIT(grp))) ||
> + (val & QS_INJ_STATUS_WMARK_REACHED(BIT(grp))))
> + return NETDEV_TX_BUSY;
> +
> + ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
> + QS_INJ_CTRL_SOF, QS_INJ_CTRL, grp);
> +
> + info.port = BIT(port->chip_port);
> + info.cpuq = 0xff;
> + ocelot_gen_ifh(ifh, &info);
> +
> + for (i = 0; i < IFH_LEN; i++)
> + ocelot_write_rix(ocelot, ifh[i], QS_INJ_WR, grp);
> +
> + count = (skb->len + 3) / 4;
> + last = skb->len % 4;
> + for (i = 0; i < count; i++) {
> + ocelot_write_rix(ocelot, cpu_to_le32(((u32 *)skb->data)[i]),
> + QS_INJ_WR, grp);
> + }
> +
> + /* Add padding */
> + while (i < (OCELOT_BUFFER_CELL_SZ / 4)) {
> + ocelot_write_rix(ocelot, 0, QS_INJ_WR, grp);
> + i++;
> + }
> +
> + /* Indicate EOF and valid bytes in last word */
> + ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
> + QS_INJ_CTRL_VLD_BYTES(skb->len < OCELOT_BUFFER_CELL_SZ ? 0 : last) |
> + QS_INJ_CTRL_EOF,
> + QS_INJ_CTRL, grp);
> +
> + /* Add dummy CRC */
> + ocelot_write_rix(ocelot, 0, QS_INJ_WR, grp);
> + skb_tx_timestamp(skb);
> +
> + dev->stats.tx_packets++;
> + dev->stats.tx_bytes += skb->len;
> + dev_kfree_skb_any(skb);
No interrupt to indicate transmit completion?
> +static int ocelot_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> + struct net_device *dev, const unsigned char *addr,
> + u16 vid, u16 flags)
> +{
> + struct ocelot_port *port = netdev_priv(dev);
> + struct ocelot *ocelot = port->ocelot;
> +
> + if (!vid) {
> + if (!port->vlan_aware)
> + /* If the bridge is not VLAN aware and no VID was
> + * provided, set it to 1 as bridges have a default VID
> + * of 1. Otherwise the MAC entry wouldn't match incoming
> + * packets as the VID would differ (0 != 1).
> + */
> + vid = 1;
> + else
> + /* If the bridge is VLAN aware a VID must be provided as
> + * otherwise the learnt entry wouldn't match any frame.
> + */
> + return -EINVAL;
> + }
So if we are targeting vid = 0 we end-up with vid = 1 possibly?
[snip]
> +static int ocelot_port_attr_stp_state_set(struct ocelot_port *ocelot_port,
> + struct switchdev_trans *trans,
> + u8 state)
> +{
> + struct ocelot *ocelot = ocelot_port->ocelot;
> + u32 port_cfg;
> + int port, i;
> +
> + if (switchdev_trans_ph_prepare(trans))
> + return 0;
> +
> + if (!(BIT(ocelot_port->chip_port) & ocelot->bridge_mask))
> + return 0;
> +
> + port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG,
> + ocelot_port->chip_port);
> +
> + switch (state) {
> + case BR_STATE_FORWARDING:
> + ocelot->bridge_fwd_mask |= BIT(ocelot_port->chip_port);
> + /* Fallthrough */
> + case BR_STATE_LEARNING:
> + port_cfg |= ANA_PORT_PORT_CFG_LEARN_ENA;
> + break;
> +
> + default:
> + port_cfg &= ~ANA_PORT_PORT_CFG_LEARN_ENA;
> + ocelot->bridge_fwd_mask &= ~BIT(ocelot_port->chip_port);
Missing break, even if this is the default case.
> + }
> +
> + ocelot_write_gix(ocelot, port_cfg, ANA_PORT_PORT_CFG,
> + ocelot_port->chip_port);
> +
> + /* Apply FWD mask. The loop is needed to add/remove the current port as
> + * a source for the other ports.
> + */
> + for (port = 0; port < ocelot->num_phys_ports; port++) {
> + if (ocelot->bridge_fwd_mask & BIT(port)) {
> + unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(port);
> +
> + for (i = 0; i < ocelot->num_phys_ports; i++) {
> + unsigned long bond_mask = ocelot->lags[i];
> +
> + if (!bond_mask)
> + continue;
> +
> + if (bond_mask & BIT(port)) {
> + mask &= ~bond_mask;
> + break;
> + }
> + }
> +
> + ocelot_write_rix(ocelot,
> + BIT(ocelot->num_phys_ports) | mask,
> + ANA_PGID_PGID, PGID_SRC + port);
> + } else {
> + /* Only the CPU port, this is compatible with link
> + * aggregation.
> + */
> + ocelot_write_rix(ocelot,
> + BIT(ocelot->num_phys_ports),
> + ANA_PGID_PGID, PGID_SRC + port);
> + }
All of this sounds like it should be moved into the br_join/leave, this
does not appear to be the right place to do that.
[snip]
> +static int ocelot_port_attr_set(struct net_device *dev,
> + const struct switchdev_attr *attr,
> + struct switchdev_trans *trans)
> +{
> + struct ocelot_port *ocelot_port = netdev_priv(dev);
> + int err = 0;
Should not this be EOPNOTSUPP by default so your cases below are
properly handled, like BRIDGE_FLAGS, MROUTER etc.
> +
> + switch (attr->id) {
> + case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
> + ocelot_port_attr_stp_state_set(ocelot_port, trans,
> + attr->u.stp_state);
> + break;
> + case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> + break;
> + case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
> + ocelot_port_attr_ageing_set(ocelot_port, attr->u.ageing_time);
> + break;
> + case SWITCHDEV_ATTR_ID_PORT_MROUTER:
> + break;
> + case SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED:
> + ocelot_port_attr_mc_set(ocelot_port, !attr->u.mc_disabled);
> + break;
> + default:
> + err = -EOPNOTSUPP;
> + break;
> + }
> +
> + return err;
> +}
> +
> +static struct ocelot_multicast *ocelot_multicast_get(struct ocelot *ocelot,
> + const unsigned char *addr,
> + u16 vid)
> +{
> + struct ocelot_multicast *mc;
> +
> + list_for_each_entry(mc, &ocelot->multicast, list) {
> + if (ether_addr_equal(mc->addr, addr) && mc->vid == vid)
> + return mc;
> + }
> +
> + return NULL;
> +}
> +static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
> +{
> + struct ocelot *ocelot = arg;
> + int i = 0, grp = 0;
> + int err = 0;
> +
> + if (!(ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp)))
> + return IRQ_NONE;
> +
> + do {
> + struct sk_buff *skb;
> + struct net_device *dev;
> + u32 *buf;
> + int sz, len;
> + u32 ifh[4];
> + u32 val;
> + struct frame_info info;
> +
> + for (i = 0; i < IFH_LEN; i++) {
> + err = ocelot_rx_frame_word(ocelot, grp, true, &ifh[i]);
> + if (err != 4)
> + break;
> + }
NAPI maybe?
[snip]
> + ocelot->targets[SYS] = ocelot_io_platform_init(ocelot, pdev, "sys");
> + if (IS_ERR(ocelot->targets[SYS]))
> + return PTR_ERR(ocelot->targets[SYS]);
You can clearly make this in a loop instead of repeating this section,
you just need an array of register names to be looking for.
[snip]
> + if (np) {
Please rework the indentation here, check for !np
> + for_each_child_of_node(np, portnp) {
for_each_available_child_of_node() you should be able to mark specific
ports as being disabled and skip over these accordingly.
[snip]
> +int ocelot_regfields_init(struct ocelot *ocelot,
> + const struct reg_field *const regfields)
> +{
> + int i;
unsigned int i
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 6/8] MIPS: mscc: Add switch to ocelot
From: Florian Fainelli @ 2018-03-23 21:44 UTC (permalink / raw)
To: Andrew Lunn, Alexandre Belloni
Cc: David S . Miller, Allan Nielsen, razvan.stefanescu, po.liu,
Thomas Petazzoni, netdev, devicetree, linux-kernel, linux-mips,
James Hogan
In-Reply-To: <20180323213344.GV24361@lunn.ch>
On 03/23/2018 02:33 PM, Andrew Lunn wrote:
> On Fri, Mar 23, 2018 at 10:22:30PM +0100, Alexandre Belloni wrote:
>> On 23/03/2018 at 14:17:48 -0700, Florian Fainelli wrote:
>>> On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
>>>> +
>>>> + phy0: ethernet-phy@0 {
>>>> + reg = <0>;
>>>> + };
>>>> + phy1: ethernet-phy@1 {
>>>> + reg = <1>;
>>>> + };
>>>> + phy2: ethernet-phy@2 {
>>>> + reg = <2>;
>>>> + };
>>>> + phy3: ethernet-phy@3 {
>>>> + reg = <3>;
>>>> + };
>>>
>>> These PHYs should be defined at the board DTS level.
>>
>> Those are internal PHYs, present on the SoC, I doubt anyone will have
>> anything different while using the same SoC.
>
> With DSA, there is no need to list internal PHYs.
>
> That is the trade off of having a standalone MDIO bus driver. Maybe
> add a phandle to the internal MDIO bus? The switch driver could then
> follow the phandle, and direct connect the internal PHYs?
This is more or less what patch 7 does, right?
--
Florian
^ permalink raw reply
* Re: rcu: Add might_sleep() check to synchronize_rcu()
From: Thomas Gleixner @ 2018-03-23 21:46 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Paul E. McKenney, Peter Zijlstra, Josh Triplett,
Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, netdev,
James Chapman
In-Reply-To: <20180323174023.60579f45@gandalf.local.home>
On Fri, 23 Mar 2018, Steven Rostedt wrote:
> On Fri, 23 Mar 2018 22:33:29 +0100 (CET)
> [ 150.741223] [<ffffffff81290287>] synchronize_rcu+0x27/0x90
> [ 150.746908] [<ffffffff83588b35>] __l2tp_session_unhash+0x3d5/0x550
>
> Looks like __l2tp_session_unhash() is the real culprit here.
Yes. I reported that to netdev already.
> [ 150.753281] [<ffffffff8358891f>] ? __l2tp_session_unhash+0x1bf/0x550
> [ 150.759828] [<ffffffff8114596a>] ? __local_bh_enable_ip+0x6a/0xd0
> [ 150.766123] [<ffffffff8358ddb0>] ? l2tp_udp_encap_recv+0xd90/0xd90
> [ 150.772497] [<ffffffff83588e97>] l2tp_tunnel_closeall+0x1e7/0x3a0
> [ 150.778782] [<ffffffff835897be>] l2tp_tunnel_destruct+0x30e/0x5a0
> [ 150.785067] [<ffffffff8358965a>] ? l2tp_tunnel_destruct+0x1aa/0x5a0
> [ 150.791537] [<ffffffff835894b0>] ? l2tp_tunnel_del_work+0x460/0x460
> [ 150.797997] [<ffffffff82ee8053>] __sk_destruct+0x53/0x570
> [ 150.803588] [<ffffffff81293918>] rcu_process_callbacks+0x898/0x1300
> [ 150.810048] [<ffffffff812939f7>] ? rcu_process_callbacks+0x977/0x1300
> [ 150.816684] [<ffffffff82ee8000>] ? __sk_dst_check+0x240/0x240
> [ 150.822625] [<ffffffff838be5d6>] __do_softirq+0x206/0x951
> [ 150.828223] [<ffffffff81147315>] irq_exit+0x165/0x190
> [ 150.833557] [<ffffffff838bd1eb>] smp_apic_timer_interrupt+0x7b/0xa0
> [ 150.840018] [<ffffffff838b9470>] apic_timer_interrupt+0xa0/0xb0
> [ 150.846132] <EOI> [ 150.848166] [<ffffffff838b6756>] ? native_safe_halt+0x6/0x10
> [ 150.854036] [<ffffffff8123bf2d>] ? trace_hardirqs_on+0xd/0x10
> [ 150.859973] [<ffffffff838b5d85>] default_idle+0x55/0x360
> [ 150.865478] [<ffffffff8106be0a>] arch_cpu_idle+0xa/0x10
>
> I think you want this instead, as __l2tp_session_unhash is what looks
> like might be hiding the call to synchronize_rcu(). It's not called in
> all instances, and I don't think your patch would have triggered the
> issues before hand. You want this:
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index 194a7483bb93..857b494bee29 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1677,6 +1677,8 @@ void __l2tp_session_unhash(struct l2tp_session *session)
> {
> struct l2tp_tunnel *tunnel = session->tunnel;
>
> + might_sleep();
> +
> /* Remove the session from core hashes */
> if (tunnel) {
> /* Remove from the per-tunnel hash */
That too :)
^ permalink raw reply
* Re: [PATCH net-next 2/8] dt-bindings: net: add DT bindings for Microsemi MIIM
From: Florian Fainelli @ 2018-03-23 21:46 UTC (permalink / raw)
To: Alexandre Belloni, David S . Miller
Cc: Allan Nielsen, razvan.stefanescu, po.liu, Thomas Petazzoni,
Andrew Lunn, netdev, devicetree, linux-kernel, linux-mips,
Rob Herring
In-Reply-To: <20180323201117.8416-3-alexandre.belloni@bootlin.com>
On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> DT bindings for the Microsemi MII Management Controller found on Microsemi
> SoCs
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> .../devicetree/bindings/net/mscc-miim.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/mscc-miim.txt
>
> diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt b/Documentation/devicetree/bindings/net/mscc-miim.txt
> new file mode 100644
> index 000000000000..711ac9ab853c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt
> @@ -0,0 +1,25 @@
> +Microsemi MII Management Controller (MIIM) / MDIO
> +=================================================
> +
> +Properties:
> +- compatible: must be "mscc,ocelot-miim"
> +- reg: The base address of the MDIO bus controller register bank. Optionally, a
> + second register bank can be defined if there is an associated reset register
> + for internal PHYs
> +- #address-cells: Must be <1>.
> +- #size-cells: Must be <0>. MDIO addresses have no size component.
Missing interrupt property documentation (sorry), other than that:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present
From: Chas Williams @ 2018-03-23 21:50 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: David Miller, netdev, Stephen Hemminger
In-Reply-To: <80b5cdab-9d5e-ef84-7cc0-2fe517001ed3@cumulusnetworks.com>
On Fri, Mar 23, 2018 at 12:20 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 23/03/18 18:17, David Miller wrote:
>> From: Chas Williams <3chas3@gmail.com>
>> Date: Thu, 22 Mar 2018 11:34:06 -0400
>>
>>> If the bridge is allowing multiple VLANs, some VLANs may have
>>> different MTUs. Instead of choosing the minimum MTU for the
>>> bridge interface, choose the maximum MTU of the bridge members.
>>> With this the user only needs to set a larger MTU on the member
>>> ports that are participating in the large MTU VLANS.
>>>
>>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>>
>> Applied, thanks.
>>
>
> Argh, this will break on builds without vlans because br->vlan_enabled shouldn't
> be accessed directly. I missed that when reviewing.
> I'll send a follow up fix in a second that uses br_vlan_enabled().
>
Sorry I missed that one. Thanks for covering!
^ permalink raw reply
* Re: [PATCH net-next 3/8] net: mscc: Add MDIO driver
From: Florian Fainelli @ 2018-03-23 21:51 UTC (permalink / raw)
To: Alexandre Belloni, David S . Miller
Cc: Allan Nielsen, razvan.stefanescu, po.liu, Thomas Petazzoni,
Andrew Lunn, netdev, devicetree, linux-kernel, linux-mips
In-Reply-To: <20180323201117.8416-4-alexandre.belloni@bootlin.com>
On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> Add a driver for the Microsemi MII Management controller (MIIM) found on
> Microsemi SoCs.
> On Ocelot, there are two controllers, one is connected to the internal
> PHYs, the other one can communicate with external PHYs.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/net/ethernet/Kconfig | 1 +
> drivers/net/ethernet/Makefile | 1 +
> drivers/net/ethernet/mscc/Kconfig | 22 ++++
> drivers/net/ethernet/mscc/Makefile | 2 +
> drivers/net/ethernet/mscc/mscc_miim.c | 210 ++++++++++++++++++++++++++++++++++
> 5 files changed, 236 insertions(+)
> create mode 100644 drivers/net/ethernet/mscc/Kconfig
> create mode 100644 drivers/net/ethernet/mscc/Makefile
> create mode 100644 drivers/net/ethernet/mscc/mscc_miim.c
>
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index b6cf4b6962f5..adf643484198 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -115,6 +115,7 @@ source "drivers/net/ethernet/mediatek/Kconfig"
> source "drivers/net/ethernet/mellanox/Kconfig"
> source "drivers/net/ethernet/micrel/Kconfig"
> source "drivers/net/ethernet/microchip/Kconfig"
> +source "drivers/net/ethernet/mscc/Kconfig"
> source "drivers/net/ethernet/moxa/Kconfig"
> source "drivers/net/ethernet/myricom/Kconfig"
>
> diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
> index 3cdf01e96e0b..ed7df22de7ff 100644
> --- a/drivers/net/ethernet/Makefile
> +++ b/drivers/net/ethernet/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_NET_VENDOR_MEDIATEK) += mediatek/
> obj-$(CONFIG_NET_VENDOR_MELLANOX) += mellanox/
> obj-$(CONFIG_NET_VENDOR_MICREL) += micrel/
> obj-$(CONFIG_NET_VENDOR_MICROCHIP) += microchip/
> +obj-$(CONFIG_NET_VENDOR_MICROSEMI) += mscc/
> obj-$(CONFIG_NET_VENDOR_MOXART) += moxa/
> obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/
> obj-$(CONFIG_FEALNX) += fealnx.o
> diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig
> new file mode 100644
> index 000000000000..2330de6e7bb6
> --- /dev/null
> +++ b/drivers/net/ethernet/mscc/Kconfig
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +config NET_VENDOR_MICROSEMI
> + bool "Microsemi devices"
> + default y
> + help
> + If you have a network (Ethernet) card belonging to this class, say Y.
> +
> + Note that the answer to this question doesn't directly affect the
> + kernel: saying N will just cause the configurator to skip all
> + the questions about Microsemi devices.
> +
> +if NET_VENDOR_MICROSEMI
> +
> +config MSCC_MIIM
> + tristate "Microsemi MIIM interface support"
> + depends on HAS_IOMEM
> + select PHYLIB
> + help
> + This driver supports the MIIM (MDIO) interface found in the network
> + switches of the Microsemi SoCs
> +
> +endif # NET_VENDOR_MICROSEMI
> diff --git a/drivers/net/ethernet/mscc/Makefile b/drivers/net/ethernet/mscc/Makefile
> new file mode 100644
> index 000000000000..4570e8fa4711
> --- /dev/null
> +++ b/drivers/net/ethernet/mscc/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +obj-$(CONFIG_MSCC_MIIM) += mscc_miim.o
> diff --git a/drivers/net/ethernet/mscc/mscc_miim.c b/drivers/net/ethernet/mscc/mscc_miim.c
> new file mode 100644
> index 000000000000..95b8d102c90f
> --- /dev/null
> +++ b/drivers/net/ethernet/mscc/mscc_miim.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Driver for the MDIO interface of Microsemi network switches.
> + *
> + * Author: Alexandre Belloni <alexandre.belloni@bootlin.com>
> + * Copyright (c) 2017 Microsemi Corporation
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/bitops.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_mdio.h>
> +
> +#define MSCC_MIIM_REG_STATUS 0x0
> +#define MSCC_MIIM_STATUS_STAT_BUSY BIT(3)
> +#define MSCC_MIIM_REG_CMD 0x8
> +#define MSCC_MIIM_CMD_OPR_WRITE BIT(1)
> +#define MSCC_MIIM_CMD_OPR_READ BIT(2)
> +#define MSCC_MIIM_CMD_WRDATA_SHIFT 4
> +#define MSCC_MIIM_CMD_REGAD_SHIFT 20
> +#define MSCC_MIIM_CMD_PHYAD_SHIFT 25
> +#define MSCC_MIIM_CMD_VLD BIT(31)
> +#define MSCC_MIIM_REG_DATA 0xC
> +#define MSCC_MIIM_DATA_ERROR (BIT(16) | BIT(17))
> +
> +#define MSCC_PHY_REG_PHY_CFG 0x0
> +#define PHY_CFG_PHY_ENA (BIT(0) | BIT(1) | BIT(2) | BIT(3))
> +#define PHY_CFG_PHY_COMMON_RESET BIT(4)
> +#define PHY_CFG_PHY_RESET (BIT(5) | BIT(6) | BIT(7) | BIT(8))
> +#define MSCC_PHY_REG_PHY_STATUS 0x4
> +
> +struct mscc_miim_dev {
> + struct mutex lock;
> + void __iomem *regs;
> + void __iomem *phy_regs;
> +};
> +
> +static int mscc_miim_wait_ready(struct mii_bus *bus)
> +{
> + struct mscc_miim_dev *miim = bus->priv;
> + u32 val;
> +
> + readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val,
> + !(val & MSCC_MIIM_STATUS_STAT_BUSY), 100, 250000);
> + if (val & MSCC_MIIM_STATUS_STAT_BUSY)
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum)
> +{
> + struct mscc_miim_dev *miim = bus->priv;
> + u32 val;
> + int ret;
> +
> + mutex_lock(&miim->lock);
What is this lock for considering that bus->lock should always be
acquired when doing these operations? As Andrew pointed out, needs to be
initialized with mutex_init(), but likely you would drop it.
> +
> + ret = mscc_miim_wait_ready(bus);
> + if (ret)
> + goto out;
> +
> + writel(MSCC_MIIM_CMD_VLD | (mii_id << MSCC_MIIM_CMD_PHYAD_SHIFT) |
> + (regnum << MSCC_MIIM_CMD_REGAD_SHIFT) | MSCC_MIIM_CMD_OPR_READ,
> + miim->regs + MSCC_MIIM_REG_CMD);
> +
> + ret = mscc_miim_wait_ready(bus);
> + if (ret)
> + goto out;
Your example had an interrupt specified, can't you use that instead of
polling?
> +
> + val = readl(miim->regs + MSCC_MIIM_REG_DATA);
> + if (val & MSCC_MIIM_DATA_ERROR) {
> + ret = -EIO;
> + goto out;
> + }
> +
> + ret = val & 0xFFFF;
> +out:
> + mutex_unlock(&miim->lock);
> + return ret;
> +}
> +
> +static int mscc_miim_write(struct mii_bus *bus, int mii_id,
> + int regnum, u16 value)
> +{
> + struct mscc_miim_dev *miim = bus->priv;
> + int ret;
> +
> + mutex_lock(&miim->lock);
> +
> + ret = mscc_miim_wait_ready(bus);
> + if (ret < 0)
> + goto out;
> +
> + writel(MSCC_MIIM_CMD_VLD | (mii_id << MSCC_MIIM_CMD_PHYAD_SHIFT) |
> + (regnum << MSCC_MIIM_CMD_REGAD_SHIFT) |
> + (value << MSCC_MIIM_CMD_WRDATA_SHIFT) |
> + MSCC_MIIM_CMD_OPR_WRITE,
> + miim->regs + MSCC_MIIM_REG_CMD);
> +
> +out:
> + mutex_unlock(&miim->lock);
> + return ret;
> +}
> +
> +static int mscc_miim_reset(struct mii_bus *bus)
> +{
> + struct mscc_miim_dev *miim = bus->priv;
> + int i;
unsigned int i
> +
> + if (miim->phy_regs) {
> + writel(0, miim->phy_regs + MSCC_PHY_REG_PHY_CFG);
> + writel(0x1ff, miim->phy_regs + MSCC_PHY_REG_PHY_CFG);
> + mdelay(500);
> + }
> +
> + for (i = 0; i < PHY_MAX_ADDR; i++) {
> + if (mscc_miim_read(bus, i, MII_PHYSID1) < 0)
> + bus->phy_mask |= BIT(i);
> + }
What is this used for? You have an OF MDIO bus which would create a
phy_device for each node specified, is this a similar workaround to what
drivers/net/phy/mdio-bcm-unimac.c has to do? If so, please document it
as such.
Other than that, this looks quite good!
--
Florian
^ permalink raw reply
* Re: [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
From: Alexander Duyck @ 2018-03-23 21:53 UTC (permalink / raw)
To: Sinan Kaya, intel-wired-lan
Cc: sulrich, Netdev, Timur Tabi, Jeff Kirsher, linux-arm-msm,
linux-arm-kernel
In-Reply-To: <1521831180-25014-1-git-send-email-okaya@codeaurora.org>
On Fri, Mar 23, 2018 at 11:52 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> Code includes wmb() followed by writel() in multiple places. writel()
> already has a barrier on some architectures like arm64.
>
> This ends up CPU observing two barriers back to back before executing the
> register write.
>
> Since code already has an explicit barrier call, changing writel() to
> writel_relaxed().
>
> I did a regex search for wmb() followed by writel() in each drivers
> directory.
> I scrubbed the ones I care about in this series.
>
> I considered "ease of change", "popular usage" and "performance critical
> path" as the determining criteria for my filtering.
>
> We used relaxed API heavily on ARM for a long time but
> it did not exist on other architectures. For this reason, relaxed
> architectures have been paying double penalty in order to use the common
> drivers.
>
> Now that relaxed API is present on all architectures, we can go and scrub
> all drivers to see what needs to change and what can remain.
>
> We start with mostly used ones and hope to increase the coverage over time.
> It will take a while to cover all drivers.
>
> Feel free to apply patches individually.
I looked over the set and they seem good.
Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
>
> Changes since v6:
> clean up between 2..6 and then make your Alex's changes on 1 and 7
> The mmiowb shouldn't be needed for Rx. Only one CPU will be running
> NAPI for the queue and we will synchronize this with a full writel
> anyway when we re-enable the interrupts.
>
> Sinan Kaya (7):
> i40e/i40evf: Eliminate duplicate barriers on weakly-ordered archs
> ixgbe: eliminate duplicate barriers on weakly-ordered archs
> igbvf: eliminate duplicate barriers on weakly-ordered archs
> igb: eliminate duplicate barriers on weakly-ordered archs
> fm10k: Eliminate duplicate barriers on weakly-ordered archs
> ixgbevf: keep writel() closer to wmb()
> ixgbevf: eliminate duplicate barriers on weakly-ordered archs
>
> drivers/net/ethernet/intel/fm10k/fm10k_main.c | 4 ++--
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 14 ++++++++++----
> drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 4 ++--
> drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
> drivers/net/ethernet/intel/igbvf/netdev.c | 4 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 8 ++++----
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 -----
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 11 ++++++++---
> 8 files changed, 30 insertions(+), 24 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH net-next 6/8] MIPS: mscc: Add switch to ocelot
From: Andrew Lunn @ 2018-03-23 22:06 UTC (permalink / raw)
To: Florian Fainelli
Cc: Alexandre Belloni, David S . Miller, Allan Nielsen,
razvan.stefanescu, po.liu, Thomas Petazzoni, netdev, devicetree,
linux-kernel, linux-mips, James Hogan
In-Reply-To: <dcac43b7-2eb7-d409-a77c-4f671a8cfc3d@gmail.com>
> > That is the trade off of having a standalone MDIO bus driver. Maybe
> > add a phandle to the internal MDIO bus? The switch driver could then
> > follow the phandle, and direct connect the internal PHYs?
>
> This is more or less what patch 7 does, right?
Patch 7 does it in DT. I'm suggesting it could be done in C. It is
hard wired, so there is no need to describe it in DT. Use the phandle
to get the mdio bus, mdiobus_get_phy(, port) to get the phydev and
then use phy_connect().
Andrew
^ permalink raw reply
* Re: [PATCH net-next 6/8] MIPS: mscc: Add switch to ocelot
From: Florian Fainelli @ 2018-03-23 22:11 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Belloni, David S . Miller, Allan Nielsen,
razvan.stefanescu, po.liu, Thomas Petazzoni, netdev, devicetree,
linux-kernel, linux-mips, James Hogan
In-Reply-To: <20180323220657.GY24361@lunn.ch>
On 03/23/2018 03:06 PM, Andrew Lunn wrote:
>>> That is the trade off of having a standalone MDIO bus driver. Maybe
>>> add a phandle to the internal MDIO bus? The switch driver could then
>>> follow the phandle, and direct connect the internal PHYs?
>>
>> This is more or less what patch 7 does, right?
>
> Patch 7 does it in DT. I'm suggesting it could be done in C. It is
> hard wired, so there is no need to describe it in DT. Use the phandle
> to get the mdio bus, mdiobus_get_phy(, port) to get the phydev and
> then use phy_connect().
That does not sound like a great idea. And to go back to your example
about DSA, it is partially true, you will see some switch bindings
defining the internal PHYs (e.g: qca8k), and most not doing it (b53,
mv88e6xxx, etc.). In either case, this resolves to the same thing
though. Being able to parse a phy-handle property is a lot more
flexible, and if it does matter that the PHY truly is internal, then the
'phy-mode' property can help reflect that.
--
Florian
^ permalink raw reply
* [PATCH v6 0/6] netdev: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 22:23 UTC (permalink / raw)
To: netdev, timur, sulrich; +Cc: Sinan Kaya, linux-arm-msm, linux-arm-kernel
Code includes wmb() followed by writel() in multiple places. writel()
already has a barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
I did a regex search for wmb() followed by writel() in each drivers
directory.
I scrubbed the ones I care about in this series.
I considered "ease of change", "popular usage" and "performance critical
path" as the determining criteria for my filtering.
We used relaxed API heavily on ARM for a long time but
it did not exist on other architectures. For this reason, relaxed
architectures have been paying double penalty in order to use the common
drivers.
Now that relaxed API is present on all architectures, we can go and scrub
all drivers to see what needs to change and what can remain.
We start with mostly used ones and hope to increase the coverage over time.
It will take a while to cover all drivers.
Feel free to apply patches individually.
Changes since v5:
- add mmiowb() for PPC architecture
- collect reviewed/acked bys
- bn2x: change doorbell barrier to wmb for observability guarantee across
all architectures.
Sinan Kaya (6):
net: qla3xxx: Eliminate duplicate barriers on weakly-ordered archs
qlcnic: Eliminate duplicate barriers on weakly-ordered archs
bnx2x: Replace doorbell barrier() with wmb()
bnx2x: Eliminate duplicate barriers on weakly-ordered archs
net: qlge: Eliminate duplicate barriers on weakly-ordered archs
bnxt_en: Eliminate duplicate barriers on weakly-ordered archs
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 12 ++++++++----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 5 +++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 5 +++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 4 +++-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 9 +++++++++
drivers/net/ethernet/qlogic/qla3xxx.c | 5 +++--
drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 2 +-
drivers/net/ethernet/qlogic/qlge/qlge.h | 16 ++++++++++++++++
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 3 ++-
12 files changed, 54 insertions(+), 18 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v6 1/6] net: qla3xxx: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 22:23 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Dept-GELinuxNICDev,
linux-kernel
In-Reply-To: <1521843791-21201-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing
the register write.
Since code already has an explicit barrier call, changing code to
wmb()
writel_relaxed()
mmiowb()
for multi-arch support.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/qlogic/qla3xxx.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
index 9e5264d..b48f761 100644
--- a/drivers/net/ethernet/qlogic/qla3xxx.c
+++ b/drivers/net/ethernet/qlogic/qla3xxx.c
@@ -1858,8 +1858,9 @@ static void ql_update_small_bufq_prod_index(struct ql3_adapter *qdev)
qdev->small_buf_release_cnt -= 8;
}
wmb();
- writel(qdev->small_buf_q_producer_index,
- &port_regs->CommonRegs.rxSmallQProducerIndex);
+ writel_relaxed(qdev->small_buf_q_producer_index,
+ &port_regs->CommonRegs.rxSmallQProducerIndex);
+ mmiowb();
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH v6 2/6] qlcnic: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 22:23 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: Dept-GELinuxNICDev, linux-arm-msm, linux-kernel, Sinan Kaya,
Harish Patil, linux-arm-kernel, Manish Chopra
In-Reply-To: <1521843791-21201-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing
the register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
Acked-by: Manish Chopra <manish.chopra@cavium.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 46b0372..97c146e7 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -478,7 +478,7 @@ irqreturn_t qlcnic_83xx_clear_legacy_intr(struct qlcnic_adapter *adapter)
wmb();
/* clear the interrupt trigger control register */
- writel(0, adapter->isr_int_vec);
+ writel_relaxed(0, adapter->isr_int_vec);
intr_val = readl(adapter->isr_int_vec);
do {
intr_val = readl(adapter->tgt_status_reg);
--
2.7.4
^ permalink raw reply related
* [PATCH v6 3/6] bnx2x: Replace doorbell barrier() with wmb()
From: Sinan Kaya @ 2018-03-23 22:23 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-kernel, Sinan Kaya, Ariel Elior,
everest-linux-l2, linux-arm-kernel
In-Reply-To: <1521843791-21201-1-git-send-email-okaya@codeaurora.org>
barrier() doesn't guarantee memory writes to be observed by the hardware on
all architectures. barrier() only tells compiler not to move this code
with respect to other read/writes.
If memory write needs to be observed by the HW, wmb() is the right choice.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 3 ++-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index d7c98e8..b97820f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4153,7 +4153,8 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
wmb();
txdata->tx_db.data.prod += nbd;
- barrier();
+ /* make sur edescriptor update is observed by HW */
+ wmb();
DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 1e33abd..39af4f8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -2591,7 +2591,8 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode)
wmb();
txdata->tx_db.data.prod += 2;
- barrier();
+ /* make sure descriptor update is observed by the HW */
+ wmb();
DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
mmiowb();
--
2.7.4
^ permalink raw reply related
* [PATCH v6 4/6] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 22:23 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Ariel Elior,
everest-linux-l2, linux-kernel
In-Reply-To: <1521843791-21201-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing
the register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 12 ++++++++----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 2 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 4 +++-
6 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 352beff..d847e1b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -166,6 +166,12 @@ do { \
#define REG_RD8(bp, offset) readb(REG_ADDR(bp, offset))
#define REG_RD16(bp, offset) readw(REG_ADDR(bp, offset))
+#define REG_WR_RELAXED(bp, offset, val) \
+ writel_relaxed((u32)val, REG_ADDR(bp, offset))
+
+#define REG_WR16_RELAXED(bp, offset, val) \
+ writew_relaxed((u16)val, REG_ADDR(bp, offset))
+
#define REG_WR(bp, offset, val) writel((u32)val, REG_ADDR(bp, offset))
#define REG_WR8(bp, offset, val) writeb((u8)val, REG_ADDR(bp, offset))
#define REG_WR16(bp, offset, val) writew((u16)val, REG_ADDR(bp, offset))
@@ -758,10 +764,8 @@ struct bnx2x_fastpath {
#if (BNX2X_DB_SHIFT < BNX2X_DB_MIN_SHIFT)
#error "Min DB doorbell stride is 8"
#endif
-#define DOORBELL(bp, cid, val) \
- do { \
- writel((u32)(val), bp->doorbells + (bp->db_size * (cid))); \
- } while (0)
+#define DOORBELL_RELAXED(bp, cid, val) \
+ writel_relaxed((u32)(val), (bp)->doorbells + ((bp)->db_size * (cid)))
/* TX CSUM helpers */
#define SKB_CS_OFF(skb) (offsetof(struct tcphdr, check) - \
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index b97820f..91d2de6 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4156,7 +4156,7 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
/* make sur edescriptor update is observed by HW */
wmb();
- DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
+ DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
mmiowb();
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
index a5265e1..a8ce5c5 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
@@ -522,8 +522,8 @@ static inline void bnx2x_update_rx_prod(struct bnx2x *bp,
wmb();
for (i = 0; i < sizeof(rx_prods)/4; i++)
- REG_WR(bp, fp->ustorm_rx_prods_offset + i*4,
- ((u32 *)&rx_prods)[i]);
+ REG_WR_RELAXED(bp, fp->ustorm_rx_prods_offset + i * 4,
+ ((u32 *)&rx_prods)[i]);
mmiowb(); /* keep prod updates ordered */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 39af4f8..da18aa2 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -2593,7 +2593,7 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode)
txdata->tx_db.data.prod += 2;
/* make sure descriptor update is observed by the HW */
wmb();
- DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
+ DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
mmiowb();
barrier();
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 74fc9af..146c40d 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -3817,8 +3817,8 @@ static void bnx2x_sp_prod_update(struct bnx2x *bp)
*/
mb();
- REG_WR16(bp, BAR_XSTRORM_INTMEM + XSTORM_SPQ_PROD_OFFSET(func),
- bp->spq_prod_idx);
+ REG_WR16_RELAXED(bp, BAR_XSTRORM_INTMEM + XSTORM_SPQ_PROD_OFFSET(func),
+ bp->spq_prod_idx);
mmiowb();
}
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
index 76a4668..8e0a317 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -170,7 +170,9 @@ static int bnx2x_send_msg2pf(struct bnx2x *bp, u8 *done, dma_addr_t msg_mapping)
wmb();
/* Trigger the PF FW */
- writeb(1, &zone_data->trigger.vf_pf_channel.addr_valid);
+ writeb_relaxed(1, &zone_data->trigger.vf_pf_channel.addr_valid);
+
+ mmiowb();
/* Wait for PF to complete */
while ((tout >= 0) && (!*done)) {
--
2.7.4
^ permalink raw reply related
* [PATCH v6 5/6] net: qlge: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 22:23 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Harish Patil,
Manish Chopra, Dept-GELinuxNICDev, linux-kernel
In-Reply-To: <1521843791-21201-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a barrier on
some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Create a new wrapper function with relaxed write operator. Use the new
wrapper when a write is following a wmb().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/qlogic/qlge/qlge.h | 16 ++++++++++++++++
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 3 ++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index 84ac50f..3e71b65 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -2185,6 +2185,22 @@ static inline void ql_write_db_reg(u32 val, void __iomem *addr)
}
/*
+ * Doorbell Registers:
+ * Doorbell registers are virtual registers in the PCI memory space.
+ * The space is allocated by the chip during PCI initialization. The
+ * device driver finds the doorbell address in BAR 3 in PCI config space.
+ * The registers are used to control outbound and inbound queues. For
+ * example, the producer index for an outbound queue. Each queue uses
+ * 1 4k chunk of memory. The lower half of the space is for outbound
+ * queues. The upper half is for inbound queues.
+ * Caller has to guarantee ordering.
+ */
+static inline void ql_write_db_reg_relaxed(u32 val, void __iomem *addr)
+{
+ writel_relaxed(val, addr);
+}
+
+/*
* Shadow Registers:
* Outbound queues have a consumer index that is maintained by the chip.
* Inbound queues have a producer index that is maintained by the chip.
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 50038d9..8293c202 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -2700,7 +2700,8 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
tx_ring->prod_idx = 0;
wmb();
- ql_write_db_reg(tx_ring->prod_idx, tx_ring->prod_idx_db_reg);
+ ql_write_db_reg_relaxed(tx_ring->prod_idx, tx_ring->prod_idx_db_reg);
+ mmiowb();
netif_printk(qdev, tx_queued, KERN_DEBUG, qdev->ndev,
"tx queued, slot %d, len %d\n",
tx_ring->prod_idx, skb->len);
--
2.7.4
^ permalink raw reply related
* [PATCH v6 6/6] bnxt_en: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 22:23 UTC (permalink / raw)
To: netdev, timur, sulrich
Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Michael Chan,
linux-kernel
In-Reply-To: <1521843791-21201-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a barrier on
some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Create a new wrapper function with relaxed write operator. Use the new
wrapper when a write is following a wmb().
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Also add mmiowb() so that write code doesn't move outside of scope.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 9 +++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1500243..fc8ea0d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1922,7 +1922,8 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
/* Sync BD data before updating doorbell */
wmb();
- bnxt_db_write(bp, db, DB_KEY_TX | prod);
+ bnxt_db_write_relaxed(bp, db, DB_KEY_TX | prod);
+ mmiowb();
}
cpr->cp_raw_cons = raw_cons;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 1989c47..5e453b9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1401,6 +1401,15 @@ static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
((txr->tx_prod - txr->tx_cons) & bp->tx_ring_mask);
}
+/* For TX and RX ring doorbells with no ordering guarantee*/
+static inline void bnxt_db_write_relaxed(struct bnxt *bp, void __iomem *db,
+ u32 val)
+{
+ writel_relaxed(val, db);
+ if (bp->flags & BNXT_FLAG_DOUBLE_DB)
+ writel_relaxed(val, db);
+}
+
/* For TX and RX ring doorbells */
static inline void bnxt_db_write(struct bnxt *bp, void __iomem *db, u32 val)
{
--
2.7.4
^ permalink raw reply related
* [net 4/8] net/mlx5e: Verify coalescing parameters in range
From: Saeed Mahameed @ 2018-03-23 22:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Moshe Shemesh, Saeed Mahameed
In-Reply-To: <20180323220534.19353-1-saeedm@mellanox.com>
From: Moshe Shemesh <moshe@mellanox.com>
Add check of coalescing parameters received through ethtool are within
range of values supported by the HW.
Driver gets the coalescing rx/tx-usecs and rx/tx-frames as set by the
users through ethtool. The ethtool support up to 32 bit value for each.
However, mlx5 modify cq limits the coalescing time parameter to 12 bit
and coalescing frames parameters to 16 bits.
Return out of range error if user tries to set these parameters to
higher values.
Fixes: f62b8bb8f2d3 ('net/mlx5: Extend mlx5_core to support ConnectX-4 Ethernet functionality')
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index cc8048f68f11..59ebfdae6695 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -477,6 +477,9 @@ static int mlx5e_get_coalesce(struct net_device *netdev,
return mlx5e_ethtool_get_coalesce(priv, coal);
}
+#define MLX5E_MAX_COAL_TIME MLX5_MAX_CQ_PERIOD
+#define MLX5E_MAX_COAL_FRAMES MLX5_MAX_CQ_COUNT
+
static void
mlx5e_set_priv_channels_coalesce(struct mlx5e_priv *priv, struct ethtool_coalesce *coal)
{
@@ -511,6 +514,20 @@ int mlx5e_ethtool_set_coalesce(struct mlx5e_priv *priv,
if (!MLX5_CAP_GEN(mdev, cq_moderation))
return -EOPNOTSUPP;
+ if (coal->tx_coalesce_usecs > MLX5E_MAX_COAL_TIME ||
+ coal->rx_coalesce_usecs > MLX5E_MAX_COAL_TIME) {
+ netdev_info(priv->netdev, "%s: maximum coalesce time supported is %lu usecs\n",
+ __func__, MLX5E_MAX_COAL_TIME);
+ return -ERANGE;
+ }
+
+ if (coal->tx_max_coalesced_frames > MLX5E_MAX_COAL_FRAMES ||
+ coal->rx_max_coalesced_frames > MLX5E_MAX_COAL_FRAMES) {
+ netdev_info(priv->netdev, "%s: maximum coalesced frames supported is %lu\n",
+ __func__, MLX5E_MAX_COAL_FRAMES);
+ return -ERANGE;
+ }
+
mutex_lock(&priv->state_lock);
new_channels.params = priv->channels.params;
--
2.14.3
^ permalink raw reply related
* [net 1/8] net/mlx5e: Don't override vport admin link state in switchdev mode
From: Saeed Mahameed @ 2018-03-23 22:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Jianbo Liu, Saeed Mahameed
In-Reply-To: <20180323220534.19353-1-saeedm@mellanox.com>
From: Jianbo Liu <jianbol@mellanox.com>
The vport admin original link state will be re-applied after returning
back to legacy mode, it is not right to change the admin link state value
when in switchdev mode.
Use direct vport commands to alter logical vport state in netdev
representor open/close flows rather than the administrative eswitch API.
Fixes: 20a1ea674783 ('net/mlx5e: Support VF vport link state control for SRIOV switchdev mode')
Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 363d8dcb7f17..0273c233bc85 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -668,7 +668,6 @@ static int mlx5e_rep_open(struct net_device *dev)
struct mlx5e_priv *priv = netdev_priv(dev);
struct mlx5e_rep_priv *rpriv = priv->ppriv;
struct mlx5_eswitch_rep *rep = rpriv->rep;
- struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
int err;
mutex_lock(&priv->state_lock);
@@ -676,8 +675,9 @@ static int mlx5e_rep_open(struct net_device *dev)
if (err)
goto unlock;
- if (!mlx5_eswitch_set_vport_state(esw, rep->vport,
- MLX5_ESW_VPORT_ADMIN_STATE_UP))
+ if (!mlx5_modify_vport_admin_state(priv->mdev,
+ MLX5_QUERY_VPORT_STATE_IN_OP_MOD_ESW_VPORT,
+ rep->vport, MLX5_ESW_VPORT_ADMIN_STATE_UP))
netif_carrier_on(dev);
unlock:
@@ -690,11 +690,12 @@ static int mlx5e_rep_close(struct net_device *dev)
struct mlx5e_priv *priv = netdev_priv(dev);
struct mlx5e_rep_priv *rpriv = priv->ppriv;
struct mlx5_eswitch_rep *rep = rpriv->rep;
- struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
int ret;
mutex_lock(&priv->state_lock);
- (void)mlx5_eswitch_set_vport_state(esw, rep->vport, MLX5_ESW_VPORT_ADMIN_STATE_DOWN);
+ mlx5_modify_vport_admin_state(priv->mdev,
+ MLX5_QUERY_VPORT_STATE_IN_OP_MOD_ESW_VPORT,
+ rep->vport, MLX5_ESW_VPORT_ADMIN_STATE_DOWN);
ret = mlx5e_close_locked(dev);
mutex_unlock(&priv->state_lock);
return ret;
--
2.14.3
^ permalink raw reply related
* [pull request][net 0/8] Mellanox, mlx5 fixes 2018-03-23
From: Saeed Mahameed @ 2018-03-23 22:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Saeed Mahameed
Hi Dave,
The following series includes fixes for mlx5 netdev and eswitch.
For -stable v4.12
('net/mlx5e: Avoid using the ipv6 stub in the TC offload neigh update path')
('net/mlx5e: Fix traffic being dropped on VF representor')
For -stable v4.13
('net/mlx5e: Fix memory usage issues in offloading TC flows')
('net/mlx5e: Verify coalescing parameters in range')
For -stable v4.14
('net/mlx5e: Don't override vport admin link state in switchdev mode')
For -stable v4.15
('108b2b6d5c02 net/mlx5e: Sync netdev vxlan ports at open')
Please pull and let me know if there's any problem.
Thanks,
Saeed.
---
The following changes since commit 1bfa26ff8c4b7512f4e4efa6df211239223033d4:
ipv6: fix possible deadlock in rt6_age_examine_exception() (2018-03-23 13:40:34 -0400)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2018-03-23
for you to fetch changes up to dcbfde231207f7b6e1f5628d59b182198e1cfae2:
net/mlx5e: Sync netdev vxlan ports at open (2018-03-23 14:23:50 -0700)
----------------------------------------------------------------
mlx5-fixes-2018-03-23
----------------------------------------------------------------
Jianbo Liu (2):
net/mlx5e: Don't override vport admin link state in switchdev mode
net/mlx5e: Fix memory usage issues in offloading TC flows
Moshe Shemesh (1):
net/mlx5e: Verify coalescing parameters in range
Or Gerlitz (3):
net/mlx5e: Use 32 bits to store VF representor SQ number
net/mlx5: Make eswitch support to depend on switchdev
net/mlx5e: Avoid using the ipv6 stub in the TC offload neigh update path
Roi Dayan (1):
net/mlx5e: Fix traffic being dropped on VF representor
Shahar Klein (1):
net/mlx5e: Sync netdev vxlan ports at open
drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 17 +++++++++++
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 13 ++++-----
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 34 ++++++++++++----------
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 18 ++++++------
5 files changed, 51 insertions(+), 33 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox