diff mbox

[RFC,v02,03/15] dmaengine: core: Introduce new, universal API to request a channel

Message ID 1448891145-10766-4-git-send-email-peter.ujfalusi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Ujfalusi Nov. 30, 2015, 1:45 p.m. UTC
The two API function can cover most, if not all current APIs used to
request a channel. With minimal effort dmaengine drivers, platforms and
dmaengine user drivers can be converted to use the two function.

struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);

To request any channel matching with the requested capabilities, can be
used to request channel for memcpy, memset, xor, etc where no hardware
synchronization is needed.

struct dma_chan *dma_request_chan(struct device *dev, const char *name);
To request a slave channel. The dma_request_chan() will try to find the
channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
it will use a filter lookup table and retrieves the needed information from
the dma_filter_map provided by the DMA drivers.
This legacy mode needs changes in platform code, in dmaengine drivers and
finally the dmaengine user drivers can be converted:

For each dmaengine driver an array of DMA device, slave and the parameter
for the filter function needs to be added:

static struct dma_filter_map da830_edma_map[] = {
	DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
	DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
	DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
	DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
	DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
	DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
	DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
	DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
	DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
	DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
	DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
	DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
};

This information is going to be needed by the dmaengine driver, so
modification to the platform_data is needed, and the driver map should be
added to the pdata of the DMA driver:

da8xx_edma0_pdata.filter_map = da830_edma_map;
da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);

The DMA driver then needs to convigure the needed device -> filter_fn
mapping before it registers with dma_async_device_register() :

if (info->filter_map) {
	ecc->dma_slave.filter_map.map = info->filter_map;
	ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
	ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
}

When neither DT or ACPI lookup is available the dma_request_chan() will
try to match the requester's device name with the filter_map's list of
device names, when a match found it will use the information from the
dma_filter_map to get the channel with the dma_get_channel() internal
function.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/dmaengine.c   | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/dmaengine.h | 31 +++++++++++++++++++
 2 files changed, 107 insertions(+)

Comments

Arnd Bergmann Nov. 30, 2015, 2:09 p.m. UTC | #1
On Monday 30 November 2015 15:45:33 Peter Ujfalusi wrote:
>                                                   const char *name);
>  struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
> +
> +struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
> +
>  void dma_release_channel(struct dma_chan *chan);
>  int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
>  #else
> @@ -1268,6 +1291,14 @@ static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
>  {
>         return NULL;
>  }
> +static inline struct dma_chan *dma_request_chan(struct device *dev)
> +{
> +       return ERR_PTR(-ENODEV);
> +}
> 

