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

Groups > linux.kernel > #1240308 > unrolled thread

Re: [PATCH v5 06/10] dmaengine: add API for getting dma controller's 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 v5 06/10] dmaengine: add API for getting dma controller's  quirk Shawn Lin <shawn.lin@rock-chips.com> - 2015-10-06 11:30 +0200
    Re: [PATCH v5 06/10] dmaengine: add API for getting dma controller's  quirk Vinod Koul <vinod.koul@intel.com> - 2015-10-07 16:40 +0200
      Re: [PATCH v5 06/10] dmaengine: add API for getting dma controller's  quirk Shawn Lin <shawn.lin@rock-chips.com> - 2015-10-09 13:30 +0200
    Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting  dma controller's quirk Lars-Peter Clausen <lars@metafoo.de> - 2015-10-08 10:40 +0200
      Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting  dma controller's quirk Lars-Peter Clausen <lars@metafoo.de> - 2015-10-09 13:40 +0200
      Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma  controller's quirk Shawn Lin <shawn.lin@rock-chips.com> - 2015-10-09 13:40 +0200
      Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma  controller's quirk Vinod Koul <vinod.koul@intel.com> - 2015-10-14 13:00 +0200
        Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma  controller's quirk Shawn Lin <shawn.lin@rock-chips.com> - 2015-10-14 16:40 +0200

#1240308 — Re: [PATCH v5 06/10] dmaengine: add API for getting dma controller's quirk

Hi Vinod,

On 2015/10/5 23:37, Vinod Koul wrote:
> On Mon, Sep 14, 2015 at 07:48:59AM +0800, Shawn Lin wrote:
>> Add dmaengine_get_quirks API for peripheral devices to query
>> quirks if they need it to make special workaround due to broken
>> dma controller design.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>> Changes in v1: None
>>
>>   include/linux/dmaengine.h | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> index e2f5eb4..5174ca4 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -704,6 +704,7 @@ struct dma_device {
>>
>>   	int (*device_config)(struct dma_chan *chan,
>>   			     struct dma_slave_config *config);
>> +	int (*device_get_quirks)(struct dma_chan *chan);
>
> And why do we want to expose this to users? THis doesnt seem right!
>

Basically I agree not to expose dma's quirk to slave controllers...But, 
the fact I mentioned on cover letter explain the reasons why I have to 
let slave controllers know that they are working with a broken dma. It's 
a dilemma that if we don't want that to be exposed(let slave 
controllers' driver get the info via a API), we have to add broken quirk 
for all of them ,here and there, which seems to be a disaster:(

I would appreciate it if you could give me some suggestions at your 
earliest convenience. :)

> A quirk may exists but should be handled inside the controller driver and do
> appropriate action. You don't tell users or expect them to handle these
>


-- 
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1241567

On Tue, Oct 06, 2015 at 05:21:13PM +0800, Shawn Lin wrote:

> >>+	int (*device_get_quirks)(struct dma_chan *chan);
> >
> >And why do we want to expose this to users? THis doesnt seem right!
> >
> 
> Basically I agree not to expose dma's quirk to slave controllers...But, the
> fact I mentioned on cover letter explain the reasons why I have to let slave
> controllers know that they are working with a broken dma. It's a dilemma
> that if we don't want that to be exposed(let slave controllers' driver get
> the info via a API), we have to add broken quirk for all of them ,here and
> there, which seems to be a disaster:(
> 
> I would appreciate it if you could give me some suggestions at your earliest
> convenience. :)
> 
> >A quirk may exists but should be handled inside the controller driver and do
> >appropriate action. You don't tell users or expect them to handle these
> >
I laready gave one re-read the above lines.

Anyway I went ahead and read the usage. You are setting the slave parameters
for this. I can see two ways:
1. Have the quirk to driver and based on quirk reset the slave settings when
they are set by client.
2. Put this in DT and set the dma properties based on these quirks and let
driver and cleint be agnostic to it

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1243269

