devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] DMA40 SRAM refactoring and cleanup
@ 2023-04-17  7:55 Linus Walleij
  2023-04-17  7:55 ` [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle Linus Walleij
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

I started out by augmenting the STE DMA40 driver to get
its LCPA SRAM memory from a proper SRAM handle in the
device tree instead of as a reg cell, and then I saw
that the driver was in a bit of sad state so I did a bit
of cleanups on top.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Linus Walleij (7):
      dt-bindings: dma: dma40: Prefer to pass sram through phandle
      dmaengine: ste_dma40: Get LCPA SRAM from SRAM node
      dmaengine: ste_dma40: Add dev helper variable
      dmaengine: ste_dma40: Remove platform data
      dmaengine: ste_dma40: Pass dev to OF function
      dmaengine: ste_dma40: Use managed resources
      dmaengine: ste_dma40: Return error codes properly

 .../devicetree/bindings/dma/stericsson,dma40.yaml  |  35 ++-
 drivers/dma/Kconfig                                |   1 +
 drivers/dma/ste_dma40.c                            | 336 +++++++++------------
 .../dma-ste-dma40.h => drivers/dma/ste_dma40.h     | 101 +------
 drivers/dma/ste_dma40_ll.c                         |   3 +-
 5 files changed, 182 insertions(+), 294 deletions(-)
---
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
change-id: 20230417-ux500-dma40-cleanup-fe4f8d9b19c5

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle
  2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
@ 2023-04-17  7:55 ` Linus Walleij
  2023-04-18 16:22   ` Krzysztof Kozlowski
  2023-04-18 22:41   ` Rob Herring
  2023-04-17  7:55 ` [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node Linus Walleij
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

Extend the DMA40 bindings so that we can pass two SRAM
segments as phandles instead of directly referring to the
memory address in the second reg cell. This enables more
granular control over the SRAM, and adds the optiona LCLA
SRAM segment as well.

Deprecate the old way of passing LCPA as a second reg cell,
make sram compulsory.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/dma/stericsson,dma40.yaml  | 35 +++++++++++++++++-----
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml b/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
index 64845347f44d..4fe0df937171 100644
--- a/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
+++ b/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
@@ -112,14 +112,23 @@ properties:
       - const: stericsson,dma40
 
   reg:
-    items:
-      - description: DMA40 memory base
-      - description: LCPA memory base
+    oneOf:
+      - items:
+          - description: DMA40 memory base
+      - items:
+          - description: DMA40 memory base
+          - description: LCPA memory base, deprecated, use eSRAM pool instead
+        deprecated: true
+
 
   reg-names:
-    items:
-      - const: base
-      - const: lcpa
+    oneOf:
+      - items:
+          - const: base
+      - items:
+          - const: base
+          - const: lcpa
+        deprecated: true
 
   interrupts:
     maxItems: 1
@@ -127,6 +136,14 @@ properties:
   clocks:
     maxItems: 1
 
+  sram:
+    $ref: '/schemas/types.yaml#/definitions/phandle-array'
+    items:
+      maxItems: 2
+    description:
+      List of phandles for the SRAM used by the DMA40 block, the first
+      phandle is the LCPA memory, the second is the LCLA memory.
+
   memcpy-channels:
     $ref: /schemas/types.yaml#/definitions/uint32-array
     description: Array of u32 elements indicating which channels on the DMA
@@ -138,6 +155,7 @@ required:
   - reg
   - interrupts
   - clocks
+  - sram
   - memcpy-channels
 
 additionalProperties: false
@@ -149,8 +167,9 @@ examples:
     #include <dt-bindings/mfd/dbx500-prcmu.h>
     dma-controller@801c0000 {
         compatible = "stericsson,db8500-dma40", "stericsson,dma40";
-        reg = <0x801c0000 0x1000>, <0x40010000 0x800>;
-        reg-names = "base", "lcpa";
+        reg = <0x801c0000 0x1000>;
+        reg-names = "base";
+        sram = <&lcpa>, <&lcla>;
         interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
         #dma-cells = <3>;
         memcpy-channels = <56 57 58 59 60>;

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node
  2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
  2023-04-17  7:55 ` [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle Linus Walleij
@ 2023-04-17  7:55 ` Linus Walleij
  2023-04-17 19:54   ` kernel test robot
  2023-04-18  7:52   ` kernel test robot
  2023-04-17  7:55 ` [PATCH 3/7] dmaengine: ste_dma40: Add dev helper variable Linus Walleij
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

Instead of passing the reserved SRAM as a "reg" field
look for a phandle to the LCPA SRAM memory so we can
use the proper SRAM device tree bindings for the SRAM.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/dma/Kconfig     |  1 +
 drivers/dma/ste_dma40.c | 47 ++++++++++++++++++++++++-----------------------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index fb7073fc034f..37db3dc9a92a 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -553,6 +553,7 @@ config STE_DMA40
 	bool "ST-Ericsson DMA40 support"
 	depends on ARCH_U8500
 	select DMA_ENGINE
+	select SRAM
 	help
 	  Support for ST-Ericsson DMA40 controller
 
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index f093e08c23b1..236269d35a53 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -19,6 +19,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/err.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_dma.h>
 #include <linux/amba/bus.h>
 #include <linux/regulator/consumer.h>
@@ -3506,9 +3507,11 @@ static int __init d40_probe(struct platform_device *pdev)
 {
 	struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
 	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np_lcpa;
 	int ret = -ENOENT;
 	struct d40_base *base;
 	struct resource *res;
+	struct resource res_lcpa;
 	int num_reserved_chans;
 	u32 val;
 
@@ -3535,37 +3538,37 @@ static int __init d40_probe(struct platform_device *pdev)
 	spin_lock_init(&base->interrupt_lock);
 	spin_lock_init(&base->execmd_lock);
 
-	/* Get IO for logical channel parameter address */
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lcpa");
-	if (!res) {
-		ret = -ENOENT;
-		d40_err(&pdev->dev, "No \"lcpa\" memory resource\n");
-		goto destroy_cache;
+	/* Get IO for logical channel parameter address (LCPA) */
+	np_lcpa = of_parse_phandle(np, "sram", 0);
+	if (!np_lcpa) {
+		dev_err(&pdev->dev, "no LCPA SRAM node\n");
+		goto report_failure;
 	}
-	base->lcpa_size = resource_size(res);
-	base->phy_lcpa = res->start;
-
-	if (request_mem_region(res->start, resource_size(res),
-			       D40_NAME " I/O lcpa") == NULL) {
-		ret = -EBUSY;
-		d40_err(&pdev->dev, "Failed to request LCPA region %pR\n", res);
-		goto destroy_cache;
+	/* This is no device so read the address directly from the node */
+	ret = of_address_to_resource(np_lcpa, 0, &res_lcpa);
+	if (ret) {
+		dev_err(&pdev->dev, "no LCPA SRAM resource\n");
+		goto report_failure;
 	}
+	base->lcpa_size = resource_size(&res_lcpa);
+	base->phy_lcpa = res_lcpa.start;
+	dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
+		 base->phy_lcpa, base->lcpa_size);
 
 	/* We make use of ESRAM memory for this. */
 	val = readl(base->virtbase + D40_DREG_LCPA);
-	if (res->start != val && val != 0) {
+	if (base->phy_lcpa != val && val != 0) {
 		dev_warn(&pdev->dev,
-			 "[%s] Mismatch LCPA dma 0x%x, def %pa\n",
-			 __func__, val, &res->start);
+			 "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
+			 __func__, val, base->phy_lcpa);
 	} else
-		writel(res->start, base->virtbase + D40_DREG_LCPA);
+		writel(base->phy_lcpa, base->virtbase + D40_DREG_LCPA);
 
-	base->lcpa_base = ioremap(res->start, resource_size(res));
+	base->lcpa_base = ioremap(base->phy_lcpa, base->lcpa_size);
 	if (!base->lcpa_base) {
 		ret = -ENOMEM;
 		d40_err(&pdev->dev, "Failed to ioremap LCPA region\n");
-		goto destroy_cache;
+		goto release_base;
 	}
 	/* If lcla has to be located in ESRAM we don't need to allocate */
 	if (base->plat_data->use_esram_lcla) {
@@ -3678,9 +3681,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	if (base->lcpa_base)
 		iounmap(base->lcpa_base);
 
-	if (base->phy_lcpa)
-		release_mem_region(base->phy_lcpa,
-				   base->lcpa_size);
+release_base:
 	if (base->phy_start)
 		release_mem_region(base->phy_start,
 				   base->phy_size);

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/7] dmaengine: ste_dma40: Add dev helper variable
  2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
  2023-04-17  7:55 ` [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle Linus Walleij
  2023-04-17  7:55 ` [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node Linus Walleij
@ 2023-04-17  7:55 ` Linus Walleij
  2023-04-17  7:55 ` [PATCH 4/7] dmaengine: ste_dma40: Remove platform data Linus Walleij
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

The &pdev->dev device pointer is used so many times in the
probe() and d40_hw_detect_init() functions that a local *dev
variable makes the code way easier to read.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/dma/ste_dma40.c | 50 +++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 236269d35a53..ef2a2fdaa82e 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3104,6 +3104,7 @@ static int __init d40_phy_res_init(struct d40_base *base)
 static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 {
 	struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
+	struct device *dev = &pdev->dev;
 	struct clk *clk;
 	void __iomem *virtbase;
 	struct resource *res;
@@ -3117,15 +3118,15 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	u32 cid;
 	u8 rev;
 
-	clk = clk_get(&pdev->dev, NULL);
+	clk = clk_get(dev, NULL);
 	if (IS_ERR(clk)) {
-		d40_err(&pdev->dev, "No matching clock found\n");
+		d40_err(dev, "No matching clock found\n");
 		goto check_prepare_enabled;
 	}
 
 	clk_ret = clk_prepare_enable(clk);
 	if (clk_ret) {
-		d40_err(&pdev->dev, "Failed to prepare/enable clock\n");
+		d40_err(dev, "Failed to prepare/enable clock\n");
 		goto disable_unprepare;
 	}
 
@@ -3151,11 +3152,11 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 			& 255) << (i * 8);
 
 	if (cid != AMBA_CID) {
-		d40_err(&pdev->dev, "Unknown hardware! No PrimeCell ID\n");
+		d40_err(dev, "Unknown hardware! No PrimeCell ID\n");
 		goto unmap_io;
 	}
 	if (AMBA_MANF_BITS(pid) != AMBA_VENDOR_ST) {
-		d40_err(&pdev->dev, "Unknown designer! Got %x wanted %x\n",
+		d40_err(dev, "Unknown designer! Got %x wanted %x\n",
 			AMBA_MANF_BITS(pid),
 			AMBA_VENDOR_ST);
 		goto unmap_io;
@@ -3171,7 +3172,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	 */
 	rev = AMBA_REV_BITS(pid);
 	if (rev < 2) {
-		d40_err(&pdev->dev, "hardware revision: %d is not supported", rev);
+		d40_err(dev, "hardware revision: %d is not supported", rev);
 		goto unmap_io;
 	}
 
@@ -3189,7 +3190,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 
 	num_log_chans = num_phy_chans * D40_MAX_LOG_CHAN_PER_PHY;
 
-	dev_info(&pdev->dev,
+	dev_info(dev,
 		 "hardware rev: %d @ %pa with %d physical and %d logical channels\n",
 		 rev, &res->start, num_phy_chans, num_log_chans);
 
@@ -3209,7 +3210,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	base->phy_size = resource_size(res);
 	base->virtbase = virtbase;
 	base->plat_data = plat_data;
-	base->dev = &pdev->dev;
+	base->dev = dev;
 	base->phy_chans = ((void *)base) + ALIGN(sizeof(struct d40_base), 4);
 	base->log_chans = &base->phy_chans[num_phy_chans];
 
@@ -3505,7 +3506,8 @@ static int __init d40_of_probe(struct platform_device *pdev,
 
 static int __init d40_probe(struct platform_device *pdev)
 {
-	struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
+	struct device *dev = &pdev->dev;
+	struct stedma40_platform_data *plat_data = dev_get_platdata(dev);
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *np_lcpa;
 	int ret = -ENOENT;
@@ -3522,7 +3524,7 @@ static int __init d40_probe(struct platform_device *pdev)
 				goto report_failure;
 			}
 		} else {
-			d40_err(&pdev->dev, "No pdata or Device Tree provided\n");
+			d40_err(dev, "No pdata or Device Tree provided\n");
 			goto report_failure;
 		}
 	}
@@ -3541,24 +3543,24 @@ static int __init d40_probe(struct platform_device *pdev)
 	/* Get IO for logical channel parameter address (LCPA) */
 	np_lcpa = of_parse_phandle(np, "sram", 0);
 	if (!np_lcpa) {
-		dev_err(&pdev->dev, "no LCPA SRAM node\n");
+		dev_err(dev, "no LCPA SRAM node\n");
 		goto report_failure;
 	}
 	/* This is no device so read the address directly from the node */
 	ret = of_address_to_resource(np_lcpa, 0, &res_lcpa);
 	if (ret) {
-		dev_err(&pdev->dev, "no LCPA SRAM resource\n");
+		dev_err(dev, "no LCPA SRAM resource\n");
 		goto report_failure;
 	}
 	base->lcpa_size = resource_size(&res_lcpa);
 	base->phy_lcpa = res_lcpa.start;
-	dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
+	dev_info(dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
 		 base->phy_lcpa, base->lcpa_size);
 
 	/* We make use of ESRAM memory for this. */
 	val = readl(base->virtbase + D40_DREG_LCPA);
 	if (base->phy_lcpa != val && val != 0) {
-		dev_warn(&pdev->dev,
+		dev_warn(dev,
 			 "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
 			 __func__, val, base->phy_lcpa);
 	} else
@@ -3567,7 +3569,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	base->lcpa_base = ioremap(base->phy_lcpa, base->lcpa_size);
 	if (!base->lcpa_base) {
 		ret = -ENOMEM;
-		d40_err(&pdev->dev, "Failed to ioremap LCPA region\n");
+		d40_err(dev, "Failed to ioremap LCPA region\n");
 		goto release_base;
 	}
 	/* If lcla has to be located in ESRAM we don't need to allocate */
@@ -3576,7 +3578,7 @@ static int __init d40_probe(struct platform_device *pdev)
 							"lcla_esram");
 		if (!res) {
 			ret = -ENOENT;
-			d40_err(&pdev->dev,
+			d40_err(dev,
 				"No \"lcla_esram\" memory resource\n");
 			goto destroy_cache;
 		}
@@ -3584,7 +3586,7 @@ static int __init d40_probe(struct platform_device *pdev)
 						resource_size(res));
 		if (!base->lcla_pool.base) {
 			ret = -ENOMEM;
-			d40_err(&pdev->dev, "Failed to ioremap LCLA region\n");
+			d40_err(dev, "Failed to ioremap LCLA region\n");
 			goto destroy_cache;
 		}
 		writel(res->start, base->virtbase + D40_DREG_LCLA);
@@ -3592,7 +3594,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	} else {
 		ret = d40_lcla_allocate(base);
 		if (ret) {
-			d40_err(&pdev->dev, "Failed to allocate LCLA area\n");
+			d40_err(dev, "Failed to allocate LCLA area\n");
 			goto destroy_cache;
 		}
 	}
@@ -3603,7 +3605,7 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	ret = request_irq(base->irq, d40_handle_interrupt, 0, D40_NAME, base);
 	if (ret) {
-		d40_err(&pdev->dev, "No IRQ defined\n");
+		d40_err(dev, "No IRQ defined\n");
 		goto destroy_cache;
 	}
 
@@ -3611,7 +3613,7 @@ static int __init d40_probe(struct platform_device *pdev)
 
 		base->lcpa_regulator = regulator_get(base->dev, "lcla_esram");
 		if (IS_ERR(base->lcpa_regulator)) {
-			d40_err(&pdev->dev, "Failed to get lcpa_regulator\n");
+			d40_err(dev, "Failed to get lcpa_regulator\n");
 			ret = PTR_ERR(base->lcpa_regulator);
 			base->lcpa_regulator = NULL;
 			goto destroy_cache;
@@ -3619,7 +3621,7 @@ static int __init d40_probe(struct platform_device *pdev)
 
 		ret = regulator_enable(base->lcpa_regulator);
 		if (ret) {
-			d40_err(&pdev->dev,
+			d40_err(dev,
 				"Failed to enable lcpa_regulator\n");
 			regulator_put(base->lcpa_regulator);
 			base->lcpa_regulator = NULL;
@@ -3642,7 +3644,7 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	ret = dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
 	if (ret) {
-		d40_err(&pdev->dev, "Failed to set dma max seg size\n");
+		d40_err(dev, "Failed to set dma max seg size\n");
 		goto destroy_cache;
 	}
 
@@ -3651,7 +3653,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	if (np) {
 		ret = of_dma_controller_register(np, d40_xlate, NULL);
 		if (ret)
-			dev_err(&pdev->dev,
+			dev_err(dev,
 				"could not register of_dma_controller\n");
 	}
 
@@ -3701,7 +3703,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	kfree(base->phy_res);
 	kfree(base);
  report_failure:
-	d40_err(&pdev->dev, "probe failed\n");
+	d40_err(dev, "probe failed\n");
 	return ret;
 }
 

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/7] dmaengine: ste_dma40: Remove platform data
  2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
                   ` (2 preceding siblings ...)
  2023-04-17  7:55 ` [PATCH 3/7] dmaengine: ste_dma40: Add dev helper variable Linus Walleij
@ 2023-04-17  7:55 ` Linus Walleij
  2023-04-17  7:55 ` [PATCH 5/7] dmaengine: ste_dma40: Pass dev to OF function Linus Walleij
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

The Ux500 is device tree-only since ages. Delete the
platform data header and push it into or next to the driver
instead.

Drop the non-DT probe path since this will not happen.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/dma/ste_dma40.c                            |  56 ++++++++----
 .../dma-ste-dma40.h => drivers/dma/ste_dma40.h     | 101 +--------------------
 drivers/dma/ste_dma40_ll.c                         |   3 +-
 3 files changed, 41 insertions(+), 119 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index ef2a2fdaa82e..e5df28cdc4c8 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -23,11 +23,39 @@
 #include <linux/of_dma.h>
 #include <linux/amba/bus.h>
 #include <linux/regulator/consumer.h>
-#include <linux/platform_data/dma-ste-dma40.h>
 
 #include "dmaengine.h"
+#include "ste_dma40.h"
 #include "ste_dma40_ll.h"
 
+/**
+ * struct stedma40_platform_data - Configuration struct for the dma device.
+ *
+ * @dev_tx: mapping between destination event line and io address
+ * @dev_rx: mapping between source event line and io address
+ * @disabled_channels: A vector, ending with -1, that marks physical channels
+ * that are for different reasons not available for the driver.
+ * @soft_lli_chans: A vector, that marks physical channels will use LLI by SW
+ * which avoids HW bug that exists in some versions of the controller.
+ * SoftLLI introduces relink overhead that could impact performace for
+ * certain use cases.
+ * @num_of_soft_lli_chans: The number of channels that needs to be configured
+ * to use SoftLLI.
+ * @use_esram_lcla: flag for mapping the lcla into esram region
+ * @num_of_memcpy_chans: The number of channels reserved for memcpy.
+ * @num_of_phy_chans: The number of physical channels implemented in HW.
+ * 0 means reading the number of channels from DMA HW but this is only valid
+ * for 'multiple of 4' channels, like 8.
+ */
+struct stedma40_platform_data {
+	int				 disabled_channels[STEDMA40_MAX_PHYS];
+	int				*soft_lli_chans;
+	int				 num_of_soft_lli_chans;
+	bool				 use_esram_lcla;
+	int				 num_of_memcpy_chans;
+	int				 num_of_phy_chans;
+};
+
 #define D40_NAME "dma40"
 
 #define D40_PHY_CHAN -1
@@ -2269,7 +2297,7 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
 	return NULL;
 }
 
-bool stedma40_filter(struct dma_chan *chan, void *data)
+static bool stedma40_filter(struct dma_chan *chan, void *data)
 {
 	struct stedma40_chan_cfg *info = data;
 	struct d40_chan *d40c =
@@ -2288,7 +2316,6 @@ bool stedma40_filter(struct dma_chan *chan, void *data)
 
 	return err == 0;
 }
-EXPORT_SYMBOL(stedma40_filter);
 
 static void __d40_set_prio_rt(struct d40_chan *d40c, int dev_type, bool src)
 {
@@ -3517,16 +3544,9 @@ static int __init d40_probe(struct platform_device *pdev)
 	int num_reserved_chans;
 	u32 val;
 
-	if (!plat_data) {
-		if (np) {
-			if (d40_of_probe(pdev, np)) {
-				ret = -ENOMEM;
-				goto report_failure;
-			}
-		} else {
-			d40_err(dev, "No pdata or Device Tree provided\n");
-			goto report_failure;
-		}
+	if (d40_of_probe(pdev, np)) {
+		ret = -ENOMEM;
+		goto report_failure;
 	}
 
 	base = d40_hw_detect_init(pdev);
@@ -3650,11 +3670,11 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	d40_hw_init(base);
 
-	if (np) {
-		ret = of_dma_controller_register(np, d40_xlate, NULL);
-		if (ret)
-			dev_err(dev,
-				"could not register of_dma_controller\n");
+	ret = of_dma_controller_register(np, d40_xlate, NULL);
+	if (ret) {
+		dev_err(dev,
+			"could not register of_dma_controller\n");
+		goto destroy_cache;
 	}
 
 	dev_info(base->dev, "initialized\n");
diff --git a/include/linux/platform_data/dma-ste-dma40.h b/drivers/dma/ste_dma40.h
similarity index 51%
rename from include/linux/platform_data/dma-ste-dma40.h
rename to drivers/dma/ste_dma40.h
index 10641633facc..c697bfe16a01 100644
--- a/include/linux/platform_data/dma-ste-dma40.h
+++ b/drivers/dma/ste_dma40.h
@@ -1,19 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) ST-Ericsson SA 2007-2010
- * Author: Per Forlin <per.forlin@stericsson.com> for ST-Ericsson
- * Author: Jonas Aaberg <jonas.aberg@stericsson.com> for ST-Ericsson
- */
-
 
 #ifndef STE_DMA40_H
 #define STE_DMA40_H
 
-#include <linux/dmaengine.h>
-#include <linux/scatterlist.h>
-#include <linux/workqueue.h>
-#include <linux/interrupt.h>
-
 /*
  * Maxium size for a single dma descriptor
  * Size is limited to 16 bits.
@@ -118,92 +107,4 @@ struct stedma40_chan_cfg {
 	int					 phy_channel;
 };
 
-/**
- * struct stedma40_platform_data - Configuration struct for the dma device.
- *
- * @dev_tx: mapping between destination event line and io address
- * @dev_rx: mapping between source event line and io address
- * @disabled_channels: A vector, ending with -1, that marks physical channels
- * that are for different reasons not available for the driver.
- * @soft_lli_chans: A vector, that marks physical channels will use LLI by SW
- * which avoids HW bug that exists in some versions of the controller.
- * SoftLLI introduces relink overhead that could impact performace for
- * certain use cases.
- * @num_of_soft_lli_chans: The number of channels that needs to be configured
- * to use SoftLLI.
- * @use_esram_lcla: flag for mapping the lcla into esram region
- * @num_of_memcpy_chans: The number of channels reserved for memcpy.
- * @num_of_phy_chans: The number of physical channels implemented in HW.
- * 0 means reading the number of channels from DMA HW but this is only valid
- * for 'multiple of 4' channels, like 8.
- */
-struct stedma40_platform_data {
-	int				 disabled_channels[STEDMA40_MAX_PHYS];
-	int				*soft_lli_chans;
-	int				 num_of_soft_lli_chans;
-	bool				 use_esram_lcla;
-	int				 num_of_memcpy_chans;
-	int				 num_of_phy_chans;
-};
-
-#ifdef CONFIG_STE_DMA40
-
-/**
- * stedma40_filter() - Provides stedma40_chan_cfg to the
- * ste_dma40 dma driver via the dmaengine framework.
- * does some checking of what's provided.
- *
- * Never directly called by client. It used by dmaengine.
- * @chan: dmaengine handle.
- * @data: Must be of type: struct stedma40_chan_cfg and is
- * the configuration of the framework.
- *
- *
- */
-
-bool stedma40_filter(struct dma_chan *chan, void *data);
-
-/**
- * stedma40_slave_mem() - Transfers a raw data buffer to or from a slave
- * (=device)
- *
- * @chan: dmaengine handle
- * @addr: source or destination physicall address.
- * @size: bytes to transfer
- * @direction: direction of transfer
- * @flags: is actually enum dma_ctrl_flags. See dmaengine.h
- */
-
-static inline struct
-dma_async_tx_descriptor *stedma40_slave_mem(struct dma_chan *chan,
-					    dma_addr_t addr,
-					    unsigned int size,
-					    enum dma_transfer_direction direction,
-					    unsigned long flags)
-{
-	struct scatterlist sg;
-	sg_init_table(&sg, 1);
-	sg.dma_address = addr;
-	sg.length = size;
-
-	return dmaengine_prep_slave_sg(chan, &sg, 1, direction, flags);
-}
-
-#else
-static inline bool stedma40_filter(struct dma_chan *chan, void *data)
-{
-	return false;
-}
-
-static inline struct
-dma_async_tx_descriptor *stedma40_slave_mem(struct dma_chan *chan,
-					    dma_addr_t addr,
-					    unsigned int size,
-					    enum dma_transfer_direction direction,
-					    unsigned long flags)
-{
-	return NULL;
-}
-#endif
-
-#endif
+#endif /* STE_DMA40_H */
diff --git a/drivers/dma/ste_dma40_ll.c b/drivers/dma/ste_dma40_ll.c
index b5287c661eb7..4c489b126cb2 100644
--- a/drivers/dma/ste_dma40_ll.c
+++ b/drivers/dma/ste_dma40_ll.c
@@ -6,8 +6,9 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/platform_data/dma-ste-dma40.h>
+#include <linux/dmaengine.h>
 
+#include "ste_dma40.h"
 #include "ste_dma40_ll.h"
 
 static u8 d40_width_to_bits(enum dma_slave_buswidth width)

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/7] dmaengine: ste_dma40: Pass dev to OF function
  2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
                   ` (3 preceding siblings ...)
  2023-04-17  7:55 ` [PATCH 4/7] dmaengine: ste_dma40: Remove platform data Linus Walleij
@ 2023-04-17  7:55 ` Linus Walleij
  2023-04-17  7:55 ` [PATCH 6/7] dmaengine: ste_dma40: Use managed resources Linus Walleij
  2023-04-17  7:55 ` [PATCH 7/7] dmaengine: ste_dma40: Return error codes properly Linus Walleij
  6 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

The OF platform data population function only wants to
use struct device *dev, so pass that instead.

This change makes the compiler realize that the local
platform data variable is unused, so drop that too.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/dma/ste_dma40.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index e5df28cdc4c8..fe98f12b8130 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3480,14 +3480,14 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 	return ret;
 }
 
-static int __init d40_of_probe(struct platform_device *pdev,
+static int __init d40_of_probe(struct device *dev,
 			       struct device_node *np)
 {
 	struct stedma40_platform_data *pdata;
 	int num_phy = 0, num_memcpy = 0, num_disabled = 0;
 	const __be32 *list;
 
-	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
 
@@ -3500,7 +3500,7 @@ static int __init d40_of_probe(struct platform_device *pdev,
 	num_memcpy /= sizeof(*list);
 
 	if (num_memcpy > D40_MEMCPY_MAX_CHANS || num_memcpy <= 0) {
-		d40_err(&pdev->dev,
+		d40_err(dev,
 			"Invalid number of memcpy channels specified (%d)\n",
 			num_memcpy);
 		return -EINVAL;
@@ -3515,7 +3515,7 @@ static int __init d40_of_probe(struct platform_device *pdev,
 	num_disabled /= sizeof(*list);
 
 	if (num_disabled >= STEDMA40_MAX_PHYS || num_disabled < 0) {
-		d40_err(&pdev->dev,
+		d40_err(dev,
 			"Invalid number of disabled channels specified (%d)\n",
 			num_disabled);
 		return -EINVAL;
@@ -3526,7 +3526,7 @@ static int __init d40_of_probe(struct platform_device *pdev,
 				   num_disabled);
 	pdata->disabled_channels[num_disabled] = -1;
 
-	pdev->dev.platform_data = pdata;
+	dev->platform_data = pdata;
 
 	return 0;
 }
@@ -3534,7 +3534,6 @@ static int __init d40_of_probe(struct platform_device *pdev,
 static int __init d40_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct stedma40_platform_data *plat_data = dev_get_platdata(dev);
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *np_lcpa;
 	int ret = -ENOENT;
@@ -3544,7 +3543,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	int num_reserved_chans;
 	u32 val;
 
-	if (d40_of_probe(pdev, np)) {
+	if (d40_of_probe(dev, np)) {
 		ret = -ENOMEM;
 		goto report_failure;
 	}

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/7] dmaengine: ste_dma40: Use managed resources
  2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
                   ` (4 preceding siblings ...)
  2023-04-17  7:55 ` [PATCH 5/7] dmaengine: ste_dma40: Pass dev to OF function Linus Walleij
@ 2023-04-17  7:55 ` Linus Walleij
  2023-04-17  7:55 ` [PATCH 7/7] dmaengine: ste_dma40: Return error codes properly Linus Walleij
  6 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

This switches the DMA40 driver to use a bunch of managed
resources and strip down the errorpath.

The result is pretty neat and makes the driver way more
readable.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/dma/ste_dma40.c | 180 ++++++++++++++++--------------------------------
 1 file changed, 61 insertions(+), 119 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index fe98f12b8130..c5991009d3e4 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -554,8 +554,6 @@ struct d40_gen_dmac {
  * @virtbase: The virtual base address of the DMA's register.
  * @rev: silicon revision detected.
  * @clk: Pointer to the DMA clock structure.
- * @phy_start: Physical memory start of the DMA registers.
- * @phy_size: Size of the DMA register map.
  * @irq: The IRQ number.
  * @num_memcpy_chans: The number of channels used for memcpy (mem-to-mem
  * transfers).
@@ -599,8 +597,6 @@ struct d40_base {
 	void __iomem			 *virtbase;
 	u8				  rev:4;
 	struct clk			 *clk;
-	phys_addr_t			  phy_start;
-	resource_size_t			  phy_size;
 	int				  irq;
 	int				  num_memcpy_chans;
 	int				  num_phy_chans;
@@ -3128,65 +3124,58 @@ static int __init d40_phy_res_init(struct d40_base *base)
 	return num_phy_chans_avail;
 }
 
+/* Called from the registered devm action */
+static void d40_drop_kmem_cache_action(void *d)
+{
+	struct kmem_cache *desc_slab = d;
+
+	kmem_cache_destroy(desc_slab);
+}
+
 static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 {
 	struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
 	struct device *dev = &pdev->dev;
 	struct clk *clk;
 	void __iomem *virtbase;
-	struct resource *res;
 	struct d40_base *base;
 	int num_log_chans;
 	int num_phy_chans;
 	int num_memcpy_chans;
-	int clk_ret = -EINVAL;
 	int i;
 	u32 pid;
 	u32 cid;
 	u8 rev;
+	int ret;
 
-	clk = clk_get(dev, NULL);
-	if (IS_ERR(clk)) {
-		d40_err(dev, "No matching clock found\n");
-		goto check_prepare_enabled;
-	}
-
-	clk_ret = clk_prepare_enable(clk);
-	if (clk_ret) {
-		d40_err(dev, "Failed to prepare/enable clock\n");
-		goto disable_unprepare;
-	}
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return NULL;
 
 	/* Get IO for DMAC base address */
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "base");
-	if (!res)
-		goto disable_unprepare;
-
-	if (request_mem_region(res->start, resource_size(res),
-			       D40_NAME " I/O base") == NULL)
-		goto release_region;
-
-	virtbase = ioremap(res->start, resource_size(res));
-	if (!virtbase)
-		goto release_region;
+	virtbase = devm_platform_ioremap_resource_byname(pdev, "base");
+	if (IS_ERR(virtbase)) {
+		dev_err(dev, "No IO base defined\n");
+		return NULL;
+	}
 
 	/* This is just a regular AMBA PrimeCell ID actually */
 	for (pid = 0, i = 0; i < 4; i++)
-		pid |= (readl(virtbase + resource_size(res) - 0x20 + 4 * i)
+		pid |= (readl(virtbase + SZ_4K - 0x20 + 4 * i)
 			& 255) << (i * 8);
 	for (cid = 0, i = 0; i < 4; i++)
-		cid |= (readl(virtbase + resource_size(res) - 0x10 + 4 * i)
+		cid |= (readl(virtbase + SZ_4K - 0x10 + 4 * i)
 			& 255) << (i * 8);
 
 	if (cid != AMBA_CID) {
 		d40_err(dev, "Unknown hardware! No PrimeCell ID\n");
-		goto unmap_io;
+		return NULL;
 	}
 	if (AMBA_MANF_BITS(pid) != AMBA_VENDOR_ST) {
 		d40_err(dev, "Unknown designer! Got %x wanted %x\n",
 			AMBA_MANF_BITS(pid),
 			AMBA_VENDOR_ST);
-		goto unmap_io;
+		return NULL;
 	}
 	/*
 	 * HW revision:
@@ -3200,7 +3189,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	rev = AMBA_REV_BITS(pid);
 	if (rev < 2) {
 		d40_err(dev, "hardware revision: %d is not supported", rev);
-		goto unmap_io;
+		return NULL;
 	}
 
 	/* The number of physical channels on this HW */
@@ -3218,23 +3207,22 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	num_log_chans = num_phy_chans * D40_MAX_LOG_CHAN_PER_PHY;
 
 	dev_info(dev,
-		 "hardware rev: %d @ %pa with %d physical and %d logical channels\n",
-		 rev, &res->start, num_phy_chans, num_log_chans);
+		 "hardware rev: %d with %d physical and %d logical channels\n",
+		 rev, num_phy_chans, num_log_chans);
 
-	base = kzalloc(ALIGN(sizeof(struct d40_base), 4) +
-		       (num_phy_chans + num_log_chans + num_memcpy_chans) *
-		       sizeof(struct d40_chan), GFP_KERNEL);
+	base = devm_kzalloc(dev,
+		ALIGN(sizeof(struct d40_base), 4) +
+		(num_phy_chans + num_log_chans + num_memcpy_chans) *
+		sizeof(struct d40_chan), GFP_KERNEL);
 
-	if (base == NULL)
-		goto unmap_io;
+	if (!base)
+		return NULL;
 
 	base->rev = rev;
 	base->clk = clk;
 	base->num_memcpy_chans = num_memcpy_chans;
 	base->num_phy_chans = num_phy_chans;
 	base->num_log_chans = num_log_chans;
-	base->phy_start = res->start;
-	base->phy_size = resource_size(res);
 	base->virtbase = virtbase;
 	base->plat_data = plat_data;
 	base->dev = dev;
@@ -3271,76 +3259,55 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 		base->gen_dmac.init_reg_size = ARRAY_SIZE(dma_init_reg_v4a);
 	}
 
-	base->phy_res = kcalloc(num_phy_chans,
-				sizeof(*base->phy_res),
-				GFP_KERNEL);
+	base->phy_res = devm_kcalloc(dev, num_phy_chans,
+				     sizeof(*base->phy_res),
+				     GFP_KERNEL);
 	if (!base->phy_res)
-		goto free_base;
+		return NULL;
 
-	base->lookup_phy_chans = kcalloc(num_phy_chans,
-					 sizeof(*base->lookup_phy_chans),
-					 GFP_KERNEL);
+	base->lookup_phy_chans = devm_kcalloc(dev, num_phy_chans,
+					      sizeof(*base->lookup_phy_chans),
+					      GFP_KERNEL);
 	if (!base->lookup_phy_chans)
-		goto free_phy_res;
+		return NULL;
 
-	base->lookup_log_chans = kcalloc(num_log_chans,
-					 sizeof(*base->lookup_log_chans),
-					 GFP_KERNEL);
+	base->lookup_log_chans = devm_kcalloc(dev, num_log_chans,
+					      sizeof(*base->lookup_log_chans),
+					      GFP_KERNEL);
 	if (!base->lookup_log_chans)
-		goto free_phy_chans;
+		return NULL;
 
-	base->reg_val_backup_chan = kmalloc_array(base->num_phy_chans,
+	base->reg_val_backup_chan = devm_kmalloc_array(dev, base->num_phy_chans,
 						  sizeof(d40_backup_regs_chan),
 						  GFP_KERNEL);
 	if (!base->reg_val_backup_chan)
-		goto free_log_chans;
+		return NULL;
 
-	base->lcla_pool.alloc_map = kcalloc(num_phy_chans
+	base->lcla_pool.alloc_map = devm_kcalloc(dev, num_phy_chans
 					    * D40_LCLA_LINK_PER_EVENT_GRP,
 					    sizeof(*base->lcla_pool.alloc_map),
 					    GFP_KERNEL);
 	if (!base->lcla_pool.alloc_map)
-		goto free_backup_chan;
+		return NULL;
 
-	base->regs_interrupt = kmalloc_array(base->gen_dmac.il_size,
+	base->regs_interrupt = devm_kmalloc_array(dev, base->gen_dmac.il_size,
 					     sizeof(*base->regs_interrupt),
 					     GFP_KERNEL);
 	if (!base->regs_interrupt)
-		goto free_map;
+		return NULL;
 
 	base->desc_slab = kmem_cache_create(D40_NAME, sizeof(struct d40_desc),
 					    0, SLAB_HWCACHE_ALIGN,
 					    NULL);
-	if (base->desc_slab == NULL)
-		goto free_regs;
+	if (!base->desc_slab)
+		return NULL;
 
+	ret = devm_add_action_or_reset(dev, d40_drop_kmem_cache_action,
+				       base->desc_slab);
+	if (ret)
+		return NULL;
 
 	return base;
- free_regs:
-	kfree(base->regs_interrupt);
- free_map:
-	kfree(base->lcla_pool.alloc_map);
- free_backup_chan:
-	kfree(base->reg_val_backup_chan);
- free_log_chans:
-	kfree(base->lookup_log_chans);
- free_phy_chans:
-	kfree(base->lookup_phy_chans);
- free_phy_res:
-	kfree(base->phy_res);
- free_base:
-	kfree(base);
- unmap_io:
-	iounmap(virtbase);
- release_region:
-	release_mem_region(res->start, resource_size(res));
- check_prepare_enabled:
-	if (!clk_ret)
- disable_unprepare:
-		clk_disable_unprepare(clk);
-	if (!IS_ERR(clk))
-		clk_put(clk);
-	return NULL;
 }
 
 static void __init d40_hw_init(struct d40_base *base)
@@ -3585,11 +3552,11 @@ static int __init d40_probe(struct platform_device *pdev)
 	} else
 		writel(base->phy_lcpa, base->virtbase + D40_DREG_LCPA);
 
-	base->lcpa_base = ioremap(base->phy_lcpa, base->lcpa_size);
+	base->lcpa_base = devm_ioremap(dev, base->phy_lcpa, base->lcpa_size);
 	if (!base->lcpa_base) {
 		ret = -ENOMEM;
 		d40_err(dev, "Failed to ioremap LCPA region\n");
-		goto release_base;
+		goto report_failure;
 	}
 	/* If lcla has to be located in ESRAM we don't need to allocate */
 	if (base->plat_data->use_esram_lcla) {
@@ -3599,14 +3566,14 @@ static int __init d40_probe(struct platform_device *pdev)
 			ret = -ENOENT;
 			d40_err(dev,
 				"No \"lcla_esram\" memory resource\n");
-			goto destroy_cache;
+			goto report_failure;
 		}
-		base->lcla_pool.base = ioremap(res->start,
-						resource_size(res));
+		base->lcla_pool.base = devm_ioremap(dev, res->start,
+						    resource_size(res));
 		if (!base->lcla_pool.base) {
 			ret = -ENOMEM;
 			d40_err(dev, "Failed to ioremap LCLA region\n");
-			goto destroy_cache;
+			goto report_failure;
 		}
 		writel(res->start, base->virtbase + D40_DREG_LCLA);
 
@@ -3678,16 +3645,8 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	dev_info(base->dev, "initialized\n");
 	return 0;
- destroy_cache:
-	kmem_cache_destroy(base->desc_slab);
-	if (base->virtbase)
-		iounmap(base->virtbase);
-
-	if (base->lcla_pool.base && base->plat_data->use_esram_lcla) {
-		iounmap(base->lcla_pool.base);
-		base->lcla_pool.base = NULL;
-	}
 
+ destroy_cache:
 	if (base->lcla_pool.dma_addr)
 		dma_unmap_single(base->dev, base->lcla_pool.dma_addr,
 				 SZ_1K * base->num_phy_chans,
@@ -3699,28 +3658,11 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	kfree(base->lcla_pool.base_unaligned);
 
-	if (base->lcpa_base)
-		iounmap(base->lcpa_base);
-
-release_base:
-	if (base->phy_start)
-		release_mem_region(base->phy_start,
-				   base->phy_size);
-	if (base->clk) {
-		clk_disable_unprepare(base->clk);
-		clk_put(base->clk);
-	}
-
 	if (base->lcpa_regulator) {
 		regulator_disable(base->lcpa_regulator);
 		regulator_put(base->lcpa_regulator);
 	}
 
-	kfree(base->lcla_pool.alloc_map);
-	kfree(base->lookup_log_chans);
-	kfree(base->lookup_phy_chans);
-	kfree(base->phy_res);
-	kfree(base);
  report_failure:
 	d40_err(dev, "probe failed\n");
 	return ret;

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/7] dmaengine: ste_dma40: Return error codes properly
  2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
                   ` (5 preceding siblings ...)
  2023-04-17  7:55 ` [PATCH 6/7] dmaengine: ste_dma40: Use managed resources Linus Walleij
@ 2023-04-17  7:55 ` Linus Walleij
  6 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2023-04-17  7:55 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel,
	Linus Walleij

This makes the probe() and its subfunction d40_hw_detect_init()
return proper error codes.

One effect of this is that deferred probe, e.g from the clock,
will start to work, would it happen. Also it is better design.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/dma/ste_dma40.c | 46 ++++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index c5991009d3e4..2911017265cf 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3132,7 +3132,8 @@ static void d40_drop_kmem_cache_action(void *d)
 	kmem_cache_destroy(desc_slab);
 }
 
-static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
+static int __init d40_hw_detect_init(struct platform_device *pdev,
+				     struct d40_base **retbase)
 {
 	struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
 	struct device *dev = &pdev->dev;
@@ -3150,14 +3151,12 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 
 	clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(clk))
-		return NULL;
+		return PTR_ERR(clk);
 
 	/* Get IO for DMAC base address */
 	virtbase = devm_platform_ioremap_resource_byname(pdev, "base");
-	if (IS_ERR(virtbase)) {
-		dev_err(dev, "No IO base defined\n");
-		return NULL;
-	}
+	if (IS_ERR(virtbase))
+		return PTR_ERR(virtbase);
 
 	/* This is just a regular AMBA PrimeCell ID actually */
 	for (pid = 0, i = 0; i < 4; i++)
@@ -3169,13 +3168,13 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 
 	if (cid != AMBA_CID) {
 		d40_err(dev, "Unknown hardware! No PrimeCell ID\n");
-		return NULL;
+		return -EINVAL;
 	}
 	if (AMBA_MANF_BITS(pid) != AMBA_VENDOR_ST) {
 		d40_err(dev, "Unknown designer! Got %x wanted %x\n",
 			AMBA_MANF_BITS(pid),
 			AMBA_VENDOR_ST);
-		return NULL;
+		return -EINVAL;
 	}
 	/*
 	 * HW revision:
@@ -3189,7 +3188,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	rev = AMBA_REV_BITS(pid);
 	if (rev < 2) {
 		d40_err(dev, "hardware revision: %d is not supported", rev);
-		return NULL;
+		return -EINVAL;
 	}
 
 	/* The number of physical channels on this HW */
@@ -3216,7 +3215,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 		sizeof(struct d40_chan), GFP_KERNEL);
 
 	if (!base)
-		return NULL;
+		return -ENOMEM;
 
 	base->rev = rev;
 	base->clk = clk;
@@ -3263,51 +3262,53 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 				     sizeof(*base->phy_res),
 				     GFP_KERNEL);
 	if (!base->phy_res)
-		return NULL;
+		return -ENOMEM;
 
 	base->lookup_phy_chans = devm_kcalloc(dev, num_phy_chans,
 					      sizeof(*base->lookup_phy_chans),
 					      GFP_KERNEL);
 	if (!base->lookup_phy_chans)
-		return NULL;
+		return -ENOMEM;
 
 	base->lookup_log_chans = devm_kcalloc(dev, num_log_chans,
 					      sizeof(*base->lookup_log_chans),
 					      GFP_KERNEL);
 	if (!base->lookup_log_chans)
-		return NULL;
+		return -ENOMEM;
 
 	base->reg_val_backup_chan = devm_kmalloc_array(dev, base->num_phy_chans,
 						  sizeof(d40_backup_regs_chan),
 						  GFP_KERNEL);
 	if (!base->reg_val_backup_chan)
-		return NULL;
+		return -ENOMEM;
 
 	base->lcla_pool.alloc_map = devm_kcalloc(dev, num_phy_chans
 					    * D40_LCLA_LINK_PER_EVENT_GRP,
 					    sizeof(*base->lcla_pool.alloc_map),
 					    GFP_KERNEL);
 	if (!base->lcla_pool.alloc_map)
-		return NULL;
+		return -ENOMEM;
 
 	base->regs_interrupt = devm_kmalloc_array(dev, base->gen_dmac.il_size,
 					     sizeof(*base->regs_interrupt),
 					     GFP_KERNEL);
 	if (!base->regs_interrupt)
-		return NULL;
+		return -ENOMEM;
 
 	base->desc_slab = kmem_cache_create(D40_NAME, sizeof(struct d40_desc),
 					    0, SLAB_HWCACHE_ALIGN,
 					    NULL);
 	if (!base->desc_slab)
-		return NULL;
+		return -ENOMEM;
 
 	ret = devm_add_action_or_reset(dev, d40_drop_kmem_cache_action,
 				       base->desc_slab);
 	if (ret)
-		return NULL;
+		return ret;
+
+	*retbase = base;
 
-	return base;
+	return 0;
 }
 
 static void __init d40_hw_init(struct d40_base *base)
@@ -3503,20 +3504,20 @@ static int __init d40_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *np_lcpa;
-	int ret = -ENOENT;
 	struct d40_base *base;
 	struct resource *res;
 	struct resource res_lcpa;
 	int num_reserved_chans;
 	u32 val;
+	int ret;
 
 	if (d40_of_probe(dev, np)) {
 		ret = -ENOMEM;
 		goto report_failure;
 	}
 
-	base = d40_hw_detect_init(pdev);
-	if (!base)
+	ret = d40_hw_detect_init(pdev, &base);
+	if (ret)
 		goto report_failure;
 
 	num_reserved_chans = d40_phy_res_init(base);
@@ -3530,6 +3531,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	np_lcpa = of_parse_phandle(np, "sram", 0);
 	if (!np_lcpa) {
 		dev_err(dev, "no LCPA SRAM node\n");
+		ret = -EINVAL;
 		goto report_failure;
 	}
 	/* This is no device so read the address directly from the node */

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node
  2023-04-17  7:55 ` [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node Linus Walleij
@ 2023-04-17 19:54   ` kernel test robot
  2023-04-18  7:52   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2023-04-17 19:54 UTC (permalink / raw)
  To: Linus Walleij, Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: oe-kbuild-all, dmaengine, devicetree, linux-kernel,
	linux-arm-kernel, Linus Walleij

Hi Linus,

kernel test robot noticed the following build warnings:

[auto build test WARNING on fe15c26ee26efa11741a7b632e9f23b01aca4cc6]

url:    https://github.com/intel-lab-lkp/linux/commits/Linus-Walleij/dt-bindings-dma-dma40-Prefer-to-pass-sram-through-phandle/20230417-160001
base:   fe15c26ee26efa11741a7b632e9f23b01aca4cc6
patch link:    https://lore.kernel.org/r/20230417-ux500-dma40-cleanup-v1-2-b26324956e47%40linaro.org
patch subject: [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node
config: arm-buildonly-randconfig-r005-20230417 (https://download.01.org/0day-ci/archive/20230418/202304180357.gfLjNPk9-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/168b5818186247983ec0f99554e13c3aaa9383bb
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Linus-Walleij/dt-bindings-dma-dma40-Prefer-to-pass-sram-through-phandle/20230417-160001
        git checkout 168b5818186247983ec0f99554e13c3aaa9383bb
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/dma/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304180357.gfLjNPk9-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from drivers/dma/ste_dma40.c:9:
   drivers/dma/ste_dma40.c: In function 'd40_probe':
>> drivers/dma/ste_dma40.c:3555:30: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'dma_addr_t' {aka 'long long unsigned int'} [-Wformat=]
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:150:58: note: in expansion of macro 'dev_fmt'
     150 |         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                          ^~~~~~~
   drivers/dma/ste_dma40.c:3555:9: note: in expansion of macro 'dev_info'
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |         ^~~~~~~~
   drivers/dma/ste_dma40.c:3555:55: note: format string is defined here
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |                                                    ~~~^
         |                                                       |
         |                                                       unsigned int
         |                                                    %08llx
   drivers/dma/ste_dma40.c:3562:26: warning: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'dma_addr_t' {aka 'long long unsigned int'} [-Wformat=]
    3562 |                          "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
         |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
     146 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/dma/ste_dma40.c:3561:17: note: in expansion of macro 'dev_warn'
    3561 |                 dev_warn(&pdev->dev,
         |                 ^~~~~~~~
   drivers/dma/ste_dma40.c:3562:63: note: format string is defined here
    3562 |                          "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
         |                                                            ~~~^
         |                                                               |
         |                                                               unsigned int
         |                                                            %08llx


vim +3555 drivers/dma/ste_dma40.c

  3505	
  3506	static int __init d40_probe(struct platform_device *pdev)
  3507	{
  3508		struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
  3509		struct device_node *np = pdev->dev.of_node;
  3510		struct device_node *np_lcpa;
  3511		int ret = -ENOENT;
  3512		struct d40_base *base;
  3513		struct resource *res;
  3514		struct resource res_lcpa;
  3515		int num_reserved_chans;
  3516		u32 val;
  3517	
  3518		if (!plat_data) {
  3519			if (np) {
  3520				if (d40_of_probe(pdev, np)) {
  3521					ret = -ENOMEM;
  3522					goto report_failure;
  3523				}
  3524			} else {
  3525				d40_err(&pdev->dev, "No pdata or Device Tree provided\n");
  3526				goto report_failure;
  3527			}
  3528		}
  3529	
  3530		base = d40_hw_detect_init(pdev);
  3531		if (!base)
  3532			goto report_failure;
  3533	
  3534		num_reserved_chans = d40_phy_res_init(base);
  3535	
  3536		platform_set_drvdata(pdev, base);
  3537	
  3538		spin_lock_init(&base->interrupt_lock);
  3539		spin_lock_init(&base->execmd_lock);
  3540	
  3541		/* Get IO for logical channel parameter address (LCPA) */
  3542		np_lcpa = of_parse_phandle(np, "sram", 0);
  3543		if (!np_lcpa) {
  3544			dev_err(&pdev->dev, "no LCPA SRAM node\n");
  3545			goto report_failure;
  3546		}
  3547		/* This is no device so read the address directly from the node */
  3548		ret = of_address_to_resource(np_lcpa, 0, &res_lcpa);
  3549		if (ret) {
  3550			dev_err(&pdev->dev, "no LCPA SRAM resource\n");
  3551			goto report_failure;
  3552		}
  3553		base->lcpa_size = resource_size(&res_lcpa);
  3554		base->phy_lcpa = res_lcpa.start;
> 3555		dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
  3556			 base->phy_lcpa, base->lcpa_size);
  3557	
  3558		/* We make use of ESRAM memory for this. */
  3559		val = readl(base->virtbase + D40_DREG_LCPA);
  3560		if (base->phy_lcpa != val && val != 0) {
  3561			dev_warn(&pdev->dev,
  3562				 "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
  3563				 __func__, val, base->phy_lcpa);
  3564		} else
  3565			writel(base->phy_lcpa, base->virtbase + D40_DREG_LCPA);
  3566	
  3567		base->lcpa_base = ioremap(base->phy_lcpa, base->lcpa_size);
  3568		if (!base->lcpa_base) {
  3569			ret = -ENOMEM;
  3570			d40_err(&pdev->dev, "Failed to ioremap LCPA region\n");
  3571			goto release_base;
  3572		}
  3573		/* If lcla has to be located in ESRAM we don't need to allocate */
  3574		if (base->plat_data->use_esram_lcla) {
  3575			res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
  3576								"lcla_esram");
  3577			if (!res) {
  3578				ret = -ENOENT;
  3579				d40_err(&pdev->dev,
  3580					"No \"lcla_esram\" memory resource\n");
  3581				goto destroy_cache;
  3582			}
  3583			base->lcla_pool.base = ioremap(res->start,
  3584							resource_size(res));
  3585			if (!base->lcla_pool.base) {
  3586				ret = -ENOMEM;
  3587				d40_err(&pdev->dev, "Failed to ioremap LCLA region\n");
  3588				goto destroy_cache;
  3589			}
  3590			writel(res->start, base->virtbase + D40_DREG_LCLA);
  3591	
  3592		} else {
  3593			ret = d40_lcla_allocate(base);
  3594			if (ret) {
  3595				d40_err(&pdev->dev, "Failed to allocate LCLA area\n");
  3596				goto destroy_cache;
  3597			}
  3598		}
  3599	
  3600		spin_lock_init(&base->lcla_pool.lock);
  3601	
  3602		base->irq = platform_get_irq(pdev, 0);
  3603	
  3604		ret = request_irq(base->irq, d40_handle_interrupt, 0, D40_NAME, base);
  3605		if (ret) {
  3606			d40_err(&pdev->dev, "No IRQ defined\n");
  3607			goto destroy_cache;
  3608		}
  3609	
  3610		if (base->plat_data->use_esram_lcla) {
  3611	
  3612			base->lcpa_regulator = regulator_get(base->dev, "lcla_esram");
  3613			if (IS_ERR(base->lcpa_regulator)) {
  3614				d40_err(&pdev->dev, "Failed to get lcpa_regulator\n");
  3615				ret = PTR_ERR(base->lcpa_regulator);
  3616				base->lcpa_regulator = NULL;
  3617				goto destroy_cache;
  3618			}
  3619	
  3620			ret = regulator_enable(base->lcpa_regulator);
  3621			if (ret) {
  3622				d40_err(&pdev->dev,
  3623					"Failed to enable lcpa_regulator\n");
  3624				regulator_put(base->lcpa_regulator);
  3625				base->lcpa_regulator = NULL;
  3626				goto destroy_cache;
  3627			}
  3628		}
  3629	
  3630		writel_relaxed(D40_DREG_GCC_ENABLE_ALL, base->virtbase + D40_DREG_GCC);
  3631	
  3632		pm_runtime_irq_safe(base->dev);
  3633		pm_runtime_set_autosuspend_delay(base->dev, DMA40_AUTOSUSPEND_DELAY);
  3634		pm_runtime_use_autosuspend(base->dev);
  3635		pm_runtime_mark_last_busy(base->dev);
  3636		pm_runtime_set_active(base->dev);
  3637		pm_runtime_enable(base->dev);
  3638	
  3639		ret = d40_dmaengine_init(base, num_reserved_chans);
  3640		if (ret)
  3641			goto destroy_cache;
  3642	
  3643		ret = dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
  3644		if (ret) {
  3645			d40_err(&pdev->dev, "Failed to set dma max seg size\n");
  3646			goto destroy_cache;
  3647		}
  3648	
  3649		d40_hw_init(base);
  3650	
  3651		if (np) {
  3652			ret = of_dma_controller_register(np, d40_xlate, NULL);
  3653			if (ret)
  3654				dev_err(&pdev->dev,
  3655					"could not register of_dma_controller\n");
  3656		}
  3657	
  3658		dev_info(base->dev, "initialized\n");
  3659		return 0;
  3660	 destroy_cache:
  3661		kmem_cache_destroy(base->desc_slab);
  3662		if (base->virtbase)
  3663			iounmap(base->virtbase);
  3664	
  3665		if (base->lcla_pool.base && base->plat_data->use_esram_lcla) {
  3666			iounmap(base->lcla_pool.base);
  3667			base->lcla_pool.base = NULL;
  3668		}
  3669	
  3670		if (base->lcla_pool.dma_addr)
  3671			dma_unmap_single(base->dev, base->lcla_pool.dma_addr,
  3672					 SZ_1K * base->num_phy_chans,
  3673					 DMA_TO_DEVICE);
  3674	
  3675		if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
  3676			free_pages((unsigned long)base->lcla_pool.base,
  3677				   base->lcla_pool.pages);
  3678	
  3679		kfree(base->lcla_pool.base_unaligned);
  3680	
  3681		if (base->lcpa_base)
  3682			iounmap(base->lcpa_base);
  3683	
  3684	release_base:
  3685		if (base->phy_start)
  3686			release_mem_region(base->phy_start,
  3687					   base->phy_size);
  3688		if (base->clk) {
  3689			clk_disable_unprepare(base->clk);
  3690			clk_put(base->clk);
  3691		}
  3692	
  3693		if (base->lcpa_regulator) {
  3694			regulator_disable(base->lcpa_regulator);
  3695			regulator_put(base->lcpa_regulator);
  3696		}
  3697	
  3698		kfree(base->lcla_pool.alloc_map);
  3699		kfree(base->lookup_log_chans);
  3700		kfree(base->lookup_phy_chans);
  3701		kfree(base->phy_res);
  3702		kfree(base);
  3703	 report_failure:
  3704		d40_err(&pdev->dev, "probe failed\n");
  3705		return ret;
  3706	}
  3707	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node
  2023-04-17  7:55 ` [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node Linus Walleij
  2023-04-17 19:54   ` kernel test robot
@ 2023-04-18  7:52   ` kernel test robot
  1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2023-04-18  7:52 UTC (permalink / raw)
  To: Linus Walleij, Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: oe-kbuild-all, dmaengine, devicetree, linux-kernel,
	linux-arm-kernel, Linus Walleij

Hi Linus,

kernel test robot noticed the following build warnings:

[auto build test WARNING on fe15c26ee26efa11741a7b632e9f23b01aca4cc6]

url:    https://github.com/intel-lab-lkp/linux/commits/Linus-Walleij/dt-bindings-dma-dma40-Prefer-to-pass-sram-through-phandle/20230417-160001
base:   fe15c26ee26efa11741a7b632e9f23b01aca4cc6
patch link:    https://lore.kernel.org/r/20230417-ux500-dma40-cleanup-v1-2-b26324956e47%40linaro.org
patch subject: [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node
config: arm-randconfig-c024-20230417 (https://download.01.org/0day-ci/archive/20230418/202304181505.5iKHmSYn-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/168b5818186247983ec0f99554e13c3aaa9383bb
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Linus-Walleij/dt-bindings-dma-dma40-Prefer-to-pass-sram-through-phandle/20230417-160001
        git checkout 168b5818186247983ec0f99554e13c3aaa9383bb
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304181505.5iKHmSYn-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/dma-mapping.h:7,
                    from drivers/dma/ste_dma40.c:9:
   drivers/dma/ste_dma40.c: In function 'd40_probe':
   drivers/dma/ste_dma40.c:3555:30: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'dma_addr_t' {aka 'long long unsigned int'} [-Wformat=]
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:150:58: note: in expansion of macro 'dev_fmt'
     150 |         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                          ^~~~~~~
   drivers/dma/ste_dma40.c:3555:9: note: in expansion of macro 'dev_info'
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |         ^~~~~~~~
   drivers/dma/ste_dma40.c:3555:55: note: format string is defined here
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |                                                    ~~~^
         |                                                       |
         |                                                       unsigned int
         |                                                    %08llx
>> drivers/dma/ste_dma40.c:3555:30: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:150:58: note: in expansion of macro 'dev_fmt'
     150 |         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                          ^~~~~~~
   drivers/dma/ste_dma40.c:3555:9: note: in expansion of macro 'dev_info'
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |         ^~~~~~~~
   drivers/dma/ste_dma40.c:3555:68: note: format string is defined here
    3555 |         dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
         |                                                                 ~~~^
         |                                                                    |
         |                                                                    unsigned int
         |                                                                 %08llx
   drivers/dma/ste_dma40.c:3562:26: warning: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'dma_addr_t' {aka 'long long unsigned int'} [-Wformat=]
    3562 |                          "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
         |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
     146 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/dma/ste_dma40.c:3561:17: note: in expansion of macro 'dev_warn'
    3561 |                 dev_warn(&pdev->dev,
         |                 ^~~~~~~~
   drivers/dma/ste_dma40.c:3562:63: note: format string is defined here
    3562 |                          "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
         |                                                            ~~~^
         |                                                               |
         |                                                               unsigned int
         |                                                            %08llx


vim +3555 drivers/dma/ste_dma40.c

  3505	
  3506	static int __init d40_probe(struct platform_device *pdev)
  3507	{
  3508		struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
  3509		struct device_node *np = pdev->dev.of_node;
  3510		struct device_node *np_lcpa;
  3511		int ret = -ENOENT;
  3512		struct d40_base *base;
  3513		struct resource *res;
  3514		struct resource res_lcpa;
  3515		int num_reserved_chans;
  3516		u32 val;
  3517	
  3518		if (!plat_data) {
  3519			if (np) {
  3520				if (d40_of_probe(pdev, np)) {
  3521					ret = -ENOMEM;
  3522					goto report_failure;
  3523				}
  3524			} else {
  3525				d40_err(&pdev->dev, "No pdata or Device Tree provided\n");
  3526				goto report_failure;
  3527			}
  3528		}
  3529	
  3530		base = d40_hw_detect_init(pdev);
  3531		if (!base)
  3532			goto report_failure;
  3533	
  3534		num_reserved_chans = d40_phy_res_init(base);
  3535	
  3536		platform_set_drvdata(pdev, base);
  3537	
  3538		spin_lock_init(&base->interrupt_lock);
  3539		spin_lock_init(&base->execmd_lock);
  3540	
  3541		/* Get IO for logical channel parameter address (LCPA) */
  3542		np_lcpa = of_parse_phandle(np, "sram", 0);
  3543		if (!np_lcpa) {
  3544			dev_err(&pdev->dev, "no LCPA SRAM node\n");
  3545			goto report_failure;
  3546		}
  3547		/* This is no device so read the address directly from the node */
  3548		ret = of_address_to_resource(np_lcpa, 0, &res_lcpa);
  3549		if (ret) {
  3550			dev_err(&pdev->dev, "no LCPA SRAM resource\n");
  3551			goto report_failure;
  3552		}
  3553		base->lcpa_size = resource_size(&res_lcpa);
  3554		base->phy_lcpa = res_lcpa.start;
> 3555		dev_info(&pdev->dev, "found LCPA SRAM at 0x%08x, size 0x%08x\n",
  3556			 base->phy_lcpa, base->lcpa_size);
  3557	
  3558		/* We make use of ESRAM memory for this. */
  3559		val = readl(base->virtbase + D40_DREG_LCPA);
  3560		if (base->phy_lcpa != val && val != 0) {
  3561			dev_warn(&pdev->dev,
  3562				 "[%s] Mismatch LCPA dma 0x%x, def %08x\n",
  3563				 __func__, val, base->phy_lcpa);
  3564		} else
  3565			writel(base->phy_lcpa, base->virtbase + D40_DREG_LCPA);
  3566	
  3567		base->lcpa_base = ioremap(base->phy_lcpa, base->lcpa_size);
  3568		if (!base->lcpa_base) {
  3569			ret = -ENOMEM;
  3570			d40_err(&pdev->dev, "Failed to ioremap LCPA region\n");
  3571			goto release_base;
  3572		}
  3573		/* If lcla has to be located in ESRAM we don't need to allocate */
  3574		if (base->plat_data->use_esram_lcla) {
  3575			res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
  3576								"lcla_esram");
  3577			if (!res) {
  3578				ret = -ENOENT;
  3579				d40_err(&pdev->dev,
  3580					"No \"lcla_esram\" memory resource\n");
  3581				goto destroy_cache;
  3582			}
  3583			base->lcla_pool.base = ioremap(res->start,
  3584							resource_size(res));
  3585			if (!base->lcla_pool.base) {
  3586				ret = -ENOMEM;
  3587				d40_err(&pdev->dev, "Failed to ioremap LCLA region\n");
  3588				goto destroy_cache;
  3589			}
  3590			writel(res->start, base->virtbase + D40_DREG_LCLA);
  3591	
  3592		} else {
  3593			ret = d40_lcla_allocate(base);
  3594			if (ret) {
  3595				d40_err(&pdev->dev, "Failed to allocate LCLA area\n");
  3596				goto destroy_cache;
  3597			}
  3598		}
  3599	
  3600		spin_lock_init(&base->lcla_pool.lock);
  3601	
  3602		base->irq = platform_get_irq(pdev, 0);
  3603	
  3604		ret = request_irq(base->irq, d40_handle_interrupt, 0, D40_NAME, base);
  3605		if (ret) {
  3606			d40_err(&pdev->dev, "No IRQ defined\n");
  3607			goto destroy_cache;
  3608		}
  3609	
  3610		if (base->plat_data->use_esram_lcla) {
  3611	
  3612			base->lcpa_regulator = regulator_get(base->dev, "lcla_esram");
  3613			if (IS_ERR(base->lcpa_regulator)) {
  3614				d40_err(&pdev->dev, "Failed to get lcpa_regulator\n");
  3615				ret = PTR_ERR(base->lcpa_regulator);
  3616				base->lcpa_regulator = NULL;
  3617				goto destroy_cache;
  3618			}
  3619	
  3620			ret = regulator_enable(base->lcpa_regulator);
  3621			if (ret) {
  3622				d40_err(&pdev->dev,
  3623					"Failed to enable lcpa_regulator\n");
  3624				regulator_put(base->lcpa_regulator);
  3625				base->lcpa_regulator = NULL;
  3626				goto destroy_cache;
  3627			}
  3628		}
  3629	
  3630		writel_relaxed(D40_DREG_GCC_ENABLE_ALL, base->virtbase + D40_DREG_GCC);
  3631	
  3632		pm_runtime_irq_safe(base->dev);
  3633		pm_runtime_set_autosuspend_delay(base->dev, DMA40_AUTOSUSPEND_DELAY);
  3634		pm_runtime_use_autosuspend(base->dev);
  3635		pm_runtime_mark_last_busy(base->dev);
  3636		pm_runtime_set_active(base->dev);
  3637		pm_runtime_enable(base->dev);
  3638	
  3639		ret = d40_dmaengine_init(base, num_reserved_chans);
  3640		if (ret)
  3641			goto destroy_cache;
  3642	
  3643		ret = dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
  3644		if (ret) {
  3645			d40_err(&pdev->dev, "Failed to set dma max seg size\n");
  3646			goto destroy_cache;
  3647		}
  3648	
  3649		d40_hw_init(base);
  3650	
  3651		if (np) {
  3652			ret = of_dma_controller_register(np, d40_xlate, NULL);
  3653			if (ret)
  3654				dev_err(&pdev->dev,
  3655					"could not register of_dma_controller\n");
  3656		}
  3657	
  3658		dev_info(base->dev, "initialized\n");
  3659		return 0;
  3660	 destroy_cache:
  3661		kmem_cache_destroy(base->desc_slab);
  3662		if (base->virtbase)
  3663			iounmap(base->virtbase);
  3664	
  3665		if (base->lcla_pool.base && base->plat_data->use_esram_lcla) {
  3666			iounmap(base->lcla_pool.base);
  3667			base->lcla_pool.base = NULL;
  3668		}
  3669	
  3670		if (base->lcla_pool.dma_addr)
  3671			dma_unmap_single(base->dev, base->lcla_pool.dma_addr,
  3672					 SZ_1K * base->num_phy_chans,
  3673					 DMA_TO_DEVICE);
  3674	
  3675		if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
  3676			free_pages((unsigned long)base->lcla_pool.base,
  3677				   base->lcla_pool.pages);
  3678	
  3679		kfree(base->lcla_pool.base_unaligned);
  3680	
  3681		if (base->lcpa_base)
  3682			iounmap(base->lcpa_base);
  3683	
  3684	release_base:
  3685		if (base->phy_start)
  3686			release_mem_region(base->phy_start,
  3687					   base->phy_size);
  3688		if (base->clk) {
  3689			clk_disable_unprepare(base->clk);
  3690			clk_put(base->clk);
  3691		}
  3692	
  3693		if (base->lcpa_regulator) {
  3694			regulator_disable(base->lcpa_regulator);
  3695			regulator_put(base->lcpa_regulator);
  3696		}
  3697	
  3698		kfree(base->lcla_pool.alloc_map);
  3699		kfree(base->lookup_log_chans);
  3700		kfree(base->lookup_phy_chans);
  3701		kfree(base->phy_res);
  3702		kfree(base);
  3703	 report_failure:
  3704		d40_err(&pdev->dev, "probe failed\n");
  3705		return ret;
  3706	}
  3707	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle
  2023-04-17  7:55 ` [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle Linus Walleij
@ 2023-04-18 16:22   ` Krzysztof Kozlowski
  2023-04-18 22:41   ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-04-18 16:22 UTC (permalink / raw)
  To: Linus Walleij, Vinod Koul, Rob Herring, Krzysztof Kozlowski
  Cc: dmaengine, devicetree, linux-kernel, linux-arm-kernel

On 17/04/2023 09:55, Linus Walleij wrote:
> Extend the DMA40 bindings so that we can pass two SRAM
> segments as phandles instead of directly referring to the
> memory address in the second reg cell. This enables more
> granular control over the SRAM, and adds the optiona LCLA
> SRAM segment as well.
> 
> Deprecate the old way of passing LCPA as a second reg cell,
> make sram compulsory.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/dma/stericsson,dma40.yaml  | 35 +++++++++++++++++-----
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml b/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
> index 64845347f44d..4fe0df937171 100644
> --- a/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
> +++ b/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
> @@ -112,14 +112,23 @@ properties:
>        - const: stericsson,dma40
>  
>    reg:
> -    items:
> -      - description: DMA40 memory base
> -      - description: LCPA memory base
> +    oneOf:
> +      - items:
> +          - description: DMA40 memory base
> +      - items:
> +          - description: DMA40 memory base
> +          - description: LCPA memory base, deprecated, use eSRAM pool instead
> +        deprecated: true
> +
>  
>    reg-names:
> -    items:
> -      - const: base
> -      - const: lcpa
> +    oneOf:
> +      - items:
> +          - const: base
> +      - items:
> +          - const: base
> +          - const: lcpa
> +        deprecated: true
>  
>    interrupts:
>      maxItems: 1
> @@ -127,6 +136,14 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  sram:
> +    $ref: '/schemas/types.yaml#/definitions/phandle-array'

Drop quotes


> +    items:

Drop items...

> +      maxItems: 2

and this...

> +    description:
> +      List of phandles for the SRAM used by the DMA40 block, the first
> +      phandle is the LCPA memory, the second is the LCLA memory.

and all this, to write everything like:

items:
  - description: LCPA SRAM memory
  - description: ....


> +

> 

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle
  2023-04-17  7:55 ` [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle Linus Walleij
  2023-04-18 16:22   ` Krzysztof Kozlowski
@ 2023-04-18 22:41   ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring @ 2023-04-18 22:41 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Vinod Koul, Krzysztof Kozlowski, dmaengine, devicetree,
	linux-kernel, linux-arm-kernel

On Mon, Apr 17, 2023 at 09:55:46AM +0200, Linus Walleij wrote:
> Extend the DMA40 bindings so that we can pass two SRAM
> segments as phandles instead of directly referring to the
> memory address in the second reg cell. This enables more
> granular control over the SRAM, and adds the optiona LCLA
> SRAM segment as well.
> 
> Deprecate the old way of passing LCPA as a second reg cell,
> make sram compulsory.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/dma/stericsson,dma40.yaml  | 35 +++++++++++++++++-----
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml b/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
> index 64845347f44d..4fe0df937171 100644
> --- a/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
> +++ b/Documentation/devicetree/bindings/dma/stericsson,dma40.yaml
> @@ -112,14 +112,23 @@ properties:
>        - const: stericsson,dma40
>  
>    reg:
> -    items:
> -      - description: DMA40 memory base
> -      - description: LCPA memory base
> +    oneOf:
> +      - items:
> +          - description: DMA40 memory base
> +      - items:
> +          - description: DMA40 memory base
> +          - description: LCPA memory base, deprecated, use eSRAM pool instead
> +        deprecated: true
> +
>  
>    reg-names:
> -    items:
> -      - const: base
> -      - const: lcpa
> +    oneOf:
> +      - items:
> +          - const: base
> +      - items:
> +          - const: base
> +          - const: lcpa
> +        deprecated: true
>  
>    interrupts:
>      maxItems: 1
> @@ -127,6 +136,14 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  sram:
> +    $ref: '/schemas/types.yaml#/definitions/phandle-array'

Drop quotes.

> +    items:
> +      maxItems: 2

phandle-array really means phandle+args array. So the inner size is 1 
plus number of arg cells. Since you have no arg cells, that would be:

maxItems: 2
items:
  maxItems: 1

> +    description:
> +      List of phandles for the SRAM used by the DMA40 block, the first
> +      phandle is the LCPA memory, the second is the LCLA memory.
> +
>    memcpy-channels:
>      $ref: /schemas/types.yaml#/definitions/uint32-array
>      description: Array of u32 elements indicating which channels on the DMA
> @@ -138,6 +155,7 @@ required:
>    - reg
>    - interrupts
>    - clocks
> +  - sram
>    - memcpy-channels
>  
>  additionalProperties: false
> @@ -149,8 +167,9 @@ examples:
>      #include <dt-bindings/mfd/dbx500-prcmu.h>
>      dma-controller@801c0000 {
>          compatible = "stericsson,db8500-dma40", "stericsson,dma40";
> -        reg = <0x801c0000 0x1000>, <0x40010000 0x800>;
> -        reg-names = "base", "lcpa";
> +        reg = <0x801c0000 0x1000>;
> +        reg-names = "base";
> +        sram = <&lcpa>, <&lcla>;
>          interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>          #dma-cells = <3>;
>          memcpy-channels = <56 57 58 59 60>;
> 
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-04-18 22:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-17  7:55 [PATCH 0/7] DMA40 SRAM refactoring and cleanup Linus Walleij
2023-04-17  7:55 ` [PATCH 1/7] dt-bindings: dma: dma40: Prefer to pass sram through phandle Linus Walleij
2023-04-18 16:22   ` Krzysztof Kozlowski
2023-04-18 22:41   ` Rob Herring
2023-04-17  7:55 ` [PATCH 2/7] dmaengine: ste_dma40: Get LCPA SRAM from SRAM node Linus Walleij
2023-04-17 19:54   ` kernel test robot
2023-04-18  7:52   ` kernel test robot
2023-04-17  7:55 ` [PATCH 3/7] dmaengine: ste_dma40: Add dev helper variable Linus Walleij
2023-04-17  7:55 ` [PATCH 4/7] dmaengine: ste_dma40: Remove platform data Linus Walleij
2023-04-17  7:55 ` [PATCH 5/7] dmaengine: ste_dma40: Pass dev to OF function Linus Walleij
2023-04-17  7:55 ` [PATCH 6/7] dmaengine: ste_dma40: Use managed resources Linus Walleij
2023-04-17  7:55 ` [PATCH 7/7] dmaengine: ste_dma40: Return error codes properly Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).