The prototypes for dma_request_chan() don't match, otherwise looks good.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko Nov. 30, 2015, 2:51 p.m. UTC | #2
On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> The two API function can cover most, if not all current APIs used to
> request a channel. With minimal effort dmaengine drivers, platforms and
> dmaengine user drivers can be converted to use the two function.
>
> struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
>
> To request any channel matching with the requested capabilities, can be
> used to request channel for memcpy, memset, xor, etc where no hardware
> synchronization is needed.
>
> struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> To request a slave channel. The dma_request_chan() will try to find the
> channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
> it will use a filter lookup table and retrieves the needed information from
> the dma_filter_map provided by the DMA drivers.
> This legacy mode needs changes in platform code, in dmaengine drivers and
> finally the dmaengine user drivers can be converted:
>
> For each dmaengine driver an array of DMA device, slave and the parameter
> for the filter function needs to be added:
>
> static struct dma_filter_map da830_edma_map[] = {
>         DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
>         DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
>         DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
>         DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
>         DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
>         DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
>         DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
>         DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
>         DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
>         DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
>         DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
>         DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> };
>
> This information is going to be needed by the dmaengine driver, so
> modification to the platform_data is needed, and the driver map should be
> added to the pdata of the DMA driver:
>
> da8xx_edma0_pdata.filter_map = da830_edma_map;
> da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);
>
> The DMA driver then needs to convigure the needed device -> filter_fn
> mapping before it registers with dma_async_device_register() :
>
> if (info->filter_map) {
>         ecc->dma_slave.filter_map.map = info->filter_map;
>         ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
>         ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
> }
>
> When neither DT or ACPI lookup is available the dma_request_chan() will
> try to match the requester's device name with the filter_map's list of
> device names, when a match found it will use the information from the
> dma_filter_map to get the channel with the dma_get_channel() internal
> function.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/dma/dmaengine.c   | 76 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/dmaengine.h | 31 +++++++++++++++++++
>  2 files changed, 107 insertions(+)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 1249165fb4b2..bd774adff697 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -43,6 +43,7 @@
>
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/platform_device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
> @@ -715,6 +716,81 @@ struct dma_chan *dma_request_slave_channel(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(dma_request_slave_channel);
>
> +static struct dma_filter_map *dma_filter_match(struct dma_device *device,
> +                                              const char *name,
> +                                              struct device *dev)
> +{
> +       struct dma_filter_map *map_found = NULL;
> +       int i;
> +
> +       if (!device->filter_map.mapcnt)
> +               return NULL;
> +
> +       for (i = 0; i < device->filter_map.mapcnt; i++) {
> +               struct dma_filter_map *map = &device->filter_map.map[i];
> +
> +               if (!strcmp(map->devname, dev_name(dev)) &&
> +                   !strcmp(map->slave, name)) {
> +                       map_found = map;
> +                       break;
> +               }
> +       }
> +
> +       return map_found;
> +}
> +
> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
> +{
> +       struct dma_device *device, *_d;
> +       struct dma_chan *chan = NULL;
> +
> +       /* If device-tree is present get slave info from here */
> +       if (dev->of_node)
> +               chan = of_dma_request_slave_channel(dev->of_node, name);
> +
> +       /* If device was enumerated by ACPI get slave info from here */
> +       if (ACPI_HANDLE(dev) && !chan)

The preferable way is to use
has_acpi_companion() instead of ACPI_HANDLE().

> +               chan = acpi_dma_request_slave_chan_by_name(dev, name);
> +
> +       if (chan)
> +               return chan;
> +
> +       /* Try to find the channel via the DMA filter map(s) */
> +       mutex_lock(&dma_list_mutex);
> +       list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
> +               struct dma_filter_map *map = dma_filter_match(device, name,
> +                                                             dev);
> +
> +               if (!map)
> +                       continue;
> +
> +               chan = dma_get_channel(device, NULL,
> +                                      device->filter_map.filter_fn,
> +                                      map->param);
> +               if (!IS_ERR(chan))
> +                       break;
> +       }
> +       mutex_unlock(&dma_list_mutex);
> +
> +       return chan ? chan : ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL_GPL(dma_request_chan);
> +
> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
> +{
> +       struct dma_chan *chan;
> +
> +       if (!mask)
> +               return ERR_PTR(-ENODEV);
> +
> +       chan = __dma_request_channel(mask, NULL, NULL);
> +       if (!chan)
> +               chan = ERR_PTR(-ENODEV);
> +
> +       return chan;
> +}
> +EXPORT_SYMBOL_GPL(dma_request_chan_by_mask);
> +
>  void dma_release_channel(struct dma_chan *chan)
>  {
>         mutex_lock(&dma_list_mutex);
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index a2b7c2071cf4..f7b06ff982c8 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -606,6 +606,24 @@ enum dmaengine_alignment {
>         DMAENGINE_ALIGN_64_BYTES = 6,
>  };
>
> +struct dma_filter_map {
> +        char *devname;
> +        char *slave;
> +        void *param;
> +};
> +#define DMA_FILTER_ENTRY(_devname, _slave, _param)     \
> +       {                                               \
> +               .devname = _devname,                    \
> +               .slave = _slave,                        \
> +               .param = (void*)_param,                 \
> +       }
> +
> +struct dma_filter {
> +        dma_filter_fn filter_fn;
> +        int mapcnt;
> +        struct dma_filter_map *map;
> +};
> +
>  /**
>   * struct dma_device - info on the entity supplying DMA services
>   * @chancnt: how many DMA channels are supported
> @@ -669,6 +687,7 @@ struct dma_device {
>         unsigned int privatecnt;
>         struct list_head channels;
>         struct list_head global_node;
> +       struct dma_filter filter_map;
>         dma_cap_mask_t  cap_mask;
>         unsigned short max_xor;
>         unsigned short max_pq;
> @@ -1235,6 +1254,10 @@ struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
>  struct dma_chan *dma_request_slave_channel_reason(struct device *dev,
>                                                   const char *name);
>  struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
> +
> +struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
> +
>  void dma_release_channel(struct dma_chan *chan);
>  int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
>  #else
> @@ -1268,6 +1291,14 @@ static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
>  {
>         return NULL;
>  }
> +static inline struct dma_chan *dma_request_chan(struct device *dev)
> +{
> +       return ERR_PTR(-ENODEV);
> +}
> +static inline struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
> +{
> +       return ERR_PTR(-ENODEV);
> +}
>  static inline void dma_release_channel(struct dma_chan *chan)
>  {
>  }
> --
> 2.6.3
>
Tony Lindgren Nov. 30, 2015, 3:51 p.m. UTC | #3
Hi,

* Peter Ujfalusi <peter.ujfalusi@ti.com> [151130 05:49]:
> 
> For each dmaengine driver an array of DMA device, slave and the parameter
> for the filter function needs to be added:
> 
> static struct dma_filter_map da830_edma_map[] = {
> 	DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> 	DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> 	DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> 	DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> 	DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> 	DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> 	DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> 	DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> 	DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> 	DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> 	DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> 	DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> };

FYI, if the EDMA_CTRL_CHAN above is just the evtmux registers, those
can be handled with the pinctrl framework. It seems that would allow
leaving out some of the built-in look up data, and have the mux parts
handled by a proper device driver. Below is a sample from the dm81xx
platform for reference.

SoC dtsi file:

evtmux: pinmux@f90 {
	compatible = "pinctrl-single";
	reg = <0xf90 0x40>;
	#address-cells = <1>;
	#size-cells = <0>;
	pinctrl-single,register-width = <8>;
	pinctrl-single,function-mask = <0x1f>;
};

Board specific dts file:

&evtmux {
	sd2_edma_pins: pinmux_sd2_edma_pins {
	pinctrl-single,pins = <
		8 1     /* use SDTXEVT1 for EDMA instead of MCASP0TX */
		9 2     /* use SDRXEVT1 for EDMA instead of MCASP0RX */
		>;
	};
};