On 2015/10/7 22:32, Vinod Koul wrote:
> On Tue, Oct 06, 2015 at 05:21:13PM +0800, Shawn Lin wrote:
>
>>>> +	int (*device_get_quirks)(struct dma_chan *chan);
>>>
>>> And why do we want to expose this to users? THis doesnt seem right!
>>>
>>
>> Basically I agree not to expose dma's quirk to slave controllers...But, the
>> fact I mentioned on cover letter explain the reasons why I have to let slave
>> controllers know that they are working with a broken dma. It's a dilemma
>> that if we don't want that to be exposed(let slave controllers' driver get
>> the info via a API), we have to add broken quirk for all of them ,here and
>> there, which seems to be a disaster:(
>>
>> I would appreciate it if you could give me some suggestions at your earliest
>> convenience. :)
>>
>>> A quirk may exists but should be handled inside the controller driver and do
>>> appropriate action. You don't tell users or expect them to handle these
>>>
> I laready gave one re-read the above lines.
>
> Anyway I went ahead and read the usage. You are setting the slave parameters
> for this. I can see two ways:
> 1. Have the quirk to driver and based on quirk reset the slave settings when
> they are set by client.
> 2. Put this in DT and set the dma properties based on these quirks and let
> driver and cleint be agnostic to it

+Mark Brown

Thanks for these.

The first one is hard for dma to distinguish "broken 
slave"(I2S/SPI/UART..) from the unbroken ones(block devices like 
mmc/SFC...). If all clients are broken for that, it's easy to reset the 
slave parameters in dma driver. Unfortunately, not always, at least for 
rockchips' Socs.

And before V4, I did pass quirks to clents from DT, but Mark thought we 
should avoid to get it from dt. Anyway, now I think about it again,

(1) dma drivers doesn't know the users who should use limited burst and 
who won't. That make it's impossible for dma drivers to handle it alone 
or expose new API of burst capabilities to clients.
(2) It's inappropriate to expose quirks to clients.

so the only way I can see is to make some tricks from DT.
I would prefer to add "burst limit" property for the broken slaves 
instead of quirks for them which I did before V4.

Something like:

&spi{
max-burst-len=<1>;
}

How about?
:)

>


-- 
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1242054 — Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma controller's quirk

On 10/06/2015 11:21 AM, Shawn Lin wrote:
> Hi Vinod,
> 
> On 2015/10/5 23:37, Vinod Koul wrote:
>> On Mon, Sep 14, 2015 at 07:48:59AM +0800, Shawn Lin wrote:
>>> Add dmaengine_get_quirks API for peripheral devices to query
>>> quirks if they need it to make special workaround due to broken
>>> dma controller design.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>> ---
>>>
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>> Changes in v1: None
>>>
>>>   include/linux/dmaengine.h | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>>> index e2f5eb4..5174ca4 100644
>>> --- a/include/linux/dmaengine.h
>>> +++ b/include/linux/dmaengine.h
>>> @@ -704,6 +704,7 @@ struct dma_device {
>>>
>>>       int (*device_config)(struct dma_chan *chan,
>>>                    struct dma_slave_config *config);
>>> +    int (*device_get_quirks)(struct dma_chan *chan);
>>
>> And why do we want to expose this to users? THis doesnt seem right!
>>
> 
> Basically I agree not to expose dma's quirk to slave controllers...But, the
> fact I mentioned on cover letter explain the reasons why I have to let slave
> controllers know that they are working with a broken dma. It's a dilemma
> that if we don't want that to be exposed(let slave controllers' driver get
> the info via a API), we have t add broken quirk for all of them ,here and
> there, which seems to be a disaster:(

The problem with this API is that it transports values with device specific
meanings over a generic API. Which is generally speaking not a good idea
because the consumer witch is supposed to be generic suddenly needs to know
which provider it is talking to.

A better solution in this case typically is either introduce a generic API
with generic values or a custom API with custom values, but don't mix the two.

> 
> I would appreciate it if you could give me some suggestions at your earliest
> convenience. :)

In this case I think the best way to handle this is not quirks, but rather
expose the actual maximum burst size using the DMA capabilities API. Since
supporting only a certain burst depth is not really a quirk. All hardware
has a limit for this and for some it might be larger or smaller than for
others and it might be the same IP core but the maximum size depends on some
IP core parameters. So this should be discoverable.

- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1243274 — Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma controller's quirk

