All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Add DMA driver for StarFive JH7110 SoC
@ 2023-03-06 14:04 ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

This patch series adds dma support for the StarFive JH7110 RISC-V
SoC. The first patch adds device tree binding. The second patch includes
dma driver. The last patch adds device node of dma to JH7110 dts.

The series has been tested on the VisionFive 2 board which equip with
JH7110 SoC and works normally.

The last patch should be applied after the following patchset:
https://lore.kernel.org/all/20230221083323.302471-1-xingyu.wu@starfivetech.com/

Changes since v3:
- Constrain the minItems of resets to 2 for jh7110 dma in the
  dt-binding.
- Replaced all uses of of_device_is_compatible with of_device_get_match_data.
- Moved the definition of struct axi_dma_chip_config to dw-axi-dmac-platform.c

v3: https://lore.kernel.org/all/20230227131042.16125-1-walker.chen@starfivetech.com/

Changes since v2:
- Added minItems with value 1 and changed the maxItems' value to 2 about
  resets properties in the dt-binding.
- Added match data for jh7110-axi-dma and executed reset call to match
  data.
- Dropped reset-names from dma node of device tree.

v2: https://lore.kernel.org/all/20230221140424.719-1-walker.chen@starfivetech.com/

Changes since v1:
- Rebased on Linux 6.2.
- Changed the compatible string to SoC specific and dropped '-rst' from
  reset-names in the dt-binding.
- Dropped 'snps,num-hs-if' in the dt-binding.
- Use different configuration on CH_CFG registers according to the compatible string.

v1: https://lore.kernel.org/all/20230206113811.23133-1-walker.chen@starfivetech.com/

Walker Chen (3):
  dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110
    dma
  dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
  riscv: dts: starfive: add dma controller node

 .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++--
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 18 +++++
 .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
 drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
 4 files changed, 99 insertions(+), 12 deletions(-)


base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
prerequisite-patch-id: 54ce870d6ea747466474b5d4105cfbc05e1b01ab
prerequisite-patch-id: ac150a8c622e858e088df8121093d448df49c245
prerequisite-patch-id: 044263ef2fb9f1e5a586edbf85d5f67814a28430
prerequisite-patch-id: 057fa35870d8d7d22a57c13362588ffb9e9df316
prerequisite-patch-id: 848332ca483b026a755639b9eefb0bf8f3fcf8be
prerequisite-patch-id: 1b2d0982b18da060c82134f05bf3ce16425bac8d
prerequisite-patch-id: 090ba4b78d47bc19204916e76fdbc70021785388
prerequisite-patch-id: a5d9e0f7d4f8163f566678894cf693015119f2d9
prerequisite-patch-id: 87cb528acd9a7f1ffe7475d7261553f6a4de5753
prerequisite-patch-id: 417736eb958e1158c60a5ed74bc2350394321a80
prerequisite-patch-id: ff9fe0b043a5f7f74a1f6af5cebc4793c6f14ce7
prerequisite-patch-id: 290602062703e666191c20ca02f2840471a6bf4f
prerequisite-patch-id: f0b29adbb18edffbfeec7292c5f33e2bbeb30945
prerequisite-patch-id: fccfad539d8455777988b709171ad97729e1a97c
prerequisite-patch-id: 929ebaffab0df158ea801661d0da74e8b5ef138c
prerequisite-patch-id: 0d9ddcaa8a867fcbc790b41d6d0349796e0c44b0
prerequisite-patch-id: 5f539ac7c96023b36489c6da7c70c31eaf64a25b
prerequisite-patch-id: 65f2aed865d88e6fa468d2923527b523d4313857
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e3b986b9c60b2b93b7812ec174c9e1b4cfb14c97
prerequisite-patch-id: 2e03eeb766aefd5d38f132d091618e9fa19a37b6
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: ea9a6d0313dd3936c8de0239dc2072c3360a2f6b
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 602c3cf8f42c8c88125defa0a8a301da51f8af49
prerequisite-patch-id: 82d2d2bc302045505a51f4ab2bf607a904d4b2d1
prerequisite-patch-id: a6df0f7d8fc2d534c06d85f17578c9134913d01b
prerequisite-patch-id: 2ddada18ab6ea5cd1da14212aaf59632f5203d40
prerequisite-patch-id: b9b8fda5e8cd2dd4c9101ec03f4c8fb8e8caa573
prerequisite-patch-id: 7acbc9c924e802712d3574dd74a6b3576089f78c
prerequisite-patch-id: f9ce88e490c2473c3c94ad63fa26bc91829ce2cc
prerequisite-patch-id: ce8a6557564ba04bd90bb41d34f520347f399887
prerequisite-patch-id: 9f71c539a241baf1e73c7e7dfde5b0b04c66a502
prerequisite-patch-id: 378a6ccc643a8bf51918cdd61876af813564c638
prerequisite-patch-id: bb8e071ed43998874b9d98292c0dcdeedc0760ca
prerequisite-patch-id: 0c04762f1d20f09cd2a1356334a86e520907d111
prerequisite-patch-id: 8867ef35e4d555491a97106db7834149309426b7
prerequisite-patch-id: e5a319ba557c8165f7620e574c79ff2ad3be1f65
prerequisite-patch-id: 2bc43b375b470f7e8bbe937b78678ba3856e3b8f
-- 
2.17.1


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

* [PATCH v4 0/3] Add DMA driver for StarFive JH7110 SoC
@ 2023-03-06 14:04 ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

This patch series adds dma support for the StarFive JH7110 RISC-V
SoC. The first patch adds device tree binding. The second patch includes
dma driver. The last patch adds device node of dma to JH7110 dts.

The series has been tested on the VisionFive 2 board which equip with
JH7110 SoC and works normally.

The last patch should be applied after the following patchset:
https://lore.kernel.org/all/20230221083323.302471-1-xingyu.wu@starfivetech.com/

Changes since v3:
- Constrain the minItems of resets to 2 for jh7110 dma in the
  dt-binding.
- Replaced all uses of of_device_is_compatible with of_device_get_match_data.
- Moved the definition of struct axi_dma_chip_config to dw-axi-dmac-platform.c

v3: https://lore.kernel.org/all/20230227131042.16125-1-walker.chen@starfivetech.com/

Changes since v2:
- Added minItems with value 1 and changed the maxItems' value to 2 about
  resets properties in the dt-binding.
- Added match data for jh7110-axi-dma and executed reset call to match
  data.
- Dropped reset-names from dma node of device tree.

v2: https://lore.kernel.org/all/20230221140424.719-1-walker.chen@starfivetech.com/

Changes since v1:
- Rebased on Linux 6.2.
- Changed the compatible string to SoC specific and dropped '-rst' from
  reset-names in the dt-binding.
- Dropped 'snps,num-hs-if' in the dt-binding.
- Use different configuration on CH_CFG registers according to the compatible string.

v1: https://lore.kernel.org/all/20230206113811.23133-1-walker.chen@starfivetech.com/

Walker Chen (3):
  dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110
    dma
  dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
  riscv: dts: starfive: add dma controller node

 .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++--
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 18 +++++
 .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
 drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
 4 files changed, 99 insertions(+), 12 deletions(-)


base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
prerequisite-patch-id: 54ce870d6ea747466474b5d4105cfbc05e1b01ab
prerequisite-patch-id: ac150a8c622e858e088df8121093d448df49c245
prerequisite-patch-id: 044263ef2fb9f1e5a586edbf85d5f67814a28430
prerequisite-patch-id: 057fa35870d8d7d22a57c13362588ffb9e9df316
prerequisite-patch-id: 848332ca483b026a755639b9eefb0bf8f3fcf8be
prerequisite-patch-id: 1b2d0982b18da060c82134f05bf3ce16425bac8d
prerequisite-patch-id: 090ba4b78d47bc19204916e76fdbc70021785388
prerequisite-patch-id: a5d9e0f7d4f8163f566678894cf693015119f2d9
prerequisite-patch-id: 87cb528acd9a7f1ffe7475d7261553f6a4de5753
prerequisite-patch-id: 417736eb958e1158c60a5ed74bc2350394321a80
prerequisite-patch-id: ff9fe0b043a5f7f74a1f6af5cebc4793c6f14ce7
prerequisite-patch-id: 290602062703e666191c20ca02f2840471a6bf4f
prerequisite-patch-id: f0b29adbb18edffbfeec7292c5f33e2bbeb30945
prerequisite-patch-id: fccfad539d8455777988b709171ad97729e1a97c
prerequisite-patch-id: 929ebaffab0df158ea801661d0da74e8b5ef138c
prerequisite-patch-id: 0d9ddcaa8a867fcbc790b41d6d0349796e0c44b0
prerequisite-patch-id: 5f539ac7c96023b36489c6da7c70c31eaf64a25b
prerequisite-patch-id: 65f2aed865d88e6fa468d2923527b523d4313857
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e3b986b9c60b2b93b7812ec174c9e1b4cfb14c97
prerequisite-patch-id: 2e03eeb766aefd5d38f132d091618e9fa19a37b6
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: ea9a6d0313dd3936c8de0239dc2072c3360a2f6b
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 602c3cf8f42c8c88125defa0a8a301da51f8af49
prerequisite-patch-id: 82d2d2bc302045505a51f4ab2bf607a904d4b2d1
prerequisite-patch-id: a6df0f7d8fc2d534c06d85f17578c9134913d01b
prerequisite-patch-id: 2ddada18ab6ea5cd1da14212aaf59632f5203d40
prerequisite-patch-id: b9b8fda5e8cd2dd4c9101ec03f4c8fb8e8caa573
prerequisite-patch-id: 7acbc9c924e802712d3574dd74a6b3576089f78c
prerequisite-patch-id: f9ce88e490c2473c3c94ad63fa26bc91829ce2cc
prerequisite-patch-id: ce8a6557564ba04bd90bb41d34f520347f399887
prerequisite-patch-id: 9f71c539a241baf1e73c7e7dfde5b0b04c66a502
prerequisite-patch-id: 378a6ccc643a8bf51918cdd61876af813564c638
prerequisite-patch-id: bb8e071ed43998874b9d98292c0dcdeedc0760ca
prerequisite-patch-id: 0c04762f1d20f09cd2a1356334a86e520907d111
prerequisite-patch-id: 8867ef35e4d555491a97106db7834149309426b7
prerequisite-patch-id: e5a319ba557c8165f7620e574c79ff2ad3be1f65
prerequisite-patch-id: 2bc43b375b470f7e8bbe937b78678ba3856e3b8f
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
  2023-03-06 14:04 ` Walker Chen
@ 2023-03-06 14:04   ` Walker Chen
  -1 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