Dynamic muxing of these channels can be done too using the pinctrl
framework named modes, but probably is not a good idea in the case of
SD card and MaASP in case something goes wrong :)

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Ujfalusi Dec. 1, 2015, 8:13 a.m. UTC | #4
On 11/30/2015 05:51 PM, Tony Lindgren wrote:
> Hi,
> 
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [151130 05:49]:
>>
>> For each dmaengine driver an array of DMA device, slave and the parameter
>> for the filter function needs to be added:
>>
>> static struct dma_filter_map da830_edma_map[] = {
>> 	DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
>> 	DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
>> 	DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
>> 	DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
>> 	DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
>> 	DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
>> 	DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
>> 	DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
>> 	DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
>> 	DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
>> 	DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
>> 	DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
>> };
> 
> FYI, if the EDMA_CTRL_CHAN above is just the evtmux registers

No, they are not. They are the eDMA event numbers. I used the EDMA_CTRL_CHAN()
macro for all board files to have uniform look for the data. The first
parameter means the eDMA instance number while the second is the event number
on that eDMA. Since most devices have only one eDMA, we have 0 as eDMA id in
most cases.
The eventmux, or crossbar is different thing and we have several versions of
the event crossbar or mux used.

> those
> can be handled with the pinctrl framework. It seems that would allow
> leaving out some of the built-in look up data, and have the mux parts
> handled by a proper device driver. Below is a sample from the dm81xx
> platform for reference.
> 
> SoC dtsi file:
> 
> evtmux: pinmux@f90 {
> 	compatible = "pinctrl-single";
> 	reg = <0xf90 0x40>;
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	pinctrl-single,register-width = <8>;
> 	pinctrl-single,function-mask = <0x1f>;
> };
> 
> Board specific dts file:
> 
> &evtmux {
> 	sd2_edma_pins: pinmux_sd2_edma_pins {
> 	pinctrl-single,pins = <
> 		8 1     /* use SDTXEVT1 for EDMA instead of MCASP0TX */
> 		9 2     /* use SDRXEVT1 for EDMA instead of MCASP0RX */
> 		>;
> 	};
> };

