linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Allwinner V3s DMA support
@ 2017-06-05 12:33 Icenowy Zheng
  2017-06-05 12:33 ` [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng
  2017-06-05 12:33 ` [PATCH 2/2] dmaengine: sun6i: support V3s SoC variant Icenowy Zheng
  0 siblings, 2 replies; 12+ messages in thread
From: Icenowy Zheng @ 2017-06-05 12:33 UTC (permalink / raw)
  To: Vinod Koul, Maxime Ripard, Chen-Yu Tsai
  Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-sunxi, Icenowy Zheng

This is a dedicated patchset of Allwinner V3s DMA support, which used
to be part of the audio codec support patchset.

It's a derivation of the DMA part of v3 of the codec patchset.

Icenowy Zheng (2):
  dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  dmaengine: sun6i: support V3s SoC variant

 .../devicetree/bindings/dma/sun6i-dma.txt          |  1 +
 drivers/dma/sun6i-dma.c                            | 33 +++++++++++++++++-----
 2 files changed, 27 insertions(+), 7 deletions(-)

-- 
2.12.2

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

* [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-05 12:33 [PATCH 0/2] Allwinner V3s DMA support Icenowy Zheng
@ 2017-06-05 12:33 ` Icenowy Zheng
  2017-06-05 13:15   ` [linux-sunxi] " Chen-Yu Tsai
  2017-06-14  8:32   ` Vinod Koul
  2017-06-05 12:33 ` [PATCH 2/2] dmaengine: sun6i: support V3s SoC variant Icenowy Zheng
  1 sibling, 2 replies; 12+ messages in thread
From: Icenowy Zheng @ 2017-06-05 12:33 UTC (permalink / raw)
  To: Vinod Koul, Maxime Ripard, Chen-Yu Tsai
  Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-sunxi, Icenowy Zheng

From: Icenowy Zheng <icenowy@aosc.xyz>

Originally we enable a special gate bit when the compatible indicates
A23/33.

But according to BSP sources and user manuals, more SoCs will need this
gate bit.

So make it a common quirk configured in the config struct.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
Changes since original codec patchset v3:
- Refactored comments to cover some words found in official documents.
- Removed the comments when toggling the gate bit.

 drivers/dma/sun6i-dma.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a2358780ab2c..252b59c1d1d5 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -101,6 +101,17 @@ struct sun6i_dma_config {
 	u32 nr_max_channels;
 	u32 nr_max_requests;
 	u32 nr_max_vchans;
+	/*
+	 * In the datasheets/user manuals of newer Allwinner SoCs, a special
+	 * bit (bit 2 at register 0x20) is present.
+	 * It's named "DMA MCLK interface circuit auto gating bit" in the
+	 * documents, and the footnote of this register says that this bit
+	 * should be set up when initializing the DMA controller.
+	 * Allwinner A23/A33 user manuals do not have this bit documented,
+	 * however these SoCs really have and need this bit, as seen in the
+	 * BSP kernel source code.
+	 */
+	bool gate_needed;
 };
 
 /*
@@ -1009,6 +1020,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.nr_max_channels = 8,
 	.nr_max_requests = 24,
 	.nr_max_vchans   = 37,
+	.gate_needed	 = true,
 };
 
 static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
@@ -1174,13 +1186,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 		goto err_dma_unregister;
 	}
 
-	/*
-	 * 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.
-	 */
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "allwinner,sun8i-a23-dma"))
+	if (sdc->cfg->gate_needed)
 		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
 
 	return 0;
-- 
2.12.2

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

* [PATCH 2/2] dmaengine: sun6i: support V3s SoC variant
  2017-06-05 12:33 [PATCH 0/2] Allwinner V3s DMA support Icenowy Zheng
  2017-06-05 12:33 ` [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng
@ 2017-06-05 12:33 ` Icenowy Zheng
  1 sibling, 0 replies; 12+ messages in thread
