| Message ID | 1611488647-12478-1-git-send-email-stefanc@marvell.com |
|---|---|
| Headers | show |
| Series | net: mvpp2: Add TX Flow Control support | expand |
On Sun, Jan 24, 2021 at 01:44:02PM +0200, stefanc@marvell.com wrote: > @@ -6407,6 +6490,29 @@ static void mvpp2_mac_link_up(struct phylink_config *config, > val); > } > > + if (tx_pause && port->priv->global_tx_fc) { > + port->tx_fc = true; > + mvpp2_rxq_enable_fc(port); > + if (port->priv->percpu_pools) { > + for (i = 0; i < port->nrxqs; i++) > + mvpp2_bm_pool_update_fc(port, &port->priv->bm_pools[i], true); > + } else { > + mvpp2_bm_pool_update_fc(port, port->pool_long, true); > + mvpp2_bm_pool_update_fc(port, port->pool_short, true); > + } > + > + } else if (port->priv->global_tx_fc) { > + port->tx_fc = false; > + mvpp2_rxq_disable_fc(port); > + if (port->priv->percpu_pools) { > + for (i = 0; i < port->nrxqs; i++) > + mvpp2_bm_pool_update_fc(port, &port->priv->bm_pools[i], false); > + } else { > + mvpp2_bm_pool_update_fc(port, port->pool_long, false); > + mvpp2_bm_pool_update_fc(port, port->pool_short, false); > + } > + } > + It seems this can be written more succinctly: if (port->priv->global_tx_fc) { port->tx_fc = tx_pause; if (tx_pause) mvpp2_rxq_enable_fc(port); else mvpp2_rxq_disable_fc(port); if (port->priv->percpu_pools) { for (i = 0; i < port->nrxqs; i++) mvpp2_bm_pool_update_fc(port, &port->priv->bm_pools[i], tx_pause); } else { mvpp2_bm_pool_update_fc(port, port->pool_long, tx_pause); mvpp2_bm_pool_update_fc(port, port->pool_short, tx_pause); } }
On Sun, Jan 24, 2021 at 01:43:52PM +0200, stefanc@marvell.com wrote: > + priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0); > + if (!priv->sram_pool) { > + if (!defer_once) { > + defer_once = true; > + /* Try defer once */ > + return -EPROBE_DEFER; > + } > + dev_warn(&pdev->dev, "DT is too old, Flow control not supported\n"); > + return -ENOMEM; > + } > + priv->cm3_base = (void __iomem *)gen_pool_alloc(priv->sram_pool, > + MSS_SRAM_SIZE); > + if (!priv->cm3_base) > + return -ENOMEM; This probably could do with a comment indicating that it is reliant on this allocation happening at offset zero into the SRAM. The only reason that is guaranteed _at the moment_ is because the SRAM mapping is 0x800 bytes in size, and you are requesting 0x800 bytes in this allocation, so allocating the full size.
On Sun, Jan 24, 2021 at 01:43:53PM +0200, stefanc@marvell.com wrote: > From: Stefan Chulski <stefanc@marvell.com> > > This patch add PPv23 version definition. > PPv23 is new packet processor in CP115. > Everything that supported by PPv22, also supported by PPv23. > No functional changes in this stage. > > Signed-off-by: Stefan Chulski <stefanc@marvell.com> > --- > drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 24 ++++++++++++-------- > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 17 +++++++++----- > 2 files changed, 25 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > index aec9179..89b3ede 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > @@ -60,6 +60,9 @@ > /* Top Registers */ > #define MVPP2_MH_REG(port) (0x5040 + 4 * (port)) > #define MVPP2_DSA_EXTENDED BIT(5) > +#define MVPP2_VER_ID_REG 0x50b0 > +#define MVPP2_VER_PP22 0x10 > +#define MVPP2_VER_PP23 0x11 Looking at the Armada 8040 docs, it seems this register exists on PPv2.1 as well, and holds the value zero there. I wonder whether we should instead read it's value directly into hw_version, and test against these values, rather than inventing our own verison enum. I've also been wondering whether your != MVPP21 comparisons should instead be >= MVPP22. Any thoughts?
On Sun, Jan 24, 2021 at 12:44:43PM +0000, Russell King - ARM Linux admin wrote: > On Sun, Jan 24, 2021 at 01:43:52PM +0200, stefanc@marvell.com wrote: > > + priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0); > > + if (!priv->sram_pool) { > > + if (!defer_once) { > > + defer_once = true; > > + /* Try defer once */ > > + return -EPROBE_DEFER; > > + } > > + dev_warn(&pdev->dev, "DT is too old, Flow control not supported\n"); > > + return -ENOMEM; > > + } > > + priv->cm3_base = (void __iomem *)gen_pool_alloc(priv->sram_pool, > > + MSS_SRAM_SIZE); > > + if (!priv->cm3_base) > > + return -ENOMEM; > > This probably could do with a comment indicating that it is reliant on > this allocation happening at offset zero into the SRAM. The only reason > that is guaranteed _at the moment_ is because the SRAM mapping is 0x800 > bytes in size, and you are requesting 0x800 bytes in this allocation, > so allocating the full size. Hi Russell I'm wondering if using a pool even makes sense. The ACPI case just ioremap() the memory region. Either this memory is dedicated, and then there is no need to use a pool, or the memory is shared, and at some point the ACPI code is going to run into problems when some other driver also wants access. Andrew
On Sun, Jan 24, 2021 at 01:55:42PM +0000, Stefan Chulski wrote: > > > Signed-off-by: Stefan Chulski <stefanc@marvell.com> > > > --- > > > drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 24 ++++++++++++------ > > -- > > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 17 +++++++++----- > > > 2 files changed, 25 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > > b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > > index aec9179..89b3ede 100644 > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > > @@ -60,6 +60,9 @@ > > > /* Top Registers */ > > > #define MVPP2_MH_REG(port) (0x5040 + 4 * (port)) > > > #define MVPP2_DSA_EXTENDED BIT(5) > > > +#define MVPP2_VER_ID_REG 0x50b0 > > > +#define MVPP2_VER_PP22 0x10 > > > +#define MVPP2_VER_PP23 0x11 > > > > Looking at the Armada 8040 docs, it seems this register exists on > > PPv2.1 as well, and holds the value zero there. > > > > I wonder whether we should instead read it's value directly into hw_version, > > and test against these values, rather than inventing our own verison enum. > > > > I've also been wondering whether your != MVPP21 comparisons should > > instead be >= MVPP22. > > > > Any thoughts? > > We cannot access PPv2 register space before enabling clocks(done in mvpp2_probe) , PP21 and PP22/23 have different sets of clocks. > So diff between PP21 and PP22/23 should be stored in device tree(in > of_device_id), with MVPP22 and MVPP21 stored as .data Hi Stefan As far as i can see, you are not adding a new compatible. So 'marvell,armada-7k-pp2' means PPv2.2 and PPv2.3? It would be good to update the comment at the beginning of marvell-pp2.txt to indicate this. Andrew
> > We cannot access PPv2 register space before enabling clocks(done in > mvpp2_probe) , PP21 and PP22/23 have different sets of clocks. > > > So diff between PP21 and PP22/23 should be stored in device tree(in > > of_device_id), with MVPP22 and MVPP21 stored as .data > > Hi Stefan > > As far as i can see, you are not adding a new compatible. So 'marvell,armada- > 7k-pp2' means PPv2.2 and PPv2.3? It would be good to update the comment > at the beginning of marvell-pp2.txt to indicate this. > > Andrew Ok, I would add comment. Thanks, Stefan.
> > #include <linux/of_net.h> > > #include <linux/of_address.h> > > #include <linux/of_device.h> > > +#include <linux/genalloc.h> > > #include <linux/phy.h> > > #include <linux/phylink.h> > > #include <linux/phy/phy.h> > > @@ -91,6 +92,18 @@ static inline u32 mvpp2_cpu_to_thread(struct > mvpp2 *priv, int cpu) > > return cpu % priv->nthreads; > > } > > > > +static inline > > +void mvpp2_cm3_write(struct mvpp2 *priv, u32 offset, u32 data) { > > + writel(data, priv->cm3_base + offset); } > > + > > +static inline > > +u32 mvpp2_cm3_read(struct mvpp2 *priv, u32 offset) { > > + return readl(priv->cm3_base + offset); } > > No inline functions in .c file please. Let the compiler decide. > > Andrew Ok, I would fix. Thanks, Stefan.
From: Stefan Chulski <stefanc@marvell.com> Armada hardware has a pause generation mechanism in GOP (MAC). The GOP generate flow control frames based on an indication programmed in Ports Control 0 Register. There is a bit per port. However assertion of the PortX Pause bits in the ports control 0 register only sends a one time pause. To complement the function the GOP has a mechanism to periodically send pause control messages based on periodic counters. This mechanism ensures that the pause is effective as long as the Appropriate PortX Pause is asserted. Problem is that Packet Processor that actually can drop packets due to lack of resources not connected to the GOP flow control generation mechanism. To solve this issue Armada has firmware running on CM3 CPU dedicated for Flow Control support. Firmware monitors Packet Processor resources and asserts XON/XOFF by writing to Ports Control 0 Register. MSS shared SRAM memory used to communicate between CM3 firmware and PP2 driver. During init PP2 driver informs firmware about used BM pools, RXQs, congestion and depletion thresholds. The pause frames are generated whenever congestion or depletion in resources is detected. The back pressure is stopped when the resource reaches a sufficient level. So the congestion/depletion and sufficient level implement a hysteresis that reduces the XON/XOFF toggle frequency. Packet Processor v23 hardware introduces support for RX FIFO fill level monitor. Patch "add PPv23 version definition" to differ between v23 and v22 hardware. Patch "add TX FC firmware check" verifies that CM3 firmware supports Flow Control monitoring. v1 --> v2 - Add memory requirements information - Add EPROBE_DEFER if of_gen_pool_get return NULL - Move Flow control configuration to mvpp2_mac_link_up callback - Add firmware version info with Flow control support Konstantin Porotchkin (1): dts: marvell: add CM3 SRAM memory to cp115 ethernet device tree Stefan Chulski (17): doc: marvell: add cm3-mem device tree bindings description net: mvpp2: add CM3 SRAM memory map net: mvpp2: add PPv23 version definition net: mvpp2: always compare hw-version vs MVPP21 net: mvpp2: increase BM pool size to 2048 buffers net: mvpp2: increase RXQ size to 1024 descriptors net: mvpp2: add FCA periodic timer configurations net: mvpp2: add FCA RXQ non occupied descriptor threshold net: mvpp2: add spinlock for FW FCA configuration path net: mvpp2: enable global flow control net: mvpp2: add RXQ flow control configurations net: mvpp2: add ethtool flow control configuration support net: mvpp2: add BM protection underrun feature support net: mvpp2: add PPv23 RX FIFO flow control net: mvpp2: set 802.3x GoP Flow Control mode net: mvpp2: limit minimum ring size to 1024 descriptors net: mvpp2: add TX FC firmware check Documentation/devicetree/bindings/net/marvell-pp2.txt | 1 + arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 10 + drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 130 ++++- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 558 +++++++++++++++++++- 4 files changed, 657 insertions(+), 42 deletions(-)