I see. The dm81xx basically am33xx/am43xx?

Actually I would prefer to use the dmaengine's event router framework and we
do have support for the am33xx/am43xx type of crossbar already implemented.
I'm going to resend the DTS series for am33xx/am43xx to convert them to use
the new DT bindings along with the dma event router support:
https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html

> Dynamic muxing of these channels can be done too using the pinctrl
> framework named modes, but probably is not a good idea in the case of
> SD card and MaASP in case something goes wrong :)

In theory it can be done, but in practice it is not possible. It is up to the
board design decision to select which DMA event is not needed to be used in
default mode and that one can be used to route the crossbar hidden request to it.
Just imaging: playing audio from MMC (in the example you have), audio needs
constant DMA, so the MMC would never get DMA request, also the drivers tend to
request the DMA channel in their probe/init and hold to it as long as they are
loaded...
Peter Ujfalusi Dec. 1, 2015, 9:48 a.m. UTC | #5
On 11/30/2015 04:09 PM, Arnd Bergmann wrote:
> On Monday 30 November 2015 15:45:33 Peter Ujfalusi wrote:
>>                                                   const char *name);
>>  struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
>> +
>> +struct dma_chan *dma_request_chan(struct device *dev, const char *name);
>> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
>> +
>>  void dma_release_channel(struct dma_chan *chan);
>>  int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
>>  #else
>> @@ -1268,6 +1291,14 @@ static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
>>  {
>>         return NULL;
>>  }
>> +static inline struct dma_chan *dma_request_chan(struct device *dev)
>> +{
>> +       return ERR_PTR(-ENODEV);
>> +}
>>
> 
> The prototypes for dma_request_chan() don't match, otherwise looks good.

Aargh, the !CONFIG_DMA_ENGINE path...
Fixed for the next RFC

