Re: [RESEND PATCH v8 net-next 03/15] net: mvpp2: add CM3 SRAM memory map
From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-02-07 16:42:32
Also in:
linux-arm-kernel, linux-devicetree, lkml
On Sun, Feb 07, 2021 at 10:19:12AM +0200, stefanc@marvell.com wrote:
quoted hunk ↗ jump to hunk
From: Stefan Chulski <redacted> This patch adds CM3 memory map and CM3 read/write callbacks. No functionality changes. Signed-off-by: Stefan Chulski <redacted> --- drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 7 +++ drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 63 +++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-)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 a07cf60..307f9fd 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>@@ -6846,6 +6847,44 @@ 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; + static bool defer_once; + 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, Flow control not supported\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) { + 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; + } + /* cm3_base allocated with offset zero into the SRAM since mapping size + * is equal to requested size. + */ + priv->cm3_base = (void __iomem *)gen_pool_alloc(priv->sram_pool, + MSS_SRAM_SIZE); + if (!priv->cm3_base) + return -ENOMEM; + }
For v2 i asked:
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.
There was never an answer to this.
Also, the defer_once stuff is odd. You don't see any other driver do
this. The core decides when to give up probing a device. This is
partially an API problem. of_gen_pool_get() gives you no idea why it
failed. Is the property missing, or has the SRAM not probed yet. If
the answer to my question is yes, a pool does make sense, it would be
good to add an of_gen_pool_get_optional() which returns
ERR_PTR(-EPROBE_DEFER) if the property is in DT, but is not yet
available, NULL if the properties does not exist, and a pointer if
everything goes well.
Andrew