Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1665575 > unrolled thread

Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng <icenowy@aosc.io> - 2017-06-14 10:40 +0200
    Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA  engines a common quirk Vinod Koul <vinod.koul@intel.com> - 2017-06-14 10:50 +0200
      Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng <icenowy@aosc.io> - 2017-06-14 11:00 +0200
      Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in  sun8i's DMA engines a common quirk Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-06-14 11:10 +0200
        Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk Icenowy Zheng <icenowy@aosc.io> - 2017-06-15 06:00 +0200
        Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in  sun8i's DMA engines a common quirk Vinod Koul <vinod.koul@intel.com> - 2017-06-15 06:00 +0200

#1665575 — Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

于 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
>> 

[toc] | [next] | [standalone]

#1665583 — Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

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

[toc] | [prev] | [next] | [standalone]

#1665592 — Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

于 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
>> >> 

[toc] | [prev] | [next] | [standalone]

#1665602 — Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1666437 — Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

于 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).

[toc] | [prev] | [next] | [standalone]

#1666438 — Re: [linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web