From: Icenowy Zheng @ 2017-06-05 12:33 UTC (permalink / raw)
  To: Vinod Koul, Maxime Ripard, Chen-Yu Tsai
  Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-sunxi, Icenowy Zheng

From: Icenowy Zheng <icenowy@aosc.xyz>

Allwinner V3s has a DMA engine similar to the ones from A31, but with
fewer channels and DRQs.

Add support for it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
Acked-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes since the original codec patchset v3:
- Added Rob's ACK.

 Documentation/devicetree/bindings/dma/sun6i-dma.txt |  1 +
 drivers/dma/sun6i-dma.c                             | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
index 6b267045f522..98fbe1a5c6dd 100644
--- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
+++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
@@ -9,6 +9,7 @@ Required properties:
 		  "allwinner,sun8i-a23-dma"
 		  "allwinner,sun8i-a83t-dma"
 		  "allwinner,sun8i-h3-dma"
+		  "allwinner,sun8i-v3s-dma"
 - reg:		Should contain the registers base address and length
 - interrupts:	Should contain a reference to the interrupt used by this device
 - clocks:	Should contain a reference to the parent AHB clock
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 252b59c1d1d5..bcd496edc70f 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -1040,11 +1040,24 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.nr_max_vchans   = 34,
 };
 
+/*
+ * The V3s have only 8 physical channels, a maximum DRQ port id of 23,
+ * and a total of 24 usable source and destination endpoints.
+ */
+
+static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
+	.nr_max_channels = 8,
+	.nr_max_requests = 23,
+	.nr_max_vchans   = 24,
+	.gate_needed	 = true,
+};
+
 static const struct of_device_id sun6i_dma_match[] = {
 	{ .compatible = "allwinner,sun6i-a31-dma", .data = &sun6i_a31_dma_cfg },
 	{ .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,sun8i-v3s-dma", .data = &sun8i_v3s_dma_cfg },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dma_match);
-- 
2.12.2

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

* Re: [linux-sunxi] [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-05 12:33 ` [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng
@ 2017-06-05 13:15   ` Chen-Yu Tsai
  2017-06-14  8:32   ` Vinod Koul
  1 sibling, 0 replies; 12+ messages in thread
From: Chen-Yu Tsai @ 2017-06-05 13:15 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Vinod Koul, Maxime Ripard, Chen-Yu Tsai, dmaengine,
	linux-arm-kernel, linux-kernel, linux-sunxi, Icenowy Zheng

On Mon, Jun 5, 2017 at 8:33 PM, Icenowy Zheng <icenowy@aosc.io> wrote:
> From: Icenowy Zheng <icenowy@aosc.xyz>
>
> Originally we enable a special gate bit when the compatible indicates
> A23/33.
>
> But according to BSP sources and user manuals, more SoCs will need this
> gate bit.
>
> So make it a common quirk configured in the config struct.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-05 12:33 ` [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng
  2017-06-05 13:15   ` [linux-sunxi] " Chen-Yu Tsai
@ 2017-06-14  8:32   ` Vinod Koul
  2017-06-14  8:32     ` Icenowy Zheng
  1 sibling, 1 reply; 12+ messages in thread
From: Vinod Koul @ 2017-06-14  8:32 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Maxime Ripard, Chen-Yu Tsai, dmaengine, linux-arm-kernel,
	linux-kernel, linux-sunxi, Icenowy Zheng

On Mon, Jun 05, 2017 at 08:33:47PM +0800, Icenowy Zheng wrote:
> From: Icenowy Zheng <icenowy@aosc.xyz>
> 
> Originally we enable a special gate bit when the compatible indicates
> A23/33.
> 
> But according to BSP sources and user manuals, more SoCs will need this
> gate bit.
> 
> So make it a common quirk configured in the config struct.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
> Changes since original codec patchset v3:
> - Refactored comments to cover some words found in official documents.
> - Removed the comments when toggling the gate bit.
> 
>  drivers/dma/sun6i-dma.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a2358780ab2c..252b59c1d1d5 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -101,6 +101,17 @@ struct sun6i_dma_config {
>  	u32 nr_max_channels;
>  	u32 nr_max_requests;
>  	u32 nr_max_vchans;
> +	/*
> +	 * In the datasheets/user manuals of newer Allwinner SoCs, a special
> +	 * bit (bit 2 at register 0x20) is present.
> +	 * It's named "DMA MCLK interface circuit auto gating bit" in the
> +	 * documents, and the footnote of this register says that this bit
> +	 * should be set up when initializing the DMA controller.
> +	 * Allwinner A23/A33 user manuals do not have this bit documented,
> +	 * however these SoCs really have and need this bit, as seen in the
> +	 * BSP kernel source code.
> +	 */
> +	bool gate_needed;

Since this is a hw property, why is this not added as an optional DT
property?

>  };
>  
>  /*
> @@ -1009,6 +1020,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
>  	.nr_max_channels = 8,
>  	.nr_max_requests = 24,
>  	.nr_max_vchans   = 37,
> +	.gate_needed	 = true,
>  };
>  
>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
> @@ -1174,13 +1186,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  		goto err_dma_unregister;
>  	}
>  
> -	/*
> -	 * 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.
> -	 */
> -	if (of_device_is_compatible(pdev->dev.of_node,
> -				    "allwinner,sun8i-a23-dma"))
> +	if (sdc->cfg->gate_needed)
>  		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
>  
>  	return 0;
> -- 
> 2.12.2
> 

-- 
~Vinod

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

* Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-14  8:32   ` Vinod Koul
@ 2017-06-14  8:32     ` Icenowy Zheng
  2017-06-14  8:45       ` Vinod Koul
  0 siblings, 1 reply; 12+ messages in thread
From: Icenowy Zheng @ 2017-06-14  8:32 UTC (permalink / raw)
  To: linux-arm-kernel, Vinod Koul
  Cc: Chen-Yu Tsai, linux-kernel, linux-sunxi, Icenowy Zheng,
	dmaengine, Maxime Ripard



于 2017年6月14日 GMT+08:00 下午4:32:52, Vinod Koul <vinod.koul@intel.com> 写到:
>On Mon, Jun 05, 2017 at 08:33:47PM +0800, Icenowy Zheng wrote:
>> From: Icenowy Zheng <icenowy@aosc.xyz>
>> 
>> Originally we enable a special gate bit when the compatible indicates
>> A23/33.
>> 
>> But according to BSP sources and user manuals, more SoCs will need
>this
>> gate bit.
>> 
>> So make it a common quirk configured in the config struct.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ---
>> Changes since original codec patchset v3:
>> - Refactored comments to cover some words found in official
>documents.
>> - Removed the comments when toggling the gate bit.
>> 
>>  drivers/dma/sun6i-dma.c | 20 +++++++++++++-------
>>  1 file changed, 13 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
>> index a2358780ab2c..252b59c1d1d5 100644
>> --- a/drivers/dma/sun6i-dma.c
>> +++ b/drivers/dma/sun6i-dma.c
>> @@ -101,6 +101,17 @@ struct sun6i_dma_config {
>>  	u32 nr_max_channels;
>>  	u32 nr_max_requests;
>>  	u32 nr_max_vchans;
>> +	/*
>> +	 * In the datasheets/user manuals of newer Allwinner SoCs, a
>special
>> +	 * bit (bit 2 at register 0x20) is present.
>> +	 * It's named "DMA MCLK interface circuit auto gating bit" in the
>> +	 * documents, and the footnote of this register says that this bit
>> +	 * should be set up when initializing the DMA controller.
>> +	 * Allwinner A23/A33 user manuals do not have this bit documented,
>> +	 * however these SoCs really have and need this bit, as seen in the
>> +	 * BSP kernel source code.
>> +	 */
>> +	bool gate_needed;
>
>Since this is a hw property, why is this not added as an optional DT
>property?

As it's SoC-specified.

Some SoCs need it, and some don't.

SoC info is in compatible, so there's no reason to make it a property.

>
>>  };
>>  
>>  /*
>> @@ -1009,6 +1020,7 @@ static struct sun6i_dma_config
>sun8i_a23_dma_cfg = {
>>  	.nr_max_channels = 8,
>>  	.nr_max_requests = 24,
>>  	.nr_max_vchans   = 37,
>> +	.gate_needed	 = true,
>>  };
>>  
>>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
>> @@ -1174,13 +1186,7 @@ static int sun6i_dma_probe(struct
>platform_device *pdev)
>>  		goto err_dma_unregister;
>>  	}
>>  
>> -	/*
>> -	 * 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.
>> -	 */
>> -	if (of_device_is_compatible(pdev->dev.of_node,
>> -				    "allwinner,sun8i-a23-dma"))
>> +	if (sdc->cfg->gate_needed)
>>  		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
>>  
>>  	return 0;
>> -- 
>> 2.12.2
>> 

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

* Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-14  8:32     ` Icenowy Zheng
@ 2017-06-14  8:45       ` Vinod Koul
  2017-06-14  8:46         ` [linux-sunxi] " Icenowy Zheng
  2017-06-14  9:04         ` Maxime Ripard
  0 siblings, 2 replies; 12+ messages in thread
From: Vinod Koul @ 2017-06-14  8:45 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: linux-arm-kernel, Chen-Yu Tsai, linux-kernel, linux-sunxi,
	Icenowy Zheng, dmaengine, Maxime Ripard

On Wed, Jun 14, 2017 at 04:32:57PM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2017年6月14日 GMT+08:00 下午4:32:52, Vinod Koul <vinod.koul@intel.com> 写到:
> >On Mon, Jun 05, 2017 at 08:33:47PM +0800, Icenowy Zheng wrote:
> >> From: Icenowy Zheng <icenowy@aosc.xyz>
> >> 
> >> Originally we enable a special gate bit when the compatible indicates
> >> A23/33.
> >> 
> >> But according to BSP sources and user manuals, more SoCs will need
> >this
> >> gate bit.
> >> 
> >> So make it a common quirk configured in the config struct.
> >> 
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> >> ---
> >> Changes since original codec patchset v3:
> >> - Refactored comments to cover some words found in official
> >documents.
> >> - Removed the comments when toggling the gate bit.
> >> 
> >>  drivers/dma/sun6i-dma.c | 20 +++++++++++++-------
> >>  1 file changed, 13 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> >> index a2358780ab2c..252b59c1d1d5 100644
> >> --- a/drivers/dma/sun6i-dma.c
> >> +++ b/drivers/dma/sun6i-dma.c
> >> @@ -101,6 +101,17 @@ struct sun6i_dma_config {
> >>  	u32 nr_max_channels;
> >>  	u32 nr_max_requests;
> >>  	u32 nr_max_vchans;
> >> +	/*
> >> +	 * In the datasheets/user manuals of newer Allwinner SoCs, a
> >special
> >> +	 * bit (bit 2 at register 0x20) is present.
> >> +	 * It's named "DMA MCLK interface circuit auto gating bit" in the
> >> +	 * documents, and the footnote of this register says that this bit
> >> +	 * should be set up when initializing the DMA controller.
> >> +	 * Allwinner A23/A33 user manuals do not have this bit documented,
> >> +	 * however these SoCs really have and need this bit, as seen in the
> >> +	 * BSP kernel source code.
> >> +	 */
> >> +	bool gate_needed;
> >
> >Since this is a hw property, why is this not added as an optional DT
> >property?
> 
> As it's SoC-specified.
> 
> Some SoCs need it, and some don't.

and that is the reason it should be a property

> 
> SoC info is in compatible, so there's no reason to make it a property.

that's why it would need to be optional for the SoC's that needs these..

> 
> >
> >>  };
> >>  
> >>  /*
> >> @@ -1009,6 +1020,7 @@ static struct sun6i_dma_config
> >sun8i_a23_dma_cfg = {
> >>  	.nr_max_channels = 8,
> >>  	.nr_max_requests = 24,
> >>  	.nr_max_vchans   = 37,
> >> +	.gate_needed	 = true,
> >>  };
> >>  
> >>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
> >> @@ -1174,13 +1186,7 @@ static int sun6i_dma_probe(struct
> >platform_device *pdev)
> >>  		goto err_dma_unregister;
> >>  	}
> >>  
> >> -	/*
> >> -	 * 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.
> >> -	 */
> >> -	if (of_device_is_compatible(pdev->dev.of_node,
> >> -				    "allwinner,sun8i-a23-dma"))
> >> +	if (sdc->cfg->gate_needed)
> >>  		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
> >>  
> >>  	return 0;
> >> -- 
> >> 2.12.2
> >> 

-- 
~Vinod

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

* Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-14  8:45       ` Vinod Koul
@ 2017-06-14  8:46         ` Icenowy Zheng
  2017-06-14  9:04         ` Maxime Ripard
  1 sibling, 0 replies; 12+ messages in thread
From: Icenowy Zheng @ 2017-06-14  8:46 UTC (permalink / raw)
  To: vinod.koul, Vinod Koul, Rob Herring
  Cc: linux-arm-kernel, Chen-Yu Tsai, linux-kernel, linux-sunxi,
	Icenowy Zheng, dmaengine, Maxime Ripard



于 2017年6月14日 GMT+08:00 下午4:45:29, Vinod Koul <vinod.koul@intel.com> 写到:
>On Wed, Jun 14, 2017 at 04:32:57PM +0800, Icenowy Zheng wrote:
>> 
>> 
>> 于 2017年6月14日 GMT+08:00 下午4:32:52, Vinod Koul <vinod.koul@intel.com>
>写到:
>> >On Mon, Jun 05, 2017 at 08:33:47PM +0800, Icenowy Zheng wrote:
>> >> From: Icenowy Zheng <icenowy@aosc.xyz>
>> >> 
>> >> Originally we enable a special gate bit when the compatible
>indicates
>> >> A23/33.
>> >> 
>> >> But according to BSP sources and user manuals, more SoCs will need
>> >this
>> >> gate bit.
>> >> 
>> >> So make it a common quirk configured in the config struct.
>> >> 
>> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> >> ---
>> >> Changes since original codec patchset v3:
>> >> - Refactored comments to cover some words found in official
>> >documents.
>> >> - Removed the comments when toggling the gate bit.
>> >> 
>> >>  drivers/dma/sun6i-dma.c | 20 +++++++++++++-------
>> >>  1 file changed, 13 insertions(+), 7 deletions(-)
>> >> 
>> >> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
>> >> index a2358780ab2c..252b59c1d1d5 100644
>> >> --- a/drivers/dma/sun6i-dma.c
>> >> +++ b/drivers/dma/sun6i-dma.c
>> >> @@ -101,6 +101,17 @@ struct sun6i_dma_config {
>> >>  	u32 nr_max_channels;
>> >>  	u32 nr_max_requests;
>> >>  	u32 nr_max_vchans;
>> >> +	/*
>> >> +	 * In the datasheets/user manuals of newer Allwinner SoCs, a
>> >special
>> >> +	 * bit (bit 2 at register 0x20) is present.
>> >> +	 * It's named "DMA MCLK interface circuit auto gating bit" in
>the
>> >> +	 * documents, and the footnote of this register says that this
>bit
>> >> +	 * should be set up when initializing the DMA controller.
>> >> +	 * Allwinner A23/A33 user manuals do not have this bit
>documented,
>> >> +	 * however these SoCs really have and need this bit, as seen in
>the
>> >> +	 * BSP kernel source code.
>> >> +	 */
>> >> +	bool gate_needed;
>> >
>> >Since this is a hw property, why is this not added as an optional DT
>> >property?
>> 
>> As it's SoC-specified.
>> 
>> Some SoCs need it, and some don't.
>
>and that is the reason it should be a property
>
>> 
>> SoC info is in compatible, so there's no reason to make it a
>property.
>
>that's why it would need to be optional for the SoC's that needs
>these..

I don't think it proper to add block-specified properties
that can be bound to compatible.

I added Rob Herring to the recipient list.

Rob, do you think this can be added as a property?

This is SoC-specific and compatibles are also SoC-specific.

>
>> 
>> >
>> >>  };
>> >>  
>> >>  /*
>> >> @@ -1009,6 +1020,7 @@ static struct sun6i_dma_config
>> >sun8i_a23_dma_cfg = {
>> >>  	.nr_max_channels = 8,
>> >>  	.nr_max_requests = 24,
>> >>  	.nr_max_vchans   = 37,
>> >> +	.gate_needed	 = true,
>> >>  };
>> >>  
>> >>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
>> >> @@ -1174,13 +1186,7 @@ static int sun6i_dma_probe(struct
>> >platform_device *pdev)
>> >>  		goto err_dma_unregister;
>> >>  	}
>> >>  
>> >> -	/*
>> >> -	 * 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.
>> >> -	 */
>> >> -	if (of_device_is_compatible(pdev->dev.of_node,
>> >> -				    "allwinner,sun8i-a23-dma"))
>> >> +	if (sdc->cfg->gate_needed)
>> >>  		writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
>> >>  
>> >>  	return 0;
>> >> -- 
>> >> 2.12.2
>> >> 

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

* Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-14  8:45       ` Vinod Koul
  2017-06-14  8:46         ` [linux-sunxi] " Icenowy Zheng
@ 2017-06-14  9:04         ` Maxime Ripard
  2017-06-15  3:54           ` Vinod Koul
  1 sibling, 1 reply; 12+ messages in thread
From: Maxime Ripard @ 2017-06-14  9:04 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Icenowy Zheng, linux-arm-kernel, Chen-Yu Tsai, linux-kernel,
	linux-sunxi, Icenowy Zheng, dmaengine

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

On Wed, Jun 14, 2017 at 02:15:29PM +0530, Vinod Koul wrote:
> > SoC info is in compatible, so there's no reason to make it a property.
> 
> that's why it would need to be optional for the SoC's that needs these..

There's nothing optional about that behaviour, it's mandatory for the
SoC that need it, and useless on the SoC that don't.

Plus, that would require changing the DT binding, which isn't
something we can do.

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] 12+ messages in thread

* Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-15  3:54           ` Vinod Koul
@ 2017-06-15  3:53             ` Icenowy Zheng
  2017-06-20  8:45             ` Maxime Ripard
  1 sibling, 0 replies; 12+ messages in thread
From: Icenowy Zheng @ 2017-06-15  3:53 UTC (permalink / raw)
  To: Vinod Koul, Maxime Ripard, Rob Herring
  Cc: linux-arm-kernel, Chen-Yu Tsai, linux-kernel, linux-sunxi,
	Icenowy Zheng, dmaengine



于 2017年6月15日 GMT+08:00 上午11:54:08, Vinod Koul <vinod.koul@intel.com> 写到:
>On Wed, Jun 14, 2017 at 11:04:39AM +0200, Maxime Ripard wrote:
>> On Wed, Jun 14, 2017 at 02:15:29PM +0530, Vinod Koul wrote:
>> > > SoC info is in compatible, so there's no reason to make it a
>property.
>> > 
>> > that's why it would need to be optional for the SoC's that needs
>these..
>> 
>> There's nothing optional about that behaviour, it's mandatory for the
>> SoC that need it, and useless on the SoC that don't.
>
>And why should kernel put strings for each hw behaviour. I am expecting
>DT
>to tell me if this SoC is a special case or not and kernel shall handle
>accordingly

I don't think this kind of behavior should be described in DT.

Rob, do you agree?

>
>> Plus, that would require changing the DT binding, which isn't
>> something we can do.
>
>Any reason why bindings can't change..? I though this was support for
>new
>SoC...

This is a behavior that exists on a SoC that is already
supported (A23/A33).

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

* Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-14  9:04         ` Maxime Ripard
@ 2017-06-15  3:54           ` Vinod Koul
  2017-06-15  3:53             ` Icenowy Zheng
  2017-06-20  8:45             ` Maxime Ripard
  0 siblings, 2 replies; 12+ messages in thread
From: Vinod Koul @ 2017-06-15  3:54 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Icenowy Zheng, linux-arm-kernel, Chen-Yu Tsai, linux-kernel,
	linux-sunxi, Icenowy Zheng, dmaengine

On Wed, Jun 14, 2017 at 11:04:39AM +0200, Maxime Ripard wrote:
> On Wed, Jun 14, 2017 at 02:15:29PM +0530, Vinod Koul wrote:
> > > SoC info is in compatible, so there's no reason to make it a property.
> > 
> > that's why it would need to be optional for the SoC's that needs these..
> 
> There's nothing optional about that behaviour, it's mandatory for the
> SoC that need it, and useless on the SoC that don't.

And why should kernel put strings for each hw behaviour. I am expecting DT
to tell me if this SoC is a special case or not and kernel shall handle
accordingly

> Plus, that would require changing the DT binding, which isn't
> something we can do.

Any reason why bindings can't change..? I though this was support for new
SoC...

-- 
~Vinod

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

* Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
  2017-06-15  3:54           ` Vinod Koul
  2017-06-15  3:53             ` Icenowy Zheng
@ 2017-06-20  8:45             ` Maxime Ripard
  1 sibling, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2017-06-20  8:45 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Icenowy Zheng, linux-arm-kernel, Chen-Yu Tsai, linux-kernel,
	linux-sunxi, Icenowy Zheng, dmaengine

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

On Thu, Jun 15, 2017 at 09:24:08AM +0530, Vinod Koul wrote:
> On Wed, Jun 14, 2017 at 11:04:39AM +0200, Maxime Ripard wrote:
> > On Wed, Jun 14, 2017 at 02:15:29PM +0530, Vinod Koul wrote:
> > > > SoC info is in compatible, so there's no reason to make it a property.
> > > 
> > > that's why it would need to be optional for the SoC's that needs these..
> > 
> > There's nothing optional about that behaviour, it's mandatory for the
> > SoC that need it, and useless on the SoC that don't.
> 
> And why should kernel put strings for each hw behaviour.

You will have strings in the kernel for each hw behaviour,
disregarding on whether you base the behaviour on the compatible or a
set of properties. In fact, you will have *much* more strings in the
kernel in the latter case.

> I am expecting DT to tell me if this SoC is a special case or not
> and kernel shall handle accordingly

How is this not the case here?

The DT tells you that this SoC is a special case through a compatible
already.

> > Plus, that would require changing the DT binding, which isn't
> > something we can do.
> 
> Any reason why bindings can't change..? I though this was support for new
> SoC...

No, this is a rework of an existing code to support a new SoC. The
code is already there, and the binding too. It has been for 3 years.

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] 12+ messages in thread

end of thread, other threads:[~2017-06-20  8:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-05 12:33 [PATCH 0/2] Allwinner V3s DMA support Icenowy Zheng
2017-06-05 12:33 ` [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng
2017-06-05 13:15   ` [linux-sunxi] " Chen-Yu Tsai
2017-06-14  8:32   ` Vinod Koul
2017-06-14  8:32     ` Icenowy Zheng
2017-06-14  8:45       ` Vinod Koul
2017-06-14  8:46         ` [linux-sunxi] " Icenowy Zheng
2017-06-14  9:04         ` Maxime Ripard
2017-06-15  3:54           ` Vinod Koul
2017-06-15  3:53             ` Icenowy Zheng
2017-06-20  8:45             ` Maxime Ripard
2017-06-05 12:33 ` [PATCH 2/2] dmaengine: sun6i: support V3s SoC variant Icenowy Zheng

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