On 10/09/2015 01:31 PM, Shawn Lin wrote:
> 在 2015/10/8 16:31, Lars-Peter Clausen 写道:
>> On 10/06/2015 11:21 AM, Shawn Lin wrote:
>>> Hi Vinod,
>>>
>>> On 2015/10/5 23:37, Vinod Koul wrote:
>>>> On Mon, Sep 14, 2015 at 07:48:59AM +0800, Shawn Lin wrote:
>>>>> Add dmaengine_get_quirks API for peripheral devices to query
>>>>> quirks if they need it to make special workaround due to broken
>>>>> dma controller design.
>>>>>
>>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v5: None
>>>>> Changes in v4: None
>>>>> Changes in v3: None
>>>>> Changes in v2: None
>>>>> Changes in v1: None
>>>>>
>>>>>    include/linux/dmaengine.h | 9 +++++++++
>>>>>    1 file changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>>>>> index e2f5eb4..5174ca4 100644
>>>>> --- a/include/linux/dmaengine.h
>>>>> +++ b/include/linux/dmaengine.h
>>>>> @@ -704,6 +704,7 @@ struct dma_device {
>>>>>
>>>>>        int (*device_config)(struct dma_chan *chan,
>>>>>                     struct dma_slave_config *config);
>>>>> +    int (*device_get_quirks)(struct dma_chan *chan);
>>>>
>>>> And why do we want to expose this to users? THis doesnt seem right!
>>>>
>>>
>>> Basically I agree not to expose dma's quirk to slave controllers...But, the
>>> fact I mentioned on cover letter explain the reasons why I have to let slave
>>> controllers know that they are working with a broken dma. It's a dilemma
>>> that if we don't want that to be exposed(let slave controllers' driver get
>>> the info via a API), we have t add broken quirk for all of them ,here and
>>> there, which seems to be a disaster:(
>>
>> The problem with this API is that it transports values with device specific
>> meanings over a generic API. Which is generally speaking not a good idea
>> because the consumer witch is supposed to be generic suddenly needs to know
>> which provider it is talking to.
>>
>> A better solution in this case typically is either introduce a generic API
>> with generic values or a custom API with custom values, but don't mix the
>> two.
>>
>>>
>>> I would appreciate it if you could give me some suggestions at your earliest
>>> convenience. :)
>>
>> In this case I think the best way to handle this is not quirks, but rather
>> expose the actual maximum burst size using the DMA capabilities API. Since
>> supporting only a certain burst depth is not really a quirk. All hardware
>> has a limit for this and for some it might be larger or smaller than for
>> others and it might be the same IP core but the maximum size depends on some
>> IP core parameters. So this should be discoverable.
>>
> 
> Hi Lars,
> 
> Thanks for looking for that.
> 
> It's a good idea if all clients of the Soc are broken, but unfortunately
> some of them work. So... max burst shoule be different for individuals.

Well, the dma_get_slave_caps() API works on a DMA channel, so I don't think
this will be a problem.

- Lars


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1243283 — Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma controller's quirk

在 2015/10/8 16:31, Lars-Peter Clausen 写道:
> On 10/06/2015 11:21 AM, Shawn Lin wrote:
>> Hi Vinod,
>>
>> On 2015/10/5 23:37, Vinod Koul wrote:
>>> On Mon, Sep 14, 2015 at 07:48:59AM +0800, Shawn Lin wrote:
>>>> Add dmaengine_get_quirks API for peripheral devices to query
>>>> quirks if they need it to make special workaround due to broken
>>>> dma controller design.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>> ---
>>>>
>>>> Changes in v5: None
>>>> Changes in v4: None
>>>> Changes in v3: None
>>>> Changes in v2: None
>>>> Changes in v1: None
>>>>
>>>>    include/linux/dmaengine.h | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>>>> index e2f5eb4..5174ca4 100644
>>>> --- a/include/linux/dmaengine.h
>>>> +++ b/include/linux/dmaengine.h
>>>> @@ -704,6 +704,7 @@ struct dma_device {
>>>>
>>>>        int (*device_config)(struct dma_chan *chan,
>>>>                     struct dma_slave_config *config);
>>>> +    int (*device_get_quirks)(struct dma_chan *chan);
>>>
>>> And why do we want to expose this to users? THis doesnt seem right!
>>>
>>
>> Basically I agree not to expose dma's quirk to slave controllers...But, the
>> fact I mentioned on cover letter explain the reasons why I have to let slave
>> controllers know that they are working with a broken dma. It's a dilemma
>> that if we don't want that to be exposed(let slave controllers' driver get
>> the info via a API), we have t add broken quirk for all of them ,here and
>> there, which seems to be a disaster:(
>
> The problem with this API is that it transports values with device specific
> meanings over a generic API. Which is generally speaking not a good idea
> because the consumer witch is supposed to be generic suddenly needs to know
> which provider it is talking to.
>
> A better solution in this case typically is either introduce a generic API
> with generic values or a custom API with custom values, but don't mix the two.
>
>>
>> I would appreciate it if you could give me some suggestions at your earliest
>> convenience. :)
>
> In this case I think the best way to handle this is not quirks, but rather
> expose the actual maximum burst size using the DMA capabilities API. Since
> supporting only a certain burst depth is not really a quirk. All hardware
> has a limit for this and for some it might be larger or smaller than for
> others and it might be the same IP core but the maximum size depends on some
> IP core parameters. So this should be discoverable.
>

