All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] dmaengine: sun6i: Fixes for H3/A83T, enable A64
@ 2017-09-03 22:40 ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

Commit 3a03ea763a67 ("dmaengine: sun6i: Add support for Allwinner A83T
(sun8i) variant") and commit f008db8c00c1 ("dmaengine: sun6i: Add support for
Allwinner H3 (sun8i) variant") added support for the A83T resp. H3, but missed
some differences between the original A31 and A83T/H3.

The first patch add a variable to group different SoC generations, i.e. A31,
A23+A83T, H3+later, and uses it to for the correct clock autogating setting.

The second and fourth patches reuse this variable to reflect changes in the
channel config register, i.e. different field offsets, new burst widths/lengths.

The third patch restructures some code required for the fourth patch.

Patch 5 restructures the code to decouple some controller details (e.g. channel
count) from the compatible string/the config.

Patches 6, 7 and 8 introduce and use the "dma-chans" property for the A64. Although
register compatible to the H3, the channel count differs and thus it requires a
new compatible. To avoid introduction of new compatibles for each minor variation,
anything but the register model is moved to devicetree properties. There
is at least one SoC (R40) which can then reuse the A64 compatible, the same
would have worked for A83T+V3s.

Patches 9 and 10 add the DMA controller node to the devicetree and add the DMA
controller reference to the SPI nodes.

This patch series could be called v2, but the patches were split and significantly
restructured, thus listing changes individually is not to meaningful.

Stefan Brüns (10):
  dmaengine: sun6i: Correct setting of clock autogating register for
    A83T/H3
  dmaengine: sun6i: Correct burst length field offsets for H3
  dmaengine: sun6i: Restructure code to allow extension for new SoCs
  dmaengine: sun6i: Enable additional burst lengths/widths on H3
  dmaengine: sun6i: Move number of pchans/vchans/request to device
    struct
  arm64: allwinner: a64: Add devicetree binding for DMA controller
  dmaengine: sun6i: Retrieve channel count/max request from devicetree
  dmaengine: sun6i: Add support for Allwinner A64 and compatibles
  arm64: allwinner: a64: Add device node for DMA controller
  arm64: allwinner: a64: add dma controller references to spi nodes

 .../devicetree/bindings/dma/sun6i-dma.txt          |  26 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      |  15 ++
 drivers/dma/sun6i-dma.c                            | 207 ++++++++++++++++-----
 3 files changed, 197 insertions(+), 51 deletions(-)

-- 
2.14.1

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

* [PATCH 00/10] dmaengine: sun6i: Fixes for H3/A83T, enable A64
@ 2017-09-03 22:40 ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper, Andre Przywara

Commit 3a03ea763a67 ("dmaengine: sun6i: Add support for Allwinner A83T
(sun8i) variant") and commit f008db8c00c1 ("dmaengine: sun6i: Add support for
Allwinner H3 (sun8i) variant") added support for the A83T resp. H3, but missed
some differences between the original A31 and A83T/H3.

The first patch add a variable to group different SoC generations, i.e. A31,
A23+A83T, H3+later, and uses it to for the correct clock autogating setting.

The second and fourth patches reuse this variable to reflect changes in the
channel config register, i.e. different field offsets, new burst widths/lengths.

The third patch restructures some code required for the fourth patch.

Patch 5 restructures the code to decouple some controller details (e.g. channel
count) from the compatible string/the config.

Patches 6, 7 and 8 introduce and use the "dma-chans" property for the A64. Although
register compatible to the H3, the channel count differs and thus it requires a
new compatible. To avoid introduction of new compatibles for each minor variation,
anything but the register model is moved to devicetree properties. There
is at least one SoC (R40) which can then reuse the A64 compatible, the same
would have worked for A83T+V3s.

Patches 9 and 10 add the DMA controller node to the devicetree and add the DMA
controller reference to the SPI nodes.

This patch series could be called v2, but the patches were split and significantly
restructured, thus listing changes individually is not to meaningful.

Stefan Brüns (10):
  dmaengine: sun6i: Correct setting of clock autogating register for
    A83T/H3
  dmaengine: sun6i: Correct burst length field offsets for H3
  dmaengine: sun6i: Restructure code to allow extension for new SoCs
  dmaengine: sun6i: Enable additional burst lengths/widths on H3
  dmaengine: sun6i: Move number of pchans/vchans/request to device
    struct
  arm64: allwinner: a64: Add devicetree binding for DMA controller
  dmaengine: sun6i: Retrieve channel count/max request from devicetree
  dmaengine: sun6i: Add support for Allwinner A64 and compatibles
  arm64: allwinner: a64: Add device node for DMA controller
  arm64: allwinner: a64: add dma controller references to spi nodes

 .../devicetree/bindings/dma/sun6i-dma.txt          |  26 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      |  15 ++
 drivers/dma/sun6i-dma.c                            | 207 ++++++++++++++++-----
 3 files changed, 197 insertions(+), 51 deletions(-)

-- 
2.14.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 00/10] dmaengine: sun6i: Fixes for H3/A83T, enable A64
@ 2017-09-03 22:40 ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 3a03ea763a67 ("dmaengine: sun6i: Add support for Allwinner A83T
(sun8i) variant") and commit f008db8c00c1 ("dmaengine: sun6i: Add support for
Allwinner H3 (sun8i) variant") added support for the A83T resp. H3, but missed
some differences between the original A31 and A83T/H3.

The first patch add a variable to group different SoC generations, i.e. A31,
A23+A83T, H3+later, and uses it to for the correct clock autogating setting.

The second and fourth patches reuse this variable to reflect changes in the
channel config register, i.e. different field offsets, new burst widths/lengths.

The third patch restructures some code required for the fourth patch.

Patch 5 restructures the code to decouple some controller details (e.g. channel
count) from the compatible string/the config.

Patches 6, 7 and 8 introduce and use the "dma-chans" property for the A64. Although
register compatible to the H3, the channel count differs and thus it requires a
new compatible. To avoid introduction of new compatibles for each minor variation,
anything but the register model is moved to devicetree properties. There
is at least one SoC (R40) which can then reuse the A64 compatible, the same
would have worked for A83T+V3s.

Patches 9 and 10 add the DMA controller node to the devicetree and add the DMA
controller reference to the SPI nodes.

This patch series could be called v2, but the patches were split and significantly
restructured, thus listing changes individually is not to meaningful.

Stefan Br?ns (10):
  dmaengine: sun6i: Correct setting of clock autogating register for
    A83T/H3
  dmaengine: sun6i: Correct burst length field offsets for H3
  dmaengine: sun6i: Restructure code to allow extension for new SoCs
  dmaengine: sun6i: Enable additional burst lengths/widths on H3
  dmaengine: sun6i: Move number of pchans/vchans/request to device
    struct
  arm64: allwinner: a64: Add devicetree binding for DMA controller
  dmaengine: sun6i: Retrieve channel count/max request from devicetree
  dmaengine: sun6i: Add support for Allwinner A64 and compatibles
  arm64: allwinner: a64: Add device node for DMA controller
  arm64: allwinner: a64: add dma controller references to spi nodes

 .../devicetree/bindings/dma/sun6i-dma.txt          |  26 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      |  15 ++
 drivers/dma/sun6i-dma.c                            | 207 ++++++++++++++++-----
 3 files changed, 197 insertions(+), 51 deletions(-)

-- 
2.14.1

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

* [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

The H83T uses a compatible string different from the A23, but requires
the same clock autogating register setting.

The H3 also requires setting the clock autogating register, but has
the register at a different offset.

Some currently available SoCs not yet supported by the sun6i-dma driver
will require new compatible strings. These SoCs either follow the A23
register model (e.g. V3s) or the H3 register model (A64, R40), so a new
variable is added to the config struct to group SoCs with common register
models.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a2358780ab2c..1d9b3be30d22 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -48,6 +48,9 @@
 #define SUN8I_DMA_GATE		0x20
 #define SUN8I_DMA_GATE_ENABLE	0x4
 
+#define SUNXI_H3_SECURITE_REG		0x20
+#define SUNXI_H3_DMA_GATE		0x28
+#define SUNXI_H3_DMA_GATE_ENABLE	0x4
 /*
  * Channels specific registers
  */
@@ -90,6 +93,21 @@
 #define NORMAL_WAIT	8
 #define DRQ_SDRAM	1
 
+/*
+ * DMA Controller generations
+ *
+ * Newer SoC generations changed or added some register definitions:
+ * - A23 added a clock gate register
+ * - H3 has a different offset for the clock gating register,
+ *   the burst length field has a different offset in the channel
+ *   configuration register, also additional burst lengths/widths.
+ */
+enum dmac_variant {
+	DMAC_VARIANT_A31,
+	DMAC_VARIANT_A23,
+	DMAC_VARIANT_H3,
+};
+
 /*
  * Hardware channels / ports representation
  *
@@ -101,6 +119,7 @@ struct sun6i_dma_config {
 	u32 nr_max_channels;
 	u32 nr_max_requests;
 	u32 nr_max_vchans;
+	enum dmac_variant dmac_variant;
 };
 
 /*
@@ -998,6 +1017,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
 	.nr_max_channels = 16,
 	.nr_max_requests = 30,
 	.nr_max_vchans   = 53,
+	.dmac_variant    = DMAC_VARIANT_A31,
 };
 
 /*
@@ -1009,23 +1029,29 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 24,
 	.nr_max_vchans   = 37,
+	.dmac_variant    = DMAC_VARIANT_A23,
 };
 
 static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 28,
 	.nr_max_vchans   = 39,
+	.dmac_variant    = DMAC_VARIANT_A23,
 };
 
 /*
  * The H3 has 12 physical channels, a maximum DRQ port id of 27,
  * and a total of 34 usable source and destination endpoints.
+ * It also supports additional burst lengths and bus widths,
+ * and the burst length fields have different offsets.
  */
 
 static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.nr_max_channels = 12,
 	.nr_max_requests = 27,
 	.nr_max_vchans   = 34,
+	.dmac_variant    = DMAC_VARIANT_H3,
+};
 };
 
 static const struct of_device_id sun6i_dma_match[] = {
@@ -1177,11 +1203,13 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	/*
 	 * sun8i variant requires us to toggle a dma gating register,
 	 * as seen in Allwinner's SDK. This register is not documented
-	 * in the A23 user manual.
+	 * in the A23 user manual, but appears in e.g. the H83T manual.
+	 * For the H3, H5 and A64, the register has a different location
 	 */
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "allwinner,sun8i-a23-dma"))
+	if (sdc->cfg->dmac_variant == DMAC_VARIANT_A23)
 		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
+	else if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3)
+		writel(SUNXI_H3_DMA_GATE_ENABLE, sdc->base + SUNXI_H3_DMA_GATE);
 
 	return 0;
 
-- 
2.14.1

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

* [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper, Andre Przywara

The H83T uses a compatible string different from the A23, but requires
the same clock autogating register setting.

The H3 also requires setting the clock autogating register, but has
the register at a different offset.

Some currently available SoCs not yet supported by the sun6i-dma driver
will require new compatible strings. These SoCs either follow the A23
register model (e.g. V3s) or the H3 register model (A64, R40), so a new
variable is added to the config struct to group SoCs with common register
models.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
 drivers/dma/sun6i-dma.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a2358780ab2c..1d9b3be30d22 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -48,6 +48,9 @@
 #define SUN8I_DMA_GATE		0x20
 #define SUN8I_DMA_GATE_ENABLE	0x4
 
+#define SUNXI_H3_SECURITE_REG		0x20
+#define SUNXI_H3_DMA_GATE		0x28
+#define SUNXI_H3_DMA_GATE_ENABLE	0x4
 /*
  * Channels specific registers
  */
@@ -90,6 +93,21 @@
 #define NORMAL_WAIT	8
 #define DRQ_SDRAM	1
 
+/*
+ * DMA Controller generations
+ *
+ * Newer SoC generations changed or added some register definitions:
+ * - A23 added a clock gate register
+ * - H3 has a different offset for the clock gating register,
+ *   the burst length field has a different offset in the channel
+ *   configuration register, also additional burst lengths/widths.
+ */
+enum dmac_variant {
+	DMAC_VARIANT_A31,
+	DMAC_VARIANT_A23,
+	DMAC_VARIANT_H3,
+};
+
 /*
  * Hardware channels / ports representation
  *
@@ -101,6 +119,7 @@ struct sun6i_dma_config {
 	u32 nr_max_channels;
 	u32 nr_max_requests;
 	u32 nr_max_vchans;
+	enum dmac_variant dmac_variant;
 };
 
 /*
@@ -998,6 +1017,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
 	.nr_max_channels = 16,
 	.nr_max_requests = 30,
 	.nr_max_vchans   = 53,
+	.dmac_variant    = DMAC_VARIANT_A31,
 };
 
 /*
@@ -1009,23 +1029,29 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 24,
 	.nr_max_vchans   = 37,
+	.dmac_variant    = DMAC_VARIANT_A23,
 };
 
 static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 28,
 	.nr_max_vchans   = 39,
+	.dmac_variant    = DMAC_VARIANT_A23,
 };
 
 /*
  * The H3 has 12 physical channels, a maximum DRQ port id of 27,
  * and a total of 34 usable source and destination endpoints.
+ * It also supports additional burst lengths and bus widths,
+ * and the burst length fields have different offsets.
  */
 
 static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.nr_max_channels = 12,
 	.nr_max_requests = 27,
 	.nr_max_vchans   = 34,
+	.dmac_variant    = DMAC_VARIANT_H3,
+};
 };
 
 static const struct of_device_id sun6i_dma_match[] = {
@@ -1177,11 +1203,13 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	/*
 	 * sun8i variant requires us to toggle a dma gating register,
 	 * as seen in Allwinner's SDK. This register is not documented
-	 * in the A23 user manual.
+	 * in the A23 user manual, but appears in e.g. the H83T manual.
+	 * For the H3, H5 and A64, the register has a different location
 	 */
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "allwinner,sun8i-a23-dma"))
+	if (sdc->cfg->dmac_variant == DMAC_VARIANT_A23)
 		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
+	else if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3)
+		writel(SUNXI_H3_DMA_GATE_ENABLE, sdc->base + SUNXI_H3_DMA_GATE);
 
 	return 0;
 
-- 
2.14.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

The H83T uses a compatible string different from the A23, but requires
the same clock autogating register setting.

The H3 also requires setting the clock autogating register, but has
the register at a different offset.

Some currently available SoCs not yet supported by the sun6i-dma driver
will require new compatible strings. These SoCs either follow the A23
register model (e.g. V3s) or the H3 register model (A64, R40), so a new
variable is added to the config struct to group SoCs with common register
models.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a2358780ab2c..1d9b3be30d22 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -48,6 +48,9 @@
 #define SUN8I_DMA_GATE		0x20
 #define SUN8I_DMA_GATE_ENABLE	0x4
 
+#define SUNXI_H3_SECURITE_REG		0x20
+#define SUNXI_H3_DMA_GATE		0x28
+#define SUNXI_H3_DMA_GATE_ENABLE	0x4
 /*
  * Channels specific registers
  */
@@ -90,6 +93,21 @@
 #define NORMAL_WAIT	8
 #define DRQ_SDRAM	1
 
+/*
+ * DMA Controller generations
+ *
+ * Newer SoC generations changed or added some register definitions:
+ * - A23 added a clock gate register
+ * - H3 has a different offset for the clock gating register,
+ *   the burst length field has a different offset in the channel
+ *   configuration register, also additional burst lengths/widths.
+ */
+enum dmac_variant {
+	DMAC_VARIANT_A31,
+	DMAC_VARIANT_A23,
+	DMAC_VARIANT_H3,
+};
+
 /*
  * Hardware channels / ports representation
  *
@@ -101,6 +119,7 @@ struct sun6i_dma_config {
 	u32 nr_max_channels;
 	u32 nr_max_requests;
 	u32 nr_max_vchans;
+	enum dmac_variant dmac_variant;
 };
 
 /*
@@ -998,6 +1017,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
 	.nr_max_channels = 16,
 	.nr_max_requests = 30,
 	.nr_max_vchans   = 53,
+	.dmac_variant    = DMAC_VARIANT_A31,
 };
 
 /*
@@ -1009,23 +1029,29 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 24,
 	.nr_max_vchans   = 37,
+	.dmac_variant    = DMAC_VARIANT_A23,
 };
 
 static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 28,
 	.nr_max_vchans   = 39,
+	.dmac_variant    = DMAC_VARIANT_A23,
 };
 
 /*
  * The H3 has 12 physical channels, a maximum DRQ port id of 27,
  * and a total of 34 usable source and destination endpoints.
+ * It also supports additional burst lengths and bus widths,
+ * and the burst length fields have different offsets.
  */
 
 static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.nr_max_channels = 12,
 	.nr_max_requests = 27,
 	.nr_max_vchans   = 34,
+	.dmac_variant    = DMAC_VARIANT_H3,
+};
 };
 
 static const struct of_device_id sun6i_dma_match[] = {
@@ -1177,11 +1203,13 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	/*
 	 * sun8i variant requires us to toggle a dma gating register,
 	 * as seen in Allwinner's SDK. This register is not documented
-	 * in the A23 user manual.
+	 * in the A23 user manual, but appears in e.g. the H83T manual.
+	 * For the H3, H5 and A64, the register has a different location
 	 */
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "allwinner,sun8i-a23-dma"))
+	if (sdc->cfg->dmac_variant == DMAC_VARIANT_A23)
 		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
+	else if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3)
+		writel(SUNXI_H3_DMA_GATE_ENABLE, sdc->base + SUNXI_H3_DMA_GATE);
 
 	return 0;
 
-- 
2.14.1

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