The DMA controller needs two reset items to work properly on JH7110 SoC,
so there is need to constrain the items' value to 2, other platforms
have 1 reset item at most.

Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
index ad107a4d3b33..d8b5439f215c 100644
--- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
+++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
@@ -12,14 +12,12 @@ maintainers:
 description:
   Synopsys DesignWare AXI DMA Controller DT Binding
 
-allOf:
-  - $ref: "dma-controller.yaml#"
-
 properties:
   compatible:
     enum:
       - snps,axi-dma-1.01a
       - intel,kmb-axi-dma
+      - starfive,jh7110-axi-dma
 
   reg:
     minItems: 1
@@ -58,7 +56,8 @@ properties:
     maximum: 8
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   snps,dma-masters:
     description: |
@@ -109,6 +108,23 @@ required:
   - snps,priority
   - snps,block-size
 
+allOf:
+  - $ref: "dma-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - starfive,jh7110-axi-dma
+    then:
+      properties:
+        resets:
+          minItems: 2
+    else:
+      properties:
+        resets:
+          maxItems: 1
+
 additionalProperties: false
 
 examples:
-- 
2.17.1


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

* [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
@ 2023-03-06 14:04   ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

The DMA controller needs two reset items to work properly on JH7110 SoC,
so there is need to constrain the items' value to 2, other platforms
have 1 reset item at most.

Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
index ad107a4d3b33..d8b5439f215c 100644
--- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
+++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
@@ -12,14 +12,12 @@ maintainers:
 description:
   Synopsys DesignWare AXI DMA Controller DT Binding
 
-allOf:
-  - $ref: "dma-controller.yaml#"
-
 properties:
   compatible:
     enum:
       - snps,axi-dma-1.01a
       - intel,kmb-axi-dma
+      - starfive,jh7110-axi-dma
 
   reg:
     minItems: 1
@@ -58,7 +56,8 @@ properties:
     maximum: 8
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   snps,dma-masters:
     description: |
@@ -109,6 +108,23 @@ required:
   - snps,priority
   - snps,block-size
 
+allOf:
+  - $ref: "dma-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - starfive,jh7110-axi-dma
+    then:
+      properties:
+        resets:
+          minItems: 2
+    else:
+      properties:
+        resets:
+          maxItems: 1
+
 additionalProperties: false
 
 examples:
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v4 2/3] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
  2023-03-06 14:04 ` Walker Chen
@ 2023-03-06 14:04   ` Walker Chen
  -1 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

Add dma reset operation in device probe and use different configuration
on CH_CFG registers according to match data. Update all uses of
of_device_is_compatible with of_device_get_match_data.

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
 drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
 2 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index bf85aa0979ec..d1148f6fbcf9 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -21,10 +21,12 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_dma.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 
@@ -46,6 +48,12 @@
 	DMA_SLAVE_BUSWIDTH_32_BYTES	| \
 	DMA_SLAVE_BUSWIDTH_64_BYTES)
 
+struct axi_dma_chip_config {
+	int (*apb_setup)(struct platform_device *pdev, struct axi_dma_chip *chip);
+	int (*reset_init)(struct platform_device *pdev);
+	bool use_cfg2;
+};
+
 static inline void
 axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
 {
@@ -86,7 +94,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan,
 
 	cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS |
 		  config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS);
-	if (chan->chip->dw->hdata->reg_map_8_channels) {
+	if (chan->chip->dw->hdata->reg_map_8_channels &&
+	    !chan->chip->dw->hdata->use_cfg2) {
 		cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS |
 			 config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS |
 			 config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS |
@@ -1142,7 +1151,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
 	axi_chan_disable(chan);
 
 	ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val,
-					!(val & chan_active), 1000, 10000);
+					!(val & chan_active), 1000, DMAC_TIMEOUT_US);
 	if (ret == -ETIMEDOUT)
 		dev_warn(dchan2dev(dchan),
 			 "%s failed to stop\n", axi_chan_name(chan));
@@ -1367,13 +1376,33 @@ static int parse_device_properties(struct axi_dma_chip *chip)
 	return 0;
 }
 
+static int intel_apb_setup(struct platform_device *pdev, struct axi_dma_chip *chip)
+{
+	chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(chip->apb_regs))
+		return PTR_ERR(chip->apb_regs);
+	else
+		return 0;
+}
+
+static int jh7110_reset_init(struct platform_device *pdev)
+{
+	struct reset_control *resets;
+
+	resets = devm_reset_control_array_get_exclusive(&pdev->dev);
+	if (IS_ERR(resets))
+		return PTR_ERR(resets);
+
+	return reset_control_deassert(resets);
+}
+
 static int dw_probe(struct platform_device *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
 	struct axi_dma_chip *chip;
 	struct resource *mem;
 	struct dw_axi_dma *dw;
 	struct dw_axi_dma_hcfg *hdata;
+	const struct axi_dma_chip_config *ccfg;
 	u32 i;
 	int ret;
 
@@ -1402,10 +1431,21 @@ static int dw_probe(struct platform_device *pdev)
 	if (IS_ERR(chip->regs))
 		return PTR_ERR(chip->regs);
 
-	if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
-		chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
-		if (IS_ERR(chip->apb_regs))
-			return PTR_ERR(chip->apb_regs);
+	ccfg = of_device_get_match_data(&pdev->dev);
+	if (ccfg) {
+		if (ccfg->apb_setup) {
+			ret = ccfg->apb_setup(pdev, chip);
+			if (ret)
+				return ret;
+		}
+
+		if (ccfg->reset_init) {
+			ret = ccfg->reset_init(pdev);
+			if (ret)
+				return ret;
+		}
+
+		chip->dw->hdata->use_cfg2 = ccfg->use_cfg2;
 	}
 
 	chip->core_clk = devm_clk_get(chip->dev, "core-clk");
@@ -1557,9 +1597,20 @@ static const struct dev_pm_ops dw_axi_dma_pm_ops = {
 	SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL)
 };
 
+static const struct axi_dma_chip_config intel_chip_config = {
+	.apb_setup = intel_apb_setup,
+	.use_cfg2 = false,
+};
+
+static const struct axi_dma_chip_config jh7110_chip_config = {
+	.reset_init = jh7110_reset_init,
+	.use_cfg2 = true,
+};
+
 static const struct of_device_id dw_dma_of_id_table[] = {
 	{ .compatible = "snps,axi-dma-1.01a" },
-	{ .compatible = "intel,kmb-axi-dma" },
+	{ .compatible = "intel,kmb-axi-dma", .data = &intel_chip_config },
+	{ .compatible = "starfive,jh7110-axi-dma", .data = &jh7110_chip_config },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dw_dma_of_id_table);
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
index e9d5eb0fd594..b906d5884efe 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
@@ -21,6 +21,7 @@
 #define DMAC_MAX_CHANNELS	16
 #define DMAC_MAX_MASTERS	2
 #define DMAC_MAX_BLK_SIZE	0x200000