Thanks,
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Ujfalusi Dec. 1, 2015, 9:56 a.m. UTC | #6
On 11/30/2015 04:51 PM, Andy Shevchenko wrote:
>> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
>> +{
>> +       struct dma_device *device, *_d;
>> +       struct dma_chan *chan = NULL;
>> +
>> +       /* If device-tree is present get slave info from here */
>> +       if (dev->of_node)
>> +               chan = of_dma_request_slave_channel(dev->of_node, name);
>> +
>> +       /* If device was enumerated by ACPI get slave info from here */
>> +       if (ACPI_HANDLE(dev) && !chan)
> 
> The preferable way is to use
> has_acpi_companion() instead of ACPI_HANDLE().

I have done this part based on the dma_request_slave_channel_reason().
Will switch to use the has_acpi_companion() for the next RFC.

>> +               chan = acpi_dma_request_slave_chan_by_name(dev, name);
>> +
>> +       if (chan)
>> +               return chan;
>> +
Andy Shevchenko Dec. 1, 2015, 10:07 a.m. UTC | #7
On Tue, Dec 1, 2015 at 11:56 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 11/30/2015 04:51 PM, Andy Shevchenko wrote:
>>> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
>>> +{
>>> +       struct dma_device *device, *_d;
>>> +       struct dma_chan *chan = NULL;
>>> +
>>> +       /* If device-tree is present get slave info from here */
>>> +       if (dev->of_node)
>>> +               chan = of_dma_request_slave_channel(dev->of_node, name);
>>> +
>>> +       /* If device was enumerated by ACPI get slave info from here */
>>> +       if (ACPI_HANDLE(dev) && !chan)
>>
>> The preferable way is to use
>> has_acpi_companion() instead of ACPI_HANDLE().
>
> I have done this part based on the dma_request_slave_channel_reason().

Understood, though that function was implemented before
has_acpi_companion() has been introduced.

> Will switch to use the has_acpi_companion() for the next RFC.

Good.
Tony Lindgren Dec. 1, 2015, 5 p.m. UTC | #8
* Peter Ujfalusi <peter.ujfalusi@ti.com> [151201 00:14]:
> On 11/30/2015 05:51 PM, Tony Lindgren wrote:
> > Hi,
> > 
> > * Peter Ujfalusi <peter.ujfalusi@ti.com> [151130 05:49]:
> >>
> >> For each dmaengine driver an array of DMA device, slave and the parameter
> >> for the filter function needs to be added:
> >>
> >> static struct dma_filter_map da830_edma_map[] = {
> >> 	DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> >> 	DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> >> 	DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> >> 	DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> >> 	DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> >> 	DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> >> 	DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> >> 	DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> >> 	DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> >> 	DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> >> 	DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> >> 	DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> >> };
> > 
> > FYI, if the EDMA_CTRL_CHAN above is just the evtmux registers
> 
> No, they are not. They are the eDMA event numbers. I used the EDMA_CTRL_CHAN()
> macro for all board files to have uniform look for the data. The first
> parameter means the eDMA instance number while the second is the event number
> on that eDMA. Since most devices have only one eDMA, we have 0 as eDMA id in
> most cases.
> The eventmux, or crossbar is different thing and we have several versions of
> the event crossbar or mux used.

OK

> > those
> > can be handled with the pinctrl framework. It seems that would allow
> > leaving out some of the built-in look up data, and have the mux parts
> > handled by a proper device driver. Below is a sample from the dm81xx
> > platform for reference.
> > 
> > SoC dtsi file:
> > 
> > evtmux: pinmux@f90 {
> > 	compatible = "pinctrl-single";
> > 	reg = <0xf90 0x40>;
> > 	#address-cells = <1>;
> > 	#size-cells = <0>;
> > 	pinctrl-single,register-width = <8>;
> > 	pinctrl-single,function-mask = <0x1f>;
> > };
> > 
> > Board specific dts file:
> > 
> > &evtmux {
> > 	sd2_edma_pins: pinmux_sd2_edma_pins {
> > 	pinctrl-single,pins = <
> > 		8 1     /* use SDTXEVT1 for EDMA instead of MCASP0TX */
> > 		9 2     /* use SDRXEVT1 for EDMA instead of MCASP0RX */
> > 		>;
> > 	};
> > };
> 
> I see. The dm81xx basically am33xx/am43xx?

Yeah similar to am33xx with different clocks and with a bunch of accelerators.

> Actually I would prefer to use the dmaengine's event router framework and we
> do have support for the am33xx/am43xx type of crossbar already implemented.
> I'm going to resend the DTS series for am33xx/am43xx to convert them to use
> the new DT bindings along with the dma event router support:
> https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html

OK yes a dmaengine event router works too when available. Good to see
them as separate driver instances now :) Are only the dts changes missing
now?

FYI, when we have separate interconnect driver instances, we don't want to
and cannot tweak registers outside the interconnect instance because of them
being in separate clock and/or power domains :p

In any case, it seems there's no harm using pinctrl for evtmux on dm81xx
until the event router is available. It's currently only needed on the
t410 emmc that I'm aware of :)

> > Dynamic muxing of these channels can be done too using the pinctrl
> > framework named modes, but probably is not a good idea in the case of
> > SD card and MaASP in case something goes wrong :)
> 
> In theory it can be done, but in practice it is not possible. It is up to the
> board design decision to select which DMA event is not needed to be used in
> default mode and that one can be used to route the crossbar hidden request to it.
> Just imaging: playing audio from MMC (in the example you have), audio needs
> constant DMA, so the MMC would never get DMA request, also the drivers tend to
> request the DMA channel in their probe/init and hold to it as long as they are
> loaded...