* [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
  2017-09-03 22:40 ` Stefan Brüns
  (?)
@ 2017-09-03 22:40   ` Stefan Brüns
  -1 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

For the H3, the burst lengths field offsets in the channel configuration
register differs from earlier SoC generations.

Using the A31 register macros actually configured the H3 controller
do to bursts of length 1 always, which although working leads to higher
bus utilisation.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 1d9b3be30d22..f1a139f0102f 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -68,13 +68,15 @@
 #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
-#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
+#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
+#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
 #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
 
 #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
 #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
 #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
-#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
+#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) << 16)
+#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 16)
 #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
 
 #define DMA_CHAN_CUR_SRC	0x10
@@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
 	if (dst_width < 0)
 		return dst_width;
 
-	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
-		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
-		DMA_CHAN_CFG_DST_BURST(dst_burst) |
+	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
 		DMA_CHAN_CFG_DST_WIDTH(dst_width);
 
+	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
+			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
+	} else {
+		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
+			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
+	}
+
 	return 0;
 }
 
@@ -601,11 +609,17 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 		DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
 		DMA_CHAN_CFG_DST_LINEAR_MODE |
 		DMA_CHAN_CFG_SRC_LINEAR_MODE |
-		DMA_CHAN_CFG_SRC_BURST(burst) |
 		DMA_CHAN_CFG_SRC_WIDTH(width) |
-		DMA_CHAN_CFG_DST_BURST(burst) |
 		DMA_CHAN_CFG_DST_WIDTH(width);
 
+	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		v_lli->cfg |= DMA_CHAN_CFG_SRC_BURST_H3(burst) |
+			      DMA_CHAN_CFG_DST_BURST_H3(burst);
+	} else {
+		v_lli->cfg |= DMA_CHAN_CFG_SRC_BURST_A31(burst) |
+			      DMA_CHAN_CFG_DST_BURST_A31(burst);
+	}
+
 	sun6i_dma_lli_add(NULL, v_lli, p_lli, txd);
 
 	sun6i_dma_dump_lli(vchan, v_lli);
-- 
2.14.1

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

* [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

For the H3, the burst lengths field offsets in the channel configuration
register differs from earlier SoC generations.

Using the A31 register macros actually configured the H3 controller
do to bursts of length 1 always, which although working leads to higher
bus utilisation.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 1d9b3be30d22..f1a139f0102f 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -68,13 +68,15 @@
 #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
-#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
+#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
+#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
 #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
 
 #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
 #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
 #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
-#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
+#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) << 16)
+#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 16)
 #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
 
 #define DMA_CHAN_CUR_SRC	0x10
@@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
 	if (dst_width < 0)
 		return dst_width;
 
-	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
-		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
-		DMA_CHAN_CFG_DST_BURST(dst_burst) |
+	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
 		DMA_CHAN_CFG_DST_WIDTH(dst_width);
 
+	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
+			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
+	} else {
+		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
+			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
+	}
+
 	return 0;
 }
 
@@ -601,11 +609,17 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 		DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
 		DMA_CHAN_CFG_DST_LINEAR_MODE |
 		DMA_CHAN_CFG_SRC_LINEAR_MODE |
-		DMA_CHAN_CFG_SRC_BURST(burst) |
 		DMA_CHAN_CFG_SRC_WIDTH(width) |
-		DMA_CHAN_CFG_DST_BURST(burst) |
 		DMA_CHAN_CFG_DST_WIDTH(width);
 
+	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		v_lli->cfg |= DMA_CHAN_CFG_SRC_BURST_H3(burst) |
+			      DMA_CHAN_CFG_DST_BURST_H3(burst);
+	} else {
+		v_lli->cfg |= DMA_CHAN_CFG_SRC_BURST_A31(burst) |
+			      DMA_CHAN_CFG_DST_BURST_A31(burst);
+	}
+
 	sun6i_dma_lli_add(NULL, v_lli, p_lli, txd);
 
 	sun6i_dma_dump_lli(vchan, v_lli);
-- 
2.14.1

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

* [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

For the H3, the burst lengths field offsets in the channel configuration
register differs from earlier SoC generations.

Using the A31 register macros actually configured the H3 controller
do to bursts of length 1 always, which although working leads to higher
bus utilisation.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 1d9b3be30d22..f1a139f0102f 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -68,13 +68,15 @@
 #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
-#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
+#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
+#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
 #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
 
 #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
 #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
 #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
-#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
+#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) << 16)
+#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 16)
 #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
 
 #define DMA_CHAN_CUR_SRC	0x10
@@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
 	if (dst_width < 0)
 		return dst_width;
 
-	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
-		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
-		DMA_CHAN_CFG_DST_BURST(dst_burst) |
+	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
 		DMA_CHAN_CFG_DST_WIDTH(dst_width);
 
+	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
+			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
+	} else {
+		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
+			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
+	}
+
 	return 0;
 }
 
@@ -601,11 +609,17 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 		DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
 		DMA_CHAN_CFG_DST_LINEAR_MODE |
 		DMA_CHAN_CFG_SRC_LINEAR_MODE |
-		DMA_CHAN_CFG_SRC_BURST(burst) |
 		DMA_CHAN_CFG_SRC_WIDTH(width) |
-		DMA_CHAN_CFG_DST_BURST(burst) |
 		DMA_CHAN_CFG_DST_WIDTH(width);
 
+	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		v_lli->cfg |= DMA_CHAN_CFG_SRC_BURST_H3(burst) |
+			      DMA_CHAN_CFG_DST_BURST_H3(burst);
+	} else {
+		v_lli->cfg |= DMA_CHAN_CFG_SRC_BURST_A31(burst) |
+			      DMA_CHAN_CFG_DST_BURST_A31(burst);
+	}
+
 	sun6i_dma_lli_add(NULL, v_lli, p_lli, txd);
 
 	sun6i_dma_dump_lli(vchan, v_lli);
-- 
2.14.1

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

* [PATCH 03/10] dmaengine: sun6i: Restructure code to allow extension for new SoCs
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

The current code mixes three distinct operations when transforming
the slave config to register settings:

  1. special handling of DMA_SLAVE_BUSWIDTH_UNDEFINED, maxburst == 0
  2. range checking
  3. conversion of raw to register values

As the range checks depend on the specific SoC, move these out of the
conversion to distinct operations.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 58 +++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index f1a139f0102f..c5644bd0f91a 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -185,6 +185,8 @@ struct sun6i_dma_dev {
 	struct sun6i_pchan	*pchans;
 	struct sun6i_vchan	*vchans;
 	const struct sun6i_dma_config *cfg;
+	u32			src_burst_lengths;
+	u32			dst_burst_lengths;
 };
 
 static struct device *chan2dev(struct dma_chan *chan)
@@ -270,10 +272,6 @@ static inline s8 convert_burst(u32 maxburst)
 
 static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 {
-	if ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||
-	    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
-		return -EINVAL;
-
 	return addr_width >> 1;
 }
 