Hi Lars,

Thanks for looking for that.

It's a good idea if all clients of the Soc are broken, but unfortunately 
some of them work. So... max burst shoule be different for individuals.

> - Lars
>
>
>
>


-- 
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1246511 — Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma controller's quirk

On Thu, Oct 08, 2015 at 10:31:18AM +0200, Lars-Peter Clausen wrote:
> > Basically I agree not to expose dma's quirk to slave controllers...But, the
> > fact I mentioned on cover letter explain the reasons why I have to let slave
> > controllers know that they are working with a broken dma. It's a dilemma
> > that if we don't want that to be exposed(let slave controllers' driver get
> > the info via a API), we have t add broken quirk for all of them ,here and
> > there, which seems to be a disaster:(
> 
> The problem with this API is that it transports values with device specific
> meanings over a generic API. Which is generally speaking not a good idea
> because the consumer witch is supposed to be generic suddenly needs to know
> which provider it is talking to.
> 
> A better solution in this case typically is either introduce a generic API
> with generic values or a custom API with custom values, but don't mix the two.
> 
> > 
> > I would appreciate it if you could give me some suggestions at your earliest
> > convenience. :)
> 
> In this case I think the best way to handle this is not quirks, but rather
> expose the actual maximum burst size using the DMA capabilities API. Since
> supporting only a certain burst depth is not really a quirk. All hardware
> has a limit for this and for some it might be larger or smaller than for
> others and it might be the same IP core but the maximum size depends on some
> IP core parameters. So this should be discoverable.

yes that makes more sense than adding quirks, exposing the right values
which should be a readable property for driver will ensure it works on
system with/without quirks

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1246832 — Re: [alsa-devel] [PATCH v5 06/10] dmaengine: add API for getting dma controller's quirk

On 2015/10/14 18:53, Vinod Koul wrote:
> On Thu, Oct 08, 2015 at 10:31:18AM +0200, Lars-Peter Clausen wrote:
>>> Basically I agree not to expose dma's quirk to slave controllers...But, the
>>> fact I mentioned on cover letter explain the reasons why I have to let slave
>>> controllers know that they are working with a broken dma. It's a dilemma
>>> that if we don't want that to be exposed(let slave controllers' driver get
>>> the info via a API), we have t add broken quirk for all of them ,here and
>>> there, which seems to be a disaster:(
>>
>> The problem with this API is that it transports values with device specific
>> meanings over a generic API. Which is generally speaking not a good idea
>> because the consumer witch is supposed to be generic suddenly needs to know
>> which provider it is talking to.
>>
>> A better solution in this case typically is either introduce a generic API
>> with generic values or a custom API with custom values, but don't mix the two.
>>
>>>
>>> I would appreciate it if you could give me some suggestions at your earliest
>>> convenience. :)
>>
>> In this case I think the best way to handle this is not quirks, but rather
>> expose the actual maximum burst size using the DMA capabilities API. Since
>> supporting only a certain burst depth is not really a quirk. All hardware
>> has a limit for this and for some it might be larger or smaller than for
>> others and it might be the same IP core but the maximum size depends on some
>> IP core parameters. So this should be discoverable.
>
> yes that makes more sense than adding quirks, exposing the right values
> which should be a readable property for driver will ensure it works on
> system with/without quirks

Sorry for late response in this thread.

Right, we can expose max-burst to clients by dma_slave_caps instead of 
quirks. I will try it and send v6 ASAP.

Thanks Lars and Vinod.

>


-- 
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web