diff mbox series

[RFC,net-next,03/19] net: mvpp2: add CM3 SRAM memory map

Message ID 1610292623-15564-4-git-send-email-stefanc@marvell.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series net: mvpp2: Add TX Flow Control support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch fail ERROR: Remove Gerrit Change-Id's before submitting upstream
netdev/build_allmodconfig_warn success Errors and warnings before: 5 this patch: 5
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Stefan Chulski Jan. 10, 2021, 3:30 p.m. UTC 
From: Stefan Chulski <stefanc@marvell.com>

This patch adds CM3 memory map and CM3 read/write callbacks.
No functionality changes.

Change-Id: Ibae3f5e6695f3454f799568308d349addc730f01
Signed-off-by: Stefan Chulski <stefanc@marvell.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h      |  7 +++
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 66 +++++++++++++++++++-
 2 files changed, 70 insertions(+), 3 deletions(-)

Comments

Andrew Lunn Jan. 10, 2021, 5:04 p.m. UTC | #1 
> +static int mvpp2_get_sram(struct platform_device *pdev,
> +			  struct mvpp2 *priv)
> +{
> +	struct device_node *dn = pdev->dev.of_node;
> +	struct resource *res;
> +
> +	if (has_acpi_companion(&pdev->dev)) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +		if (!res) {
> +			dev_warn(&pdev->dev, "ACPI is too old, TX FC disabled\n");
> +			return 0;
> +		}
> +		priv->cm3_base = devm_ioremap_resource(&pdev->dev, res);
> +		if (IS_ERR(priv->cm3_base))
> +			return PTR_ERR(priv->cm3_base);
> +	} else {
> +		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> +		if (!priv->sram_pool) {
> +			dev_warn(&pdev->dev, "DT is too old, TX FC disabled\n");
> +			return 0;
> +		}
> +		priv->cm3_base = (void __iomem *)gen_pool_alloc(priv->sram_pool,
> +								MSS_SRAM_SIZE);
> +		if (!priv->cm3_base)
> +			return -ENOMEM;

Should there be -EPROBE_DEFER handling in here somewhere? The SRAM is
a device, so it might not of been probed yet?

  Andrew
Stefan Chulski Jan. 10, 2021, 5:10 p.m. UTC | #2 
> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Sunday, January 10, 2021 7:05 PM
> To: Stefan Chulski <stefanc@marvell.com>
> Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> linux@armlinux.org.uk; mw@semihalf.com; rmk+kernel@armlinux.org.uk;
> atenart@kernel.org
> Subject: [EXT] Re: [PATCH RFC net-next 03/19] net: mvpp2: add CM3 SRAM
> memory map
> 
> External Email
> 
> ----------------------------------------------------------------------
> > +static int mvpp2_get_sram(struct platform_device *pdev,
> > +			  struct mvpp2 *priv)
> > +{
> > +	struct device_node *dn = pdev->dev.of_node;
> > +	struct resource *res;
> > +
> > +	if (has_acpi_companion(&pdev->dev)) {
> > +		res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> > +		if (!res) {
> > +			dev_warn(&pdev->dev, "ACPI is too old, TX FC
> disabled\n");
> > +			return 0;
> > +		}
> > +		priv->cm3_base = devm_ioremap_resource(&pdev->dev,
> res);
> > +		if (IS_ERR(priv->cm3_base))
> > +			return PTR_ERR(priv->cm3_base);
> > +	} else {
> > +		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > +		if (!priv->sram_pool) {
> > +			dev_warn(&pdev->dev, "DT is too old, TX FC
> disabled\n");
> > +			return 0;
> > +		}
> > +		priv->cm3_base = (void __iomem *)gen_pool_alloc(priv-
> >sram_pool,
> > +
> 	MSS_SRAM_SIZE);
> > +		if (!priv->cm3_base)
> > +			return -ENOMEM;
> 
> Should there be -EPROBE_DEFER handling in here somewhere? The SRAM is a
> device, so it might not of been probed yet?

No, firmware probed during bootloader boot and we can use SRAM. SRAM memory can be safely used. 

Regards.
Andrew Lunn Jan. 10, 2021, 5:43 p.m. UTC | #3 
> > Should there be -EPROBE_DEFER handling in here somewhere? The SRAM is a
> > device, so it might not of been probed yet?
> 

> No, firmware probed during bootloader boot and we can use SRAM. SRAM
> memory can be safely used.

A previous patch added:

+               CP11X_LABEL(cm3_sram): cm3@220000 {
+                       compatible = "mmio-sram";
+                       reg = <0x220000 0x800>;
+                       #address-cells = <1>;
+                       #size-cells = <1>;
+                       ranges = <0 0x220000 0x800>;
+               };
+

So it looks like the SRAM is a device, in the linux driver model. And
there is a driver for this, driver/misc/sram.c. How do you know this
device has been probed before the Ethernet driver?

       Andrew
Stefan Chulski Jan. 10, 2021, 5:49 p.m. UTC | #4 
> 
> > > Should there be -EPROBE_DEFER handling in here somewhere? The SRAM
> > > is a device, so it might not of been probed yet?
> >
> 
> > No, firmware probed during bootloader boot and we can use SRAM. SRAM
> > memory can be safely used.
> 
> A previous patch added:
> 
> +               CP11X_LABEL(cm3_sram): cm3@220000 {
> +                       compatible = "mmio-sram";
> +                       reg = <0x220000 0x800>;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges = <0 0x220000 0x800>;
> +               };
> +
> 
> So it looks like the SRAM is a device, in the linux driver model. And there is a
> driver for this, driver/misc/sram.c. How do you know this device has been
> probed before the Ethernet driver?
> 
>        Andrew

You right, I would add EPROBE_DEFER if of_gen_pool_get return NULL.

Thanks.
Russell King (Oracle) Jan. 10, 2021, 5:55 p.m. UTC | #5 
On Sun, Jan 10, 2021 at 05:30:07PM +0200, stefanc@marvell.com wrote:
> +	} else {
> +		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> +		if (!priv->sram_pool) {
> +			dev_warn(&pdev->dev, "DT is too old, TX FC disabled\n");

I don't see anything in this patch that disables TX flow control, which
means this warning message is misleading.
Stefan Chulski Jan. 10, 2021, 5:57 p.m. UTC | #6 
> > +	} else {
> > +		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > +		if (!priv->sram_pool) {
> > +			dev_warn(&pdev->dev, "DT is too old, TX FC
> disabled\n");
> 
> I don't see anything in this patch that disables TX flow control, which means
> this warning message is misleading.

OK, I would change to TX FC not supported.

Stefan.
Andrew Lunn Jan. 10, 2021, 6:03 p.m. UTC | #7 
On Sun, Jan 10, 2021 at 05:57:14PM +0000, Stefan Chulski wrote:
> > > +	} else {
> > > +		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > > +		if (!priv->sram_pool) {
> > > +			dev_warn(&pdev->dev, "DT is too old, TX FC
> > disabled\n");
> > 
> > I don't see anything in this patch that disables TX flow control, which means
> > this warning message is misleading.
> 
> OK, I would change to TX FC not supported.

And you should tell phlylink, so it knows to disable it in autoneg.

Which make me wonder, do we need a fix for stable? Has flow control
never been support in this device up until these patches get merged?
It should not be negotiated if it is not supported, which means
telling phylink.

   Andrew
Stefan Chulski Jan. 10, 2021, 6:09 p.m. UTC | #8 
> > > > +	} else {
> > > > +		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > > > +		if (!priv->sram_pool) {
> > > > +			dev_warn(&pdev->dev, "DT is too old, TX FC
> > > disabled\n");
> > >
> > > I don't see anything in this patch that disables TX flow control,
> > > which means this warning message is misleading.
> >
> > OK, I would change to TX FC not supported.
> 
> And you should tell phlylink, so it knows to disable it in autoneg.
> 
> Which make me wonder, do we need a fix for stable? Has flow control never
> been support in this device up until these patches get merged?
> It should not be negotiated if it is not supported, which means telling phylink.
> 
>    Andrew

TX FC never were really supported. MAC or PHY can negotiated flow control.
But MAC would never trigger FC frame.

Should I prepare separate patch that disable TX FC till we merge this patches?

Regards,
Stefan.
Russell King (Oracle) Jan. 10, 2021, 6:27 p.m. UTC | #9 
On Sun, Jan 10, 2021 at 06:09:39PM +0000, Stefan Chulski wrote:
> > > > > +	} else {
> > > > > +		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > > > > +		if (!priv->sram_pool) {
> > > > > +			dev_warn(&pdev->dev, "DT is too old, TX FC
> > > > disabled\n");
> > > >
> > > > I don't see anything in this patch that disables TX flow control,
> > > > which means this warning message is misleading.
> > >
> > > OK, I would change to TX FC not supported.
> > 
> > And you should tell phlylink, so it knows to disable it in autoneg.
> > 
> > Which make me wonder, do we need a fix for stable? Has flow control never
> > been support in this device up until these patches get merged?
> > It should not be negotiated if it is not supported, which means telling phylink.
> > 
> >    Andrew
> 
> TX FC never were really supported. MAC or PHY can negotiated flow control.
> But MAC would never trigger FC frame.

That really sucks.

> Should I prepare separate patch that disable TX FC till we merge this patches?

From what I see in table 28B in 802.3, there is no way to advertise
that you only support RX flow control. If you advertise ASM_DIR=1
PAUSE=0, it basically means you support sending FC frames, but not
receiving them. Advertising anything with PAUSE=1 means you support
both sending and receiving FC frames, irrespective of the state of
ASM_DIR.

So, our only option would be to completely disable pause frames.
Yes, I think we need a separate patch for that for the net tree,
and it should be backported to stable kernels, IMHO.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 6bd7e40..aec9179 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -748,6 +748,9 @@ 
 #define MVPP2_TX_FIFO_THRESHOLD(kb)	\
 		((kb) * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
 
+/* MSS Flow control */
+#define MSS_SRAM_SIZE	0x800
+
 /* RX buffer constants */
 #define MVPP2_SKB_SHINFO_SIZE \
 	SKB_DATA_ALIGN(sizeof(struct skb_shared_info))
@@ -925,6 +928,7 @@  struct mvpp2 {
 	/* Shared registers' base addresses */
 	void __iomem *lms_base;
 	void __iomem *iface_base;
+	void __iomem *cm3_base;
 
 	/* On PPv2.2, each "software thread" can access the base
 	 * register through a separate address space, each 64 KB apart
@@ -996,6 +1000,9 @@  struct mvpp2 {
 
 	/* page_pool allocator */
 	struct page_pool *page_pool[MVPP2_PORT_MAX_RXQ];
+
+	/* CM3 SRAM pool */
+	struct gen_pool *sram_pool;
 };
 
 struct mvpp2_pcpu_stats {
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 3982956..5267746 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -25,6 +25,7 @@ 
 #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);
+}
+
 static struct page_pool *
 mvpp2_create_page_pool(struct device *dev, int num, int len,
 		       enum dma_data_direction dma_dir)
@@ -6848,6 +6861,35 @@  static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	return 0;
 }
 
+static int mvpp2_get_sram(struct platform_device *pdev,
+			  struct mvpp2 *priv)
+{
+	struct device_node *dn = pdev->dev.of_node;
+	struct resource *res;
+
+	if (has_acpi_companion(&pdev->dev)) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+		if (!res) {
+			dev_warn(&pdev->dev, "ACPI is too old, TX FC disabled\n");
+			return 0;
+		}
+		priv->cm3_base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->cm3_base))
+			return PTR_ERR(priv->cm3_base);
+	} else {
+		priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
+		if (!priv->sram_pool) {
+			dev_warn(&pdev->dev, "DT is too old, TX FC disabled\n");
+			return 0;
+		}
+		priv->cm3_base = (void __iomem *)gen_pool_alloc(priv->sram_pool,
+								MSS_SRAM_SIZE);
+		if (!priv->cm3_base)
+			return -ENOMEM;
+	}
+	return 0;
+}
+
 static int mvpp2_probe(struct platform_device *pdev)
 {
 	const struct acpi_device_id *acpi_id;
@@ -6904,6 +6946,11 @@  static int mvpp2_probe(struct platform_device *pdev)
 		priv->iface_base = devm_ioremap_resource(&pdev->dev, res);
 		if (IS_ERR(priv->iface_base))
 			return PTR_ERR(priv->iface_base);
+
+		/* Map CM3 SRAM */
+		err = mvpp2_get_sram(pdev, priv);
+		if (err)
+			dev_warn(&pdev->dev, "Fail to alloc CM3 SRAM\n");
 	}
 
 	if (priv->hw_version == MVPP22 && dev_of_node(&pdev->dev)) {
@@ -6949,11 +6996,13 @@  static int mvpp2_probe(struct platform_device *pdev)
 
 	if (dev_of_node(&pdev->dev)) {
 		priv->pp_clk = devm_clk_get(&pdev->dev, "pp_clk");
-		if (IS_ERR(priv->pp_clk))
-			return PTR_ERR(priv->pp_clk);
+		if (IS_ERR(priv->pp_clk)) {
+			err = PTR_ERR(priv->pp_clk);
+			goto err_cm3;
+		}
 		err = clk_prepare_enable(priv->pp_clk);
 		if (err < 0)
-			return err;
+			goto err_cm3;
 
 		priv->gop_clk = devm_clk_get(&pdev->dev, "gop_clk");
 		if (IS_ERR(priv->gop_clk)) {
@@ -7089,6 +7138,11 @@  static int mvpp2_probe(struct platform_device *pdev)
 	clk_disable_unprepare(priv->gop_clk);
 err_pp_clk:
 	clk_disable_unprepare(priv->pp_clk);
+err_cm3:
+	if (!has_acpi_companion(&pdev->dev) && priv->cm3_base)
+		gen_pool_free(priv->sram_pool, (unsigned long)priv->cm3_base,
+			      MSS_SRAM_SIZE);
+
 	return err;
 }
 
@@ -7129,6 +7183,12 @@  static int mvpp2_remove(struct platform_device *pdev)
 				  aggr_txq->descs_dma);
 	}
 
+	if (!has_acpi_companion(&pdev->dev) && priv->cm3_base) {
+		gen_pool_free(priv->sram_pool, (unsigned long)priv->cm3_base,
+			      MSS_SRAM_SIZE);
+		gen_pool_destroy(priv->sram_pool);
+	}
+
 	if (is_acpi_node(port_fwnode))
 		return 0;