@@ -520,41 +518,43 @@ static int set_config(struct sun6i_dma_dev *sdev,
 			enum dma_transfer_direction direction,
 			u32 *p_cfg)
 {
+	enum dma_slave_buswidth src_addr_width, dst_addr_width;
+	u32 src_maxburst, dst_maxburst;
 	s8 src_width, dst_width, src_burst, dst_burst;
 
+	src_addr_width = sconfig->src_addr_width;
+	dst_addr_width = sconfig->dst_addr_width;
+	src_maxburst = sconfig->src_maxburst;
+	dst_maxburst = sconfig->dst_maxburst;
+
 	switch (direction) {
 	case DMA_MEM_TO_DEV:
-		src_burst = convert_burst(sconfig->src_maxburst ?
-					sconfig->src_maxburst : 8);
-		src_width = convert_buswidth(sconfig->src_addr_width !=
-						DMA_SLAVE_BUSWIDTH_UNDEFINED ?
-				sconfig->src_addr_width :
-				DMA_SLAVE_BUSWIDTH_4_BYTES);
-		dst_burst = convert_burst(sconfig->dst_maxburst);
-		dst_width = convert_buswidth(sconfig->dst_addr_width);
+		if (src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		src_maxburst = src_maxburst ? src_maxburst : 8;
 		break;
 	case DMA_DEV_TO_MEM:
-		src_burst = convert_burst(sconfig->src_maxburst);
-		src_width = convert_buswidth(sconfig->src_addr_width);
-		dst_burst = convert_burst(sconfig->dst_maxburst ?
-					sconfig->dst_maxburst : 8);
-		dst_width = convert_buswidth(sconfig->dst_addr_width !=
-						DMA_SLAVE_BUSWIDTH_UNDEFINED ?
-				sconfig->dst_addr_width :
-				DMA_SLAVE_BUSWIDTH_4_BYTES);
+		if (dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		dst_maxburst = dst_maxburst ? dst_maxburst : 8;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	if (src_burst < 0)
-		return src_burst;
-	if (src_width < 0)
-		return src_width;
-	if (dst_burst < 0)
-		return dst_burst;
-	if (dst_width < 0)
-		return dst_width;
+	if (!(BIT(src_addr_width) & sdev->slave.src_addr_widths))
+		return -EINVAL;
+	if (!(BIT(dst_addr_width) & sdev->slave.dst_addr_widths))
+		return -EINVAL;
+	if (!(BIT(src_maxburst) & sdev->src_burst_lengths))
+		return -EINVAL;
+	if (!(BIT(dst_maxburst) & sdev->dst_burst_lengths))
+		return -EINVAL;
+
+	src_width = convert_buswidth(src_addr_width);
+	dst_width = convert_buswidth(dst_addr_width);
+	dst_burst = convert_burst(dst_maxburst);
+	src_burst = convert_burst(src_maxburst);
 
 	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
 		DMA_CHAN_CFG_DST_WIDTH(dst_width);
@@ -1150,6 +1150,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.dst_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
 						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
 						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+	sdc->src_burst_lengths			= BIT(1) | BIT(8);
+	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
 	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
 						  BIT(DMA_MEM_TO_DEV);
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
-- 
2.14.1

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

* [PATCH 03/10] dmaengine: sun6i: Restructure code to allow extension for new SoCs
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper, Andre Przywara

The current code mixes three distinct operations when transforming
the slave config to register settings:

  1. special handling of DMA_SLAVE_BUSWIDTH_UNDEFINED, maxburst == 0
  2. range checking
  3. conversion of raw to register values

As the range checks depend on the specific SoC, move these out of the
conversion to distinct operations.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
 drivers/dma/sun6i-dma.c | 58 +++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index f1a139f0102f..c5644bd0f91a 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -185,6 +185,8 @@ struct sun6i_dma_dev {
 	struct sun6i_pchan	*pchans;
 	struct sun6i_vchan	*vchans;
 	const struct sun6i_dma_config *cfg;
+	u32			src_burst_lengths;
+	u32			dst_burst_lengths;
 };
 
 static struct device *chan2dev(struct dma_chan *chan)
@@ -270,10 +272,6 @@ static inline s8 convert_burst(u32 maxburst)
 
 static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 {
-	if ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||
-	    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
-		return -EINVAL;
-
 	return addr_width >> 1;
 }
 
@@ -520,41 +518,43 @@ static int set_config(struct sun6i_dma_dev *sdev,
 			enum dma_transfer_direction direction,
 			u32 *p_cfg)
 {
+	enum dma_slave_buswidth src_addr_width, dst_addr_width;
+	u32 src_maxburst, dst_maxburst;
 	s8 src_width, dst_width, src_burst, dst_burst;
 
+	src_addr_width = sconfig->src_addr_width;
+	dst_addr_width = sconfig->dst_addr_width;
+	src_maxburst = sconfig->src_maxburst;
+	dst_maxburst = sconfig->dst_maxburst;
+
 	switch (direction) {
 	case DMA_MEM_TO_DEV:
-		src_burst = convert_burst(sconfig->src_maxburst ?
-					sconfig->src_maxburst : 8);
-		src_width = convert_buswidth(sconfig->src_addr_width !=
-						DMA_SLAVE_BUSWIDTH_UNDEFINED ?
-				sconfig->src_addr_width :
-				DMA_SLAVE_BUSWIDTH_4_BYTES);
-		dst_burst = convert_burst(sconfig->dst_maxburst);
-		dst_width = convert_buswidth(sconfig->dst_addr_width);
+		if (src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		src_maxburst = src_maxburst ? src_maxburst : 8;
 		break;
 	case DMA_DEV_TO_MEM:
-		src_burst = convert_burst(sconfig->src_maxburst);
-		src_width = convert_buswidth(sconfig->src_addr_width);
-		dst_burst = convert_burst(sconfig->dst_maxburst ?
-					sconfig->dst_maxburst : 8);
-		dst_width = convert_buswidth(sconfig->dst_addr_width !=
-						DMA_SLAVE_BUSWIDTH_UNDEFINED ?
-				sconfig->dst_addr_width :
-				DMA_SLAVE_BUSWIDTH_4_BYTES);
+		if (dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		dst_maxburst = dst_maxburst ? dst_maxburst : 8;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	if (src_burst < 0)
-		return src_burst;
-	if (src_width < 0)
-		return src_width;
-	if (dst_burst < 0)
-		return dst_burst;
-	if (dst_width < 0)
-		return dst_width;
+	if (!(BIT(src_addr_width) & sdev->slave.src_addr_widths))
+		return -EINVAL;
+	if (!(BIT(dst_addr_width) & sdev->slave.dst_addr_widths))
+		return -EINVAL;
+	if (!(BIT(src_maxburst) & sdev->src_burst_lengths))
+		return -EINVAL;
+	if (!(BIT(dst_maxburst) & sdev->dst_burst_lengths))
+		return -EINVAL;
+
+	src_width = convert_buswidth(src_addr_width);
+	dst_width = convert_buswidth(dst_addr_width);
+	dst_burst = convert_burst(dst_maxburst);
+	src_burst = convert_burst(src_maxburst);
 
 	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
 		DMA_CHAN_CFG_DST_WIDTH(dst_width);
@@ -1150,6 +1150,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.dst_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
 						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
 						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+	sdc->src_burst_lengths			= BIT(1) | BIT(8);
+	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
 	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
 						  BIT(DMA_MEM_TO_DEV);
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
-- 
2.14.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 03/10] dmaengine: sun6i: Restructure code to allow extension for new SoCs
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

The current code mixes three distinct operations when transforming
the slave config to register settings:

  1. special handling of DMA_SLAVE_BUSWIDTH_UNDEFINED, maxburst == 0
  2. range checking
  3. conversion of raw to register values

As the range checks depend on the specific SoC, move these out of the
conversion to distinct operations.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 58 +++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index f1a139f0102f..c5644bd0f91a 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -185,6 +185,8 @@ struct sun6i_dma_dev {
 	struct sun6i_pchan	*pchans;
 	struct sun6i_vchan	*vchans;
 	const struct sun6i_dma_config *cfg;
+	u32			src_burst_lengths;
+	u32			dst_burst_lengths;
 };
 
 static struct device *chan2dev(struct dma_chan *chan)
@@ -270,10 +272,6 @@ static inline s8 convert_burst(u32 maxburst)
 
 static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 {
-	if ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||
-	    (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
-		return -EINVAL;
-
 	return addr_width >> 1;
 }
 
@@ -520,41 +518,43 @@ static int set_config(struct sun6i_dma_dev *sdev,
 			enum dma_transfer_direction direction,
 			u32 *p_cfg)
 {
+	enum dma_slave_buswidth src_addr_width, dst_addr_width;
+	u32 src_maxburst, dst_maxburst;
 	s8 src_width, dst_width, src_burst, dst_burst;
 
+	src_addr_width = sconfig->src_addr_width;
+	dst_addr_width = sconfig->dst_addr_width;
+	src_maxburst = sconfig->src_maxburst;
+	dst_maxburst = sconfig->dst_maxburst;
+
 	switch (direction) {
 	case DMA_MEM_TO_DEV:
-		src_burst = convert_burst(sconfig->src_maxburst ?
-					sconfig->src_maxburst : 8);
-		src_width = convert_buswidth(sconfig->src_addr_width !=
-						DMA_SLAVE_BUSWIDTH_UNDEFINED ?
-				sconfig->src_addr_width :
-				DMA_SLAVE_BUSWIDTH_4_BYTES);
-		dst_burst = convert_burst(sconfig->dst_maxburst);
-		dst_width = convert_buswidth(sconfig->dst_addr_width);
+		if (src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		src_maxburst = src_maxburst ? src_maxburst : 8;
 		break;
 	case DMA_DEV_TO_MEM:
-		src_burst = convert_burst(sconfig->src_maxburst);
-		src_width = convert_buswidth(sconfig->src_addr_width);
-		dst_burst = convert_burst(sconfig->dst_maxburst ?
-					sconfig->dst_maxburst : 8);
-		dst_width = convert_buswidth(sconfig->dst_addr_width !=
-						DMA_SLAVE_BUSWIDTH_UNDEFINED ?
-				sconfig->dst_addr_width :
-				DMA_SLAVE_BUSWIDTH_4_BYTES);
+		if (dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		dst_maxburst = dst_maxburst ? dst_maxburst : 8;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	if (src_burst < 0)
-		return src_burst;
-	if (src_width < 0)
-		return src_width;
-	if (dst_burst < 0)
-		return dst_burst;
-	if (dst_width < 0)
-		return dst_width;
+	if (!(BIT(src_addr_width) & sdev->slave.src_addr_widths))
+		return -EINVAL;
+	if (!(BIT(dst_addr_width) & sdev->slave.dst_addr_widths))
+		return -EINVAL;
+	if (!(BIT(src_maxburst) & sdev->src_burst_lengths))
+		return -EINVAL;
+	if (!(BIT(dst_maxburst) & sdev->dst_burst_lengths))
+		return -EINVAL;
+
+	src_width = convert_buswidth(src_addr_width);
+	dst_width = convert_buswidth(dst_addr_width);
+	dst_burst = convert_burst(dst_maxburst);
+	src_burst = convert_burst(src_maxburst);
 
 	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
 		DMA_CHAN_CFG_DST_WIDTH(dst_width);
@@ -1150,6 +1150,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.dst_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
 						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
 						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+	sdc->src_burst_lengths			= BIT(1) | BIT(8);
+	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
 	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
 						  BIT(DMA_MEM_TO_DEV);
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
-- 
2.14.1

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

* [PATCH 04/10] dmaengine: sun6i: Enable additional burst lengths/widths on H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

The H3 supports bursts lengths of 1, 4, 8 and 16 transfers, each with
a width of 1, 2, 4 or 8 bytes.

The register value for the the width is log2-encoded, change the
conversion function to provide the correct value for width == 8.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c5644bd0f91a..335a8ec88b0b 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -263,8 +263,12 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;
 	case 8:
 		return 2;
+	case 16:
+		return 3;
 	default:
 		return -EINVAL;
 	}
@@ -272,7 +276,7 @@ static inline s8 convert_burst(u32 maxburst)
 
 static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 {
-	return addr_width >> 1;
+	return ilog2(addr_width);
 }
 
 static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
@@ -1152,6 +1156,12 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
 	sdc->src_burst_lengths			= BIT(1) | BIT(8);
 	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
+	if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		sdc->slave.src_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
+		sdc->slave.dst_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
+		sdc->src_burst_lengths     |= BIT(4) | BIT(16);
+		sdc->dst_burst_lengths     |= BIT(4) | BIT(16);
+	}
 	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
 						  BIT(DMA_MEM_TO_DEV);
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
-- 
2.14.1

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

* [PATCH 04/10] dmaengine: sun6i: Enable additional burst lengths/widths on H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper, Andre Przywara

The H3 supports bursts lengths of 1, 4, 8 and 16 transfers, each with
a width of 1, 2, 4 or 8 bytes.

The register value for the the width is log2-encoded, change the
conversion function to provide the correct value for width == 8.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
 drivers/dma/sun6i-dma.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c5644bd0f91a..335a8ec88b0b 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -263,8 +263,12 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;
 	case 8:
 		return 2;
+	case 16:
+		return 3;
 	default:
 		return -EINVAL;
 	}
@@ -272,7 +276,7 @@ static inline s8 convert_burst(u32 maxburst)
 
 static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 {
-	return addr_width >> 1;
+	return ilog2(addr_width);
 }
 
 static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
@@ -1152,6 +1156,12 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
 	sdc->src_burst_lengths			= BIT(1) | BIT(8);
 	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
+	if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		sdc->slave.src_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
+		sdc->slave.dst_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
+		sdc->src_burst_lengths     |= BIT(4) | BIT(16);
+		sdc->dst_burst_lengths     |= BIT(4) | BIT(16);
+	}
 	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
 						  BIT(DMA_MEM_TO_DEV);
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
-- 
2.14.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 04/10] dmaengine: sun6i: Enable additional burst lengths/widths on H3
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

The H3 supports bursts lengths of 1, 4, 8 and 16 transfers, each with
a width of 1, 2, 4 or 8 bytes.

The register value for the the width is log2-encoded, change the
conversion function to provide the correct value for width == 8.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c5644bd0f91a..335a8ec88b0b 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -263,8 +263,12 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;
 	case 8:
 		return 2;
+	case 16:
+		return 3;
 	default:
 		return -EINVAL;
 	}
@@ -272,7 +276,7 @@ static inline s8 convert_burst(u32 maxburst)
 
 static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 {
-	return addr_width >> 1;
+	return ilog2(addr_width);
 }
 
 static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
@@ -1152,6 +1156,12 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
 	sdc->src_burst_lengths			= BIT(1) | BIT(8);
 	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
+	if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3) {
+		sdc->slave.src_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
+		sdc->slave.dst_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
+		sdc->src_burst_lengths     |= BIT(4) | BIT(16);
+		sdc->dst_burst_lengths     |= BIT(4) | BIT(16);
+	}
 	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
 						  BIT(DMA_MEM_TO_DEV);
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
-- 
2.14.1

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

* [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

Preparatory patch: If the same compatible is used for different SoCs which
have a common register layout, but different number of channels, the
channel count can no longer be stored in the config. Store it in the
device structure instead.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 335a8ec88b0b..c69dadb853d2 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -187,6 +187,9 @@ struct sun6i_dma_dev {
 	const struct sun6i_dma_config *cfg;
 	u32			src_burst_lengths;
 	u32			dst_burst_lengths;
+	u32			num_pchans;
+	u32			num_vchans;
+	u32			max_request;
 };
 
 static struct device *chan2dev(struct dma_chan *chan)
@@ -411,7 +414,6 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 static void sun6i_dma_tasklet(unsigned long data)
 {
 	struct sun6i_dma_dev *sdev = (struct sun6i_dma_dev *)data;
-	const struct sun6i_dma_config *cfg = sdev->cfg;
 	struct sun6i_vchan *vchan;
 	struct sun6i_pchan *pchan;
 	unsigned int pchan_alloc = 0;
@@ -439,7 +441,7 @@ static void sun6i_dma_tasklet(unsigned long data)
 	}
 
 	spin_lock_irq(&sdev->lock);
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
+	for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
 		pchan = &sdev->pchans[pchan_idx];
 
 		if (pchan->vchan || list_empty(&sdev->pending))
@@ -460,7 +462,7 @@ static void sun6i_dma_tasklet(unsigned long data)
 	}
 	spin_unlock_irq(&sdev->lock);
 
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
+	for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
 		if (!(pchan_alloc & BIT(pchan_idx)))
 			continue;
 
@@ -482,7 +484,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 	int i, j, ret = IRQ_NONE;
 	u32 status;
 
-	for (i = 0; i < sdev->cfg->nr_max_channels / DMA_IRQ_CHAN_NR; i++) {
+	for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
 		status = readl(sdev->base + DMA_IRQ_STAT(i));
 		if (!status)
 			continue;
@@ -974,7 +976,7 @@ static struct dma_chan *sun6i_dma_of_xlate(struct of_phandle_args *dma_spec,
 	struct dma_chan *chan;
 	u8 port = dma_spec->args[0];
 
-	if (port > sdev->cfg->nr_max_requests)
+	if (port > sdev->max_request)
 		return NULL;
 
 	chan = dma_get_any_slave_channel(&sdev->slave);
@@ -1007,7 +1009,7 @@ static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
 {
 	int i;
 
-	for (i = 0; i < sdev->cfg->nr_max_vchans; i++) {
+	for (i = 0; i < sdev->num_vchans; i++) {
 		struct sun6i_vchan *vchan = &sdev->vchans[i];
 
 		list_del(&vchan->vc.chan.device_node);
@@ -1167,26 +1169,30 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
 	sdc->slave.dev = &pdev->dev;
 
-	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_channels,
+	sdc->num_pchans = sdc->cfg->nr_max_channels;
+	sdc->num_vchans = sdc->cfg->nr_max_vchans;
+	sdc->max_request = sdc->cfg->nr_max_requests;
+
+	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
 				   sizeof(struct sun6i_pchan), GFP_KERNEL);
 	if (!sdc->pchans)
 		return -ENOMEM;
 
-	sdc->vchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_vchans,
+	sdc->vchans = devm_kcalloc(&pdev->dev, sdc->num_vchans,
 				   sizeof(struct sun6i_vchan), GFP_KERNEL);
 	if (!sdc->vchans)
 		return -ENOMEM;
 
 	tasklet_init(&sdc->task, sun6i_dma_tasklet, (unsigned long)sdc);
 
-	for (i = 0; i < sdc->cfg->nr_max_channels; i++) {
+	for (i = 0; i < sdc->num_pchans; i++) {
 		struct sun6i_pchan *pchan = &sdc->pchans[i];
 
 		pchan->idx = i;
 		pchan->base = sdc->base + 0x100 + i * 0x40;
 	}
 
-	for (i = 0; i < sdc->cfg->nr_max_vchans; i++) {
+	for (i = 0; i < sdc->num_vchans; i++) {
 		struct sun6i_vchan *vchan = &sdc->vchans[i];
 
 		INIT_LIST_HEAD(&vchan->node);
-- 
2.14.1

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

* [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper, Andre Przywara

Preparatory patch: If the same compatible is used for different SoCs which
have a common register layout, but different number of channels, the
channel count can no longer be stored in the config. Store it in the
device structure instead.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
 drivers/dma/sun6i-dma.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 335a8ec88b0b..c69dadb853d2 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -187,6 +187,9 @@ struct sun6i_dma_dev {
 	const struct sun6i_dma_config *cfg;
 	u32			src_burst_lengths;
 	u32			dst_burst_lengths;
+	u32			num_pchans;
+	u32			num_vchans;
+	u32			max_request;
 };
 
 static struct device *chan2dev(struct dma_chan *chan)
@@ -411,7 +414,6 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 static void sun6i_dma_tasklet(unsigned long data)
 {
 	struct sun6i_dma_dev *sdev = (struct sun6i_dma_dev *)data;
-	const struct sun6i_dma_config *cfg = sdev->cfg;
 	struct sun6i_vchan *vchan;
 	struct sun6i_pchan *pchan;
 	unsigned int pchan_alloc = 0;
@@ -439,7 +441,7 @@ static void sun6i_dma_tasklet(unsigned long data)
 	}
 
 	spin_lock_irq(&sdev->lock);
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
+	for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
 		pchan = &sdev->pchans[pchan_idx];
 
 		if (pchan->vchan || list_empty(&sdev->pending))
@@ -460,7 +462,7 @@ static void sun6i_dma_tasklet(unsigned long data)
 	}
 	spin_unlock_irq(&sdev->lock);
 
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
+	for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
 		if (!(pchan_alloc & BIT(pchan_idx)))
 			continue;
 
@@ -482,7 +484,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 	int i, j, ret = IRQ_NONE;
 	u32 status;
 
-	for (i = 0; i < sdev->cfg->nr_max_channels / DMA_IRQ_CHAN_NR; i++) {
+	for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
 		status = readl(sdev->base + DMA_IRQ_STAT(i));
 		if (!status)
 			continue;
@@ -974,7 +976,7 @@ static struct dma_chan *sun6i_dma_of_xlate(struct of_phandle_args *dma_spec,
 	struct dma_chan *chan;
 	u8 port = dma_spec->args[0];
 
-	if (port > sdev->cfg->nr_max_requests)
+	if (port > sdev->max_request)
 		return NULL;
 
 	chan = dma_get_any_slave_channel(&sdev->slave);
@@ -1007,7 +1009,7 @@ static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
 {
 	int i;
 
-	for (i = 0; i < sdev->cfg->nr_max_vchans; i++) {
+	for (i = 0; i < sdev->num_vchans; i++) {
 		struct sun6i_vchan *vchan = &sdev->vchans[i];
 
 		list_del(&vchan->vc.chan.device_node);
@@ -1167,26 +1169,30 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
 	sdc->slave.dev = &pdev->dev;
 
-	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_channels,
+	sdc->num_pchans = sdc->cfg->nr_max_channels;
+	sdc->num_vchans = sdc->cfg->nr_max_vchans;
+	sdc->max_request = sdc->cfg->nr_max_requests;
+
+	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
 				   sizeof(struct sun6i_pchan), GFP_KERNEL);
 	if (!sdc->pchans)
 		return -ENOMEM;
 
-	sdc->vchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_vchans,
+	sdc->vchans = devm_kcalloc(&pdev->dev, sdc->num_vchans,
 				   sizeof(struct sun6i_vchan), GFP_KERNEL);
 	if (!sdc->vchans)
 		return -ENOMEM;
 
 	tasklet_init(&sdc->task, sun6i_dma_tasklet, (unsigned long)sdc);
 
-	for (i = 0; i < sdc->cfg->nr_max_channels; i++) {
+	for (i = 0; i < sdc->num_pchans; i++) {
 		struct sun6i_pchan *pchan = &sdc->pchans[i];
 
 		pchan->idx = i;
 		pchan->base = sdc->base + 0x100 + i * 0x40;
 	}
 
-	for (i = 0; i < sdc->cfg->nr_max_vchans; i++) {
+	for (i = 0; i < sdc->num_vchans; i++) {
 		struct sun6i_vchan *vchan = &sdc->vchans[i];
 
 		INIT_LIST_HEAD(&vchan->node);
-- 
2.14.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

Preparatory patch: If the same compatible is used for different SoCs which
have a common register layout, but different number of channels, the
channel count can no longer be stored in the config. Store it in the
device structure instead.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 335a8ec88b0b..c69dadb853d2 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -187,6 +187,9 @@ struct sun6i_dma_dev {
 	const struct sun6i_dma_config *cfg;
 	u32			src_burst_lengths;
 	u32			dst_burst_lengths;
+	u32			num_pchans;
+	u32			num_vchans;
+	u32			max_request;
 };
 
 static struct device *chan2dev(struct dma_chan *chan)
@@ -411,7 +414,6 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 static void sun6i_dma_tasklet(unsigned long data)
 {
 	struct sun6i_dma_dev *sdev = (struct sun6i_dma_dev *)data;
-	const struct sun6i_dma_config *cfg = sdev->cfg;
 	struct sun6i_vchan *vchan;
 	struct sun6i_pchan *pchan;
 	unsigned int pchan_alloc = 0;
@@ -439,7 +441,7 @@ static void sun6i_dma_tasklet(unsigned long data)
 	}
 
 	spin_lock_irq(&sdev->lock);
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
+	for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
 		pchan = &sdev->pchans[pchan_idx];
 
 		if (pchan->vchan || list_empty(&sdev->pending))
@@ -460,7 +462,7 @@ static void sun6i_dma_tasklet(unsigned long data)
 	}
 	spin_unlock_irq(&sdev->lock);
 
-	for (pchan_idx = 0; pchan_idx < cfg->nr_max_channels; pchan_idx++) {
+	for (pchan_idx = 0; pchan_idx < sdev->num_pchans; pchan_idx++) {
 		if (!(pchan_alloc & BIT(pchan_idx)))
 			continue;
 
@@ -482,7 +484,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 	int i, j, ret = IRQ_NONE;
 	u32 status;
 
-	for (i = 0; i < sdev->cfg->nr_max_channels / DMA_IRQ_CHAN_NR; i++) {
+	for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
 		status = readl(sdev->base + DMA_IRQ_STAT(i));
 		if (!status)
 			continue;
@@ -974,7 +976,7 @@ static struct dma_chan *sun6i_dma_of_xlate(struct of_phandle_args *dma_spec,
 	struct dma_chan *chan;
 	u8 port = dma_spec->args[0];
 
-	if (port > sdev->cfg->nr_max_requests)
+	if (port > sdev->max_request)
 		return NULL;
 
 	chan = dma_get_any_slave_channel(&sdev->slave);
@@ -1007,7 +1009,7 @@ static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
 {
 	int i;
 
-	for (i = 0; i < sdev->cfg->nr_max_vchans; i++) {
+	for (i = 0; i < sdev->num_vchans; i++) {
 		struct sun6i_vchan *vchan = &sdev->vchans[i];
 
 		list_del(&vchan->vc.chan.device_node);
@@ -1167,26 +1169,30 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
 	sdc->slave.dev = &pdev->dev;
 
-	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_channels,
+	sdc->num_pchans = sdc->cfg->nr_max_channels;
+	sdc->num_vchans = sdc->cfg->nr_max_vchans;
+	sdc->max_request = sdc->cfg->nr_max_requests;
+
+	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
 				   sizeof(struct sun6i_pchan), GFP_KERNEL);
 	if (!sdc->pchans)
 		return -ENOMEM;
 
-	sdc->vchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_vchans,
+	sdc->vchans = devm_kcalloc(&pdev->dev, sdc->num_vchans,
 				   sizeof(struct sun6i_vchan), GFP_KERNEL);
 	if (!sdc->vchans)
 		return -ENOMEM;
 
 	tasklet_init(&sdc->task, sun6i_dma_tasklet, (unsigned long)sdc);
 
-	for (i = 0; i < sdc->cfg->nr_max_channels; i++) {
+	for (i = 0; i < sdc->num_pchans; i++) {
 		struct sun6i_pchan *pchan = &sdc->pchans[i];
 
 		pchan->idx = i;
 		pchan->base = sdc->base + 0x100 + i * 0x40;
 	}
 
-	for (i = 0; i < sdc->cfg->nr_max_vchans; i++) {
+	for (i = 0; i < sdc->num_vchans; i++) {
 		struct sun6i_vchan *vchan = &sdc->vchans[i];
 
 		INIT_LIST_HEAD(&vchan->node);
-- 
2.14.1

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

* [PATCH 06/10] arm64: allwinner: a64: Add devicetree binding for DMA controller
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

The A64 is register compatible with the H3, but has a different number
of dma channels and request ports.

Attach additional properties to the node to allow future reuse of the
compatible for controllers with different number of channels/requests.

If dma-requests is not specified, the register layout defined maximum
of 32 is used.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 .../devicetree/bindings/dma/sun6i-dma.txt          | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
index 6b267045f522..66195fb31296 100644
--- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
+++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
@@ -26,6 +26,32 @@ Example:
 		#dma-cells = <1>;
 	};
 
+------------------------------------------------------------------------------
+For A64 DMA controller:
+
+Required properties:
+- compatible:	"allwinner,sun50i-a64-dma"
+- dma-channels: Number of DMA channels supported by the controller.
+		Refer to Documentation/devicetree/bindings/dma/dma.txt
+- all properties above, i.e. reg, interrupts, clocks, resets and #dma-cells
+
+Optional properties:
+- dma-requests: Number of DMA request signals supported by the controller.
+		Refer to Documentation/devicetree/bindings/dma/dma.txt
+
+Example:
+	dma: dma-controller@01c02000 {
+		compatible = "allwinner,sun50i-a64-dma";
+		reg = <0x01c02000 0x1000>;
+		interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&ccu CLK_BUS_DMA>;
+		dma-channels = <8>;
+		dma-requests = <27>;
+		resets = <&ccu RST_BUS_DMA>;
+		#dma-cells = <1>;
+	};
+------------------------------------------------------------------------------
+
 Clients:
 
 DMA clients connected to the A31 DMA controller must use the format
-- 
2.14.1

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

* [PATCH 06/10] arm64: allwinner: a64: Add devicetree binding for DMA controller
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper, Andre Przywara

The A64 is register compatible with the H3, but has a different number
of dma channels and request ports.

Attach additional properties to the node to allow future reuse of the
compatible for controllers with different number of channels/requests.

If dma-requests is not specified, the register layout defined maximum
of 32 is used.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
 .../devicetree/bindings/dma/sun6i-dma.txt          | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
index 6b267045f522..66195fb31296 100644
--- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
+++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
@@ -26,6 +26,32 @@ Example:
 		#dma-cells = <1>;
 	};
 
+------------------------------------------------------------------------------
+For A64 DMA controller:
+
+Required properties:
+- compatible:	"allwinner,sun50i-a64-dma"
+- dma-channels: Number of DMA channels supported by the controller.
+		Refer to Documentation/devicetree/bindings/dma/dma.txt
+- all properties above, i.e. reg, interrupts, clocks, resets and #dma-cells
+
+Optional properties:
+- dma-requests: Number of DMA request signals supported by the controller.
+		Refer to Documentation/devicetree/bindings/dma/dma.txt
+
+Example:
+	dma: dma-controller@01c02000 {
+		compatible = "allwinner,sun50i-a64-dma";
+		reg = <0x01c02000 0x1000>;
+		interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&ccu CLK_BUS_DMA>;
+		dma-channels = <8>;
+		dma-requests = <27>;
+		resets = <&ccu RST_BUS_DMA>;
+		#dma-cells = <1>;
+	};
+------------------------------------------------------------------------------
+
 Clients:
 
 DMA clients connected to the A31 DMA controller must use the format
-- 
2.14.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 06/10] arm64: allwinner: a64: Add devicetree binding for DMA controller
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

The A64 is register compatible with the H3, but has a different number
of dma channels and request ports.

Attach additional properties to the node to allow future reuse of the
compatible for controllers with different number of channels/requests.

If dma-requests is not specified, the register layout defined maximum
of 32 is used.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 .../devicetree/bindings/dma/sun6i-dma.txt          | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
index 6b267045f522..66195fb31296 100644
--- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
+++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
@@ -26,6 +26,32 @@ Example:
 		#dma-cells = <1>;
 	};
 
+------------------------------------------------------------------------------
+For A64 DMA controller:
+
+Required properties:
+- compatible:	"allwinner,sun50i-a64-dma"
+- dma-channels: Number of DMA channels supported by the controller.
+		Refer to Documentation/devicetree/bindings/dma/dma.txt
+- all properties above, i.e. reg, interrupts, clocks, resets and #dma-cells
+
+Optional properties:
+- dma-requests: Number of DMA request signals supported by the controller.
+		Refer to Documentation/devicetree/bindings/dma/dma.txt
+
+Example:
+	dma: dma-controller at 01c02000 {
+		compatible = "allwinner,sun50i-a64-dma";
+		reg = <0x01c02000 0x1000>;
+		interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&ccu CLK_BUS_DMA>;
+		dma-channels = <8>;
+		dma-requests = <27>;
+		resets = <&ccu RST_BUS_DMA>;
+		#dma-cells = <1>;
+	};
+------------------------------------------------------------------------------
+
 Clients:
 
 DMA clients connected to the A31 DMA controller must use the format
-- 
2.14.1

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

* [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
  2017-09-03 22:40 ` Stefan Brüns
  (?)
@ 2017-09-03 22:40   ` Stefan Brüns
  -1 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

To avoid introduction of a new compatible for each small SoC/DMA controller
variation, move the definition of the channel count to the devicetree.

The number of vchans is no longer explicit, but limited by the highest
port/DMA request number. The result is a slight overallocation for SoCs
with a sparse port mapping.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c69dadb853d2..bd4c2e4a759b 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -42,6 +42,9 @@
 
 #define DMA_STAT		0x30
 
+/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
+#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
+
 /*
  * sun8i specific registers
  */
@@ -65,7 +68,8 @@
 #define DMA_CHAN_LLI_ADDR	0x08
 
 #define DMA_CHAN_CUR_CFG	0x0c
-#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
+#define DMA_CHAN_MAX_DRQ		0x1f
+#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
 #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
@@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->num_vchans = sdc->cfg->nr_max_vchans;
 	sdc->max_request = sdc->cfg->nr_max_requests;
 
+	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
+	if (ret && !sdc->num_pchans) {
+		dev_err(&pdev->dev, "Can't get dma-channels.\n");
+		return ret;
+	}
+
+	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
+		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
+	if (ret && !sdc->max_request) {
+		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
+			 DMA_CHAN_MAX_DRQ);
+	}
+
+	if (sdc->max_request > DMA_CHAN_MAX_DRQ) {
+		dev_err(&pdev->dev, "Value of dma-requests out of range.\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * If the number of vchans is not specified, derive it from the
+	 * highest port number, at most one channel per port and direction.
+	 */
+	if (!sdc->num_vchans)
+		sdc->num_vchans = 2 * (sdc->max_request + 1);
+
 	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
 				   sizeof(struct sun6i_pchan), GFP_KERNEL);
 	if (!sdc->pchans)
-- 
2.14.1

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

* [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

To avoid introduction of a new compatible for each small SoC/DMA controller
variation, move the definition of the channel count to the devicetree.

The number of vchans is no longer explicit, but limited by the highest
port/DMA request number. The result is a slight overallocation for SoCs
with a sparse port mapping.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c69dadb853d2..bd4c2e4a759b 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -42,6 +42,9 @@
 
 #define DMA_STAT		0x30
 
+/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
+#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
+
 /*
  * sun8i specific registers
  */
@@ -65,7 +68,8 @@
 #define DMA_CHAN_LLI_ADDR	0x08
 
 #define DMA_CHAN_CUR_CFG	0x0c
-#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
+#define DMA_CHAN_MAX_DRQ		0x1f
+#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
 #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
@@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->num_vchans = sdc->cfg->nr_max_vchans;
 	sdc->max_request = sdc->cfg->nr_max_requests;
 
+	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
+	if (ret && !sdc->num_pchans) {
+		dev_err(&pdev->dev, "Can't get dma-channels.\n");
+		return ret;
+	}
+
+	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
+		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
+	if (ret && !sdc->max_request) {
+		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
+			 DMA_CHAN_MAX_DRQ);
+	}
+
+	if (sdc->max_request > DMA_CHAN_MAX_DRQ) {
+		dev_err(&pdev->dev, "Value of dma-requests out of range.\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * If the number of vchans is not specified, derive it from the
+	 * highest port number, at most one channel per port and direction.
+	 */
+	if (!sdc->num_vchans)
+		sdc->num_vchans = 2 * (sdc->max_request + 1);
+
 	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
 				   sizeof(struct sun6i_pchan), GFP_KERNEL);
 	if (!sdc->pchans)
-- 
2.14.1

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

* [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

To avoid introduction of a new compatible for each small SoC/DMA controller
variation, move the definition of the channel count to the devicetree.

The number of vchans is no longer explicit, but limited by the highest
port/DMA request number. The result is a slight overallocation for SoCs
with a sparse port mapping.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index c69dadb853d2..bd4c2e4a759b 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -42,6 +42,9 @@
 
 #define DMA_STAT		0x30
 
+/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
+#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
+
 /*
  * sun8i specific registers
  */
@@ -65,7 +68,8 @@
 #define DMA_CHAN_LLI_ADDR	0x08
 
 #define DMA_CHAN_CUR_CFG	0x0c
-#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
+#define DMA_CHAN_MAX_DRQ		0x1f
+#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
 #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
@@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->num_vchans = sdc->cfg->nr_max_vchans;
 	sdc->max_request = sdc->cfg->nr_max_requests;
 
+	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
+	if (ret && !sdc->num_pchans) {
+		dev_err(&pdev->dev, "Can't get dma-channels.\n");
+		return ret;
+	}
+
+	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
+		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
+	if (ret && !sdc->max_request) {
+		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
+			 DMA_CHAN_MAX_DRQ);
+	}
+
+	if (sdc->max_request > DMA_CHAN_MAX_DRQ) {
+		dev_err(&pdev->dev, "Value of dma-requests out of range.\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * If the number of vchans is not specified, derive it from the
+	 * highest port number, at most one channel per port and direction.
+	 */
+	if (!sdc->num_vchans)
+		sdc->num_vchans = 2 * (sdc->max_request + 1);
+
 	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
 				   sizeof(struct sun6i_pchan), GFP_KERNEL);
 	if (!sdc->pchans)
-- 
2.14.1

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

* [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

The A64 SoC has the same dma engine as the H3 (sun8i), with a
reduced amount of physical channels. To allow future reuse of the
compatible, leave the channel count etc. in the config data blank
and retrieve it from the devicetree.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index bd4c2e4a759b..4fae7ffad549 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -1076,6 +1076,16 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.nr_max_vchans   = 34,
 	.dmac_variant    = DMAC_VARIANT_H3,
 };
+
+/*
+ * The A64 binding uses the number of dma channels from the
+ * device tree node.
+ */
+static struct sun6i_dma_config sun50i_a64_dma_cfg = {
+	.nr_max_channels = 0,
+	.nr_max_requests = 0,
+	.nr_max_vchans   = 0,
+	.dmac_variant    = DMAC_VARIANT_H3,
 };
 
 static const struct of_device_id sun6i_dma_match[] = {
@@ -1083,6 +1093,7 @@ static const struct of_device_id sun6i_dma_match[] = {
 	{ .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
 	{ .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
 	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
+	{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dma_match);
@@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
 static int sun6i_dma_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *device;
+	struct device_node *np = pdev->dev.of_node;
 	struct sun6i_dma_dev *sdc;
 	struct resource *res;
 	int ret, i;
-- 
2.14.1

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

* [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper, Andre Przywara

The A64 SoC has the same dma engine as the H3 (sun8i), with a
reduced amount of physical channels. To allow future reuse of the
compatible, leave the channel count etc. in the config data blank
and retrieve it from the devicetree.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
 drivers/dma/sun6i-dma.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index bd4c2e4a759b..4fae7ffad549 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -1076,6 +1076,16 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.nr_max_vchans   = 34,
 	.dmac_variant    = DMAC_VARIANT_H3,
 };
+
+/*
+ * The A64 binding uses the number of dma channels from the
+ * device tree node.
+ */
+static struct sun6i_dma_config sun50i_a64_dma_cfg = {
+	.nr_max_channels = 0,
+	.nr_max_requests = 0,
+	.nr_max_vchans   = 0,
+	.dmac_variant    = DMAC_VARIANT_H3,
 };
 
 static const struct of_device_id sun6i_dma_match[] = {
@@ -1083,6 +1093,7 @@ static const struct of_device_id sun6i_dma_match[] = {
 	{ .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
 	{ .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
 	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
+	{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dma_match);
@@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
 static int sun6i_dma_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *device;
+	struct device_node *np = pdev->dev.of_node;
 	struct sun6i_dma_dev *sdc;
 	struct resource *res;
 	int ret, i;
-- 
2.14.1

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-03 22:40   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

The A64 SoC has the same dma engine as the H3 (sun8i), with a
reduced amount of physical channels. To allow future reuse of the
compatible, leave the channel count etc. in the config data blank
and retrieve it from the devicetree.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 drivers/dma/sun6i-dma.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index bd4c2e4a759b..4fae7ffad549 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -1076,6 +1076,16 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.nr_max_vchans   = 34,
 	.dmac_variant    = DMAC_VARIANT_H3,
 };
+
+/*
+ * The A64 binding uses the number of dma channels from the
+ * device tree node.
+ */
+static struct sun6i_dma_config sun50i_a64_dma_cfg = {
+	.nr_max_channels = 0,
+	.nr_max_requests = 0,
+	.nr_max_vchans   = 0,
+	.dmac_variant    = DMAC_VARIANT_H3,
 };
 
 static const struct of_device_id sun6i_dma_match[] = {
@@ -1083,6 +1093,7 @@ static const struct of_device_id sun6i_dma_match[] = {
 	{ .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
 	{ .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
 	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
+	{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dma_match);
@@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
 static int sun6i_dma_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *device;
+	struct device_node *np = pdev->dev.of_node;
 	struct sun6i_dma_dev *sdc;
 	struct resource *res;
 	int ret, i;
-- 
2.14.1

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

* [PATCH 09/10] arm64: allwinner: a64: Add device node for DMA controller
  2017-09-03 22:40 ` Stefan Brüns
  (?)
@ 2017-09-03 22:41   ` Stefan Brüns
  -1 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:41 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

The A64 SoC has a DMA controller that supports 8 DMA channels
to and from various peripherals. The last used DRQ port is 27.

Add a device node for it.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 882e2155f0aa..ccec81c4e9d2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -136,6 +136,17 @@
 			reg = <0x01c00000 0x1000>;
 		};
 
+		dma: dma-controller@01c02000 {
+			compatible = "allwinner,sun50i-a64-dma";
+			reg = <0x01c02000 0x1000>;
+			interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_DMA>;
+			dma-channels = <8>;
+			dma-requests = <27>;
+			resets = <&ccu RST_BUS_DMA>;
+			#dma-cells = <1>;
+		};
+
 		mmc0: mmc@1c0f000 {
 			compatible = "allwinner,sun50i-a64-mmc";
 			reg = <0x01c0f000 0x1000>;
-- 
2.14.1

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

* [PATCH 09/10] arm64: allwinner: a64: Add device node for DMA controller
@ 2017-09-03 22:41   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:41 UTC (permalink / raw)
  To: linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

The A64 SoC has a DMA controller that supports 8 DMA channels
to and from various peripherals. The last used DRQ port is 27.

Add a device node for it.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 882e2155f0aa..ccec81c4e9d2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -136,6 +136,17 @@
 			reg = <0x01c00000 0x1000>;
 		};
 
+		dma: dma-controller@01c02000 {
+			compatible = "allwinner,sun50i-a64-dma";
+			reg = <0x01c02000 0x1000>;
+			interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_DMA>;
+			dma-channels = <8>;
+			dma-requests = <27>;
+			resets = <&ccu RST_BUS_DMA>;
+			#dma-cells = <1>;
+		};
+
 		mmc0: mmc@1c0f000 {
 			compatible = "allwinner,sun50i-a64-mmc";
 			reg = <0x01c0f000 0x1000>;
-- 
2.14.1

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

* [PATCH 09/10] arm64: allwinner: a64: Add device node for DMA controller
@ 2017-09-03 22:41   ` Stefan Brüns
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Brüns @ 2017-09-03 22:41 UTC (permalink / raw)
  To: linux-arm-kernel

The A64 SoC has a DMA controller that supports 8 DMA channels
to and from various peripherals. The last used DRQ port is 27.

Add a device node for it.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 882e2155f0aa..ccec81c4e9d2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -136,6 +136,17 @@
 			reg = <0x01c00000 0x1000>;
 		};
 
+		dma: dma-controller at 01c02000 {
+			compatible = "allwinner,sun50i-a64-dma";
+			reg = <0x01c02000 0x1000>;
+			interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_DMA>;
+			dma-channels = <8>;
+			dma-requests = <27>;
+			resets = <&ccu RST_BUS_DMA>;
+			#dma-cells = <1>;
+		};
+
 		mmc0: mmc at 1c0f000 {
 			compatible = "allwinner,sun50i-a64-mmc";
 			reg = <0x01c0f000 0x1000>;
-- 
2.14.1

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

* Re: [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
  2017-09-03 22:40   ` Stefan Brüns
@ 2017-09-03 23:23     ` André Przywara
  -1 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:23 UTC (permalink / raw)
  To: Stefan Brüns, linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper

On 03/09/17 23:40, Stefan Brüns wrote:
> The H83T uses a compatible string different from the A23, but requires

      A83T

> the same clock autogating register setting.
> 
> The H3 also requires setting the clock autogating register, but has
> the register at a different offset.
> 
> Some currently available SoCs not yet supported by the sun6i-dma driver
> will require new compatible strings. These SoCs either follow the A23
> register model (e.g. V3s) or the H3 register model (A64, R40), so a new
> variable is added to the config struct to group SoCs with common register
> models.

As mentioned in that other mail, using the actual properties as names
here instead of grouping them to rather arbitrary groups seems more
useful and future-proof to me and should be easier to read.
In this case this should simplify this patch:
sun8i_a23_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 24,
 	.nr_max_vchans   = 37,
+	.auto_clock_gate = 0x20,
...
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "allwinner,sun8i-a23-dma"))
-		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
+	if (sdc->cfg->auto_clock_gate)
+		writel(SUN8I_DMA_GATE_ENABLE,
+		       sdc->base + sdc->cfg->auto_clock_gate);

> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a2358780ab2c..1d9b3be30d22 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -48,6 +48,9 @@
>  #define SUN8I_DMA_GATE		0x20
>  #define SUN8I_DMA_GATE_ENABLE	0x4
>  
> +#define SUNXI_H3_SECURITE_REG		0x20

typo?	   SUNXI_H3_SECURITY_REG ?

Cheers,
Andre.

> +#define SUNXI_H3_DMA_GATE		0x28
> +#define SUNXI_H3_DMA_GATE_ENABLE	0x4
>  /*
>   * Channels specific registers
>   */
> @@ -90,6 +93,21 @@
>  #define NORMAL_WAIT	8
>  #define DRQ_SDRAM	1
>  
> +/*
> + * DMA Controller generations
> + *
> + * Newer SoC generations changed or added some register definitions:
> + * - A23 added a clock gate register
> + * - H3 has a different offset for the clock gating register,
> + *   the burst length field has a different offset in the channel
> + *   configuration register, also additional burst lengths/widths.
> + */
> +enum dmac_variant {
> +	DMAC_VARIANT_A31,
> +	DMAC_VARIANT_A23,
> +	DMAC_VARIANT_H3,
> +};
> +
>  /*
>   * Hardware channels / ports representation
>   *
> @@ -101,6 +119,7 @@ struct sun6i_dma_config {
>  	u32 nr_max_channels;
>  	u32 nr_max_requests;
>  	u32 nr_max_vchans;
> +	enum dmac_variant dmac_variant;
>  };
>  
>  /*
> @@ -998,6 +1017,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
>  	.nr_max_channels = 16,
>  	.nr_max_requests = 30,
>  	.nr_max_vchans   = 53,
> +	.dmac_variant    = DMAC_VARIANT_A31,
>  };
>  
>  /*
> @@ -1009,23 +1029,29 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 24,
>  	.nr_max_vchans   = 37,
> +	.dmac_variant    = DMAC_VARIANT_A23,
>  };
>  
>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 28,
>  	.nr_max_vchans   = 39,
> +	.dmac_variant    = DMAC_VARIANT_A23,
>  };
>  
>  /*
>   * The H3 has 12 physical channels, a maximum DRQ port id of 27,
>   * and a total of 34 usable source and destination endpoints.
> + * It also supports additional burst lengths and bus widths,
> + * and the burst length fields have different offsets.
>   */
>  
>  static struct sun6i_dma_config sun8i_h3_dma_cfg = {
>  	.nr_max_channels = 12,
>  	.nr_max_requests = 27,
>  	.nr_max_vchans   = 34,
> +	.dmac_variant    = DMAC_VARIANT_H3,
> +};
>  };
>  
>  static const struct of_device_id sun6i_dma_match[] = {
> @@ -1177,11 +1203,13 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	/*
>  	 * sun8i variant requires us to toggle a dma gating register,
>  	 * as seen in Allwinner's SDK. This register is not documented
> -	 * in the A23 user manual.
> +	 * in the A23 user manual, but appears in e.g. the H83T manual.
> +	 * For the H3, H5 and A64, the register has a different location
>  	 */
> -	if (of_device_is_compatible(pdev->dev.of_node,
> -				    "allwinner,sun8i-a23-dma"))
> +	if (sdc->cfg->dmac_variant == DMAC_VARIANT_A23)
>  		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
> +	else if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3)
> +		writel(SUNXI_H3_DMA_GATE_ENABLE, sdc->base + SUNXI_H3_DMA_GATE);
>  
>  	return 0;
>  
> 

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

* [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
@ 2017-09-03 23:23     ` André Przywara
  0 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/09/17 23:40, Stefan Br?ns wrote:
> The H83T uses a compatible string different from the A23, but requires

      A83T

> the same clock autogating register setting.
> 
> The H3 also requires setting the clock autogating register, but has
> the register at a different offset.
> 
> Some currently available SoCs not yet supported by the sun6i-dma driver
> will require new compatible strings. These SoCs either follow the A23
> register model (e.g. V3s) or the H3 register model (A64, R40), so a new
> variable is added to the config struct to group SoCs with common register
> models.

As mentioned in that other mail, using the actual properties as names
here instead of grouping them to rather arbitrary groups seems more
useful and future-proof to me and should be easier to read.
In this case this should simplify this patch:
sun8i_a23_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 24,
 	.nr_max_vchans   = 37,
+	.auto_clock_gate = 0x20,
...
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "allwinner,sun8i-a23-dma"))
-		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
+	if (sdc->cfg->auto_clock_gate)
+		writel(SUN8I_DMA_GATE_ENABLE,
+		       sdc->base + sdc->cfg->auto_clock_gate);

> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a2358780ab2c..1d9b3be30d22 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -48,6 +48,9 @@
>  #define SUN8I_DMA_GATE		0x20
>  #define SUN8I_DMA_GATE_ENABLE	0x4
>  
> +#define SUNXI_H3_SECURITE_REG		0x20

typo?	   SUNXI_H3_SECURITY_REG ?

Cheers,
Andre.

> +#define SUNXI_H3_DMA_GATE		0x28
> +#define SUNXI_H3_DMA_GATE_ENABLE	0x4
>  /*
>   * Channels specific registers
>   */
> @@ -90,6 +93,21 @@
>  #define NORMAL_WAIT	8
>  #define DRQ_SDRAM	1
>  
> +/*
> + * DMA Controller generations
> + *
> + * Newer SoC generations changed or added some register definitions:
> + * - A23 added a clock gate register
> + * - H3 has a different offset for the clock gating register,
> + *   the burst length field has a different offset in the channel
> + *   configuration register, also additional burst lengths/widths.
> + */
> +enum dmac_variant {
> +	DMAC_VARIANT_A31,
> +	DMAC_VARIANT_A23,
> +	DMAC_VARIANT_H3,
> +};
> +
>  /*
>   * Hardware channels / ports representation
>   *
> @@ -101,6 +119,7 @@ struct sun6i_dma_config {
>  	u32 nr_max_channels;
>  	u32 nr_max_requests;
>  	u32 nr_max_vchans;
> +	enum dmac_variant dmac_variant;
>  };
>  
>  /*
> @@ -998,6 +1017,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
>  	.nr_max_channels = 16,
>  	.nr_max_requests = 30,
>  	.nr_max_vchans   = 53,
> +	.dmac_variant    = DMAC_VARIANT_A31,
>  };
>  
>  /*
> @@ -1009,23 +1029,29 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 24,
>  	.nr_max_vchans   = 37,
> +	.dmac_variant    = DMAC_VARIANT_A23,
>  };
>  
>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 28,
>  	.nr_max_vchans   = 39,
> +	.dmac_variant    = DMAC_VARIANT_A23,
>  };
>  
>  /*
>   * The H3 has 12 physical channels, a maximum DRQ port id of 27,
>   * and a total of 34 usable source and destination endpoints.
> + * It also supports additional burst lengths and bus widths,
> + * and the burst length fields have different offsets.
>   */
>  
>  static struct sun6i_dma_config sun8i_h3_dma_cfg = {
>  	.nr_max_channels = 12,
>  	.nr_max_requests = 27,
>  	.nr_max_vchans   = 34,
> +	.dmac_variant    = DMAC_VARIANT_H3,
> +};
>  };
>  
>  static const struct of_device_id sun6i_dma_match[] = {
> @@ -1177,11 +1203,13 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	/*
>  	 * sun8i variant requires us to toggle a dma gating register,
>  	 * as seen in Allwinner's SDK. This register is not documented
> -	 * in the A23 user manual.
> +	 * in the A23 user manual, but appears in e.g. the H83T manual.
> +	 * For the H3, H5 and A64, the register has a different location
>  	 */
> -	if (of_device_is_compatible(pdev->dev.of_node,
> -				    "allwinner,sun8i-a23-dma"))
> +	if (sdc->cfg->dmac_variant == DMAC_VARIANT_A23)
>  		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
> +	else if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3)
> +		writel(SUNXI_H3_DMA_GATE_ENABLE, sdc->base + SUNXI_H3_DMA_GATE);
>  
>  	return 0;
>  
> 

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

* Re: [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-03 23:37     ` André Przywara
  0 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:37 UTC (permalink / raw)
  To: Stefan Brüns, linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper

Hi,

On 03/09/17 23:40, Stefan Brüns wrote:
> The A64 SoC has the same dma engine as the H3 (sun8i), with a
> reduced amount of physical channels. To allow future reuse of the
> compatible, leave the channel count etc. in the config data blank
> and retrieve it from the devicetree.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index bd4c2e4a759b..4fae7ffad549 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -1076,6 +1076,16 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
>  	.nr_max_vchans   = 34,
>  	.dmac_variant    = DMAC_VARIANT_H3,
>  };
> +
> +/*
> + * The A64 binding uses the number of dma channels from the
> + * device tree node.
> + */
> +static struct sun6i_dma_config sun50i_a64_dma_cfg = {
> +	.nr_max_channels = 0,
> +	.nr_max_requests = 0,

But this does not work with the "dma-requests" property being optional
according to the binding spec? Either we put the value for the A64 in
here (and thus force the R40 and others to specify this in the DT) or we
map the "0" from struct config to DMA_CHAN_MAX_DRQ in the probe function.

> +	.nr_max_vchans   = 0,
> +	.dmac_variant    = DMAC_VARIANT_H3,
>  };
>  
>  static const struct of_device_id sun6i_dma_match[] = {
> @@ -1083,6 +1093,7 @@ static const struct of_device_id sun6i_dma_match[] = {
>  	{ .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
>  	{ .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
>  	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
> +	{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> @@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
>  static int sun6i_dma_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *device;
> +	struct device_node *np = pdev->dev.of_node;

Is this some rebase/split artefact?

Cheers,
Andre.

>  	struct sun6i_dma_dev *sdc;
>  	struct resource *res;
>  	int ret, i;
> 

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

* Re: [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-03 23:37     ` André Przywara
  0 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:37 UTC (permalink / raw)
  To: Stefan Brüns, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper

Hi,

On 03/09/17 23:40, Stefan Brüns wrote:
> The A64 SoC has the same dma engine as the H3 (sun8i), with a
> reduced amount of physical channels. To allow future reuse of the
> compatible, leave the channel count etc. in the config data blank
> and retrieve it from the devicetree.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> ---
>  drivers/dma/sun6i-dma.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index bd4c2e4a759b..4fae7ffad549 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -1076,6 +1076,16 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
>  	.nr_max_vchans   = 34,
>  	.dmac_variant    = DMAC_VARIANT_H3,
>  };
> +
> +/*
> + * The A64 binding uses the number of dma channels from the
> + * device tree node.
> + */
> +static struct sun6i_dma_config sun50i_a64_dma_cfg = {
> +	.nr_max_channels = 0,
> +	.nr_max_requests = 0,

But this does not work with the "dma-requests" property being optional
according to the binding spec? Either we put the value for the A64 in
here (and thus force the R40 and others to specify this in the DT) or we
map the "0" from struct config to DMA_CHAN_MAX_DRQ in the probe function.

> +	.nr_max_vchans   = 0,
> +	.dmac_variant    = DMAC_VARIANT_H3,
>  };
>  
>  static const struct of_device_id sun6i_dma_match[] = {
> @@ -1083,6 +1093,7 @@ static const struct of_device_id sun6i_dma_match[] = {
>  	{ .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
>  	{ .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
>  	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
> +	{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> @@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
>  static int sun6i_dma_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *device;
> +	struct device_node *np = pdev->dev.of_node;

Is this some rebase/split artefact?

Cheers,
Andre.

>  	struct sun6i_dma_dev *sdc;
>  	struct resource *res;
>  	int ret, i;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-03 23:37     ` André Przywara
  0 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:37 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 03/09/17 23:40, Stefan Br?ns wrote:
> The A64 SoC has the same dma engine as the H3 (sun8i), with a
> reduced amount of physical channels. To allow future reuse of the
> compatible, leave the channel count etc. in the config data blank
> and retrieve it from the devicetree.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index bd4c2e4a759b..4fae7ffad549 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -1076,6 +1076,16 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
>  	.nr_max_vchans   = 34,
>  	.dmac_variant    = DMAC_VARIANT_H3,
>  };
> +
> +/*
> + * The A64 binding uses the number of dma channels from the
> + * device tree node.
> + */
> +static struct sun6i_dma_config sun50i_a64_dma_cfg = {
> +	.nr_max_channels = 0,
> +	.nr_max_requests = 0,

But this does not work with the "dma-requests" property being optional
according to the binding spec? Either we put the value for the A64 in
here (and thus force the R40 and others to specify this in the DT) or we
map the "0" from struct config to DMA_CHAN_MAX_DRQ in the probe function.

> +	.nr_max_vchans   = 0,
> +	.dmac_variant    = DMAC_VARIANT_H3,
>  };
>  
>  static const struct of_device_id sun6i_dma_match[] = {
> @@ -1083,6 +1093,7 @@ static const struct of_device_id sun6i_dma_match[] = {
>  	{ .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
>  	{ .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
>  	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
> +	{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> @@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
>  static int sun6i_dma_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *device;
> +	struct device_node *np = pdev->dev.of_node;

Is this some rebase/split artefact?

Cheers,
Andre.

>  	struct sun6i_dma_dev *sdc;
>  	struct resource *res;
>  	int ret, i;
> 

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

* Re: [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-03 23:44     ` André Przywara
  0 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:44 UTC (permalink / raw)
  To: Stefan Brüns, linux-sunxi
  Cc: devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper

Hi,

On 03/09/17 23:40, Stefan Brüns wrote:
> To avoid introduction of a new compatible for each small SoC/DMA controller
> variation, move the definition of the channel count to the devicetree.
> 
> The number of vchans is no longer explicit, but limited by the highest
> port/DMA request number. The result is a slight overallocation for SoCs
> with a sparse port mapping.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index c69dadb853d2..bd4c2e4a759b 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -42,6 +42,9 @@
>  
>  #define DMA_STAT		0x30
>  
> +/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
> +#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
> +
>  /*
>   * sun8i specific registers
>   */
> @@ -65,7 +68,8 @@
>  #define DMA_CHAN_LLI_ADDR	0x08
>  
>  #define DMA_CHAN_CUR_CFG	0x0c
> -#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> +#define DMA_CHAN_MAX_DRQ		0x1f
> +#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
>  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
>  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
>  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> @@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	sdc->num_vchans = sdc->cfg->nr_max_vchans;
>  	sdc->max_request = sdc->cfg->nr_max_requests;
>  
> +	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
> +	if (ret && !sdc->num_pchans) {
> +		dev_err(&pdev->dev, "Can't get dma-channels.\n");
> +		return ret;
> +	}
> +
> +	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
> +		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
> +	if (ret && !sdc->max_request) {
> +		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
> +			 DMA_CHAN_MAX_DRQ);

Mmmh, is this mapping of "!sdc->max_request" -> DMA_CHAN_MAX_DRQ
implemented somewhere else? Or is it just missing here:
		sdc->max_request = DMA_CHAN_MAX_DRQ;

Otherwise this is looking good, thanks for picking up the DT property
approach!

Cheers,
Andre.

> +	}
> +
> +	if (sdc->max_request > DMA_CHAN_MAX_DRQ) {
> +		dev_err(&pdev->dev, "Value of dma-requests out of range.\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * If the number of vchans is not specified, derive it from the
> +	 * highest port number, at most one channel per port and direction.
> +	 */
> +	if (!sdc->num_vchans)
> +		sdc->num_vchans = 2 * (sdc->max_request + 1);
> +
>  	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
>  				   sizeof(struct sun6i_pchan), GFP_KERNEL);
>  	if (!sdc->pchans)
> 

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

* Re: [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-03 23:44     ` André Przywara
  0 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:44 UTC (permalink / raw)
  To: Stefan Brüns, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper

Hi,

On 03/09/17 23:40, Stefan Brüns wrote:
> To avoid introduction of a new compatible for each small SoC/DMA controller
> variation, move the definition of the channel count to the devicetree.
> 
> The number of vchans is no longer explicit, but limited by the highest
> port/DMA request number. The result is a slight overallocation for SoCs
> with a sparse port mapping.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> ---
>  drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index c69dadb853d2..bd4c2e4a759b 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -42,6 +42,9 @@
>  
>  #define DMA_STAT		0x30
>  
> +/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
> +#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
> +
>  /*
>   * sun8i specific registers
>   */
> @@ -65,7 +68,8 @@
>  #define DMA_CHAN_LLI_ADDR	0x08
>  
>  #define DMA_CHAN_CUR_CFG	0x0c
> -#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> +#define DMA_CHAN_MAX_DRQ		0x1f
> +#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
>  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
>  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
>  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> @@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	sdc->num_vchans = sdc->cfg->nr_max_vchans;
>  	sdc->max_request = sdc->cfg->nr_max_requests;
>  
> +	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
> +	if (ret && !sdc->num_pchans) {
> +		dev_err(&pdev->dev, "Can't get dma-channels.\n");
> +		return ret;
> +	}
> +
> +	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
> +		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
> +	if (ret && !sdc->max_request) {
> +		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
> +			 DMA_CHAN_MAX_DRQ);

Mmmh, is this mapping of "!sdc->max_request" -> DMA_CHAN_MAX_DRQ
implemented somewhere else? Or is it just missing here:
		sdc->max_request = DMA_CHAN_MAX_DRQ;

Otherwise this is looking good, thanks for picking up the DT property
approach!

Cheers,
Andre.

> +	}
> +
> +	if (sdc->max_request > DMA_CHAN_MAX_DRQ) {
> +		dev_err(&pdev->dev, "Value of dma-requests out of range.\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * If the number of vchans is not specified, derive it from the
> +	 * highest port number, at most one channel per port and direction.
> +	 */
> +	if (!sdc->num_vchans)
> +		sdc->num_vchans = 2 * (sdc->max_request + 1);
> +
>  	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
>  				   sizeof(struct sun6i_pchan), GFP_KERNEL);
>  	if (!sdc->pchans)
> 

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-03 23:44     ` André Przywara
  0 siblings, 0 replies; 68+ messages in thread
From: André Przywara @ 2017-09-03 23:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 03/09/17 23:40, Stefan Br?ns wrote:
> To avoid introduction of a new compatible for each small SoC/DMA controller
> variation, move the definition of the channel count to the devicetree.
> 
> The number of vchans is no longer explicit, but limited by the highest
> port/DMA request number. The result is a slight overallocation for SoCs
> with a sparse port mapping.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index c69dadb853d2..bd4c2e4a759b 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -42,6 +42,9 @@
>  
>  #define DMA_STAT		0x30
>  
> +/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
> +#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
> +
>  /*
>   * sun8i specific registers
>   */
> @@ -65,7 +68,8 @@
>  #define DMA_CHAN_LLI_ADDR	0x08
>  
>  #define DMA_CHAN_CUR_CFG	0x0c
> -#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> +#define DMA_CHAN_MAX_DRQ		0x1f
> +#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
>  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
>  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
>  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> @@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	sdc->num_vchans = sdc->cfg->nr_max_vchans;
>  	sdc->max_request = sdc->cfg->nr_max_requests;
>  
> +	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
> +	if (ret && !sdc->num_pchans) {
> +		dev_err(&pdev->dev, "Can't get dma-channels.\n");
> +		return ret;
> +	}
> +
> +	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
> +		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
> +	if (ret && !sdc->max_request) {
> +		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
> +			 DMA_CHAN_MAX_DRQ);

Mmmh, is this mapping of "!sdc->max_request" -> DMA_CHAN_MAX_DRQ
implemented somewhere else? Or is it just missing here:
		sdc->max_request = DMA_CHAN_MAX_DRQ;

Otherwise this is looking good, thanks for picking up the DT property
approach!

Cheers,
Andre.

> +	}
> +
> +	if (sdc->max_request > DMA_CHAN_MAX_DRQ) {
> +		dev_err(&pdev->dev, "Value of dma-requests out of range.\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * If the number of vchans is not specified, derive it from the
> +	 * highest port number, at most one channel per port and direction.
> +	 */
> +	if (!sdc->num_vchans)
> +		sdc->num_vchans = 2 * (sdc->max_request + 1);
> +
>  	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->num_pchans,
>  				   sizeof(struct sun6i_pchan), GFP_KERNEL);
>  	if (!sdc->pchans)
> 

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

* Re: [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-04  0:12       ` Stefan Bruens
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Bruens @ 2017-09-04  0:12 UTC (permalink / raw)
  To: André Przywara
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper

On Montag, 4. September 2017 01:44:54 CEST André Przywara wrote:
> Hi,
> 
> On 03/09/17 23:40, Stefan Brüns wrote:
> > To avoid introduction of a new compatible for each small SoC/DMA
> > controller
> > variation, move the definition of the channel count to the devicetree.
> > 
> > The number of vchans is no longer explicit, but limited by the highest
> > port/DMA request number. The result is a slight overallocation for SoCs
> > with a sparse port mapping.
> > 
> > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index c69dadb853d2..bd4c2e4a759b 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -42,6 +42,9 @@
> > 
> >  #define DMA_STAT		0x30
> > 
> > +/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels
> > */
> > +#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
> > +
> > 
> >  /*
> >  
> >   * sun8i specific registers
> >   */
> > 
> > @@ -65,7 +68,8 @@
> > 
> >  #define DMA_CHAN_LLI_ADDR	0x08
> >  
> >  #define DMA_CHAN_CUR_CFG	0x0c
> > 
> > -#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> > +#define DMA_CHAN_MAX_DRQ		0x1f
> > +#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
> > 
> >  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
> >  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> >  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> > 
> > @@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)> 
> >  	sdc->num_vchans = sdc->cfg->nr_max_vchans;
> >  	sdc->max_request = sdc->cfg->nr_max_requests;
> > 
> > +	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
> > +	if (ret && !sdc->num_pchans) {
> > +		dev_err(&pdev->dev, "Can't get dma-channels.\n");
> > +		return ret;
> > +	}
> > +
> > +	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
> > +		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
> > +	if (ret && !sdc->max_request) {
> > +		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
> > +			 DMA_CHAN_MAX_DRQ);
> 
> Mmmh, is this mapping of "!sdc->max_request" -> DMA_CHAN_MAX_DRQ
> implemented somewhere else? Or is it just missing here:
> 		sdc->max_request = DMA_CHAN_MAX_DRQ;

Well spotted, that assignment is actually missing.

With this line added, your comment for patch 8/10 should also be addressed 
(regarding the default value).
 
> Otherwise this is looking good, thanks for picking up the DT property
> approach!
> 
> Cheers,
> Andre.
> 

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

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

* Re: [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-04  0:12       ` Stefan Bruens
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Bruens @ 2017-09-04  0:12 UTC (permalink / raw)
  To: André Przywara
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper

On Montag, 4. September 2017 01:44:54 CEST André Przywara wrote:
> Hi,
> 
> On 03/09/17 23:40, Stefan Brüns wrote:
> > To avoid introduction of a new compatible for each small SoC/DMA
> > controller
> > variation, move the definition of the channel count to the devicetree.
> > 
> > The number of vchans is no longer explicit, but limited by the highest
> > port/DMA request number. The result is a slight overallocation for SoCs
> > with a sparse port mapping.
> > 
> > Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index c69dadb853d2..bd4c2e4a759b 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -42,6 +42,9 @@
> > 
> >  #define DMA_STAT		0x30
> > 
> > +/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels
> > */
> > +#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
> > +
> > 
> >  /*
> >  
> >   * sun8i specific registers
> >   */
> > 
> > @@ -65,7 +68,8 @@
> > 
> >  #define DMA_CHAN_LLI_ADDR	0x08
> >  
> >  #define DMA_CHAN_CUR_CFG	0x0c
> > 
> > -#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> > +#define DMA_CHAN_MAX_DRQ		0x1f
> > +#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
> > 
> >  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
> >  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> >  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> > 
> > @@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)> 
> >  	sdc->num_vchans = sdc->cfg->nr_max_vchans;
> >  	sdc->max_request = sdc->cfg->nr_max_requests;
> > 
> > +	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
> > +	if (ret && !sdc->num_pchans) {
> > +		dev_err(&pdev->dev, "Can't get dma-channels.\n");
> > +		return ret;
> > +	}
> > +
> > +	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
> > +		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
> > +	if (ret && !sdc->max_request) {
> > +		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
> > +			 DMA_CHAN_MAX_DRQ);
> 
> Mmmh, is this mapping of "!sdc->max_request" -> DMA_CHAN_MAX_DRQ
> implemented somewhere else? Or is it just missing here:
> 		sdc->max_request = DMA_CHAN_MAX_DRQ;

Well spotted, that assignment is actually missing.

With this line added, your comment for patch 8/10 should also be addressed 
(regarding the default value).
 
> Otherwise this is looking good, thanks for picking up the DT property
> approach!
> 
> Cheers,
> Andre.
> 

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree
@ 2017-09-04  0:12       ` Stefan Bruens
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Bruens @ 2017-09-04  0:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Montag, 4. September 2017 01:44:54 CEST Andr? Przywara wrote:
> Hi,
> 
> On 03/09/17 23:40, Stefan Br?ns wrote:
> > To avoid introduction of a new compatible for each small SoC/DMA
> > controller
> > variation, move the definition of the channel count to the devicetree.
> > 
> > The number of vchans is no longer explicit, but limited by the highest
> > port/DMA request number. The result is a slight overallocation for SoCs
> > with a sparse port mapping.
> > 
> > Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index c69dadb853d2..bd4c2e4a759b 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -42,6 +42,9 @@
> > 
> >  #define DMA_STAT		0x30
> > 
> > +/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels
> > */
> > +#define DMA_MAX_CHANNELS	(DMA_IRQ_CHAN_NR * 0x10 / 4)
> > +
> > 
> >  /*
> >  
> >   * sun8i specific registers
> >   */
> > 
> > @@ -65,7 +68,8 @@
> > 
> >  #define DMA_CHAN_LLI_ADDR	0x08
> >  
> >  #define DMA_CHAN_CUR_CFG	0x0c
> > 
> > -#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> > +#define DMA_CHAN_MAX_DRQ		0x1f
> > +#define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & DMA_CHAN_MAX_DRQ)
> > 
> >  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
> >  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> >  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> > 
> > @@ -1173,6 +1177,35 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)> 
> >  	sdc->num_vchans = sdc->cfg->nr_max_vchans;
> >  	sdc->max_request = sdc->cfg->nr_max_requests;
> > 
> > +	ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans);
> > +	if (ret && !sdc->num_pchans) {
> > +		dev_err(&pdev->dev, "Can't get dma-channels.\n");
> > +		return ret;
> > +	}
> > +
> > +	if (sdc->num_pchans > DMA_MAX_CHANNELS) {
> > +		dev_err(&pdev->dev, "Number of dma-channels out of range.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
> > +	if (ret && !sdc->max_request) {
> > +		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
> > +			 DMA_CHAN_MAX_DRQ);
> 
> Mmmh, is this mapping of "!sdc->max_request" -> DMA_CHAN_MAX_DRQ
> implemented somewhere else? Or is it just missing here:
> 		sdc->max_request = DMA_CHAN_MAX_DRQ;

Well spotted, that assignment is actually missing.

With this line added, your comment for patch 8/10 should also be addressed 
(regarding the default value).
 
> Otherwise this is looking good, thanks for picking up the DT property
> approach!
> 
> Cheers,
> Andre.
> 

Kind regards,

Stefan

-- 
Stefan Br?ns  /  Bergstra?e 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

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

* Re: [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-04  0:13       ` Stefan Bruens
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Bruens @ 2017-09-04  0:13 UTC (permalink / raw)
  To: André Przywara
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Code Kipper

On Montag, 4. September 2017 01:37:58 CEST André Przywara wrote:

> > @@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> > 
> >  static int sun6i_dma_probe(struct platform_device *pdev)
> >  {
> >  
> >  	const struct of_device_id *device;
> > 
> > +	struct device_node *np = pdev->dev.of_node;
> 
> Is this some rebase/split artefact?
> 
> Cheers,
> Andre.
> 

Yes, that one should be in patch 7/10 ...

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

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

* Re: [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-04  0:13       ` Stefan Bruens
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Bruens @ 2017-09-04  0:13 UTC (permalink / raw)
  To: André Przywara
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
	Rob Herring, Code Kipper

On Montag, 4. September 2017 01:37:58 CEST André Przywara wrote:

> > @@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> > 
> >  static int sun6i_dma_probe(struct platform_device *pdev)
> >  {
> >  
> >  	const struct of_device_id *device;
> > 
> > +	struct device_node *np = pdev->dev.of_node;
> 
> Is this some rebase/split artefact?
> 
> Cheers,
> Andre.
> 

Yes, that one should be in patch 7/10 ...

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles
@ 2017-09-04  0:13       ` Stefan Bruens
  0 siblings, 0 replies; 68+ messages in thread
From: Stefan Bruens @ 2017-09-04  0:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Montag, 4. September 2017 01:37:58 CEST Andr? Przywara wrote:

> > @@ -1090,6 +1101,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> > 
> >  static int sun6i_dma_probe(struct platform_device *pdev)
> >  {
> >  
> >  	const struct of_device_id *device;
> > 
> > +	struct device_node *np = pdev->dev.of_node;
> 
> Is this some rebase/split artefact?
> 
> Cheers,
> Andre.
> 

Yes, that one should be in patch 7/10 ...

Kind regards,

Stefan

-- 
Stefan Br?ns  /  Bergstra?e 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

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

* Re: [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
@ 2017-09-04  7:06       ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:06 UTC (permalink / raw)
  To: André Przywara
  Cc: Stefan Brüns, linux-sunxi, devicetree, dmaengine,
	Vinod Koul, linux-arm-kernel, linux-kernel, Chen-Yu Tsai,
	Rob Herring, Code Kipper

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

On Mon, Sep 04, 2017 at 12:23:09AM +0100, André Przywara wrote:
> On 03/09/17 23:40, Stefan Brüns wrote:
> > The H83T uses a compatible string different from the A23, but requires
> 
>       A83T
> 
> > the same clock autogating register setting.
> > 
> > The H3 also requires setting the clock autogating register, but has
> > the register at a different offset.
> > 
> > Some currently available SoCs not yet supported by the sun6i-dma driver
> > will require new compatible strings. These SoCs either follow the A23
> > register model (e.g. V3s) or the H3 register model (A64, R40), so a new
> > variable is added to the config struct to group SoCs with common register
> > models.
> 
> As mentioned in that other mail, using the actual properties as names
> here instead of grouping them to rather arbitrary groups seems more
> useful and future-proof to me and should be easier to read.
> In this case this should simplify this patch:
> sun8i_a23_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 24,
>  	.nr_max_vchans   = 37,
> +	.auto_clock_gate = 0x20,
> ...
> -	if (of_device_is_compatible(pdev->dev.of_node,
> -				    "allwinner,sun8i-a23-dma"))
> -		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
> +	if (sdc->cfg->auto_clock_gate)
> +		writel(SUN8I_DMA_GATE_ENABLE,
> +		       sdc->base + sdc->cfg->auto_clock_gate);

I agree.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
@ 2017-09-04  7:06       ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:06 UTC (permalink / raw)
  To: André Przywara
  Cc: Stefan Brüns, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
	Code Kipper

[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]

On Mon, Sep 04, 2017 at 12:23:09AM +0100, André Przywara wrote:
> On 03/09/17 23:40, Stefan Brüns wrote:
> > The H83T uses a compatible string different from the A23, but requires
> 
>       A83T
> 
> > the same clock autogating register setting.
> > 
> > The H3 also requires setting the clock autogating register, but has
> > the register at a different offset.
> > 
> > Some currently available SoCs not yet supported by the sun6i-dma driver
> > will require new compatible strings. These SoCs either follow the A23
> > register model (e.g. V3s) or the H3 register model (A64, R40), so a new
> > variable is added to the config struct to group SoCs with common register
> > models.
> 
> As mentioned in that other mail, using the actual properties as names
> here instead of grouping them to rather arbitrary groups seems more
> useful and future-proof to me and should be easier to read.
> In this case this should simplify this patch:
> sun8i_a23_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 24,
>  	.nr_max_vchans   = 37,
> +	.auto_clock_gate = 0x20,
> ...
> -	if (of_device_is_compatible(pdev->dev.of_node,
> -				    "allwinner,sun8i-a23-dma"))
> -		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
> +	if (sdc->cfg->auto_clock_gate)
> +		writel(SUN8I_DMA_GATE_ENABLE,
> +		       sdc->base + sdc->cfg->auto_clock_gate);

I agree.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3
@ 2017-09-04  7:06       ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 04, 2017 at 12:23:09AM +0100, Andr? Przywara wrote:
> On 03/09/17 23:40, Stefan Br?ns wrote:
> > The H83T uses a compatible string different from the A23, but requires
> 
>       A83T
> 
> > the same clock autogating register setting.
> > 
> > The H3 also requires setting the clock autogating register, but has
> > the register at a different offset.
> > 
> > Some currently available SoCs not yet supported by the sun6i-dma driver
> > will require new compatible strings. These SoCs either follow the A23
> > register model (e.g. V3s) or the H3 register model (A64, R40), so a new
> > variable is added to the config struct to group SoCs with common register
> > models.
> 
> As mentioned in that other mail, using the actual properties as names
> here instead of grouping them to rather arbitrary groups seems more
> useful and future-proof to me and should be easier to read.
> In this case this should simplify this patch:
> sun8i_a23_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 24,
>  	.nr_max_vchans   = 37,
> +	.auto_clock_gate = 0x20,
> ...
> -	if (of_device_is_compatible(pdev->dev.of_node,
> -				    "allwinner,sun8i-a23-dma"))
> -		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
> +	if (sdc->cfg->auto_clock_gate)
> +		writel(SUN8I_DMA_GATE_ENABLE,
> +		       sdc->base + sdc->cfg->auto_clock_gate);

I agree.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170904/858b1032/attachment.sig>

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

* Re: [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-04  7:43     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:43 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Chen-Yu Tsai, Rob Herring, Code Kipper,
	Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Brüns wrote:
> Preparatory patch: If the same compatible is used for different SoCs which
> have a common register layout, but different number of channels, the
> channel count can no longer be stored in the config. Store it in the
> device structure instead.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

As stated already, we already are going to have a different
compatible, and this is not something that will change from one
instance to the other. Having code is therefore:
  A) Making the code more complex
  B) For no particular reason.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-04  7:43     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:43 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Brüns wrote:
> Preparatory patch: If the same compatible is used for different SoCs which
> have a common register layout, but different number of channels, the
> channel count can no longer be stored in the config. Store it in the
> device structure instead.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>

As stated already, we already are going to have a different
compatible, and this is not something that will change from one
instance to the other. Having code is therefore:
  A) Making the code more complex
  B) For no particular reason.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-04  7:43     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Br?ns wrote:
> Preparatory patch: If the same compatible is used for different SoCs which
> have a common register layout, but different number of channels, the
> channel count can no longer be stored in the config. Store it in the
> device structure instead.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>

As stated already, we already are going to have a different
compatible, and this is not something that will change from one
instance to the other. Having code is therefore:
  A) Making the code more complex
  B) For no particular reason.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170904/98b6865d/attachment-0001.sig>

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

* Re: [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
@ 2017-09-04  7:59     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:59 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Chen-Yu Tsai, Rob Herring, Code Kipper,
	Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]

On Mon, Sep 04, 2017 at 12:40:53AM +0200, Stefan Brüns wrote:
> For the H3, the burst lengths field offsets in the channel configuration
> register differs from earlier SoC generations.
> 
> Using the A31 register macros actually configured the H3 controller
> do to bursts of length 1 always, which although working leads to higher
> bus utilisation.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 1d9b3be30d22..f1a139f0102f 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -68,13 +68,15 @@
>  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
>  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
>  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> +#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> +#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
>  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
>  
>  #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
>  #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
>  #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
> -#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
> +#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) << 16)
> +#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 16)
>  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
>  
>  #define DMA_CHAN_CUR_SRC	0x10
> @@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
>  	if (dst_width < 0)
>  		return dst_width;
>  
> -	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
> -		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> -		DMA_CHAN_CFG_DST_BURST(dst_burst) |
> +	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
>  		DMA_CHAN_CFG_DST_WIDTH(dst_width);
>  
> +	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
> +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
> +			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
> +	} else {
> +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
> +			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
> +	}
> +

I guess we have two options to support that properly. We could either
have a different function that would generate that register value
based on the parameters we have, or duplicate the set_config function
entirely, with function pointer stored in the configuration.

I think I prefer the former, as it reduces the code duplication.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
@ 2017-09-04  7:59     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:59 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 2816 bytes --]

On Mon, Sep 04, 2017 at 12:40:53AM +0200, Stefan Brüns wrote:
> For the H3, the burst lengths field offsets in the channel configuration
> register differs from earlier SoC generations.
> 
> Using the A31 register macros actually configured the H3 controller
> do to bursts of length 1 always, which although working leads to higher
> bus utilisation.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> ---
>  drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 1d9b3be30d22..f1a139f0102f 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -68,13 +68,15 @@
>  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
>  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
>  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> +#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> +#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
>  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
>  
>  #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
>  #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
>  #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
> -#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
> +#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) << 16)
> +#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 16)
>  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
>  
>  #define DMA_CHAN_CUR_SRC	0x10
> @@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
>  	if (dst_width < 0)
>  		return dst_width;
>  
> -	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
> -		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> -		DMA_CHAN_CFG_DST_BURST(dst_burst) |
> +	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
>  		DMA_CHAN_CFG_DST_WIDTH(dst_width);
>  
> +	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
> +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
> +			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
> +	} else {
> +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
> +			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
> +	}
> +

I guess we have two options to support that properly. We could either
have a different function that would generate that register value
based on the parameters we have, or duplicate the set_config function
entirely, with function pointer stored in the configuration.

I think I prefer the former, as it reduces the code duplication.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
@ 2017-09-04  7:59     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 04, 2017 at 12:40:53AM +0200, Stefan Br?ns wrote:
> For the H3, the burst lengths field offsets in the channel configuration
> register differs from earlier SoC generations.
> 
> Using the A31 register macros actually configured the H3 controller
> do to bursts of length 1 always, which although working leads to higher
> bus utilisation.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 1d9b3be30d22..f1a139f0102f 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -68,13 +68,15 @@
>  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
>  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
>  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> +#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> +#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
>  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
>  
>  #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
>  #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
>  #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
> -#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
> +#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) << 16)
> +#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 16)
>  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
>  
>  #define DMA_CHAN_CUR_SRC	0x10
> @@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
>  	if (dst_width < 0)
>  		return dst_width;
>  
> -	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
> -		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> -		DMA_CHAN_CFG_DST_BURST(dst_burst) |
> +	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
>  		DMA_CHAN_CFG_DST_WIDTH(dst_width);
>  
> +	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
> +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
> +			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
> +	} else {
> +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
> +			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
> +	}
> +

I guess we have two options to support that properly. We could either
have a different function that would generate that register value
based on the parameters we have, or duplicate the set_config function
entirely, with function pointer stored in the configuration.

I think I prefer the former, as it reduces the code duplication.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170904/e90a7e7b/attachment-0001.sig>

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

* Re: [PATCH 04/10] dmaengine: sun6i: Enable additional burst lengths/widths on H3
@ 2017-09-04  8:00     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  8:00 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Chen-Yu Tsai, Rob Herring, Code Kipper,
	Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]

On Mon, Sep 04, 2017 at 12:40:55AM +0200, Stefan Brüns wrote:
> The H3 supports bursts lengths of 1, 4, 8 and 16 transfers, each with
> a width of 1, 2, 4 or 8 bytes.
> 
> The register value for the the width is log2-encoded, change the
> conversion function to provide the correct value for width == 8.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index c5644bd0f91a..335a8ec88b0b 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -263,8 +263,12 @@ static inline s8 convert_burst(u32 maxburst)
>  	switch (maxburst) {
>  	case 1:
>  		return 0;
> +	case 4:
> +		return 1;
>  	case 8:
>  		return 2;
> +	case 16:
> +		return 3;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -272,7 +276,7 @@ static inline s8 convert_burst(u32 maxburst)
>  
>  static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
>  {
> -	return addr_width >> 1;
> +	return ilog2(addr_width);
>  }
>  
>  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
> @@ -1152,6 +1156,12 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
>  	sdc->src_burst_lengths			= BIT(1) | BIT(8);
>  	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
> +	if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3) {
> +		sdc->slave.src_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> +		sdc->slave.dst_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> +		sdc->src_burst_lengths     |= BIT(4) | BIT(16);
> +		sdc->dst_burst_lengths     |= BIT(4) | BIT(16);
> +	}

The rest looks good, but that should be stored in the sun6i_dma_config
structure too.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [PATCH 04/10] dmaengine: sun6i: Enable additional burst lengths/widths on H3
@ 2017-09-04  8:00     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  8:00 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 1974 bytes --]

On Mon, Sep 04, 2017 at 12:40:55AM +0200, Stefan Brüns wrote:
> The H3 supports bursts lengths of 1, 4, 8 and 16 transfers, each with
> a width of 1, 2, 4 or 8 bytes.
> 
> The register value for the the width is log2-encoded, change the
> conversion function to provide the correct value for width == 8.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> ---
>  drivers/dma/sun6i-dma.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index c5644bd0f91a..335a8ec88b0b 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -263,8 +263,12 @@ static inline s8 convert_burst(u32 maxburst)
>  	switch (maxburst) {
>  	case 1:
>  		return 0;
> +	case 4:
> +		return 1;
>  	case 8:
>  		return 2;
> +	case 16:
> +		return 3;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -272,7 +276,7 @@ static inline s8 convert_burst(u32 maxburst)
>  
>  static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
>  {
> -	return addr_width >> 1;
> +	return ilog2(addr_width);
>  }
>  
>  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
> @@ -1152,6 +1156,12 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
>  	sdc->src_burst_lengths			= BIT(1) | BIT(8);
>  	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
> +	if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3) {
> +		sdc->slave.src_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> +		sdc->slave.dst_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> +		sdc->src_burst_lengths     |= BIT(4) | BIT(16);
> +		sdc->dst_burst_lengths     |= BIT(4) | BIT(16);
> +	}

The rest looks good, but that should be stored in the sun6i_dma_config
structure too.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* [PATCH 04/10] dmaengine: sun6i: Enable additional burst lengths/widths on H3
@ 2017-09-04  8:00     ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-04  8:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 04, 2017 at 12:40:55AM +0200, Stefan Br?ns wrote:
> The H3 supports bursts lengths of 1, 4, 8 and 16 transfers, each with
> a width of 1, 2, 4 or 8 bytes.
> 
> The register value for the the width is log2-encoded, change the
> conversion function to provide the correct value for width == 8.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/dma/sun6i-dma.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index c5644bd0f91a..335a8ec88b0b 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -263,8 +263,12 @@ static inline s8 convert_burst(u32 maxburst)
>  	switch (maxburst) {
>  	case 1:
>  		return 0;
> +	case 4:
> +		return 1;
>  	case 8:
>  		return 2;
> +	case 16:
> +		return 3;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -272,7 +276,7 @@ static inline s8 convert_burst(u32 maxburst)
>  
>  static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
>  {
> -	return addr_width >> 1;
> +	return ilog2(addr_width);
>  }
>  
>  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
> @@ -1152,6 +1156,12 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
>  	sdc->src_burst_lengths			= BIT(1) | BIT(8);
>  	sdc->dst_burst_lengths			= BIT(1) | BIT(8);
> +	if (sdc->cfg->dmac_variant == DMAC_VARIANT_H3) {
> +		sdc->slave.src_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> +		sdc->slave.dst_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> +		sdc->src_burst_lengths     |= BIT(4) | BIT(16);
> +		sdc->dst_burst_lengths     |= BIT(4) | BIT(16);
> +	}

The rest looks good, but that should be stored in the sun6i_dma_config
structure too.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170904/61f302d8/attachment.sig>

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

* Re: [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
  2017-09-04  7:43     ` Maxime Ripard
  (?)
@ 2017-09-04 14:30       ` Brüns, Stefan
  -1 siblings, 0 replies; 68+ messages in thread
From: Brüns, Stefan @ 2017-09-04 14:30 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Chen-Yu Tsai, Rob Herring, Code Kipper,
	Andre Przywara

On Montag, 4. September 2017 09:43:55 CEST Maxime Ripard wrote:
> On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Brüns wrote:
> > Preparatory patch: If the same compatible is used for different SoCs which
> > have a common register layout, but different number of channels, the
> > channel count can no longer be stored in the config. Store it in the
> > device structure instead.
> > 
> > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> 
> As stated already, we already are going to have a different
> compatible, and this is not something that will change from one
> instance to the other. Having code is therefore:
>   A) Making the code more complex
>   B) For no particular reason.

If the dma channel count (which is a standard dma binding, likely for a 
reason) goes into the devicetree, it has to be moved out of the config.

The R40 (which has register manuals available) has the same register layout as 
the A64, but *does* have a different channel count. So you think it is a good 
idea to introduce a new compatible again?

If you had been half as picky when merging the H3 and A83T support, we would 
not have this mess now.

There is also the H6, where there is no register manual available yet, but I  
bet it has the H3 (and A64, H5, R40) register layout, but unlikely the same 
number of DMA channels *and* the same number of ports.

Regards,

Stefan

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

* Re: [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-04 14:30       ` Brüns, Stefan
  0 siblings, 0 replies; 68+ messages in thread
From: Brüns, Stefan @ 2017-09-04 14:30 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

On Montag, 4. September 2017 09:43:55 CEST Maxime Ripard wrote:
> On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Brüns wrote:
> > Preparatory patch: If the same compatible is used for different SoCs which
> > have a common register layout, but different number of channels, the
> > channel count can no longer be stored in the config. Store it in the
> > device structure instead.
> > 
> > Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> 
> As stated already, we already are going to have a different
> compatible, and this is not something that will change from one
> instance to the other. Having code is therefore:
>   A) Making the code more complex
>   B) For no particular reason.

If the dma channel count (which is a standard dma binding, likely for a 
reason) goes into the devicetree, it has to be moved out of the config.

The R40 (which has register manuals available) has the same register layout as 
the A64, but *does* have a different channel count. So you think it is a good 
idea to introduce a new compatible again?

If you had been half as picky when merging the H3 and A83T support, we would 
not have this mess now.

There is also the H6, where there is no register manual available yet, but I  
bet it has the H3 (and A64, H5, R40) register layout, but unlikely the same 
number of DMA channels *and* the same number of ports.

Regards,

Stefan

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-04 14:30       ` Brüns, Stefan
  0 siblings, 0 replies; 68+ messages in thread
From: Brüns, Stefan @ 2017-09-04 14:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Montag, 4. September 2017 09:43:55 CEST Maxime Ripard wrote:
> On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Br?ns wrote:
> > Preparatory patch: If the same compatible is used for different SoCs which
> > have a common register layout, but different number of channels, the
> > channel count can no longer be stored in the config. Store it in the
> > device structure instead.
> > 
> > Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> 
> As stated already, we already are going to have a different
> compatible, and this is not something that will change from one
> instance to the other. Having code is therefore:
>   A) Making the code more complex
>   B) For no particular reason.

If the dma channel count (which is a standard dma binding, likely for a 
reason) goes into the devicetree, it has to be moved out of the config.

The R40 (which has register manuals available) has the same register layout as 
the A64, but *does* have a different channel count. So you think it is a good 
idea to introduce a new compatible again?

If you had been half as picky when merging the H3 and A83T support, we would 
not have this mess now.

There is also the H6, where there is no register manual available yet, but I  
bet it has the H3 (and A64, H5, R40) register layout, but unlikely the same 
number of DMA channels *and* the same number of ports.

Regards,

Stefan

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

* Re: [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
  2017-09-04  7:59     ` Maxime Ripard
  (?)
@ 2017-09-04 14:47       ` Brüns, Stefan
  -1 siblings, 0 replies; 68+ messages in thread
From: Brüns, Stefan @ 2017-09-04 14:47 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Chen-Yu Tsai, Rob Herring, Code Kipper,
	Andre Przywara

On Montag, 4. September 2017 09:59:24 CEST Maxime Ripard wrote:
> On Mon, Sep 04, 2017 at 12:40:53AM +0200, Stefan Brüns wrote:
> > For the H3, the burst lengths field offsets in the channel configuration
> > register differs from earlier SoC generations.
> > 
> > Using the A31 register macros actually configured the H3 controller
> > do to bursts of length 1 always, which although working leads to higher
> > bus utilisation.
> > 
> > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 1d9b3be30d22..f1a139f0102f 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -68,13 +68,15 @@
> > 
> >  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> >  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
> >  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> > 
> > -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> > +#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> > +#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
> > 
> >  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
> >  
> >  #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
> >  #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
> >  #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
> > 
> > -#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
> > +#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) <<
> > 16) +#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x)
> > << 16)> 
> >  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
> >  
> >  #define DMA_CHAN_CUR_SRC	0x10
> > 
> > @@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
> > 
> >  	if (dst_width < 0)
> >  	
> >  		return dst_width;
> > 
> > -	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
> > -		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> > -		DMA_CHAN_CFG_DST_BURST(dst_burst) |
> > +	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> > 
> >  		DMA_CHAN_CFG_DST_WIDTH(dst_width);
> > 
> > +	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
> > +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
> > +			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
> > +	} else {
> > +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
> > +			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
> > +	}
> > +
> 
> I guess we have two options to support that properly. We could either
> have a different function that would generate that register value
> based on the parameters we have, or duplicate the set_config function
> entirely, with function pointer stored in the configuration.
> 
> I think I prefer the former, as it reduces the code duplication.

Duplicating "set_config" would also mean duplicating sun6i_dma_prep_dma_memcpy 
- there are two hunks which change setting of the burst length register value.

A function pointer in the config would work.

Kind regards,

Stefan

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

* Re: [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
@ 2017-09-04 14:47       ` Brüns, Stefan
  0 siblings, 0 replies; 68+ messages in thread
From: Brüns, Stefan @ 2017-09-04 14:47 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

On Montag, 4. September 2017 09:59:24 CEST Maxime Ripard wrote:
> On Mon, Sep 04, 2017 at 12:40:53AM +0200, Stefan Brüns wrote:
> > For the H3, the burst lengths field offsets in the channel configuration
> > register differs from earlier SoC generations.
> > 
> > Using the A31 register macros actually configured the H3 controller
> > do to bursts of length 1 always, which although working leads to higher
> > bus utilisation.
> > 
> > Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 1d9b3be30d22..f1a139f0102f 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -68,13 +68,15 @@
> > 
> >  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> >  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
> >  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> > 
> > -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> > +#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> > +#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
> > 
> >  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
> >  
> >  #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
> >  #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
> >  #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
> > 
> > -#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
> > +#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) <<
> > 16) +#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x)
> > << 16)> 
> >  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
> >  
> >  #define DMA_CHAN_CUR_SRC	0x10
> > 
> > @@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
> > 
> >  	if (dst_width < 0)
> >  	
> >  		return dst_width;
> > 
> > -	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
> > -		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> > -		DMA_CHAN_CFG_DST_BURST(dst_burst) |
> > +	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> > 
> >  		DMA_CHAN_CFG_DST_WIDTH(dst_width);
> > 
> > +	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
> > +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
> > +			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
> > +	} else {
> > +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
> > +			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
> > +	}
> > +
> 
> I guess we have two options to support that properly. We could either
> have a different function that would generate that register value
> based on the parameters we have, or duplicate the set_config function
> entirely, with function pointer stored in the configuration.
> 
> I think I prefer the former, as it reduces the code duplication.

Duplicating "set_config" would also mean duplicating sun6i_dma_prep_dma_memcpy 
- there are two hunks which change setting of the burst length register value.

A function pointer in the config would work.

Kind regards,

Stefan

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

* [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3
@ 2017-09-04 14:47       ` Brüns, Stefan
  0 siblings, 0 replies; 68+ messages in thread
From: Brüns, Stefan @ 2017-09-04 14:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Montag, 4. September 2017 09:59:24 CEST Maxime Ripard wrote:
> On Mon, Sep 04, 2017 at 12:40:53AM +0200, Stefan Br?ns wrote:
> > For the H3, the burst lengths field offsets in the channel configuration
> > register differs from earlier SoC generations.
> > 
> > Using the A31 register macros actually configured the H3 controller
> > do to bursts of length 1 always, which although working leads to higher
> > bus utilisation.
> > 
> > Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 1d9b3be30d22..f1a139f0102f 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -68,13 +68,15 @@
> > 
> >  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> >  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
> >  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> > 
> > -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> > +#define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> > +#define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
> > 
> >  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
> >  
> >  #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
> >  #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)
> >  #define DMA_CHAN_CFG_DST_LINEAR_MODE	(DMA_CHAN_CFG_SRC_LINEAR_MODE << 16)
> > 
> > -#define DMA_CHAN_CFG_DST_BURST(x)	(DMA_CHAN_CFG_SRC_BURST(x) << 16)
> > +#define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) <<
> > 16) +#define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x)
> > << 16)> 
> >  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
> >  
> >  #define DMA_CHAN_CUR_SRC	0x10
> > 
> > @@ -554,11 +556,17 @@ static int set_config(struct sun6i_dma_dev *sdev,
> > 
> >  	if (dst_width < 0)
> >  	
> >  		return dst_width;
> > 
> > -	*p_cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
> > -		DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> > -		DMA_CHAN_CFG_DST_BURST(dst_burst) |
> > +	*p_cfg = DMA_CHAN_CFG_SRC_WIDTH(src_width) |
> > 
> >  		DMA_CHAN_CFG_DST_WIDTH(dst_width);
> > 
> > +	if (sdev->cfg->dmac_variant == DMAC_VARIANT_H3) {
> > +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_H3(src_burst) |
> > +			  DMA_CHAN_CFG_DST_BURST_H3(dst_burst);
> > +	} else {
> > +		*p_cfg |= DMA_CHAN_CFG_SRC_BURST_A31(src_burst) |
> > +			  DMA_CHAN_CFG_DST_BURST_A31(dst_burst);
> > +	}
> > +
> 
> I guess we have two options to support that properly. We could either
> have a different function that would generate that register value
> based on the parameters we have, or duplicate the set_config function
> entirely, with function pointer stored in the configuration.
> 
> I think I prefer the former, as it reduces the code duplication.

Duplicating "set_config" would also mean duplicating sun6i_dma_prep_dma_memcpy 
- there are two hunks which change setting of the burst length register value.

A function pointer in the config would work.

Kind regards,

Stefan

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

* Re: [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
  2017-09-04 14:30       ` Brüns, Stefan
  (?)
@ 2017-09-08 14:37         ` Maxime Ripard
  -1 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-08 14:37 UTC (permalink / raw)
  To: Brüns, Stefan
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Chen-Yu Tsai, Rob Herring, Code Kipper,
	Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

On Mon, Sep 04, 2017 at 02:30:59PM +0000, Brüns, Stefan wrote:
> On Montag, 4. September 2017 09:43:55 CEST Maxime Ripard wrote:
> > On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Brüns wrote:
> > > Preparatory patch: If the same compatible is used for different SoCs which
> > > have a common register layout, but different number of channels, the
> > > channel count can no longer be stored in the config. Store it in the
> > > device structure instead.
> > > 
> > > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> > 
> > As stated already, we already are going to have a different
> > compatible, and this is not something that will change from one
> > instance to the other. Having code is therefore:
> >   A) Making the code more complex
> >   B) For no particular reason.
> 
> If the dma channel count (which is a standard dma binding, likely for a 
> reason) goes into the devicetree, it has to be moved out of the config.
>
> The R40 (which has register manuals available) has the same register layout as 
> the A64, but *does* have a different channel count. So you think it is a good 
> idea to introduce a new compatible again?
> 
> If you had been half as picky when merging the H3 and A83T support, we would 
> not have this mess now.
> 
> There is also the H6, where there is no register manual available yet, but I  
> bet it has the H3 (and A64, H5, R40) register layout, but unlikely the same 
> number of DMA channels *and* the same number of ports.

The thing is that this kind of things usually grow organically until
you can't just add a simple if case any more.

I'm sorry you were at the tipping point, but I'm sure you also
understand that adding more to the mess until the next one shows up
isn't viable either.

That being said, thinking a bit more on that one, if you add to the
binding that we need to have both the current SoC compatible (to
workaround any variation / bugs we might encounter in the future) and
the "generation" one (to avoid adding one for each new IP), plus the
mandatory dma-channels / dma-requests properties, that would work for
me.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-08 14:37         ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-08 14:37 UTC (permalink / raw)
  To: Brüns, Stefan
  Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Chen-Yu Tsai, Rob Herring,
	Code Kipper, Andre Przywara

[-- Attachment #1: Type: text/plain, Size: 2606 bytes --]

On Mon, Sep 04, 2017 at 02:30:59PM +0000, Brüns, Stefan wrote:
> On Montag, 4. September 2017 09:43:55 CEST Maxime Ripard wrote:
> > On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Brüns wrote:
> > > Preparatory patch: If the same compatible is used for different SoCs which
> > > have a common register layout, but different number of channels, the
> > > channel count can no longer be stored in the config. Store it in the
> > > device structure instead.
> > > 
> > > Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
> > 
> > As stated already, we already are going to have a different
> > compatible, and this is not something that will change from one
> > instance to the other. Having code is therefore:
> >   A) Making the code more complex
> >   B) For no particular reason.
> 
> If the dma channel count (which is a standard dma binding, likely for a 
> reason) goes into the devicetree, it has to be moved out of the config.
>
> The R40 (which has register manuals available) has the same register layout as 
> the A64, but *does* have a different channel count. So you think it is a good 
> idea to introduce a new compatible again?
> 
> If you had been half as picky when merging the H3 and A83T support, we would 
> not have this mess now.
> 
> There is also the H6, where there is no register manual available yet, but I  
> bet it has the H3 (and A64, H5, R40) register layout, but unlikely the same 
> number of DMA channels *and* the same number of ports.

The thing is that this kind of things usually grow organically until
you can't just add a simple if case any more.

I'm sorry you were at the tipping point, but I'm sure you also
understand that adding more to the mess until the next one shows up
isn't viable either.

That being said, thinking a bit more on that one, if you add to the
binding that we need to have both the current SoC compatible (to
workaround any variation / bugs we might encounter in the future) and
the "generation" one (to avoid adding one for each new IP), plus the
mandatory dma-channels / dma-requests properties, that would work for
me.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct
@ 2017-09-08 14:37         ` Maxime Ripard
  0 siblings, 0 replies; 68+ messages in thread
From: Maxime Ripard @ 2017-09-08 14:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 04, 2017 at 02:30:59PM +0000, Br?ns, Stefan wrote:
> On Montag, 4. September 2017 09:43:55 CEST Maxime Ripard wrote:
> > On Mon, Sep 04, 2017 at 12:40:56AM +0200, Stefan Br?ns wrote:
> > > Preparatory patch: If the same compatible is used for different SoCs which
> > > have a common register layout, but different number of channels, the
> > > channel count can no longer be stored in the config. Store it in the
> > > device structure instead.
> > > 
> > > Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> > 
> > As stated already, we already are going to have a different
> > compatible, and this is not something that will change from one
> > instance to the other. Having code is therefore:
> >   A) Making the code more complex
> >   B) For no particular reason.
> 
> If the dma channel count (which is a standard dma binding, likely for a 
> reason) goes into the devicetree, it has to be moved out of the config.
>
> The R40 (which has register manuals available) has the same register layout as 
> the A64, but *does* have a different channel count. So you think it is a good 
> idea to introduce a new compatible again?
> 
> If you had been half as picky when merging the H3 and A83T support, we would 
> not have this mess now.
> 
> There is also the H6, where there is no register manual available yet, but I  
> bet it has the H3 (and A64, H5, R40) register layout, but unlikely the same 
> number of DMA channels *and* the same number of ports.

The thing is that this kind of things usually grow organically until
you can't just add a simple if case any more.

I'm sorry you were at the tipping point, but I'm sure you also
understand that adding more to the mess until the next one shows up
isn't viable either.

That being said, thinking a bit more on that one, if you add to the
binding that we need to have both the current SoC compatible (to
workaround any variation / bugs we might encounter in the future) and
the "generation" one (to avoid adding one for each new IP), plus the
mandatory dma-channels / dma-requests properties, that would work for
me.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170908/67b516d8/attachment.sig>

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

* Re: [PATCH 06/10] arm64: allwinner: a64: Add devicetree binding for DMA controller
  2017-09-03 22:40   ` Stefan Brüns
  (?)
@ 2017-09-12 16:52     ` Rob Herring
  -1 siblings, 0 replies; 68+ messages in thread
From: Rob Herring @ 2017-09-12 16:52 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: linux-sunxi, devicetree, dmaengine, Vinod Koul, linux-arm-kernel,
	linux-kernel, Maxime Ripard, Chen-Yu Tsai, Code Kipper,
	Andre Przywara

On Mon, Sep 04, 2017 at 12:40:57AM +0200, Stefan Brüns wrote:
> The A64 is register compatible with the H3, but has a different number
> of dma channels and request ports.
> 
> Attach additional properties to the node to allow future reuse of the
> compatible for controllers with different number of channels/requests.
> 
> If dma-requests is not specified, the register layout defined maximum
> of 32 is used.

This belongs in the binding.

> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  .../devicetree/bindings/dma/sun6i-dma.txt          | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> index 6b267045f522..66195fb31296 100644
> --- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> @@ -26,6 +26,32 @@ Example:
>  		#dma-cells = <1>;
>  	};
>  
> +------------------------------------------------------------------------------
> +For A64 DMA controller:
> +
> +Required properties:
> +- compatible:	"allwinner,sun50i-a64-dma"
> +- dma-channels: Number of DMA channels supported by the controller.
> +		Refer to Documentation/devicetree/bindings/dma/dma.txt

dma.txt already explains what these properties are. You just need to 
state what are valid values.

> +- all properties above, i.e. reg, interrupts, clocks, resets and #dma-cells
> +
> +Optional properties:
> +- dma-requests: Number of DMA request signals supported by the controller.
> +		Refer to Documentation/devicetree/bindings/dma/dma.txt
> +
> +Example:
> +	dma: dma-controller@01c02000 {

Drop the leading 0. Building dtbs with W=2 will tell you this.

> +		compatible = "allwinner,sun50i-a64-dma";
> +		reg = <0x01c02000 0x1000>;
> +		interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&ccu CLK_BUS_DMA>;
> +		dma-channels = <8>;
> +		dma-requests = <27>;
> +		resets = <&ccu RST_BUS_DMA>;
> +		#dma-cells = <1>;
> +	};
> +------------------------------------------------------------------------------
> +
>  Clients:
>  
>  DMA clients connected to the A31 DMA controller must use the format
> -- 
> 2.14.1
> 

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

* Re: [PATCH 06/10] arm64: allwinner: a64: Add devicetree binding for DMA controller
@ 2017-09-12 16:52     ` Rob Herring
  0 siblings, 0 replies; 68+ messages in thread
From: Rob Herring @ 2017-09-12 16:52 UTC (permalink / raw)
  To: Stefan Brüns
  Cc: devicetree, Vinod Koul, Andre Przywara, linux-kernel,
	Code Kipper, linux-sunxi, dmaengine, Maxime Ripard, Chen-Yu Tsai,
	linux-arm-kernel

On Mon, Sep 04, 2017 at 12:40:57AM +0200, Stefan Brüns wrote:
> The A64 is register compatible with the H3, but has a different number
> of dma channels and request ports.
> 
> Attach additional properties to the node to allow future reuse of the
> compatible for controllers with different number of channels/requests.
> 
> If dma-requests is not specified, the register layout defined maximum
> of 32 is used.

This belongs in the binding.

> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  .../devicetree/bindings/dma/sun6i-dma.txt          | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> index 6b267045f522..66195fb31296 100644
> --- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> @@ -26,6 +26,32 @@ Example:
>  		#dma-cells = <1>;
>  	};
>  
> +------------------------------------------------------------------------------
> +For A64 DMA controller:
> +
> +Required properties:
> +- compatible:	"allwinner,sun50i-a64-dma"
> +- dma-channels: Number of DMA channels supported by the controller.
> +		Refer to Documentation/devicetree/bindings/dma/dma.txt

dma.txt already explains what these properties are. You just need to 
state what are valid values.

> +- all properties above, i.e. reg, interrupts, clocks, resets and #dma-cells
> +
> +Optional properties:
> +- dma-requests: Number of DMA request signals supported by the controller.
> +		Refer to Documentation/devicetree/bindings/dma/dma.txt
> +
> +Example:
> +	dma: dma-controller@01c02000 {

Drop the leading 0. Building dtbs with W=2 will tell you this.

> +		compatible = "allwinner,sun50i-a64-dma";
> +		reg = <0x01c02000 0x1000>;
> +		interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&ccu CLK_BUS_DMA>;
> +		dma-channels = <8>;
> +		dma-requests = <27>;
> +		resets = <&ccu RST_BUS_DMA>;
> +		#dma-cells = <1>;
> +	};
> +------------------------------------------------------------------------------
> +
>  Clients:
>  
>  DMA clients connected to the A31 DMA controller must use the format
> -- 
> 2.14.1
> 

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

* [PATCH 06/10] arm64: allwinner: a64: Add devicetree binding for DMA controller
@ 2017-09-12 16:52     ` Rob Herring
  0 siblings, 0 replies; 68+ messages in thread
From: Rob Herring @ 2017-09-12 16:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 04, 2017 at 12:40:57AM +0200, Stefan Br?ns wrote:
> The A64 is register compatible with the H3, but has a different number
> of dma channels and request ports.
> 
> Attach additional properties to the node to allow future reuse of the
> compatible for controllers with different number of channels/requests.
> 
> If dma-requests is not specified, the register layout defined maximum
> of 32 is used.

This belongs in the binding.

> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  .../devicetree/bindings/dma/sun6i-dma.txt          | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> index 6b267045f522..66195fb31296 100644
> --- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> @@ -26,6 +26,32 @@ Example:
>  		#dma-cells = <1>;
>  	};
>  
> +------------------------------------------------------------------------------
> +For A64 DMA controller:
> +
> +Required properties:
> +- compatible:	"allwinner,sun50i-a64-dma"
> +- dma-channels: Number of DMA channels supported by the controller.
> +		Refer to Documentation/devicetree/bindings/dma/dma.txt

dma.txt already explains what these properties are. You just need to 
state what are valid values.

> +- all properties above, i.e. reg, interrupts, clocks, resets and #dma-cells
> +
> +Optional properties:
> +- dma-requests: Number of DMA request signals supported by the controller.
> +		Refer to Documentation/devicetree/bindings/dma/dma.txt
> +
> +Example:
> +	dma: dma-controller at 01c02000 {

Drop the leading 0. Building dtbs with W=2 will tell you this.

> +		compatible = "allwinner,sun50i-a64-dma";
> +		reg = <0x01c02000 0x1000>;
> +		interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&ccu CLK_BUS_DMA>;
> +		dma-channels = <8>;
> +		dma-requests = <27>;
> +		resets = <&ccu RST_BUS_DMA>;
> +		#dma-cells = <1>;
> +	};
> +------------------------------------------------------------------------------
> +
>  Clients:
>  
>  DMA clients connected to the A31 DMA controller must use the format
> -- 
> 2.14.1
> 

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

end of thread, other threads:[~2017-09-12 16:52 UTC | newest]

Thread overview: 68+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-03 22:40 [PATCH 00/10] dmaengine: sun6i: Fixes for H3/A83T, enable A64 Stefan Brüns
2017-09-03 22:40 ` Stefan Brüns
2017-09-03 22:40 ` Stefan Brüns
2017-09-03 22:40 ` [PATCH 01/10] dmaengine: sun6i: Correct setting of clock autogating register for A83T/H3 Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 23:23   ` André Przywara
2017-09-03 23:23     ` André Przywara
2017-09-04  7:06     ` Maxime Ripard
2017-09-04  7:06       ` Maxime Ripard
2017-09-04  7:06       ` Maxime Ripard
2017-09-03 22:40 ` [PATCH 02/10] dmaengine: sun6i: Correct burst length field offsets for H3 Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-04  7:59   ` Maxime Ripard
2017-09-04  7:59     ` Maxime Ripard
2017-09-04  7:59     ` Maxime Ripard
2017-09-04 14:47     ` Brüns, Stefan
2017-09-04 14:47       ` Brüns, Stefan
2017-09-04 14:47       ` Brüns, Stefan
2017-09-03 22:40 ` [PATCH 03/10] dmaengine: sun6i: Restructure code to allow extension for new SoCs Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40 ` [PATCH 04/10] dmaengine: sun6i: Enable additional burst lengths/widths on H3 Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-04  8:00   ` Maxime Ripard
2017-09-04  8:00     ` Maxime Ripard
2017-09-04  8:00     ` Maxime Ripard
2017-09-03 22:40 ` [PATCH 05/10] dmaengine: sun6i: Move number of pchans/vchans/request to device struct Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-04  7:43   ` Maxime Ripard
2017-09-04  7:43     ` Maxime Ripard
2017-09-04  7:43     ` Maxime Ripard
2017-09-04 14:30     ` Brüns, Stefan
2017-09-04 14:30       ` Brüns, Stefan
2017-09-04 14:30       ` Brüns, Stefan
2017-09-08 14:37       ` Maxime Ripard
2017-09-08 14:37         ` Maxime Ripard
2017-09-08 14:37         ` Maxime Ripard
2017-09-03 22:40 ` [PATCH 06/10] arm64: allwinner: a64: Add devicetree binding for DMA controller Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-12 16:52   ` Rob Herring
2017-09-12 16:52     ` Rob Herring
2017-09-12 16:52     ` Rob Herring
2017-09-03 22:40 ` [PATCH 07/10] dmaengine: sun6i: Retrieve channel count/max request from devicetree Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 23:44   ` André Przywara
2017-09-03 23:44     ` André Przywara
2017-09-03 23:44     ` André Przywara
2017-09-04  0:12     ` Stefan Bruens
2017-09-04  0:12       ` Stefan Bruens
2017-09-04  0:12       ` Stefan Bruens
2017-09-03 22:40 ` [PATCH 08/10] dmaengine: sun6i: Add support for Allwinner A64 and compatibles Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 22:40   ` Stefan Brüns
2017-09-03 23:37   ` André Przywara
2017-09-03 23:37     ` André Przywara
2017-09-03 23:37     ` André Przywara
2017-09-04  0:13     ` Stefan Bruens
2017-09-04  0:13       ` Stefan Bruens
2017-09-04  0:13       ` Stefan Bruens
2017-09-03 22:41 ` [PATCH 09/10] arm64: allwinner: a64: Add device node for DMA controller Stefan Brüns
2017-09-03 22:41   ` Stefan Brüns
2017-09-03 22:41   ` Stefan Brüns

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.