Yes dynamic muxing of the dma channels sounds like file corruption waiting
to happen :) Let's stay away from that.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Ujfalusi Dec. 2, 2015, 10 a.m. UTC | #9
On 12/01/2015 07:00 PM, Tony Lindgren wrote:
>> I see. The dm81xx basically am33xx/am43xx?
> 
> Yeah similar to am33xx with different clocks and with a bunch of accelerators.
> 
>> Actually I would prefer to use the dmaengine's event router framework and we
>> do have support for the am33xx/am43xx type of crossbar already implemented.
>> I'm going to resend the DTS series for am33xx/am43xx to convert them to use
>> the new DT bindings along with the dma event router support:
>> https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html
> 
> OK yes a dmaengine event router works too when available. Good to see
> them as separate driver instances now :) Are only the dts changes missing
> now?
> 
> FYI, when we have separate interconnect driver instances, we don't want to
> and cannot tweak registers outside the interconnect instance because of them
> being in separate clock and/or power domains :p

What does this mean in practice? We can not touch these registers? The DMA
crossbar is a separate driver from the eDMA driver.

> In any case, it seems there's no harm using pinctrl for evtmux on dm81xx
> until the event router is available. It's currently only needed on the
> t410 emmc that I'm aware of :)

The AM33xx/AM43xx DMA crossbar support is in 4.4 already:
42dbdcc6bf96 dmaengine: ti-dma-crossbar: Add support for crossbar on AM33xx/AM43xx
Tony Lindgren Dec. 2, 2015, 3 p.m. UTC | #10
* Peter Ujfalusi <peter.ujfalusi@ti.com> [151202 02:01]:
> On 12/01/2015 07:00 PM, Tony Lindgren wrote:
> >> I see. The dm81xx basically am33xx/am43xx?
> > 
> > Yeah similar to am33xx with different clocks and with a bunch of accelerators.
> > 
> >> Actually I would prefer to use the dmaengine's event router framework and we
> >> do have support for the am33xx/am43xx type of crossbar already implemented.
> >> I'm going to resend the DTS series for am33xx/am43xx to convert them to use
> >> the new DT bindings along with the dma event router support:
> >> https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html
> > 
> > OK yes a dmaengine event router works too when available. Good to see
> > them as separate driver instances now :) Are only the dts changes missing
> > now?
> > 
> > FYI, when we have separate interconnect driver instances, we don't want to
> > and cannot tweak registers outside the interconnect instance because of them
> > being in separate clock and/or power domains :p
> 
> What does this mean in practice? We can not touch these registers? The DMA
> crossbar is a separate driver from the eDMA driver.

Seem like it should not affect DMA crossbar as it's a separate driver.

What I meant is we need a separate driver for clocks, pinctrl, regulators,
PHY, crossbar or syscon to use the SCM registers from drivers sitting on a
L4 interonnect.

> > In any case, it seems there's no harm using pinctrl for evtmux on dm81xx
> > until the event router is available. It's currently only needed on the
> > t410 emmc that I'm aware of :)
> 
> The AM33xx/AM43xx DMA crossbar support is in 4.4 already:
> 42dbdcc6bf96 dmaengine: ti-dma-crossbar: Add support for crossbar on AM33xx/AM43xx