+#define DMAC_TIMEOUT_US		200000
 
 struct dw_axi_dma_hcfg {
 	u32	nr_channels;
@@ -33,6 +34,7 @@ struct dw_axi_dma_hcfg {
 	/* Register map for DMAX_NUM_CHANNELS <= 8 */
 	bool	reg_map_8_channels;
 	bool	restrict_axi_burst_len;
+	bool	use_cfg2;
 };
 
 struct axi_dma_chan {
-- 
2.17.1


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

* [PATCH v4 2/3] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
@ 2023-03-06 14:04   ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

Add dma reset operation in device probe and use different configuration
on CH_CFG registers according to match data. Update all uses of
of_device_is_compatible with of_device_get_match_data.

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
 drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
 2 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index bf85aa0979ec..d1148f6fbcf9 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -21,10 +21,12 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_dma.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 
@@ -46,6 +48,12 @@
 	DMA_SLAVE_BUSWIDTH_32_BYTES	| \
 	DMA_SLAVE_BUSWIDTH_64_BYTES)
 
+struct axi_dma_chip_config {
+	int (*apb_setup)(struct platform_device *pdev, struct axi_dma_chip *chip);
+	int (*reset_init)(struct platform_device *pdev);
+	bool use_cfg2;
+};
+
 static inline void
 axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
 {
@@ -86,7 +94,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan,
 
 	cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS |
 		  config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS);
-	if (chan->chip->dw->hdata->reg_map_8_channels) {
+	if (chan->chip->dw->hdata->reg_map_8_channels &&
+	    !chan->chip->dw->hdata->use_cfg2) {
 		cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS |
 			 config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS |
 			 config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS |
@@ -1142,7 +1151,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
 	axi_chan_disable(chan);
 
 	ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val,
-					!(val & chan_active), 1000, 10000);
+					!(val & chan_active), 1000, DMAC_TIMEOUT_US);
 	if (ret == -ETIMEDOUT)
 		dev_warn(dchan2dev(dchan),
 			 "%s failed to stop\n", axi_chan_name(chan));
@@ -1367,13 +1376,33 @@ static int parse_device_properties(struct axi_dma_chip *chip)
 	return 0;
 }
 
+static int intel_apb_setup(struct platform_device *pdev, struct axi_dma_chip *chip)
+{
+	chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(chip->apb_regs))
+		return PTR_ERR(chip->apb_regs);
+	else
+		return 0;
+}
+
+static int jh7110_reset_init(struct platform_device *pdev)
+{
+	struct reset_control *resets;
+
+	resets = devm_reset_control_array_get_exclusive(&pdev->dev);
+	if (IS_ERR(resets))
+		return PTR_ERR(resets);
+
+	return reset_control_deassert(resets);
+}
+
 static int dw_probe(struct platform_device *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
 	struct axi_dma_chip *chip;
 	struct resource *mem;
 	struct dw_axi_dma *dw;
 	struct dw_axi_dma_hcfg *hdata;
+	const struct axi_dma_chip_config *ccfg;
 	u32 i;
 	int ret;
 
@@ -1402,10 +1431,21 @@ static int dw_probe(struct platform_device *pdev)
 	if (IS_ERR(chip->regs))
 		return PTR_ERR(chip->regs);
 
-	if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
-		chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
-		if (IS_ERR(chip->apb_regs))
-			return PTR_ERR(chip->apb_regs);
+	ccfg = of_device_get_match_data(&pdev->dev);
+	if (ccfg) {
+		if (ccfg->apb_setup) {
+			ret = ccfg->apb_setup(pdev, chip);
+			if (ret)
+				return ret;
+		}
+
+		if (ccfg->reset_init) {
+			ret = ccfg->reset_init(pdev);
+			if (ret)
+				return ret;
+		}
+
+		chip->dw->hdata->use_cfg2 = ccfg->use_cfg2;
 	}
 
 	chip->core_clk = devm_clk_get(chip->dev, "core-clk");
@@ -1557,9 +1597,20 @@ static const struct dev_pm_ops dw_axi_dma_pm_ops = {
 	SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL)
 };
 
+static const struct axi_dma_chip_config intel_chip_config = {
+	.apb_setup = intel_apb_setup,
+	.use_cfg2 = false,
+};
+
+static const struct axi_dma_chip_config jh7110_chip_config = {
+	.reset_init = jh7110_reset_init,
+	.use_cfg2 = true,
+};
+
 static const struct of_device_id dw_dma_of_id_table[] = {
 	{ .compatible = "snps,axi-dma-1.01a" },
-	{ .compatible = "intel,kmb-axi-dma" },
+	{ .compatible = "intel,kmb-axi-dma", .data = &intel_chip_config },
+	{ .compatible = "starfive,jh7110-axi-dma", .data = &jh7110_chip_config },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dw_dma_of_id_table);
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
index e9d5eb0fd594..b906d5884efe 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
@@ -21,6 +21,7 @@
 #define DMAC_MAX_CHANNELS	16
 #define DMAC_MAX_MASTERS	2
 #define DMAC_MAX_BLK_SIZE	0x200000
+#define DMAC_TIMEOUT_US		200000
 
 struct dw_axi_dma_hcfg {
 	u32	nr_channels;
@@ -33,6 +34,7 @@ struct dw_axi_dma_hcfg {
 	/* Register map for DMAX_NUM_CHANNELS <= 8 */
 	bool	reg_map_8_channels;
 	bool	restrict_axi_burst_len;
+	bool	use_cfg2;
 };
 
 struct axi_dma_chan {
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v4 3/3] riscv: dts: starfive: add dma controller node
  2023-03-06 14:04 ` Walker Chen
@ 2023-03-06 14:04   ` Walker Chen
  -1 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

Add the dma controller node for the Starfive JH7110 SoC.

Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 697ab59191a1..191b6add72c8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -510,6 +510,24 @@
 			#gpio-cells = <2>;
 		};
 
+		dma: dma-controller@16050000 {
+			compatible = "starfive,jh7110-axi-dma";
+			reg = <0x0 0x16050000 0x0 0x10000>;
+			clocks = <&stgcrg JH7110_STGCLK_DMA1P_AXI>,
+				 <&stgcrg JH7110_STGCLK_DMA1P_AHB>;
+			clock-names = "core-clk", "cfgr-clk";
+			resets = <&stgcrg JH7110_STGRST_DMA1P_AXI>,
+				 <&stgcrg JH7110_STGRST_DMA1P_AHB>;
+			interrupts = <73>;
+			#dma-cells = <1>;
+			dma-channels = <4>;
+			snps,dma-masters = <1>;
+			snps,data-width = <3>;
+			snps,block-size = <65536 65536 65536 65536>;
+			snps,priority = <0 1 2 3>;
+			snps,axi-max-burst-len = <16>;
+		};
+
 		aoncrg: clock-controller@17000000 {
 			compatible = "starfive,jh7110-aoncrg";
 			reg = <0x0 0x17000000 0x0 0x10000>;
-- 
2.17.1


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

* [PATCH v4 3/3] riscv: dts: starfive: add dma controller node
@ 2023-03-06 14:04   ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-06 14:04 UTC (permalink / raw)
  To: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, Emil Renner Berthing, Walker Chen
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

Add the dma controller node for the Starfive JH7110 SoC.

Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 697ab59191a1..191b6add72c8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -510,6 +510,24 @@
 			#gpio-cells = <2>;
 		};
 
+		dma: dma-controller@16050000 {
+			compatible = "starfive,jh7110-axi-dma";
+			reg = <0x0 0x16050000 0x0 0x10000>;
+			clocks = <&stgcrg JH7110_STGCLK_DMA1P_AXI>,
+				 <&stgcrg JH7110_STGCLK_DMA1P_AHB>;
+			clock-names = "core-clk", "cfgr-clk";
+			resets = <&stgcrg JH7110_STGRST_DMA1P_AXI>,
+				 <&stgcrg JH7110_STGRST_DMA1P_AHB>;
+			interrupts = <73>;
+			#dma-cells = <1>;
+			dma-channels = <4>;
+			snps,dma-masters = <1>;
+			snps,data-width = <3>;
+			snps,block-size = <65536 65536 65536 65536>;
+			snps,priority = <0 1 2 3>;
+			snps,axi-max-burst-len = <16>;
+		};
+
 		aoncrg: clock-controller@17000000 {
 			compatible = "starfive,jh7110-aoncrg";
 			reg = <0x0 0x17000000 0x0 0x10000>;
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 2/3] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
  2023-03-06 14:04   ` Walker Chen
@ 2023-03-06 14:39     ` Emil Renner Berthing
  -1 siblings, 0 replies; 24+ messages in thread
From: Emil Renner Berthing @ 2023-03-06 14:39 UTC (permalink / raw)
  To: Walker Chen
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv

On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
>
> Add dma reset operation in device probe and use different configuration
> on CH_CFG registers according to match data. Update all uses of
> of_device_is_compatible with of_device_get_match_data.
>
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Hi Walker,

Again please remove my Reviewed-by when you're adding a bunch of new
code as you're doing here.

> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> ---
>  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
>  drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
>  2 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index bf85aa0979ec..d1148f6fbcf9 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -21,10 +21,12 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/of_dma.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
> +#include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>
> @@ -46,6 +48,12 @@
>         DMA_SLAVE_BUSWIDTH_32_BYTES     | \
>         DMA_SLAVE_BUSWIDTH_64_BYTES)
>
> +struct axi_dma_chip_config {
> +       int (*apb_setup)(struct platform_device *pdev, struct axi_dma_chip *chip);
> +       int (*reset_init)(struct platform_device *pdev);
> +       bool use_cfg2;
> +};
> +
>  static inline void
>  axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
>  {
> @@ -86,7 +94,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan,
>
>         cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS |
>                   config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS);
> -       if (chan->chip->dw->hdata->reg_map_8_channels) {
> +       if (chan->chip->dw->hdata->reg_map_8_channels &&
> +           !chan->chip->dw->hdata->use_cfg2) {
>                 cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS |
>                          config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS |
>                          config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS |
> @@ -1142,7 +1151,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
>         axi_chan_disable(chan);
>
>         ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val,
> -                                       !(val & chan_active), 1000, 10000);
> +                                       !(val & chan_active), 1000, DMAC_TIMEOUT_US);
>         if (ret == -ETIMEDOUT)
>                 dev_warn(dchan2dev(dchan),
>                          "%s failed to stop\n", axi_chan_name(chan));
> @@ -1367,13 +1376,33 @@ static int parse_device_properties(struct axi_dma_chip *chip)
>         return 0;
>  }
>
> +static int intel_apb_setup(struct platform_device *pdev, struct axi_dma_chip *chip)
> +{
> +       chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
> +       if (IS_ERR(chip->apb_regs))
> +               return PTR_ERR(chip->apb_regs);
> +       else
> +               return 0;
> +}
> +
> +static int jh7110_reset_init(struct platform_device *pdev)
> +{
> +       struct reset_control *resets;
> +
> +       resets = devm_reset_control_array_get_exclusive(&pdev->dev);
> +       if (IS_ERR(resets))
> +               return PTR_ERR(resets);
> +
> +       return reset_control_deassert(resets);
> +}
> +
>  static int dw_probe(struct platform_device *pdev)
>  {
> -       struct device_node *node = pdev->dev.of_node;
>         struct axi_dma_chip *chip;
>         struct resource *mem;
>         struct dw_axi_dma *dw;
>         struct dw_axi_dma_hcfg *hdata;
> +       const struct axi_dma_chip_config *ccfg;
>         u32 i;
>         int ret;
>
> @@ -1402,10 +1431,21 @@ static int dw_probe(struct platform_device *pdev)
>         if (IS_ERR(chip->regs))
>                 return PTR_ERR(chip->regs);
>
> -       if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
> -               chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
> -               if (IS_ERR(chip->apb_regs))
> -                       return PTR_ERR(chip->apb_regs);
> +       ccfg = of_device_get_match_data(&pdev->dev);
> +       if (ccfg) {
> +               if (ccfg->apb_setup) {
> +                       ret = ccfg->apb_setup(pdev, chip);
> +                       if (ret)
> +                               return ret;
> +               }
> +
> +               if (ccfg->reset_init) {
> +                       ret = ccfg->reset_init(pdev);
> +                       if (ret)
> +                               return ret;
> +               }
> +
> +               chip->dw->hdata->use_cfg2 = ccfg->use_cfg2;

This claims and deasserts the resets before the clocks, whereas your
previous versions did it after turning the clocks on. Which is the
correct order?

Also this certainly gets rid of of_device_is_compatible calls, but
seems like a lot of code to do that. Did you consider something like

+#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0)
+#define AXI_DMA_FLAG_HAS_RESETS BIT(1)
+#define AXI_DMA_FLAG_USE_CFG2 BIT(2)

+unsigned int flags = (uintptr_t)device_get_match_data(&pdev->dev);

-if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
+if (flags & AXI_DMA_FLAG_HAS_APB_REGS) {

-if (of_device_is_compatible(node, "starfive,jh7110-axi-dma)) {
+if (flags & AXI_DMA_FLAG_HAS_RESETS) {

+chip->dw->hwdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);

-{ .compatible = "intel,kmb-axi-dma" },
+{ .compatible = "intel,kmb-axi-dma", .data = (void
*)AXI_DMA_FLAG_HAS_APB_REGS },
+{ .compatible = "starive,jh7110-axi-dma", .data = (void
*)(AXI_DMA_FLAG_HAS_RESETS | AXI_DMA_FLAG_USE_CFG2) },

>         }
>
>         chip->core_clk = devm_clk_get(chip->dev, "core-clk");
> @@ -1557,9 +1597,20 @@ static const struct dev_pm_ops dw_axi_dma_pm_ops = {
>         SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL)
>  };
>
> +static const struct axi_dma_chip_config intel_chip_config = {
> +       .apb_setup = intel_apb_setup,
> +       .use_cfg2 = false,
> +};
> +
> +static const struct axi_dma_chip_config jh7110_chip_config = {
> +       .reset_init = jh7110_reset_init,
> +       .use_cfg2 = true,
> +};
> +
>  static const struct of_device_id dw_dma_of_id_table[] = {
>         { .compatible = "snps,axi-dma-1.01a" },
> -       { .compatible = "intel,kmb-axi-dma" },
> +       { .compatible = "intel,kmb-axi-dma", .data = &intel_chip_config },
> +       { .compatible = "starfive,jh7110-axi-dma", .data = &jh7110_chip_config },
>         {}
>  };
>  MODULE_DEVICE_TABLE(of, dw_dma_of_id_table);
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
> index e9d5eb0fd594..b906d5884efe 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
> @@ -21,6 +21,7 @@
>  #define DMAC_MAX_CHANNELS      16
>  #define DMAC_MAX_MASTERS       2
>  #define DMAC_MAX_BLK_SIZE      0x200000
> +#define DMAC_TIMEOUT_US                200000
>
>  struct dw_axi_dma_hcfg {
>         u32     nr_channels;
> @@ -33,6 +34,7 @@ struct dw_axi_dma_hcfg {
>         /* Register map for DMAX_NUM_CHANNELS <= 8 */
>         bool    reg_map_8_channels;
>         bool    restrict_axi_burst_len;
> +       bool    use_cfg2;
>  };
>
>  struct axi_dma_chan {
> --
> 2.17.1
>

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

* Re: [PATCH v4 2/3] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
@ 2023-03-06 14:39     ` Emil Renner Berthing
  0 siblings, 0 replies; 24+ messages in thread
From: Emil Renner Berthing @ 2023-03-06 14:39 UTC (permalink / raw)
  To: Walker Chen
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv

On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
>
> Add dma reset operation in device probe and use different configuration
> on CH_CFG registers according to match data. Update all uses of
> of_device_is_compatible with of_device_get_match_data.
>
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Hi Walker,

Again please remove my Reviewed-by when you're adding a bunch of new
code as you're doing here.

> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> ---
>  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
>  drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
>  2 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index bf85aa0979ec..d1148f6fbcf9 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -21,10 +21,12 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/of_dma.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
> +#include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>
> @@ -46,6 +48,12 @@
>         DMA_SLAVE_BUSWIDTH_32_BYTES     | \
>         DMA_SLAVE_BUSWIDTH_64_BYTES)
>
> +struct axi_dma_chip_config {
> +       int (*apb_setup)(struct platform_device *pdev, struct axi_dma_chip *chip);
> +       int (*reset_init)(struct platform_device *pdev);
> +       bool use_cfg2;
> +};
> +
>  static inline void
>  axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
>  {
> @@ -86,7 +94,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan,
>
>         cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS |
>                   config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS);
> -       if (chan->chip->dw->hdata->reg_map_8_channels) {
> +       if (chan->chip->dw->hdata->reg_map_8_channels &&
> +           !chan->chip->dw->hdata->use_cfg2) {
>                 cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS |
>                          config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS |
>                          config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS |
> @@ -1142,7 +1151,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
>         axi_chan_disable(chan);
>
>         ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val,
> -                                       !(val & chan_active), 1000, 10000);
> +                                       !(val & chan_active), 1000, DMAC_TIMEOUT_US);
>         if (ret == -ETIMEDOUT)
>                 dev_warn(dchan2dev(dchan),
>                          "%s failed to stop\n", axi_chan_name(chan));
> @@ -1367,13 +1376,33 @@ static int parse_device_properties(struct axi_dma_chip *chip)
>         return 0;
>  }
>
> +static int intel_apb_setup(struct platform_device *pdev, struct axi_dma_chip *chip)
> +{
> +       chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
> +       if (IS_ERR(chip->apb_regs))
> +               return PTR_ERR(chip->apb_regs);
> +       else
> +               return 0;
> +}
> +
> +static int jh7110_reset_init(struct platform_device *pdev)
> +{
> +       struct reset_control *resets;
> +
> +       resets = devm_reset_control_array_get_exclusive(&pdev->dev);
> +       if (IS_ERR(resets))
> +               return PTR_ERR(resets);
> +
> +       return reset_control_deassert(resets);
> +}
> +
>  static int dw_probe(struct platform_device *pdev)
>  {
> -       struct device_node *node = pdev->dev.of_node;
>         struct axi_dma_chip *chip;
>         struct resource *mem;
>         struct dw_axi_dma *dw;
>         struct dw_axi_dma_hcfg *hdata;
> +       const struct axi_dma_chip_config *ccfg;
>         u32 i;
>         int ret;
>
> @@ -1402,10 +1431,21 @@ static int dw_probe(struct platform_device *pdev)
>         if (IS_ERR(chip->regs))
>                 return PTR_ERR(chip->regs);
>
> -       if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
> -               chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
> -               if (IS_ERR(chip->apb_regs))
> -                       return PTR_ERR(chip->apb_regs);
> +       ccfg = of_device_get_match_data(&pdev->dev);
> +       if (ccfg) {
> +               if (ccfg->apb_setup) {
> +                       ret = ccfg->apb_setup(pdev, chip);
> +                       if (ret)
> +                               return ret;
> +               }
> +
> +               if (ccfg->reset_init) {
> +                       ret = ccfg->reset_init(pdev);
> +                       if (ret)
> +                               return ret;
> +               }
> +
> +               chip->dw->hdata->use_cfg2 = ccfg->use_cfg2;

This claims and deasserts the resets before the clocks, whereas your
previous versions did it after turning the clocks on. Which is the
correct order?

Also this certainly gets rid of of_device_is_compatible calls, but
seems like a lot of code to do that. Did you consider something like

+#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0)
+#define AXI_DMA_FLAG_HAS_RESETS BIT(1)
+#define AXI_DMA_FLAG_USE_CFG2 BIT(2)

+unsigned int flags = (uintptr_t)device_get_match_data(&pdev->dev);

-if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
+if (flags & AXI_DMA_FLAG_HAS_APB_REGS) {

-if (of_device_is_compatible(node, "starfive,jh7110-axi-dma)) {
+if (flags & AXI_DMA_FLAG_HAS_RESETS) {

+chip->dw->hwdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);

-{ .compatible = "intel,kmb-axi-dma" },
+{ .compatible = "intel,kmb-axi-dma", .data = (void
*)AXI_DMA_FLAG_HAS_APB_REGS },
+{ .compatible = "starive,jh7110-axi-dma", .data = (void
*)(AXI_DMA_FLAG_HAS_RESETS | AXI_DMA_FLAG_USE_CFG2) },

>         }
>
>         chip->core_clk = devm_clk_get(chip->dev, "core-clk");
> @@ -1557,9 +1597,20 @@ static const struct dev_pm_ops dw_axi_dma_pm_ops = {
>         SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend, axi_dma_runtime_resume, NULL)
>  };
>
> +static const struct axi_dma_chip_config intel_chip_config = {
> +       .apb_setup = intel_apb_setup,
> +       .use_cfg2 = false,
> +};
> +
> +static const struct axi_dma_chip_config jh7110_chip_config = {
> +       .reset_init = jh7110_reset_init,
> +       .use_cfg2 = true,
> +};
> +
>  static const struct of_device_id dw_dma_of_id_table[] = {
>         { .compatible = "snps,axi-dma-1.01a" },
> -       { .compatible = "intel,kmb-axi-dma" },
> +       { .compatible = "intel,kmb-axi-dma", .data = &intel_chip_config },
> +       { .compatible = "starfive,jh7110-axi-dma", .data = &jh7110_chip_config },
>         {}
>  };
>  MODULE_DEVICE_TABLE(of, dw_dma_of_id_table);
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
> index e9d5eb0fd594..b906d5884efe 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
> @@ -21,6 +21,7 @@
>  #define DMAC_MAX_CHANNELS      16
>  #define DMAC_MAX_MASTERS       2
>  #define DMAC_MAX_BLK_SIZE      0x200000
> +#define DMAC_TIMEOUT_US                200000
>
>  struct dw_axi_dma_hcfg {
>         u32     nr_channels;
> @@ -33,6 +34,7 @@ struct dw_axi_dma_hcfg {
>         /* Register map for DMAX_NUM_CHANNELS <= 8 */
>         bool    reg_map_8_channels;
>         bool    restrict_axi_burst_len;
> +       bool    use_cfg2;
>  };
>
>  struct axi_dma_chan {
> --
> 2.17.1
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 3/3] riscv: dts: starfive: add dma controller node
  2023-03-06 14:04   ` Walker Chen
@ 2023-03-06 14:51     ` Emil Renner Berthing
  -1 siblings, 0 replies; 24+ messages in thread
From: Emil Renner Berthing @ 2023-03-06 14:51 UTC (permalink / raw)
  To: Walker Chen
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv

On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
> Add the dma controller node for the Starfive JH7110 SoC.
>
> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>

Thanks!
Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> ---
>  arch/riscv/boot/dts/starfive/jh7110.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 697ab59191a1..191b6add72c8 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -510,6 +510,24 @@
>                         #gpio-cells = <2>;
>                 };
>
> +               dma: dma-controller@16050000 {
> +                       compatible = "starfive,jh7110-axi-dma";
> +                       reg = <0x0 0x16050000 0x0 0x10000>;
> +                       clocks = <&stgcrg JH7110_STGCLK_DMA1P_AXI>,
> +                                <&stgcrg JH7110_STGCLK_DMA1P_AHB>;
> +                       clock-names = "core-clk", "cfgr-clk";
> +                       resets = <&stgcrg JH7110_STGRST_DMA1P_AXI>,
> +                                <&stgcrg JH7110_STGRST_DMA1P_AHB>;
> +                       interrupts = <73>;
> +                       #dma-cells = <1>;
> +                       dma-channels = <4>;
> +                       snps,dma-masters = <1>;
> +                       snps,data-width = <3>;
> +                       snps,block-size = <65536 65536 65536 65536>;
> +                       snps,priority = <0 1 2 3>;
> +                       snps,axi-max-burst-len = <16>;
> +               };
> +
>                 aoncrg: clock-controller@17000000 {
>                         compatible = "starfive,jh7110-aoncrg";
>                         reg = <0x0 0x17000000 0x0 0x10000>;
> --
> 2.17.1
>

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

* Re: [PATCH v4 3/3] riscv: dts: starfive: add dma controller node
@ 2023-03-06 14:51     ` Emil Renner Berthing
  0 siblings, 0 replies; 24+ messages in thread
From: Emil Renner Berthing @ 2023-03-06 14:51 UTC (permalink / raw)
  To: Walker Chen
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv

On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
> Add the dma controller node for the Starfive JH7110 SoC.
>
> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>

Thanks!
Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> ---
>  arch/riscv/boot/dts/starfive/jh7110.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 697ab59191a1..191b6add72c8 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -510,6 +510,24 @@
>                         #gpio-cells = <2>;
>                 };
>
> +               dma: dma-controller@16050000 {
> +                       compatible = "starfive,jh7110-axi-dma";
> +                       reg = <0x0 0x16050000 0x0 0x10000>;
> +                       clocks = <&stgcrg JH7110_STGCLK_DMA1P_AXI>,
> +                                <&stgcrg JH7110_STGCLK_DMA1P_AHB>;
> +                       clock-names = "core-clk", "cfgr-clk";
> +                       resets = <&stgcrg JH7110_STGRST_DMA1P_AXI>,
> +                                <&stgcrg JH7110_STGRST_DMA1P_AHB>;
> +                       interrupts = <73>;
> +                       #dma-cells = <1>;
> +                       dma-channels = <4>;
> +                       snps,dma-masters = <1>;
> +                       snps,data-width = <3>;
> +                       snps,block-size = <65536 65536 65536 65536>;
> +                       snps,priority = <0 1 2 3>;
> +                       snps,axi-max-burst-len = <16>;
> +               };
> +
>                 aoncrg: clock-controller@17000000 {
>                         compatible = "starfive,jh7110-aoncrg";
>                         reg = <0x0 0x17000000 0x0 0x10000>;
> --
> 2.17.1
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 2/3] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
  2023-03-06 14:39     ` Emil Renner Berthing
@ 2023-03-07  1:38       ` Walker Chen
  -1 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-07  1:38 UTC (permalink / raw)
  To: Emil Renner Berthing
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv



On 2023/3/6 22:39, Emil Renner Berthing wrote:
> On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
>>
>> Add dma reset operation in device probe and use different configuration
>> on CH_CFG registers according to match data. Update all uses of
>> of_device_is_compatible with of_device_get_match_data.
>>
>> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> 
> Hi Walker,
> 
> Again please remove my Reviewed-by when you're adding a bunch of new
> code as you're doing here.

Sorry, I made another simple mistake.

> 
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>> ---
>>  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
>>  drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
>>  2 files changed, 61 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> index bf85aa0979ec..d1148f6fbcf9 100644
>> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> @@ -21,10 +21,12 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>> +#include <linux/of_device.h>
>>  #include <linux/of_dma.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/property.h>
>> +#include <linux/reset.h>
>>  #include <linux/slab.h>
>>  #include <linux/types.h>
>>
>> @@ -46,6 +48,12 @@
>>         DMA_SLAVE_BUSWIDTH_32_BYTES     | \
>>         DMA_SLAVE_BUSWIDTH_64_BYTES)
>>
>> +struct axi_dma_chip_config {
>> +       int (*apb_setup)(struct platform_device *pdev, struct axi_dma_chip *chip);
>> +       int (*reset_init)(struct platform_device *pdev);
>> +       bool use_cfg2;
>> +};
>> +
>>  static inline void
>>  axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
>>  {
>> @@ -86,7 +94,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan,
>>
>>         cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS |
>>                   config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS);
>> -       if (chan->chip->dw->hdata->reg_map_8_channels) {
>> +       if (chan->chip->dw->hdata->reg_map_8_channels &&
>> +           !chan->chip->dw->hdata->use_cfg2) {
>>                 cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS |
>>                          config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS |
>>                          config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS |
>> @@ -1142,7 +1151,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
>>         axi_chan_disable(chan);
>>
>>         ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val,
>> -                                       !(val & chan_active), 1000, 10000);
>> +                                       !(val & chan_active), 1000, DMAC_TIMEOUT_US);
>>         if (ret == -ETIMEDOUT)
>>                 dev_warn(dchan2dev(dchan),
>>                          "%s failed to stop\n", axi_chan_name(chan));
>> @@ -1367,13 +1376,33 @@ static int parse_device_properties(struct axi_dma_chip *chip)
>>         return 0;
>>  }
>>
>> +static int intel_apb_setup(struct platform_device *pdev, struct axi_dma_chip *chip)
>> +{
>> +       chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
>> +       if (IS_ERR(chip->apb_regs))
>> +               return PTR_ERR(chip->apb_regs);
>> +       else
>> +               return 0;
>> +}
>> +
>> +static int jh7110_reset_init(struct platform_device *pdev)
>> +{
>> +       struct reset_control *resets;
>> +
>> +       resets = devm_reset_control_array_get_exclusive(&pdev->dev);
>> +       if (IS_ERR(resets))
>> +               return PTR_ERR(resets);
>> +
>> +       return reset_control_deassert(resets);
>> +}
>> +
>>  static int dw_probe(struct platform_device *pdev)
>>  {
>> -       struct device_node *node = pdev->dev.of_node;
>>         struct axi_dma_chip *chip;
>>         struct resource *mem;
>>         struct dw_axi_dma *dw;
>>         struct dw_axi_dma_hcfg *hdata;
>> +       const struct axi_dma_chip_config *ccfg;
>>         u32 i;
>>         int ret;
>>
>> @@ -1402,10 +1431,21 @@ static int dw_probe(struct platform_device *pdev)
>>         if (IS_ERR(chip->regs))
>>                 return PTR_ERR(chip->regs);
>>
>> -       if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
>> -               chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
>> -               if (IS_ERR(chip->apb_regs))
>> -                       return PTR_ERR(chip->apb_regs);
>> +       ccfg = of_device_get_match_data(&pdev->dev);
>> +       if (ccfg) {
>> +               if (ccfg->apb_setup) {
>> +                       ret = ccfg->apb_setup(pdev, chip);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +
>> +               if (ccfg->reset_init) {
>> +                       ret = ccfg->reset_init(pdev);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +
>> +               chip->dw->hdata->use_cfg2 = ccfg->use_cfg2;
> 
> This claims and deasserts the resets before the clocks, whereas your
> previous versions did it after turning the clocks on. Which is the
> correct order?

The order has no problem, I have tested it with audio pwmdac on VisionFive2 board.

> 
> Also this certainly gets rid of of_device_is_compatible calls, but
> seems like a lot of code to do that. Did you consider something like
> 
> +#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0)
> +#define AXI_DMA_FLAG_HAS_RESETS BIT(1)
> +#define AXI_DMA_FLAG_USE_CFG2 BIT(2)
> 
> +unsigned int flags = (uintptr_t)device_get_match_data(&pdev->dev);
> 
> -if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
> +if (flags & AXI_DMA_FLAG_HAS_APB_REGS) {
> 
> -if (of_device_is_compatible(node, "starfive,jh7110-axi-dma)) {
> +if (flags & AXI_DMA_FLAG_HAS_RESETS) {
> 
> +chip->dw->hwdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);
> 
> -{ .compatible = "intel,kmb-axi-dma" },
> +{ .compatible = "intel,kmb-axi-dma", .data = (void
> *)AXI_DMA_FLAG_HAS_APB_REGS },
> +{ .compatible = "starive,jh7110-axi-dma", .data = (void
> *)(AXI_DMA_FLAG_HAS_RESETS | AXI_DMA_FLAG_USE_CFG2) },

Of course, it's better to have less code. I will try your method.

Thank you for taking the time to review and comment.

Best regards,
Walker

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

* Re: [PATCH v4 2/3] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA
@ 2023-03-07  1:38       ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-07  1:38 UTC (permalink / raw)
  To: Emil Renner Berthing
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv



On 2023/3/6 22:39, Emil Renner Berthing wrote:
> On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
>>
>> Add dma reset operation in device probe and use different configuration
>> on CH_CFG registers according to match data. Update all uses of
>> of_device_is_compatible with of_device_get_match_data.
>>
>> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> 
> Hi Walker,
> 
> Again please remove my Reviewed-by when you're adding a bunch of new
> code as you're doing here.

Sorry, I made another simple mistake.

> 
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>> ---
>>  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 67 ++++++++++++++++---
>>  drivers/dma/dw-axi-dmac/dw-axi-dmac.h         |  2 +
>>  2 files changed, 61 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> index bf85aa0979ec..d1148f6fbcf9 100644
>> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> @@ -21,10 +21,12 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>> +#include <linux/of_device.h>
>>  #include <linux/of_dma.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/property.h>
>> +#include <linux/reset.h>
>>  #include <linux/slab.h>
>>  #include <linux/types.h>
>>
>> @@ -46,6 +48,12 @@
>>         DMA_SLAVE_BUSWIDTH_32_BYTES     | \
>>         DMA_SLAVE_BUSWIDTH_64_BYTES)
>>
>> +struct axi_dma_chip_config {
>> +       int (*apb_setup)(struct platform_device *pdev, struct axi_dma_chip *chip);
>> +       int (*reset_init)(struct platform_device *pdev);
>> +       bool use_cfg2;
>> +};
>> +
>>  static inline void
>>  axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
>>  {
>> @@ -86,7 +94,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan,
>>
>>         cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS |
>>                   config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS);
>> -       if (chan->chip->dw->hdata->reg_map_8_channels) {
>> +       if (chan->chip->dw->hdata->reg_map_8_channels &&
>> +           !chan->chip->dw->hdata->use_cfg2) {
>>                 cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS |
>>                          config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS |
>>                          config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS |
>> @@ -1142,7 +1151,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
>>         axi_chan_disable(chan);
>>
>>         ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val,
>> -                                       !(val & chan_active), 1000, 10000);
>> +                                       !(val & chan_active), 1000, DMAC_TIMEOUT_US);
>>         if (ret == -ETIMEDOUT)
>>                 dev_warn(dchan2dev(dchan),
>>                          "%s failed to stop\n", axi_chan_name(chan));
>> @@ -1367,13 +1376,33 @@ static int parse_device_properties(struct axi_dma_chip *chip)
>>         return 0;
>>  }
>>
>> +static int intel_apb_setup(struct platform_device *pdev, struct axi_dma_chip *chip)
>> +{
>> +       chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
>> +       if (IS_ERR(chip->apb_regs))
>> +               return PTR_ERR(chip->apb_regs);
>> +       else
>> +               return 0;
>> +}
>> +
>> +static int jh7110_reset_init(struct platform_device *pdev)
>> +{
>> +       struct reset_control *resets;
>> +
>> +       resets = devm_reset_control_array_get_exclusive(&pdev->dev);
>> +       if (IS_ERR(resets))
>> +               return PTR_ERR(resets);
>> +
>> +       return reset_control_deassert(resets);
>> +}
>> +
>>  static int dw_probe(struct platform_device *pdev)
>>  {
>> -       struct device_node *node = pdev->dev.of_node;
>>         struct axi_dma_chip *chip;
>>         struct resource *mem;
>>         struct dw_axi_dma *dw;
>>         struct dw_axi_dma_hcfg *hdata;
>> +       const struct axi_dma_chip_config *ccfg;
>>         u32 i;
>>         int ret;
>>
>> @@ -1402,10 +1431,21 @@ static int dw_probe(struct platform_device *pdev)
>>         if (IS_ERR(chip->regs))
>>                 return PTR_ERR(chip->regs);
>>
>> -       if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
>> -               chip->apb_regs = devm_platform_ioremap_resource(pdev, 1);
>> -               if (IS_ERR(chip->apb_regs))
>> -                       return PTR_ERR(chip->apb_regs);
>> +       ccfg = of_device_get_match_data(&pdev->dev);
>> +       if (ccfg) {
>> +               if (ccfg->apb_setup) {
>> +                       ret = ccfg->apb_setup(pdev, chip);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +
>> +               if (ccfg->reset_init) {
>> +                       ret = ccfg->reset_init(pdev);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +
>> +               chip->dw->hdata->use_cfg2 = ccfg->use_cfg2;
> 
> This claims and deasserts the resets before the clocks, whereas your
> previous versions did it after turning the clocks on. Which is the
> correct order?

The order has no problem, I have tested it with audio pwmdac on VisionFive2 board.

> 
> Also this certainly gets rid of of_device_is_compatible calls, but
> seems like a lot of code to do that. Did you consider something like
> 
> +#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0)
> +#define AXI_DMA_FLAG_HAS_RESETS BIT(1)
> +#define AXI_DMA_FLAG_USE_CFG2 BIT(2)
> 
> +unsigned int flags = (uintptr_t)device_get_match_data(&pdev->dev);
> 
> -if (of_device_is_compatible(node, "intel,kmb-axi-dma")) {
> +if (flags & AXI_DMA_FLAG_HAS_APB_REGS) {
> 
> -if (of_device_is_compatible(node, "starfive,jh7110-axi-dma)) {
> +if (flags & AXI_DMA_FLAG_HAS_RESETS) {
> 
> +chip->dw->hwdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);
> 
> -{ .compatible = "intel,kmb-axi-dma" },
> +{ .compatible = "intel,kmb-axi-dma", .data = (void
> *)AXI_DMA_FLAG_HAS_APB_REGS },
> +{ .compatible = "starive,jh7110-axi-dma", .data = (void
> *)(AXI_DMA_FLAG_HAS_RESETS | AXI_DMA_FLAG_USE_CFG2) },

Of course, it's better to have less code. I will try your method.

Thank you for taking the time to review and comment.

Best regards,
Walker

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 3/3] riscv: dts: starfive: add dma controller node
  2023-03-06 14:51     ` Emil Renner Berthing
@ 2023-03-07  1:40       ` Walker Chen
  -1 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-07  1:40 UTC (permalink / raw)
  To: Emil Renner Berthing
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv



On 2023/3/6 22:51, Emil Renner Berthing wrote:
> On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
>> Add the dma controller node for the Starfive JH7110 SoC.
>>
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> 
> Thanks!
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Thank you for your review!

Best regards,
Walker


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

* Re: [PATCH v4 3/3] riscv: dts: starfive: add dma controller node
@ 2023-03-07  1:40       ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-07  1:40 UTC (permalink / raw)
  To: Emil Renner Berthing
  Cc: Eugeniy Paltsev, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Palmer Dabbelt, dmaengine, devicetree,
	linux-kernel, linux-riscv



On 2023/3/6 22:51, Emil Renner Berthing wrote:
> On Mon, 6 Mar 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote:
>> Add the dma controller node for the Starfive JH7110 SoC.
>>
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> 
> Thanks!
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

Thank you for your review!

Best regards,
Walker


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
  2023-03-06 14:04   ` Walker Chen
@ 2023-03-07  9:03     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-07  9:03 UTC (permalink / raw)
  To: Walker Chen, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

On 06/03/2023 15:04, Walker Chen wrote:
> The DMA controller needs two reset items to work properly on JH7110 SoC,
> so there is need to constrain the items' value to 2, other platforms
> have 1 reset item at most.
> 
> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> ---
>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> index ad107a4d3b33..d8b5439f215c 100644
> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> @@ -12,14 +12,12 @@ maintainers:
>  description:
>    Synopsys DesignWare AXI DMA Controller DT Binding
>  
> -allOf:
> -  - $ref: "dma-controller.yaml#"
> -
>  properties:
>    compatible:
>      enum:
>        - snps,axi-dma-1.01a
>        - intel,kmb-axi-dma
> +      - starfive,jh7110-axi-dma
>  
>    reg:
>      minItems: 1
> @@ -58,7 +56,8 @@ properties:
>      maximum: 8
>  
>    resets:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    snps,dma-masters:
>      description: |
> @@ -109,6 +108,23 @@ required:
>    - snps,priority
>    - snps,block-size
>  
> +allOf:
> +  - $ref: "dma-controller.yaml#"

Rebase your patches on something recent... I would argue that it should
be based on maintainer's (or linux-next) tree, but that would be too
good to be true.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - starfive,jh7110-axi-dma
> +    then:
> +      properties:
> +        resets:
> +          minItems: 2

What are the items expected here?

> +    else:
> +      properties:
> +        resets:

Best regards,
Krzysztof


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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
@ 2023-03-07  9:03     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-07  9:03 UTC (permalink / raw)
  To: Walker Chen, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

On 06/03/2023 15:04, Walker Chen wrote:
> The DMA controller needs two reset items to work properly on JH7110 SoC,
> so there is need to constrain the items' value to 2, other platforms
> have 1 reset item at most.
> 
> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> ---
>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> index ad107a4d3b33..d8b5439f215c 100644
> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> @@ -12,14 +12,12 @@ maintainers:
>  description:
>    Synopsys DesignWare AXI DMA Controller DT Binding
>  
> -allOf:
> -  - $ref: "dma-controller.yaml#"
> -
>  properties:
>    compatible:
>      enum:
>        - snps,axi-dma-1.01a
>        - intel,kmb-axi-dma
> +      - starfive,jh7110-axi-dma
>  
>    reg:
>      minItems: 1
> @@ -58,7 +56,8 @@ properties:
>      maximum: 8
>  
>    resets:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    snps,dma-masters:
>      description: |
> @@ -109,6 +108,23 @@ required:
>    - snps,priority
>    - snps,block-size
>  
> +allOf:
> +  - $ref: "dma-controller.yaml#"

Rebase your patches on something recent... I would argue that it should
be based on maintainer's (or linux-next) tree, but that would be too
good to be true.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - starfive,jh7110-axi-dma
> +    then:
> +      properties:
> +        resets:
> +          minItems: 2

What are the items expected here?

> +    else:
> +      properties:
> +        resets:

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
  2023-03-07  9:03     ` Krzysztof Kozlowski
@ 2023-03-07 10:30       ` Walker Chen
  -1 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-07 10:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv



On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
> On 06/03/2023 15:04, Walker Chen wrote:
>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>> so there is need to constrain the items' value to 2, other platforms
>> have 1 reset item at most.
>> 
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>> ---
>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> index ad107a4d3b33..d8b5439f215c 100644
>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> @@ -12,14 +12,12 @@ maintainers:
>>  description:
>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>  
>> -allOf:
>> -  - $ref: "dma-controller.yaml#"
>> -
>>  properties:
>>    compatible:
>>      enum:
>>        - snps,axi-dma-1.01a
>>        - intel,kmb-axi-dma
>> +      - starfive,jh7110-axi-dma
>>  
>>    reg:
>>      minItems: 1
>> @@ -58,7 +56,8 @@ properties:
>>      maximum: 8
>>  
>>    resets:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>>  
>>    snps,dma-masters:
>>      description: |
>> @@ -109,6 +108,23 @@ required:
>>    - snps,priority
>>    - snps,block-size
>>  
>> +allOf:
>> +  - $ref: "dma-controller.yaml#"
> 
> Rebase your patches on something recent... I would argue that it should
> be based on maintainer's (or linux-next) tree, but that would be too
> good to be true.

This was written by referring to the syntax of other dt-binding, but your suggestion 
is a good one, the next version of patches will be rebased on kernel 6.3.

> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - starfive,jh7110-axi-dma
>> +    then:
>> +      properties:
>> +        resets:
>> +          minItems: 2
> 
> What are the items expected here?

Do you mean to add descriptions for items here ?

Thanks!

Best regards,
Walker

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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
@ 2023-03-07 10:30       ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-07 10:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv



On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
> On 06/03/2023 15:04, Walker Chen wrote:
>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>> so there is need to constrain the items' value to 2, other platforms
>> have 1 reset item at most.
>> 
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>> ---
>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> index ad107a4d3b33..d8b5439f215c 100644
>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> @@ -12,14 +12,12 @@ maintainers:
>>  description:
>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>  
>> -allOf:
>> -  - $ref: "dma-controller.yaml#"
>> -
>>  properties:
>>    compatible:
>>      enum:
>>        - snps,axi-dma-1.01a
>>        - intel,kmb-axi-dma
>> +      - starfive,jh7110-axi-dma
>>  
>>    reg:
>>      minItems: 1
>> @@ -58,7 +56,8 @@ properties:
>>      maximum: 8
>>  
>>    resets:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>>  
>>    snps,dma-masters:
>>      description: |
>> @@ -109,6 +108,23 @@ required:
>>    - snps,priority
>>    - snps,block-size
>>  
>> +allOf:
>> +  - $ref: "dma-controller.yaml#"
> 
> Rebase your patches on something recent... I would argue that it should
> be based on maintainer's (or linux-next) tree, but that would be too
> good to be true.

This was written by referring to the syntax of other dt-binding, but your suggestion 
is a good one, the next version of patches will be rebased on kernel 6.3.

> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - starfive,jh7110-axi-dma
>> +    then:
>> +      properties:
>> +        resets:
>> +          minItems: 2
> 
> What are the items expected here?

Do you mean to add descriptions for items here ?

Thanks!

Best regards,
Walker

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
  2023-03-07 10:30       ` Walker Chen
@ 2023-03-07 15:51         ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-07 15:51 UTC (permalink / raw)
  To: Walker Chen, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

On 07/03/2023 11:30, Walker Chen wrote:
> 
> 
> On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
>> On 06/03/2023 15:04, Walker Chen wrote:
>>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>>> so there is need to constrain the items' value to 2, other platforms
>>> have 1 reset item at most.
>>>
>>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>>> ---
>>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> index ad107a4d3b33..d8b5439f215c 100644
>>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> @@ -12,14 +12,12 @@ maintainers:
>>>  description:
>>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>>  
>>> -allOf:
>>> -  - $ref: "dma-controller.yaml#"
>>> -
>>>  properties:
>>>    compatible:
>>>      enum:
>>>        - snps,axi-dma-1.01a
>>>        - intel,kmb-axi-dma
>>> +      - starfive,jh7110-axi-dma
>>>  
>>>    reg:
>>>      minItems: 1
>>> @@ -58,7 +56,8 @@ properties:
>>>      maximum: 8
>>>  
>>>    resets:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 2
>>>  
>>>    snps,dma-masters:
>>>      description: |
>>> @@ -109,6 +108,23 @@ required:
>>>    - snps,priority
>>>    - snps,block-size
>>>  
>>> +allOf:
>>> +  - $ref: "dma-controller.yaml#"
>>
>> Rebase your patches on something recent... I would argue that it should
>> be based on maintainer's (or linux-next) tree, but that would be too
>> good to be true.
> 
> This was written by referring to the syntax of other dt-binding, but your suggestion 
> is a good one, the next version of patches will be rebased on kernel 6.3.

Rebasing on old kernel was referring to syntax of other binding? I don't
understand this. How old code which you copied is related anyhow to
other binding? You are expected to send patches always based on recent
one, not something old.

> 
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - starfive,jh7110-axi-dma
>>> +    then:
>>> +      properties:
>>> +        resets:
>>> +          minItems: 2
>>
>> What are the items expected here?
> 
> Do you mean to add descriptions for items here ?

Yes, because order of the items is fixed.



Best regards,
Krzysztof


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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
@ 2023-03-07 15:51         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-07 15:51 UTC (permalink / raw)
  To: Walker Chen, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv

On 07/03/2023 11:30, Walker Chen wrote:
> 
> 
> On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
>> On 06/03/2023 15:04, Walker Chen wrote:
>>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>>> so there is need to constrain the items' value to 2, other platforms
>>> have 1 reset item at most.
>>>
>>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>>> ---
>>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> index ad107a4d3b33..d8b5439f215c 100644
>>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> @@ -12,14 +12,12 @@ maintainers:
>>>  description:
>>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>>  
>>> -allOf:
>>> -  - $ref: "dma-controller.yaml#"
>>> -
>>>  properties:
>>>    compatible:
>>>      enum:
>>>        - snps,axi-dma-1.01a
>>>        - intel,kmb-axi-dma
>>> +      - starfive,jh7110-axi-dma
>>>  
>>>    reg:
>>>      minItems: 1
>>> @@ -58,7 +56,8 @@ properties:
>>>      maximum: 8
>>>  
>>>    resets:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 2
>>>  
>>>    snps,dma-masters:
>>>      description: |
>>> @@ -109,6 +108,23 @@ required:
>>>    - snps,priority
>>>    - snps,block-size
>>>  
>>> +allOf:
>>> +  - $ref: "dma-controller.yaml#"
>>
>> Rebase your patches on something recent... I would argue that it should
>> be based on maintainer's (or linux-next) tree, but that would be too
>> good to be true.
> 
> This was written by referring to the syntax of other dt-binding, but your suggestion 
> is a good one, the next version of patches will be rebased on kernel 6.3.

Rebasing on old kernel was referring to syntax of other binding? I don't
understand this. How old code which you copied is related anyhow to
other binding? You are expected to send patches always based on recent
one, not something old.

> 
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - starfive,jh7110-axi-dma
>>> +    then:
>>> +      properties:
>>> +        resets:
>>> +          minItems: 2
>>
>> What are the items expected here?
> 
> Do you mean to add descriptions for items here ?

Yes, because order of the items is fixed.



Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
  2023-03-07 15:51         ` Krzysztof Kozlowski
@ 2023-03-08  1:08           ` Walker Chen
  -1 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-08  1:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv



On 2023/3/7 23:51, Krzysztof Kozlowski wrote:
> On 07/03/2023 11:30, Walker Chen wrote:
>> 
>> 
>> On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
>>> On 06/03/2023 15:04, Walker Chen wrote:
>>>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>>>> so there is need to constrain the items' value to 2, other platforms
>>>> have 1 reset item at most.
>>>>
>>>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>>>> ---
>>>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> index ad107a4d3b33..d8b5439f215c 100644
>>>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> @@ -12,14 +12,12 @@ maintainers:
>>>>  description:
>>>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>>>  
>>>> -allOf:
>>>> -  - $ref: "dma-controller.yaml#"
>>>> -
>>>>  properties:
>>>>    compatible:
>>>>      enum:
>>>>        - snps,axi-dma-1.01a
>>>>        - intel,kmb-axi-dma
>>>> +      - starfive,jh7110-axi-dma
>>>>  
>>>>    reg:
>>>>      minItems: 1
>>>> @@ -58,7 +56,8 @@ properties:
>>>>      maximum: 8
>>>>  
>>>>    resets:
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>>  
>>>>    snps,dma-masters:
>>>>      description: |
>>>> @@ -109,6 +108,23 @@ required:
>>>>    - snps,priority
>>>>    - snps,block-size
>>>>  
>>>> +allOf:
>>>> +  - $ref: "dma-controller.yaml#"
>>>
>>> Rebase your patches on something recent... I would argue that it should
>>> be based on maintainer's (or linux-next) tree, but that would be too
>>> good to be true.
>> 
>> This was written by referring to the syntax of other dt-binding, but your suggestion 
>> is a good one, the next version of patches will be rebased on kernel 6.3.
> 
> Rebasing on old kernel was referring to syntax of other binding? I don't
> understand this. How old code which you copied is related anyhow to
> other binding? You are expected to send patches always based on recent
> one, not something old.

Okay, I understand what you mean.

> 
>> 
>>>
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - starfive,jh7110-axi-dma
>>>> +    then:
>>>> +      properties:
>>>> +        resets:
>>>> +          minItems: 2
>>>
>>> What are the items expected here?
>> 
>> Do you mean to add descriptions for items here ?
> 
> Yes, because order of the items is fixed.

Thanks!

Best regards,
Walker

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

* Re: [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma
@ 2023-03-08  1:08           ` Walker Chen
  0 siblings, 0 replies; 24+ messages in thread
From: Walker Chen @ 2023-03-08  1:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Eugeniy Paltsev, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Palmer Dabbelt,
	Emil Renner Berthing
  Cc: dmaengine, devicetree, linux-kernel, linux-riscv



On 2023/3/7 23:51, Krzysztof Kozlowski wrote:
> On 07/03/2023 11:30, Walker Chen wrote:
>> 
>> 
>> On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
>>> On 06/03/2023 15:04, Walker Chen wrote:
>>>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>>>> so there is need to constrain the items' value to 2, other platforms
>>>> have 1 reset item at most.
>>>>
>>>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>>>> ---
>>>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> index ad107a4d3b33..d8b5439f215c 100644
>>>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> @@ -12,14 +12,12 @@ maintainers:
>>>>  description:
>>>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>>>  
>>>> -allOf:
>>>> -  - $ref: "dma-controller.yaml#"
>>>> -
>>>>  properties:
>>>>    compatible:
>>>>      enum:
>>>>        - snps,axi-dma-1.01a
>>>>        - intel,kmb-axi-dma
>>>> +      - starfive,jh7110-axi-dma
>>>>  
>>>>    reg:
>>>>      minItems: 1
>>>> @@ -58,7 +56,8 @@ properties:
>>>>      maximum: 8
>>>>  
>>>>    resets:
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>>  
>>>>    snps,dma-masters:
>>>>      description: |
>>>> @@ -109,6 +108,23 @@ required:
>>>>    - snps,priority
>>>>    - snps,block-size
>>>>  
>>>> +allOf:
>>>> +  - $ref: "dma-controller.yaml#"
>>>
>>> Rebase your patches on something recent... I would argue that it should
>>> be based on maintainer's (or linux-next) tree, but that would be too
>>> good to be true.
>> 
>> This was written by referring to the syntax of other dt-binding, but your suggestion 
>> is a good one, the next version of patches will be rebased on kernel 6.3.
> 
> Rebasing on old kernel was referring to syntax of other binding? I don't
> understand this. How old code which you copied is related anyhow to
> other binding? You are expected to send patches always based on recent
> one, not something old.

Okay, I understand what you mean.

> 
>> 
>>>
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - starfive,jh7110-axi-dma
>>>> +    then:
>>>> +      properties:
>>>> +        resets:
>>>> +          minItems: 2
>>>
>>> What are the items expected here?
>> 
>> Do you mean to add descriptions for items here ?
> 
> Yes, because order of the items is fixed.

Thanks!

Best regards,
Walker

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

end of thread, other threads:[~2023-03-08  1:09 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-06 14:04 [PATCH v4 0/3] Add DMA driver for StarFive JH7110 SoC Walker Chen
2023-03-06 14:04 ` Walker Chen
2023-03-06 14:04 ` [PATCH v4 1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma Walker Chen
2023-03-06 14:04   ` Walker Chen
2023-03-07  9:03   ` Krzysztof Kozlowski
2023-03-07  9:03     ` Krzysztof Kozlowski
2023-03-07 10:30     ` Walker Chen
2023-03-07 10:30       ` Walker Chen
2023-03-07 15:51       ` Krzysztof Kozlowski
2023-03-07 15:51         ` Krzysztof Kozlowski
2023-03-08  1:08         ` Walker Chen
2023-03-08  1:08           ` Walker Chen
2023-03-06 14:04 ` [PATCH v4 2/3] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA Walker Chen
2023-03-06 14:04   ` Walker Chen
2023-03-06 14:39   ` Emil Renner Berthing
2023-03-06 14:39     ` Emil Renner Berthing
2023-03-07  1:38     ` Walker Chen
2023-03-07  1:38       ` Walker Chen
2023-03-06 14:04 ` [PATCH v4 3/3] riscv: dts: starfive: add dma controller node Walker Chen
2023-03-06 14:04   ` Walker Chen
2023-03-06 14:51   ` Emil Renner Berthing
2023-03-06 14:51     ` Emil Renner Berthing
2023-03-07  1:40     ` Walker Chen
2023-03-07  1:40       ` Walker Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.