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

Groups > linux.kernel > #1272214 > unrolled thread

Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

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 02/13] dmaengine: Introduce  dma_request_slave_channel_compat_reason() Peter Ujfalusi <peter.ujfalusi@ti.com> - 2015-11-18 15:30 +0100
    Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason() Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-18 16:50 +0100
      Re: [PATCH 02/13] dmaengine: Introduce  dma_request_slave_channel_compat_reason() Peter Ujfalusi <peter.ujfalusi@ti.com> - 2015-11-19 11:40 +0100

#1272214 — Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

Hi Vinod,

bringing this old thread back to life as I just started to work on this.

On 06/24/2015 07:24 PM, Vinod Koul wrote:

>> We would end up with the following APIs, all returning with error code on failure:
>> dma_request_slave_channel(dev, name);
>> dma_request_channel_legacy(mask, fn, fn_param);
>> dma_request_slave_channel_compat(mask, fn, fn_param, dev, name);
>> dma_request_any_channel(mask);
> This is good idea but still we end up with 4 APIs. Why not just converge to
> two API, one legacy + memcpy + filer fn and one untimate API for slave?

Looked at the current API and it's use and, well, it is a bit confusing.

What I hoped that we can separate users to two category:
1. Slave channel requests, where we request a specific channel to handle HW
requests/triggers.
For this we could have:
dma_request_slave_channel(dev, name, fn, fn_param);

In DT/ACPI only drivers we can NULL out the fn and fn_param, in pure legacy
mode we null out the name, I would keep the dev so we could print dev specific
error in dmaengine core, but it could be optional, IN case of drivers used
both DT/ACPI and legacy mode all parameter can be filled and the core will
decide what to do.
For the legacy needs the dmaengine code would provide the mask dows with
DMA_SLAVE flag set.

2. non slave channel requests, where only the functionality matters, like
memcpy, interleaved, memset, etc.
We could have a simple:
dma_request_channel(mask);

But looking at the drivers using dmaengine legacy dma_request_channel() API:
Some sets DMA_INTERRUPT or DMA_PRIVATE or DMA_SG along with DMA_SLAVE:
drivers/misc/carma/carma-fpga.c			DMA_INTERRUPT|DMA_SLAVE|DMA_SG
drivers/misc/carma/carma-fpga-program.c		DMA_MEMCPY|DMA_SLAVE|DMA_SG
drivers/media/platform/soc_camera/mx3_camera.c	DMA_SLAVE|DMA_PRIVATE
sound/soc/intel/common/sst-firmware.c		DMA_SLAVE|DMA_MEMCPY

as examples.
Not sure how valid are these...

Some drivers do pass fn and fn_param when requesting channel for DMA_MEMCPY
drivers/misc/mic/host/mic_device.h
drivers/mtd/nand/fsmc_nand.c
sound/soc/intel/common/sst-firmware.c (well, this request DMA_SLAVE capability
at the same time).

Some driver sets the fn_param w/o fn, which means fn_param is ignored.

So the
dma_request_slave_channel(dev, name, fn, fn_param);
dma_request_channel(mask);

almost covers the current users and would be pretty clean ;)

If we add the mask to the slave channel API - which will become universal,
drop in replacement for dma_request_channel, and we might have only one API:

dma_request_channel(dev, name, mask, fn, fn_param);

> 
> Internally we may have 4 APIs for cleaner handling...
> 
> Thoughts... ??

Yes, as we need to arrange the code internally to keep things neat.

-- 
Péter
--
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]

#1272309 — Re: [PATCH 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

On Wed, Nov 18, 2015 at 4:21 PM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> Hi Vinod,
>
> bringing this old thread back to life as I just started to work on this.

What I remember we need to convert drivers to use new API meanwhile it
is good to keep old one to avoid patch storm which does nothing useful
(IIRC Russel's opinion).

On the other hand there are a lot of drivers that are used on the set
of platforms starting from legacy and abandoned ones (like AVR32) to
relatively new and newest.

And I'm not a fan of those thousands of API calls either.

-- 
With Best Regards,
Andy Shevchenko
--
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]

#1273017

On 11/18/2015 05:46 PM, Andy Shevchenko wrote:
> On Wed, Nov 18, 2015 at 4:21 PM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
>> Hi Vinod,
>>
>> bringing this old thread back to life as I just started to work on this.
> 
> What I remember we need to convert drivers to use new API meanwhile it
> is good to keep old one to avoid patch storm which does nothing useful
> (IIRC Russel's opinion).

I tend to agree. But we need to start converting the users at some point
either way.
Another issue is the fact that the current dmaengine API is using all the good
names I can think of ;)

> On the other hand there are a lot of drivers that are used on the set
> of platforms starting from legacy and abandoned ones (like AVR32) to
> relatively new and newest.
> 
> And I'm not a fan of those thousands of API calls either.
> 

-- 
Péter
--
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