OK great.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 1249165fb4b2..bd774adff697 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -43,6 +43,7 @@ 
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -715,6 +716,81 @@  struct dma_chan *dma_request_slave_channel(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(dma_request_slave_channel);
 
+static struct dma_filter_map *dma_filter_match(struct dma_device *device,
+					       const char *name,
+					       struct device *dev)
+{
+	struct dma_filter_map *map_found = NULL;
+	int i;
+
+	if (!device->filter_map.mapcnt)
+		return NULL;
+
+	for (i = 0; i < device->filter_map.mapcnt; i++) {
+		struct dma_filter_map *map = &device->filter_map.map[i];
+
+		if (!strcmp(map->devname, dev_name(dev)) &&
+		    !strcmp(map->slave, name)) {
+			map_found = map;
+			break;
+		}
+	}
+
+	return map_found;
+}
+
+struct dma_chan *dma_request_chan(struct device *dev, const char *name)
+{
+	struct dma_device *device, *_d;
+	struct dma_chan *chan = NULL;
+
+	/* If device-tree is present get slave info from here */
+	if (dev->of_node)
+		chan = of_dma_request_slave_channel(dev->of_node, name);
+
+	/* If device was enumerated by ACPI get slave info from here */
+	if (ACPI_HANDLE(dev) && !chan)
+		chan = acpi_dma_request_slave_chan_by_name(dev, name);
+
+	if (chan)
+		return chan;
+
+	/* Try to find the channel via the DMA filter map(s) */
+	mutex_lock(&dma_list_mutex);
+	list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
+		struct dma_filter_map *map = dma_filter_match(device, name,
+							      dev);
+
+		if (!map)
+			continue;
+
+		chan = dma_get_channel(device, NULL,
+				       device->filter_map.filter_fn,
+				       map->param);
+		if (!IS_ERR(chan))
+			break;
+	}
+	mutex_unlock(&dma_list_mutex);
+
+	return chan ? chan : ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL_GPL(dma_request_chan);
+
+struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
+{
+	struct dma_chan *chan;
+
+	if (!mask)
+		return ERR_PTR(-ENODEV);
+
+	chan = __dma_request_channel(mask, NULL, NULL);
+	if (!chan)
+		chan = ERR_PTR(-ENODEV);
+
+	return chan;
+}
+EXPORT_SYMBOL_GPL(dma_request_chan_by_mask);
+
 void dma_release_channel(struct dma_chan *chan)
 {
 	mutex_lock(&dma_list_mutex);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index a2b7c2071cf4..f7b06ff982c8 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -606,6 +606,24 @@  enum dmaengine_alignment {
 	DMAENGINE_ALIGN_64_BYTES = 6,
 };
 
+struct dma_filter_map {
+        char *devname;
+        char *slave;
+        void *param;
+};
+#define DMA_FILTER_ENTRY(_devname, _slave, _param)	\
+	{						\
+		.devname = _devname,			\
+		.slave = _slave,			\
+		.param = (void*)_param,			\
+	}
+
+struct dma_filter {
+        dma_filter_fn filter_fn;
+        int mapcnt;
+        struct dma_filter_map *map;
+};
+
 /**
  * struct dma_device - info on the entity supplying DMA services
  * @chancnt: how many DMA channels are supported
@@ -669,6 +687,7 @@  struct dma_device {
 	unsigned int privatecnt;
 	struct list_head channels;
 	struct list_head global_node;
+	struct dma_filter filter_map;
 	dma_cap_mask_t  cap_mask;
 	unsigned short max_xor;
 	unsigned short max_pq;
@@ -1235,6 +1254,10 @@  struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
 struct dma_chan *dma_request_slave_channel_reason(struct device *dev,
 						  const char *name);
 struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
+
+struct dma_chan *dma_request_chan(struct device *dev, const char *name);
+struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
+
 void dma_release_channel(struct dma_chan *chan);
 int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
 #else
@@ -1268,6 +1291,14 @@  static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
 {
 	return NULL;
 }
+static inline struct dma_chan *dma_request_chan(struct device *dev)
+{
+	return ERR_PTR(-ENODEV);
+}
+static inline struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
+{
+	return ERR_PTR(-ENODEV);
+}
 static inline void dma_release_channel(struct dma_chan *chan)
 {
 }