[PATCH 0/6] dmaengine: add ACPI DMA helpers and use them in dw_dmac

[PATCH 0/6] dmaengine: add ACPI DMA helpers and use them in dw_dmac

[Andy Shevchenko]

There is a patch series which introduces ACPI DMA helpers in similar way like
we have for DeviceTree.

In addition it applies this to the first user, namely dw_dmac driver.

Andy Shevchenko (6):
  dma: acpi-dma: introduce ACPI DMA helpers
  dmaengine: call acpi_dma_request_slave_channel as well
  dma: acpi-dma: parse CSRT to extract additional resources
  dw_dmac: add ACPI support
  ACPI / LPSS: return no error from register_device_clock in special
    cases
  ACPI / LPSS: add Lynxpoint DMA controller to the list

 Documentation/acpi/enumeration.txt |  77 +++++++
 drivers/acpi/Makefile              |   1 -
 drivers/acpi/acpi_lpss.c           |  11 +
 drivers/acpi/csrt.c                | 159 -------------
 drivers/acpi/internal.h            |   1 -
 drivers/acpi/scan.c                |   1 -
 drivers/clk/x86/clk-lpt.c          |   2 +-
 drivers/dma/Kconfig                |   4 +
 drivers/dma/Makefile               |   1 +
 drivers/dma/acpi-dma.c             | 447 +++++++++++++++++++++++++++++++++++++
 drivers/dma/dmaengine.c            |   6 +
 drivers/dma/dw_dmac.c              |  68 ++++--
 drivers/dma/dw_dmac_regs.h         |   1 -
 include/linux/acpi_dma.h           | 120 ++++++++++
 14 files changed, 717 insertions(+), 182 deletions(-)
 delete mode 100644 drivers/acpi/csrt.c
 create mode 100644 drivers/dma/acpi-dma.c
 create mode 100644 include/linux/acpi_dma.h

-- 
1.8.2.rc0.22.gb3600c3

[PATCH 1/6] dma: acpi-dma: introduce ACPI DMA helpers

[Andy Shevchenko]

There is a new generic API to get a DMA channel for a slave device (commit
9a6cecc8 "dmaengine: add helper function to request a slave DMA channel"). In
similar fashion to the DT case (commit aa3da644 "of: Add generic device tree
DMA helpers") we introduce helpers to the DMAC drivers which are enumerated by
ACPI.

The proposed extension provides the following API calls:
	acpi_dma_controller_register(), devm_acpi_dma_controller_register()
	acpi_dma_controller_free(), devm_acpi_dma_controller_free()
	acpi_dma_simple_xlate()
	acpi_dma_request_slave_chan_by_index()
	acpi_dma_request_slave_chan_by_name()

The first two should be used, for example, at probe() and remove() of the
corresponding DMAC driver. At the register stage the DMAC driver supplies a
custom xlate() function to translate a struct dma_spec into struct dma_chan.

Accordingly to the ACPI Fixed DMA resource specification the only two pieces of
information the slave device has are the channel id and the request line (slave
id). Those two are represented by struct dma_spec. The
acpi_dma_request_slave_chan_by_index() provides access to the specifix FixedDMA
resource by its index. Whereas dma_request_slave_channel() takes a string
parameter to identify the DMA resources required by the slave device. To make a
slave device driver work with both DeviceTree and ACPI enumeration a simple
convention is established: "tx" corresponds to the index 0 and "rx" to the
index 1. In case of robust configuration the slave device driver unfortunately
needs to call acpi_dma_request_slave_chan_by_index() directly.

Additionally the patch provides "managed" version of the register/free pair
i.e. devm_acpi_dma_controller_register() and devm_acpi_dma_controller_free().
Usually, the driver uses only devm_acpi_dma_controller_register().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/acpi/enumeration.txt |  77 ++++++++++
 drivers/dma/Kconfig                |   4 +
 drivers/dma/Makefile               |   1 +
 drivers/dma/acpi-dma.c             | 281 +++++++++++++++++++++++++++++++++++++
 include/linux/acpi_dma.h           | 116 +++++++++++++++
 5 files changed, 479 insertions(+)
 create mode 100644 drivers/dma/acpi-dma.c
 create mode 100644 include/linux/acpi_dma.h

diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt
index 94a6561..2874c90 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -66,6 +66,83 @@ the ACPI device explicitly to acpi_platform_device_ids list defined in
 drivers/acpi/acpi_platform.c. This limitation is only for the platform
 devices, SPI and I2C devices are created automatically as described below.
 
+DMA support
+~~~~~~~~~~~
+DMA controllers enumerated via ACPI should be registered in the system to
+provide generic access to their resources. For example, a driver that would
+like to be accessible to slave devices via generic API call
+dma_request_slave_channel() must register itself at the end of the probe
+function like this:
+
+	err = devm_acpi_dma_controller_register(dev, xlate_func, dw);
+	/* Handle the error if it's not a case of !CONFIG_ACPI */
+
+and implement custom xlate function if needed (usually acpi_dma_simple_xlate()
+is enough) which converts the FixedDMA resource provided by struct
+acpi_dma_spec into the corresponding DMA channel. A piece of code for that case
+could look like:
+
+	#ifdef CONFIG_ACPI
+	struct filter_args {
+		/* Provide necessary information for the filter_func */
+		...
+	};
+
+	static bool filter_func(struct dma_chan *chan, void *param)
+	{
+		/* Choose the proper channel */
+		...
+	}
+
+	static struct dma_chan *xlate_func(struct acpi_dma_spec *dma_spec,
+			struct acpi_dma *adma)
+	{
+		dma_cap_mask_t cap;
+		struct filter_args args;
+
+		/* Prepare arguments for filter_func */
+		...
+		return dma_request_channel(cap, filter_func, &args);
+	}
+	#else
+	static struct dma_chan *xlate_func(struct acpi_dma_spec *dma_spec,
+			struct acpi_dma *adma)
+	{
+		return NULL;
+	}
+	#endif
+
+dma_request_slave_channel() will call xlate_func() for each registered DMA
+controller. In the xlate function the proper channel must be chosen based on
+information in struct acpi_dma_spec and the properties of the controller
+provided by struct acpi_dma.
+
+Clients must call dma_request_slave_channel() with the string parameter that
+corresponds to a specific FixedDMA resource. By default "tx" means the first
+entry of the FixedDMA resource array, "rx" means the second entry. The table
+below shows a layout:
+
+	Device (I2C0)
+	{
+		...
+		Method (_CRS, 0, NotSerialized)
+		{
+			Name (DBUF, ResourceTemplate ()
+			{
+				FixedDMA (0x0018, 0x0004, Width32bit, _Y48)
+				FixedDMA (0x0019, 0x0005, Width32bit, )
+			})
+		...
+		}
+	}
+
+So, the FixedDMA with request line 0x0018 is "tx" and next one is "rx" in
+this example.
+
+In robust cases the client unfortunately needs to call
+acpi_dma_request_slave_chan_by_index() directly and therefore choose the
+specific FixedDMA resource by its index.
+
 SPI serial bus support
 ~~~~~~~~~~~~~~~~~~~~~~
 Slave devices behind SPI bus have SpiSerialBus resource attached to them.
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index aeaea32..2ad5ed9 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -328,6 +328,10 @@ config DMA_ENGINE
 config DMA_VIRTUAL_CHANNELS
 	tristate
 
+config DMA_ACPI
+	def_bool y
+	depends on ACPI
+
 config DMA_OF
 	def_bool y
 	depends on OF
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 488e3ff..268e626 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -3,6 +3,7 @@ ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
 
 obj-$(CONFIG_DMA_ENGINE) += dmaengine.o
 obj-$(CONFIG_DMA_VIRTUAL_CHANNELS) += virt-dma.o
+obj-$(CONFIG_DMA_ACPI) += acpi-dma.o
 obj-$(CONFIG_DMA_OF) += of-dma.o
 
 obj-$(CONFIG_NET_DMA) += iovlock.o
diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
new file mode 100644
index 0000000..af4626e
--- /dev/null
+++ b/drivers/dma/acpi-dma.c
@@ -0,0 +1,281 @@
+/*
+ * ACPI helpers for DMA request / controller
+ *
+ * Based on of-dma.c
+ *
+ * Copyright (C) 2013, Intel Corporation
+ * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/acpi_dma.h>
+
+static LIST_HEAD(acpi_dma_list);
+static DEFINE_MUTEX(acpi_dma_lock);
+
+/**
+ * acpi_dma_controller_register - Register a DMA controller to ACPI DMA helpers
+ * @dev:		struct device of DMA controller
+ * @acpi_dma_xlate:	translation function which converts a dma specifier
+ *			into a dma_chan structure
+ * @data		pointer to controller specific data to be used by
+ *			translation function
+ *
+ * Returns 0 on success or appropriate errno value on error.
+ *
+ * Allocated memory should be freed with appropriate acpi_dma_controller_free()
+ * call.
+ */
+int acpi_dma_controller_register(struct device *dev,
+		struct dma_chan *(*acpi_dma_xlate)
+		(struct acpi_dma_spec *, struct acpi_dma *),
+		void *data)
+{
+	struct acpi_device *adev;
+	struct acpi_dma	*adma;
+
+	if (!dev || !acpi_dma_xlate)
+		return -EINVAL;
+
+	/* Check if the device was enumerated by ACPI */
+	if (!ACPI_HANDLE(dev))
+		return -EINVAL;
+
+	if (acpi_bus_get_device(ACPI_HANDLE(dev), &adev))
+		return -EINVAL;
+
+	adma = kzalloc(sizeof(*adma), GFP_KERNEL);
+	if (!adma)
+		return -ENOMEM;
+
+	adma->dev = dev;
+	adma->acpi_dma_xlate = acpi_dma_xlate;
+	adma->data = data;
+
+	/* Now queue acpi_dma controller structure in list */
+	mutex_lock(&acpi_dma_lock);
+	list_add_tail(&adma->dma_controllers, &acpi_dma_list);
+	mutex_unlock(&acpi_dma_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_dma_controller_register);
+
+/**
+ * acpi_dma_controller_free - Remove a DMA controller from ACPI DMA helpers list
+ * @dev:	struct device of DMA controller
+ *
+ * Memory allocated by acpi_dma_controller_register() is freed here.
+ */
+int acpi_dma_controller_free(struct device *dev)
+{
+	struct acpi_dma *adma;
+
+	if (!dev)
+		return -EINVAL;
+
+	mutex_lock(&acpi_dma_lock);
+
+	list_for_each_entry(adma, &acpi_dma_list, dma_controllers)
+		if (adma->dev == dev) {
+			list_del(&adma->dma_controllers);
+			mutex_unlock(&acpi_dma_lock);
+			kfree(adma);
+			return 0;
+		}
+
+	mutex_unlock(&acpi_dma_lock);
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(acpi_dma_controller_free);
+
+static void devm_acpi_dma_release(struct device *dev, void *res)
+{
+	acpi_dma_controller_free(dev);
+}
+
+/**
+ * devm_acpi_dma_controller_register - resource managed acpi_dma_controller_register()
+ * @dev:		device that is registering this DMA controller
+ * @acpi_dma_xlate:	translation function
+ * @data		pointer to controller specific data
+ *
+ * Managed acpi_dma_controller_register(). DMA controller registered by this
+ * function are automatically freed on driver detach. See
+ * acpi_dma_controller_register() for more information.
+ */
+int devm_acpi_dma_controller_register(struct device *dev,
+		struct dma_chan *(*acpi_dma_xlate)
+		(struct acpi_dma_spec *, struct acpi_dma *),
+		void *data)
+{
+	void *res;
+	int ret;
+
+	res = devres_alloc(devm_acpi_dma_release, 0, GFP_KERNEL);
+	if (!res)
+		return -ENOMEM;
+
+	ret = acpi_dma_controller_register(dev, acpi_dma_xlate, data);
+	if (ret) {
+		devres_free(res);
+		return ret;
+	}
+	devres_add(dev, res);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_acpi_dma_controller_register);
+
+/**
+ * devm_acpi_dma_controller_free - resource managed acpi_dma_controller_free()
+ *
+ * Unregister a DMA controller registered with
+ * devm_acpi_dma_controller_register(). Normally this function will not need to
+ * be called and the resource management code will ensure that the resource is
+ * freed.
+ */
+void devm_acpi_dma_controller_free(struct device *dev)
+{
+	WARN_ON(devres_destroy(dev, devm_acpi_dma_release, NULL, NULL));
+}
+EXPORT_SYMBOL_GPL(devm_acpi_dma_controller_free);
+
+struct acpi_dma_parser_data {
+	struct acpi_dma_spec dma_spec;
+	size_t index;
+	size_t n;
+};
+
+/**
+ * acpi_dma_parse_fixed_dma - Parse FixedDMA ACPI resources to a DMA specifier
+ * @res:	struct acpi_resource to get FixedDMA resources from
+ * @data:	pointer to a helper struct acpi_dma_parser_data
+ */
+static int acpi_dma_parse_fixed_dma(struct acpi_resource *res, void *data)
+{
+	struct acpi_dma_parser_data *pdata = data;
+
+	if (res->type == ACPI_RESOURCE_TYPE_FIXED_DMA) {
+		struct acpi_resource_fixed_dma *dma = &res->data.fixed_dma;
+
+		if (pdata->n++ == pdata->index) {
+			pdata->dma_spec.chan_id = dma->channels;
+			pdata->dma_spec.slave_id = dma->request_lines;
+		}
+	}
+
+	/* Tell the ACPI core to skip this resource */
+	return 1;
+}
+
+/**
+ * acpi_dma_request_slave_chan_by_index - Get the DMA slave channel
+ * @dev:	struct device to get DMA request from
+ * @index:	index of FixedDMA descriptor for @dev
+ *
+ * Returns pointer to appropriate dma channel on success or NULL on error.
+ */
+struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
+		size_t index)
+{
+	struct acpi_dma_parser_data pdata;
+	struct acpi_dma_spec *dma_spec = &pdata.dma_spec;
+	struct list_head resource_list;
+	struct acpi_device *adev;
+	struct acpi_dma *adma;
+	struct dma_chan *chan;
+
+	/* Check if the device was enumerated by ACPI */
+	if (!dev || !ACPI_HANDLE(dev))
+		return NULL;
+
+	if (acpi_bus_get_device(ACPI_HANDLE(dev), &adev))
+		return NULL;
+
+	memset(&pdata, 0, sizeof(pdata));
+	pdata.index = index;
+
+	/* Initial values for the request line and channel */
+	dma_spec->chan_id = -1;
+	dma_spec->slave_id = -1;
+
+	INIT_LIST_HEAD(&resource_list);
+	acpi_dev_get_resources(adev, &resource_list,
+			acpi_dma_parse_fixed_dma, &pdata);
+	acpi_dev_free_resource_list(&resource_list);
+
+	if (dma_spec->slave_id < 0 || dma_spec->chan_id < 0)
+		return NULL;
+
+	mutex_lock(&acpi_dma_lock);
+
+	list_for_each_entry(adma, &acpi_dma_list, dma_controllers) {
+		dma_spec->dev = adma->dev;
+		chan = adma->acpi_dma_xlate(dma_spec, adma);
+		if (chan) {
+			mutex_unlock(&acpi_dma_lock);
+			return chan;
+		}
+	}
+
+	mutex_unlock(&acpi_dma_lock);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
+
+/**
+ * acpi_dma_request_slave_chan_by_name - Get the DMA slave channel
+ * @dev:	struct device to get DMA request from
+ * @name:	represents corresponding FixedDMA descriptor for @dev
+ *
+ * In order to support both Device Tree and ACPI in a single driver we
+ * translate the names "tx" and "rx" here based on the most common case where
+ * the first FixedDMA descriptor is TX and second is RX.
+ *
+ * Returns pointer to appropriate dma channel on success or NULL on error.
+ */
+struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
+		const char *name)
+{
+	size_t index;
+
+	if (!strcmp(name, "tx"))
+		index = 0;
+	else if (!strcmp(name, "rx"))
+		index = 1;
+	else
+		return NULL;
+
+	return acpi_dma_request_slave_chan_by_index(dev, index);
+}
+EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);
+
+/**
+ * acpi_dma_simple_xlate - Simple ACPI DMA engine translation helper
+ * @dma_spec: pointer to ACPI DMA specifier
+ * @adma: pointer to ACPI DMA controller data
+ *
+ * A simple translation function for ACPI based devices. Passes &struct
+ * dma_spec to the DMA controller driver provided filter function. Returns
+ * pointer to the channel if found or %NULL otherwise.
+ */
+struct dma_chan *acpi_dma_simple_xlate(struct acpi_dma_spec *dma_spec,
+		struct acpi_dma *adma)
+{
+	struct acpi_dma_filter_info *info = adma->data;
+
+	if (!info || !info->filter_fn)
+		return NULL;
+
+	return dma_request_channel(info->dma_cap, info->filter_fn, dma_spec);
+}
+EXPORT_SYMBOL_GPL(acpi_dma_simple_xlate);
diff --git a/include/linux/acpi_dma.h b/include/linux/acpi_dma.h
new file mode 100644
index 0000000..d09deab
--- /dev/null
+++ b/include/linux/acpi_dma.h
@@ -0,0 +1,116 @@
+/*
+ * ACPI helpers for DMA request / controller
+ *
+ * Based on of_dma.h
+ *
+ * Copyright (C) 2013, Intel Corporation
+ * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX_ACPI_DMA_H
+#define __LINUX_ACPI_DMA_H
+
+#include <linux/list.h>
+#include <linux/device.h>
+#include <linux/dmaengine.h>
+
+/**
+ * struct acpi_dma_spec - slave device DMA resources
+ * @chan_id:	channel unique id
+ * @slave_id:	request line unique id
+ * @dev:	struct device of the DMA controller to be used in the filter
+ *		function
+ */
+struct acpi_dma_spec {
+	int		chan_id;
+	int		slave_id;
+	struct device	*dev;
+};
+
+/**
+ * struct acpi_dma - representation of the registered DMAC
+ * @dma_controllers:	linked list node
+ * @dev:		struct device of this controller
+ * @acpi_dma_xlate:	callback function to find a suitable channel
+ * @data:		private data used by a callback function
+ */
+struct acpi_dma {
+	struct list_head	dma_controllers;
+	struct device		*dev;
+	struct dma_chan		*(*acpi_dma_xlate)
+				(struct acpi_dma_spec *, struct acpi_dma *);
+	void			*data;
+};
+
+/* Used with acpi_dma_simple_xlate() */
+struct acpi_dma_filter_info {
+	dma_cap_mask_t	dma_cap;
+	dma_filter_fn	filter_fn;
+};
+
+#ifdef CONFIG_DMA_ACPI
+
+int acpi_dma_controller_register(struct device *dev,
+		struct dma_chan *(*acpi_dma_xlate)
+		(struct acpi_dma_spec *, struct acpi_dma *),
+		void *data);
+int acpi_dma_controller_free(struct device *dev);
+int devm_acpi_dma_controller_register(struct device *dev,
+		struct dma_chan *(*acpi_dma_xlate)
+		(struct acpi_dma_spec *, struct acpi_dma *),
+		void *data);
+void devm_acpi_dma_controller_free(struct device *dev);
+
+struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
+						      size_t index);
+struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
+						     const char *name);
+
+struct dma_chan *acpi_dma_simple_xlate(struct acpi_dma_spec *dma_spec,
+				       struct acpi_dma *adma);
+#else
+
+static inline int acpi_dma_controller_register(struct device *dev,
+		struct dma_chan *(*acpi_dma_xlate)
+		(struct acpi_dma_spec *, struct acpi_dma *),
+		void *data)
+{
+	return -ENODEV;
+}
+static inline int acpi_dma_controller_free(struct device *dev)
+{
+	return -ENODEV;
+}
+static inline int devm_acpi_dma_controller_register(struct device *dev,
+		struct dma_chan *(*acpi_dma_xlate)
+		(struct acpi_dma_spec *, struct acpi_dma *),
+		void *data)
+{
+	return -ENODEV;
+}
+static inline void devm_acpi_dma_controller_free(struct device *dev)
+{
+}
+
+static inline struct dma_chan *acpi_dma_request_slave_chan_by_index(
+		struct device *dev, size_t index)
+{
+	return NULL;
+}
+static inline struct dma_chan *acpi_dma_request_slave_chan_by_name(
+		struct device *dev, const char *name)
+{
+	return NULL;
+}
+
+#define acpi_dma_simple_xlate	NULL
+
+#endif
+
+#define acpi_dma_request_slave_channel	acpi_dma_request_slave_chan_by_index
+
+#endif /* __LINUX_ACPI_DMA_H */
-- 
1.8.2.rc0.22.gb3600c3

[PATCH 2/6] dmaengine: call acpi_dma_request_slave_channel as well

[Andy Shevchenko]

The slave device could be enumerated by ACPI. In that case the
dma_request_slave_channel should use the acpi_dma_request_slave_channel()
helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/dma/dmaengine.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 2cbfefe..8bbd6aa 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -62,6 +62,8 @@
 #include <linux/rculist.h>
 #include <linux/idr.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/acpi_dma.h>
 #include <linux/of_dma.h>
 
 static DEFINE_MUTEX(dma_list_mutex);
@@ -561,6 +563,10 @@ struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name)
 	if (dev->of_node)
 		return of_dma_request_slave_channel(dev->of_node, name);
 
+	/* If device was enumerated by ACPI get slave info from here */
+	if (ACPI_HANDLE(dev))
+		return acpi_dma_request_slave_chan_by_name(dev, name);
+
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(dma_request_slave_channel);
-- 
1.8.2.rc0.22.gb3600c3

[PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources

[Andy Shevchenko]

Since we have CSRT only to get additional DMA controller resources, let's get
rid of drivers/acpi/csrt.c and move its logic inside ACPI DMA helpers code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/Makefile    |   1 -
 drivers/acpi/csrt.c      | 159 -------------------------------------------
 drivers/acpi/internal.h  |   1 -
 drivers/acpi/scan.c      |   1 -
 drivers/dma/acpi-dma.c   | 172 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/acpi_dma.h |   4 ++
 6 files changed, 173 insertions(+), 165 deletions(-)
 delete mode 100644 drivers/acpi/csrt.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index ecb743b..6050c80 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -38,7 +38,6 @@ acpi-y				+= processor_core.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o
-acpi-y				+= csrt.o
 acpi-$(CONFIG_X86_INTEL_LPSS)	+= acpi_lpss.o
 acpi-y				+= acpi_platform.o
 acpi-y				+= power.o
diff --git a/drivers/acpi/csrt.c b/drivers/acpi/csrt.c
deleted file mode 100644
index 5c15a91..0000000
--- a/drivers/acpi/csrt.c
+++ /dev/null
@@ -1,159 +0,0 @@
-/*
- * Support for Core System Resources Table (CSRT)
- *
- * Copyright (C) 2013, Intel Corporation
- * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
- *	    Andy Shevchenko <andriy.shevchenko@linux.intel.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#define pr_fmt(fmt) "ACPI: CSRT: " fmt
-
-#include <linux/acpi.h>
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/sizes.h>
-
-ACPI_MODULE_NAME("CSRT");
-
-static int __init acpi_csrt_parse_shared_info(struct platform_device *pdev,
-					      const struct acpi_csrt_group *grp)
-{
-	const struct acpi_csrt_shared_info *si;
-	struct resource res[3];
-	size_t nres;
-	int ret;
-
-	memset(res, 0, sizeof(res));
-	nres = 0;
-
-	si = (const struct acpi_csrt_shared_info *)&grp[1];
-	/*
-	 * The peripherals that are listed on CSRT typically support only
-	 * 32-bit addresses so we only use the low part of MMIO base for
-	 * now.
-	 */
-	if (!si->mmio_base_high && si->mmio_base_low) {
-		/*
-		 * There is no size of the memory resource in shared_info
-		 * so we assume that it is 4k here.
-		 */
-		res[nres].start = si->mmio_base_low;
-		res[nres].end = res[0].start + SZ_4K - 1;
-		res[nres++].flags = IORESOURCE_MEM;
-	}
-
-	if (si->gsi_interrupt) {
-		int irq = acpi_register_gsi(NULL, si->gsi_interrupt,
-					    si->interrupt_mode,
-					    si->interrupt_polarity);
-		res[nres].start = irq;
-		res[nres].end = irq;
-		res[nres++].flags = IORESOURCE_IRQ;
-	}
-
-	if (si->base_request_line || si->num_handshake_signals) {
-		/*
-		 * We pass the driver a DMA resource describing the range
-		 * of request lines the device supports.
-		 */
-		res[nres].start = si->base_request_line;
-		res[nres].end = res[nres].start + si->num_handshake_signals - 1;
-		res[nres++].flags = IORESOURCE_DMA;
-	}
-
-	ret = platform_device_add_resources(pdev, res, nres);
-	if (ret) {
-		if (si->gsi_interrupt)
-			acpi_unregister_gsi(si->gsi_interrupt);
-		return ret;
-	}
-
-	return 0;
-}
-
-static int __init
-acpi_csrt_parse_resource_group(const struct acpi_csrt_group *grp)
-{
-	struct platform_device *pdev;
-	char vendor[5], name[16];
-	int ret, i;
-
-	vendor[0] = grp->vendor_id;
-	vendor[1] = grp->vendor_id >> 8;
-	vendor[2] = grp->vendor_id >> 16;
-	vendor[3] = grp->vendor_id >> 24;
-	vendor[4] = '\0';
-
-	if (grp->shared_info_length != sizeof(struct acpi_csrt_shared_info))
-		return -ENODEV;
-
-	snprintf(name, sizeof(name), "%s%04X", vendor, grp->device_id);
-	pdev = platform_device_alloc(name, PLATFORM_DEVID_AUTO);
-	if (!pdev)
-		return -ENOMEM;
-
-	/* Add resources based on the shared info */
-	ret = acpi_csrt_parse_shared_info(pdev, grp);
-	if (ret)
-		goto fail;
-
-	ret = platform_device_add(pdev);
-	if (ret)
-		goto fail;
-
-	for (i = 0; i < pdev->num_resources; i++)
-		dev_dbg(&pdev->dev, "%pR\n", &pdev->resource[i]);
-
-	return 0;
-
-fail:
-	platform_device_put(pdev);
-	return ret;
-}
-
-/*
- * CSRT or Core System Resources Table is a proprietary ACPI table
- * introduced by Microsoft. This table can contain devices that are not in
- * the system DSDT table. In particular DMA controllers might be described
- * here.
- *
- * We present these devices as normal platform devices that don't have ACPI
- * IDs or handle. The platform device name will be something like
- * <VENDOR><DEVID>.<n>.auto for example: INTL9C06.0.auto.
- */
-void __init acpi_csrt_init(void)
-{
-	struct acpi_csrt_group *grp, *end;
-	struct acpi_table_csrt *csrt;
-	acpi_status status;
-	int ret;
-
-	status = acpi_get_table(ACPI_SIG_CSRT, 0,
-				(struct acpi_table_header **)&csrt);
-	if (ACPI_FAILURE(status)) {
-		if (status != AE_NOT_FOUND)
-			pr_warn("failed to get the CSRT table\n");
-		return;
-	}
-
-	pr_debug("parsing CSRT table for devices\n");
-
-	grp = (struct acpi_csrt_group *)(csrt + 1);
-	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
-
-	while (grp < end) {
-		ret = acpi_csrt_parse_resource_group(grp);
-		if (ret) {
-			pr_warn("error in parsing resource group: %d\n", ret);
-			return;
-		}
-
-		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
-	}
-}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 6f1afd9..297cbf4 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -35,7 +35,6 @@ void acpi_pci_link_init(void);
 void acpi_pci_root_hp_init(void);
 void acpi_platform_init(void);
 int acpi_sysfs_init(void);
-void acpi_csrt_init(void);
 #ifdef CONFIG_ACPI_CONTAINER
 void acpi_container_init(void);
 #else
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d7f3c8b..6742aa1 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2042,7 +2042,6 @@ int __init acpi_scan_init(void)
 	acpi_pci_link_init();
 	acpi_platform_init();
 	acpi_lpss_init();
-	acpi_csrt_init();
 	acpi_container_init();
 	acpi_pci_slot_init();
 	acpi_memory_hotplug_init();
diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
index af4626e..2b41df9 100644
--- a/drivers/dma/acpi-dma.c
+++ b/drivers/dma/acpi-dma.c
@@ -4,7 +4,8 @@
  * Based on of-dma.c
  *
  * Copyright (C) 2013, Intel Corporation
- * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ * Authors: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *	    Mika Westerberg <mika.westerberg@linux.intel.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -16,6 +17,7 @@
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/ioport.h>
 #include <linux/acpi.h>
 #include <linux/acpi_dma.h>
 
@@ -23,6 +25,117 @@ static LIST_HEAD(acpi_dma_list);
 static DEFINE_MUTEX(acpi_dma_lock);
 
 /**
+ * acpi_dma_parse_resource_group - match device and parse resource group
+ * @grp:	CSRT resource group
+ * @adev:	ACPI device to match with
+ * @adma:	struct acpi_dma of the given DMA controller
+ *
+ * Returns 1 on success, 0 when no information is available, or appropriate
+ * errno value on error.
+ *
+ * In order to match a device from DSDT table to the corresponding CSRT device
+ * we use MMIO address and IRQ.
+ */
+static int acpi_dma_parse_resource_group(const struct acpi_csrt_group *grp,
+		struct acpi_device *adev, struct acpi_dma *adma)
+{
+	const struct acpi_csrt_shared_info *si;
+	struct list_head resource_list;
+	struct resource_list_entry *rentry;
+	resource_size_t mem = 0, irq = 0;
+	u32 vendor_id;
+	int ret;
+
+	if (grp->shared_info_length != sizeof(struct acpi_csrt_shared_info))
+		return -ENODEV;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
+	if (ret <= 0)
+		return 0;
+
+	list_for_each_entry(rentry, &resource_list, node) {
+		if (resource_type(&rentry->res) == IORESOURCE_MEM)
+			mem = rentry->res.start;
+		else if (resource_type(&rentry->res) == IORESOURCE_IRQ)
+			irq = rentry->res.start;
+	}
+
+	acpi_dev_free_resource_list(&resource_list);
+
+	/* Consider initial zero values as resource not found */
+	if (mem == 0 && irq == 0)
+		return 0;
+
+	si = (const struct acpi_csrt_shared_info *)&grp[1];
+
+	/* Match device by MMIO and IRQ */
+	if (si->mmio_base_low != mem || si->gsi_interrupt != irq)
+		return 0;
+
+	vendor_id = le32_to_cpu(grp->vendor_id);
+	dev_dbg(&adev->dev, "matches with %.4s%04X (rev %u)\n",
+		(char *)&vendor_id, grp->device_id, grp->revision);
+
+	/* Check if the request line range is available */
+	if (si->base_request_line == 0 && si->num_handshake_signals == 0)
+		return 0;
+
+	adma->base_request_line = si->base_request_line;
+	adma->end_request_line = si->base_request_line +
+				 si->num_handshake_signals - 1;
+
+	dev_dbg(&adev->dev, "request line base: 0x%04x end: 0x%04x\n",
+		adma->base_request_line, adma->end_request_line);
+
+	return 1;
+}
+
+/**
+ * acpi_dma_parse_csrt - parse CSRT to exctract additional DMA resources
+ * @adev:	ACPI device to match with
+ * @adma:	struct acpi_dma of the given DMA controller
+ *
+ * CSRT or Core System Resources Table is a proprietary ACPI table
+ * introduced by Microsoft. This table can contain devices that are not in
+ * the system DSDT table. In particular DMA controllers might be described
+ * here.
+ *
+ * We are using this table to get the request line range of the specific DMA
+ * controller to be used later.
+ *
+ */
+static void acpi_dma_parse_csrt(struct acpi_device *adev, struct acpi_dma *adma)
+{
+	struct acpi_csrt_group *grp, *end;
+	struct acpi_table_csrt *csrt;
+	acpi_status status;
+	int ret;
+
+	status = acpi_get_table(ACPI_SIG_CSRT, 0,
+				(struct acpi_table_header **)&csrt);
+	if (ACPI_FAILURE(status)) {
+		if (status != AE_NOT_FOUND)
+			dev_warn(&adev->dev, "failed to get the CSRT table\n");
+		return;
+	}
+
+	grp = (struct acpi_csrt_group *)(csrt + 1);
+	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
+
+	while (grp < end) {
+		ret = acpi_dma_parse_resource_group(grp, adev, adma);
+		if (ret < 0) {
+			dev_warn(&adev->dev,
+				 "error in parsing resource group\n");
+			return;
+		}
+
+		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
+	}
+}
+
+/**
  * acpi_dma_controller_register - Register a DMA controller to ACPI DMA helpers
  * @dev:		struct device of DMA controller
  * @acpi_dma_xlate:	translation function which converts a dma specifier
@@ -61,6 +174,8 @@ int acpi_dma_controller_register(struct device *dev,
 	adma->acpi_dma_xlate = acpi_dma_xlate;
 	adma->data = data;
 
+	acpi_dma_parse_csrt(adev, adma);
+
 	/* Now queue acpi_dma controller structure in list */
 	mutex_lock(&acpi_dma_lock);
 	list_add_tail(&adma->dma_controllers, &acpi_dma_list);
@@ -149,6 +264,45 @@ void devm_acpi_dma_controller_free(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(devm_acpi_dma_controller_free);
 
+/**
+ * acpi_dma_update_dma_spec - prepare dma specifier to pass to translation function
+ * @adma:	struct acpi_dma of DMA controller
+ * @dma_spec:	dma specifier to update
+ *
+ * Returns 0, if no information is avaiable, -1 on mismatch, and 1 otherwise.
+ *
+ * Accordingly to ACPI 5.0 Specification Table 6-170 "Fixed DMA Resource
+ * Descriptor":
+ *	DMA Request Line bits is a platform-relative number uniquely
+ *	identifying the request line assigned. Request line-to-Controller
+ *	mapping is done in a controller-specific OS driver.
+ * That's why we can safely adjust slave_id when the appropriate controller is
+ * found.
+ */
+static int acpi_dma_update_dma_spec(struct acpi_dma *adma,
+		struct acpi_dma_spec *dma_spec)
+{
+	/* Set link to the DMA controller device */
+	dma_spec->dev = adma->dev;
+
+	/* Check if the request line range is available */
+	if (adma->base_request_line == 0 && adma->end_request_line == 0)
+		return 0;
+
+	/* Check if slave_id falls to the range */
+	if (dma_spec->slave_id < adma->base_request_line ||
+	    dma_spec->slave_id > adma->end_request_line)
+		return -1;
+
+	/*
+	 * Here we adjust slave_id. It should be a relative number to the base
+	 * request line.
+	 */
+	dma_spec->slave_id -= adma->base_request_line;
+
+	return 1;
+}
+
 struct acpi_dma_parser_data {
 	struct acpi_dma_spec dma_spec;
 	size_t index;
@@ -193,6 +347,7 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
 	struct acpi_device *adev;
 	struct acpi_dma *adma;
 	struct dma_chan *chan;
+	int found;
 
 	/* Check if the device was enumerated by ACPI */
 	if (!dev || !ACPI_HANDLE(dev))
@@ -219,9 +374,20 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
 	mutex_lock(&acpi_dma_lock);
 
 	list_for_each_entry(adma, &acpi_dma_list, dma_controllers) {
-		dma_spec->dev = adma->dev;
+		/*
+		 * We are not going to call translation function if slave_id
+		 * doesn't fall to the request range.
+		 */
+		found = acpi_dma_update_dma_spec(adma, dma_spec);
+		if (found < 0)
+			continue;
 		chan = adma->acpi_dma_xlate(dma_spec, adma);
-		if (chan) {
+		/*
+		 * Try to get a channel only from the DMA controller that
+		 * matches the slave_id. See acpi_dma_update_dma_spec()
+		 * description for the details.
+		 */
+		if (found > 0 || chan) {
 			mutex_unlock(&acpi_dma_lock);
 			return chan;
 		}
diff --git a/include/linux/acpi_dma.h b/include/linux/acpi_dma.h
index d09deab..fb02980 100644
--- a/include/linux/acpi_dma.h
+++ b/include/linux/acpi_dma.h
@@ -37,6 +37,8 @@ struct acpi_dma_spec {
  * @dev:		struct device of this controller
  * @acpi_dma_xlate:	callback function to find a suitable channel
  * @data:		private data used by a callback function
+ * @base_request_line:	first supported request line (CSRT)
+ * @end_request_line:	last supported request line (CSRT)
  */
 struct acpi_dma {
 	struct list_head	dma_controllers;
@@ -44,6 +46,8 @@ struct acpi_dma {
 	struct dma_chan		*(*acpi_dma_xlate)
 				(struct acpi_dma_spec *, struct acpi_dma *);
 	void			*data;
+	unsigned short		base_request_line;
+	unsigned short		end_request_line;
 };
 
 /* Used with acpi_dma_simple_xlate() */
-- 
1.8.2.rc0.22.gb3600c3

[PATCH 4/6] dw_dmac: add ACPI support

[Andy Shevchenko]

Since we have proper ACPI DMA helpers implemented, the driver may use it. This
patch introduces custom filter function together with acpi_device_id table.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw_dmac.c      | 68 ++++++++++++++++++++++++++++++++++------------
 drivers/dma/dw_dmac_regs.h |  1 -
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index e33dc3b..2e5deaa 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -25,6 +25,8 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/acpi_dma.h>
 
 #include "dw_dmac_regs.h"
 #include "dmaengine.h"
@@ -983,13 +985,6 @@ static inline void convert_burst(u32 *maxburst)
 		*maxburst = 0;
 }
 
-static inline void convert_slave_id(struct dw_dma_chan *dwc)
-{
-	struct dw_dma *dw = to_dw_dma(dwc->chan.device);
-
-	dwc->dma_sconfig.slave_id -= dw->request_line_base;
-}
-
 static int
 set_runtime_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
 {
@@ -1008,7 +1003,6 @@ set_runtime_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
 
 	convert_burst(&dwc->dma_sconfig.src_maxburst);
 	convert_burst(&dwc->dma_sconfig.dst_maxburst);
-	convert_slave_id(dwc);
 
 	return 0;
 }
@@ -1284,6 +1278,46 @@ static struct dma_chan *dw_dma_of_xlate(struct of_phandle_args *dma_spec,
 	return dma_request_channel(cap, dw_dma_of_filter, &fargs);
 }
 
+#ifdef CONFIG_ACPI
+static bool dw_dma_acpi_filter(struct dma_chan *chan, void *param)
+{
+	struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
+	struct acpi_dma_spec *dma_spec = param;
+
+	if (chan->device->dev != dma_spec->dev ||
+	    chan->chan_id != dma_spec->chan_id)
+		return false;
+
+	dwc->request_line = dma_spec->slave_id;
+	dwc->src_master = dwc_get_sms(NULL);
+	dwc->dst_master = dwc_get_dms(NULL);
+
+	return true;
+}
+
+static void dw_dma_acpi_controller_register(struct dw_dma *dw)
+{
+	struct device *dev = dw->dma.dev;
+	struct acpi_dma_filter_info *info;
+	int ret;
+
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return;
+
+	dma_cap_zero(info->dma_cap);
+	dma_cap_set(DMA_SLAVE, info->dma_cap);
+	info->filter_fn = dw_dma_acpi_filter;
+
+	ret = devm_acpi_dma_controller_register(dev, acpi_dma_simple_xlate,
+						info);
+	if (ret)
+		dev_err(dev, "could not register acpi_dma_controller\n");
+}
+#else /* !CONFIG_ACPI */
+static inline void dw_dma_acpi_controller_register(struct dw_dma *dw) {}
+#endif /* !CONFIG_ACPI */
+
 /* --------------------- Cyclic DMA API extensions -------------------- */
 
 /**
@@ -1620,7 +1654,6 @@ dw_dma_parse_dt(struct platform_device *pdev)
 
 static int dw_probe(struct platform_device *pdev)
 {
-	const struct platform_device_id *match;
 	struct dw_dma_platform_data *pdata;
 	struct resource		*io;
 	struct dw_dma		*dw;
@@ -1704,11 +1737,6 @@ static int dw_probe(struct platform_device *pdev)
 		memcpy(dw->data_width, pdata->data_width, 4);
 	}
 
-	/* Get the base request line if set */
-	match = platform_get_device_id(pdev);
-	if (match)
-		dw->request_line_base = (unsigned int)match->driver_data;
-
 	/* Calculate all channel mask before DMA setup */
 	dw->all_chan_mask = (1 << nr_channels) - 1;
 
@@ -1833,6 +1861,9 @@ static int dw_probe(struct platform_device *pdev)
 				"could not register of_dma_controller\n");
 	}
 
+	if (ACPI_HANDLE(&pdev->dev))
+		dw_dma_acpi_controller_register(dw);
+
 	return 0;
 }
 
@@ -1904,11 +1935,12 @@ static const struct of_device_id dw_dma_of_id_table[] = {
 MODULE_DEVICE_TABLE(of, dw_dma_of_id_table);
 #endif
 
-static const struct platform_device_id dw_dma_ids[] = {
-	/* Name,	Request Line Base */
-	{ "INTL9C60",	(kernel_ulong_t)16 },
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id dw_dma_acpi_id_table[] = {
+	{ "INTL9C60", 0 },
 	{ }
 };
+#endif
 
 static struct platform_driver dw_driver = {
 	.probe		= dw_probe,
@@ -1918,8 +1950,8 @@ static struct platform_driver dw_driver = {
 		.name	= "dw_dmac",
 		.pm	= &dw_dev_pm_ops,
 		.of_match_table = of_match_ptr(dw_dma_of_id_table),
+		.acpi_match_table = ACPI_PTR(dw_dma_acpi_id_table),
 	},
-	.id_table	= dw_dma_ids,
 };
 
 static int __init dw_init(void)
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index 9b0e12e..9d41720 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -250,7 +250,6 @@ struct dw_dma {
 	/* hardware configuration */
 	unsigned char		nr_masters;
 	unsigned char		data_width[4];
-	unsigned int		request_line_base;
 
 	struct dw_dma_chan	chan[0];
 };
-- 
1.8.2.rc0.22.gb3600c3

[PATCH 5/6] ACPI / LPSS: return no error from register_device_clock in special cases

[Andy Shevchenko]

When device uses the fixed clock and has no private space of LTR, we have to
create main LPSS clock and register platform device. This is normally a case
for LPSS DMA controller.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/acpi_lpss.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index b1c9542..4015929 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -95,6 +95,10 @@ static int register_device_clock(struct acpi_device *adev,
 	if (!lpss_clk_dev)
 		lpt_register_clock_device();
 
+	if (!dev_desc->prv_offset && dev_desc->clk_required)
+		/* There is no error when device uses fixed clock */
+		return 0;
+
 	if (!dev_desc->clk_parent || !pdata->mmio_base
 	    || pdata->mmio_size < dev_desc->prv_offset + LPSS_CLK_SIZE)
 		return -ENODATA;
-- 
1.8.2.rc0.22.gb3600c3

[PATCH 6/6] ACPI / LPSS: add Lynxpoint DMA controller to the list

[Andy Shevchenko]

In Lynxpoint we have to enable clock per each LPSS device. That's why we have
to enumerate them from drivers/acpi/acpi_lpss.c. The DMA controller is one of
such devices.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/acpi_lpss.c  | 7 +++++++
 drivers/clk/x86/clk-lpt.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 4015929..d80c81e 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -47,6 +47,10 @@ struct lpss_private_data {
 	const struct lpss_device_desc *dev_desc;
 };
 
+static struct lpss_device_desc lpss_dev_desc = {
+	.clk_required = true,
+};
+
 static struct lpss_device_desc lpt_dev_desc = {
 	.clk_required = true,
 	.clk_parent = "lpss_clk",
@@ -60,6 +64,9 @@ static struct lpss_device_desc lpt_sdio_dev_desc = {
 };
 
 static const struct acpi_device_id acpi_lpss_device_ids[] = {
+	/* Generic LPSS devices */
+	{ "INTL9C60", (unsigned long)&lpss_dev_desc },
+
 	/* Lynxpoint LPSS devices */
 	{ "INT33C0", (unsigned long)&lpt_dev_desc },
 	{ "INT33C1", (unsigned long)&lpt_dev_desc },
diff --git a/drivers/clk/x86/clk-lpt.c b/drivers/clk/x86/clk-lpt.c
index 5cf4f46..719817e 100644
--- a/drivers/clk/x86/clk-lpt.c
+++ b/drivers/clk/x86/clk-lpt.c
@@ -30,7 +30,7 @@ static int lpt_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(clk);
 
 	/* Shared DMA clock */
-	clk_register_clkdev(clk, "hclk", "INTL9C60.0.auto");
+	clk_register_clkdev(clk, "hclk", "INTL9C60:00");
 	return 0;
 }
 
-- 
1.8.2.rc0.22.gb3600c3

Re: [PATCH 1/6] dma: acpi-dma: introduce ACPI DMA helpers

[Vinod Koul]

On Wed, Mar 27, 2013 at 10:57:57AM +0200, Andy Shevchenko wrote:
> + * @dev:	struct device to get DMA request from
> + * @index:	index of FixedDMA descriptor for @dev
> + *
> + * Returns pointer to appropriate dma channel on success or NULL on error.
> + */
> +struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
> +		size_t index)
> +{
So i think this will be called by client driver to request a channel right?

If so how does client find the device pointer for dma controller.

And index is a global one, right? What is it use ?

> +	struct acpi_dma_parser_data pdata;
> +	struct acpi_dma_spec *dma_spec = &pdata.dma_spec;
> +	struct list_head resource_list;
> +	struct acpi_device *adev;
> +	struct acpi_dma *adma;
> +	struct dma_chan *chan;
> +
> +	/* Check if the device was enumerated by ACPI */
> +	if (!dev || !ACPI_HANDLE(dev))
> +		return NULL;
> +
> +	if (acpi_bus_get_device(ACPI_HANDLE(dev), &adev))
> +		return NULL;
> +
> +	memset(&pdata, 0, sizeof(pdata));
> +	pdata.index = index;
> +
> +	/* Initial values for the request line and channel */
> +	dma_spec->chan_id = -1;
> +	dma_spec->slave_id = -1;
> +
> +	INIT_LIST_HEAD(&resource_list);
> +	acpi_dev_get_resources(adev, &resource_list,
> +			acpi_dma_parse_fixed_dma, &pdata);
> +	acpi_dev_free_resource_list(&resource_list);
> +
> +	if (dma_spec->slave_id < 0 || dma_spec->chan_id < 0)
> +		return NULL;
> +
> +	mutex_lock(&acpi_dma_lock);
> +
> +	list_for_each_entry(adma, &acpi_dma_list, dma_controllers) {
> +		dma_spec->dev = adma->dev;
> +		chan = adma->acpi_dma_xlate(dma_spec, adma);
> +		if (chan) {
> +			mutex_unlock(&acpi_dma_lock);
> +			return chan;
> +		}
> +	}
> +
> +	mutex_unlock(&acpi_dma_lock);
> +	return NULL;
in this and error handling you are not doing anything different so why code
duplication?

> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
> +
> +/**
> + * acpi_dma_request_slave_chan_by_name - Get the DMA slave channel
> + * @dev:	struct device to get DMA request from
> + * @name:	represents corresponding FixedDMA descriptor for @dev
> + *
> + * In order to support both Device Tree and ACPI in a single driver we
> + * translate the names "tx" and "rx" here based on the most common case where
> + * the first FixedDMA descriptor is TX and second is RX.
> + *
> + * Returns pointer to appropriate dma channel on success or NULL on error.
> + */
> +struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
> +		const char *name)
> +{
> +	size_t index;
> +
> +	if (!strcmp(name, "tx"))
> +		index = 0;
> +	else if (!strcmp(name, "rx"))
> +		index = 1;
> +	else
> +		return NULL;
> +
> +	return acpi_dma_request_slave_chan_by_index(dev, index);
are you going to a have a different descriptor for tx and rx? Is index only for
tx = 0 and rx =1 always. How would a controller be represented which has 8
channels, each channel can do DMA in either direction

> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);
> +
> +/**
> + * acpi_dma_simple_xlate - Simple ACPI DMA engine translation helper
> + * @dma_spec: pointer to ACPI DMA specifier
> + * @adma: pointer to ACPI DMA controller data
> + *
> + * A simple translation function for ACPI based devices. Passes &struct
> + * dma_spec to the DMA controller driver provided filter function. Returns
> + * pointer to the channel if found or %NULL otherwise.
> + */
> +struct dma_chan *acpi_dma_simple_xlate(struct acpi_dma_spec *dma_spec,
> +		struct acpi_dma *adma)
> +{
> +	struct acpi_dma_filter_info *info = adma->data;
what is the purpose of filter here?
> +
> +	if (!info || !info->filter_fn)
> +		return NULL;
> +
> +	return dma_request_channel(info->dma_cap, info->filter_fn, dma_spec);
> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_simple_xlate);
> diff --git a/include/linux/acpi_dma.h b/include/linux/acpi_dma.h
> new file mode 100644
> index 0000000..d09deab
> --- /dev/null
> +++ b/include/linux/acpi_dma.h
> @@ -0,0 +1,116 @@
> +/*
> + * ACPI helpers for DMA request / controller
> + *
> + * Based on of_dma.h
> + *
> + * Copyright (C) 2013, Intel Corporation
> + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __LINUX_ACPI_DMA_H
> +#define __LINUX_ACPI_DMA_H
> +
> +#include <linux/list.h>
> +#include <linux/device.h>
> +#include <linux/dmaengine.h>
> +
> +/**
> + * struct acpi_dma_spec - slave device DMA resources
> + * @chan_id:	channel unique id
> + * @slave_id:	request line unique id
> + * @dev:	struct device of the DMA controller to be used in the filter
> + *		function
> + */
> +struct acpi_dma_spec {
> +	int		chan_id;
> +	int		slave_id;
> +	struct device	*dev;
> +};
is this the representation of Fixed DMA, if so why have you omitted transfer widths?
I can see obvious benefits of using that 

--
~Vinod

Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources

[Vinod Koul]

On Wed, Mar 27, 2013 at 10:57:59AM +0200, Andy Shevchenko wrote:
> Since we have CSRT only to get additional DMA controller resources, let's get
> rid of drivers/acpi/csrt.c and move its logic inside ACPI DMA helpers code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
for such a patch git format-patch -M is your friend. It generates patch to show
file movement. It helps review greatly if you just move the file and then do
additions on second patch, as diffstat tells me some changes have been done.

> @@ -23,6 +25,117 @@ static LIST_HEAD(acpi_dma_list);
>  static DEFINE_MUTEX(acpi_dma_lock);
>  
>  /**
> + * acpi_dma_parse_resource_group - match device and parse resource group
> + * @grp:	CSRT resource group
> + * @adev:	ACPI device to match with
> + * @adma:	struct acpi_dma of the given DMA controller
> + *
> + * Returns 1 on success, 0 when no information is available, or appropriate
> + * errno value on error.
> + *
> + * In order to match a device from DSDT table to the corresponding CSRT device
> + * we use MMIO address and IRQ.
> + */
>  
> +/**
> + * acpi_dma_update_dma_spec - prepare dma specifier to pass to translation function
> + * @adma:	struct acpi_dma of DMA controller
> + * @dma_spec:	dma specifier to update
> + *
> + * Returns 0, if no information is avaiable, -1 on mismatch, and 1 otherwise.
> + *
> + * Accordingly to ACPI 5.0 Specification Table 6-170 "Fixed DMA Resource
> + * Descriptor":
> + *	DMA Request Line bits is a platform-relative number uniquely
> + *	identifying the request line assigned. Request line-to-Controller
> + *	mapping is done in a controller-specific OS driver.
> + * That's why we can safely adjust slave_id when the appropriate controller is
> + * found.
> + */
> +static int acpi_dma_update_dma_spec(struct acpi_dma *adma,
> +		struct acpi_dma_spec *dma_spec)
> +{
> +	/* Set link to the DMA controller device */
> +	dma_spec->dev = adma->dev;
> +
> +	/* Check if the request line range is available */
> +	if (adma->base_request_line == 0 && adma->end_request_line == 0)
> +		return 0;
> +
> +	/* Check if slave_id falls to the range */
> +	if (dma_spec->slave_id < adma->base_request_line ||
> +	    dma_spec->slave_id > adma->end_request_line)
> +		return -1;
> +
> +	/*
> +	 * Here we adjust slave_id. It should be a relative number to the base
> +	 * request line.
> +	 */
> +	dma_spec->slave_id -= adma->base_request_line;
where are you getting the base_request_line, i didnt see anything for this in
ACPI spec?
> +
> +	return 1;
> +}
> +
>  struct acpi_dma_parser_data {
>  	struct acpi_dma_spec dma_spec;
>  	size_t index;
> @@ -193,6 +347,7 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
>  	struct acpi_device *adev;
>  	struct acpi_dma *adma;
>  	struct dma_chan *chan;
> +	int found;
>  
>  	/* Check if the device was enumerated by ACPI */
>  	if (!dev || !ACPI_HANDLE(dev))
> @@ -219,9 +374,20 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
>  	mutex_lock(&acpi_dma_lock);
>  
>  	list_for_each_entry(adma, &acpi_dma_list, dma_controllers) {
> -		dma_spec->dev = adma->dev;
> +		/*
> +		 * We are not going to call translation function if slave_id
> +		 * doesn't fall to the request range.
> +		 */
> +		found = acpi_dma_update_dma_spec(adma, dma_spec);
> +		if (found < 0)
> +			continue;
>  		chan = adma->acpi_dma_xlate(dma_spec, adma);
> -		if (chan) {
> +		/*
> +		 * Try to get a channel only from the DMA controller that
> +		 * matches the slave_id. See acpi_dma_update_dma_spec()
> +		 * description for the details.
> +		 */
> +		if (found > 0 || chan) {
>  			mutex_unlock(&acpi_dma_lock);
>  			return chan;
>  		}
> diff --git a/include/linux/acpi_dma.h b/include/linux/acpi_dma.h
> index d09deab..fb02980 100644
> --- a/include/linux/acpi_dma.h
> +++ b/include/linux/acpi_dma.h
> @@ -37,6 +37,8 @@ struct acpi_dma_spec {
>   * @dev:		struct device of this controller
>   * @acpi_dma_xlate:	callback function to find a suitable channel
>   * @data:		private data used by a callback function
> + * @base_request_line:	first supported request line (CSRT)
> + * @end_request_line:	last supported request line (CSRT)
okay here it is, can you add the width here as well
>   */
>  struct acpi_dma {
>  	struct list_head	dma_controllers;
> @@ -44,6 +46,8 @@ struct acpi_dma {
>  	struct dma_chan		*(*acpi_dma_xlate)
>  				(struct acpi_dma_spec *, struct acpi_dma *);
>  	void			*data;
> +	unsigned short		base_request_line;
> +	unsigned short		end_request_line;
how do you define these two?
>  };
>  
>  /* Used with acpi_dma_simple_xlate() */
> -- 
> 1.8.2.rc0.22.gb3600c3
> 

Re: [PATCH 5/6] ACPI / LPSS: return no error from register_device_clock in special cases

[Rafael J. Wysocki]

On Wednesday, March 27, 2013 10:58:01 AM Andy Shevchenko wrote:
> When device uses the fixed clock and has no private space of LTR, we have to
> create main LPSS clock and register platform device. This is normally a case
> for LPSS DMA controller.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/acpi_lpss.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index b1c9542..4015929 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -95,6 +95,10 @@ static int register_device_clock(struct acpi_device *adev,
>  	if (!lpss_clk_dev)
>  		lpt_register_clock_device();
>  
> +	if (!dev_desc->prv_offset && dev_desc->clk_required)

If clk_required is not set, register_device_clock() won't be called at all for
the device.  Moreover, prv_offset may be zero in principle.

You'd need to add support for shared clocks for that to really work.  I have code
for that, please ping me off-list.

> +		/* There is no error when device uses fixed clock */
> +		return 0;
> +
>  	if (!dev_desc->clk_parent || !pdata->mmio_base
>  	    || pdata->mmio_size < dev_desc->prv_offset + LPSS_CLK_SIZE)
>  		return -ENODATA;

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 6/6] ACPI / LPSS: add Lynxpoint DMA controller to the list

[Rafael J. Wysocki]

On Wednesday, March 27, 2013 10:58:02 AM Andy Shevchenko wrote:
> In Lynxpoint we have to enable clock per each LPSS device. That's why we have
> to enumerate them from drivers/acpi/acpi_lpss.c. The DMA controller is one of
> such devices.

We'll do that after we've sorted out [5/6].

Thanks,
Rafael


> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/acpi_lpss.c  | 7 +++++++
>  drivers/clk/x86/clk-lpt.c | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index 4015929..d80c81e 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -47,6 +47,10 @@ struct lpss_private_data {
>  	const struct lpss_device_desc *dev_desc;
>  };
>  
> +static struct lpss_device_desc lpss_dev_desc = {
> +	.clk_required = true,
> +};
> +
>  static struct lpss_device_desc lpt_dev_desc = {
>  	.clk_required = true,
>  	.clk_parent = "lpss_clk",
> @@ -60,6 +64,9 @@ static struct lpss_device_desc lpt_sdio_dev_desc = {
>  };
>  
>  static const struct acpi_device_id acpi_lpss_device_ids[] = {
> +	/* Generic LPSS devices */
> +	{ "INTL9C60", (unsigned long)&lpss_dev_desc },
> +
>  	/* Lynxpoint LPSS devices */
>  	{ "INT33C0", (unsigned long)&lpt_dev_desc },
>  	{ "INT33C1", (unsigned long)&lpt_dev_desc },
> diff --git a/drivers/clk/x86/clk-lpt.c b/drivers/clk/x86/clk-lpt.c
> index 5cf4f46..719817e 100644
> --- a/drivers/clk/x86/clk-lpt.c
> +++ b/drivers/clk/x86/clk-lpt.c
> @@ -30,7 +30,7 @@ static int lpt_clk_probe(struct platform_device *pdev)
>  		return PTR_ERR(clk);
>  
>  	/* Shared DMA clock */
> -	clk_register_clkdev(clk, "hclk", "INTL9C60.0.auto");
> +	clk_register_clkdev(clk, "hclk", "INTL9C60:00");
>  	return 0;
>  }
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/6] dma: acpi-dma: introduce ACPI DMA helpers

[Mika Westerberg]

On Sat, Mar 30, 2013 at 02:05:43AM +0530, Vinod Koul wrote:
> On Wed, Mar 27, 2013 at 10:57:57AM +0200, Andy Shevchenko wrote:
> > + * @dev:	struct device to get DMA request from
> > + * @index:	index of FixedDMA descriptor for @dev
> > + *
> > + * Returns pointer to appropriate dma channel on success or NULL on error.
> > + */
> > +struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
> > +		size_t index)
> > +{
> So i think this will be called by client driver to request a channel right?
> 
> If so how does client find the device pointer for dma controller.

It doesn't have to. the client passes pointer to it's own device to this
function.

> And index is a global one, right? What is it use ?

It is the index in the client device's ACPI DMA resources. It is explained
in the documentation but it refers to something like:

	Device (SPI0) 
	{
		Method (_CRS, 0, NotSerialized)
		{
			Name (RBUF, ResourceTemplate ()
			{
				// Bunch of resources...
				FixedDMA(...)	// This will be index 0
				FixedDMA(...)	// and this is 1
				// More resources..
			}

			Return (RBUF)
		}
	}

So we use the index to refer the correct DMA resource in the resource list.

> > +	struct acpi_dma_parser_data pdata;
> > +	struct acpi_dma_spec *dma_spec = &pdata.dma_spec;
> > +	struct list_head resource_list;
> > +	struct acpi_device *adev;
> > +	struct acpi_dma *adma;
> > +	struct dma_chan *chan;
> > +
> > +	/* Check if the device was enumerated by ACPI */
> > +	if (!dev || !ACPI_HANDLE(dev))
> > +		return NULL;
> > +
> > +	if (acpi_bus_get_device(ACPI_HANDLE(dev), &adev))
> > +		return NULL;
> > +
> > +	memset(&pdata, 0, sizeof(pdata));
> > +	pdata.index = index;
> > +
> > +	/* Initial values for the request line and channel */
> > +	dma_spec->chan_id = -1;
> > +	dma_spec->slave_id = -1;
> > +
> > +	INIT_LIST_HEAD(&resource_list);
> > +	acpi_dev_get_resources(adev, &resource_list,
> > +			acpi_dma_parse_fixed_dma, &pdata);
> > +	acpi_dev_free_resource_list(&resource_list);
> > +
> > +	if (dma_spec->slave_id < 0 || dma_spec->chan_id < 0)
> > +		return NULL;
> > +
> > +	mutex_lock(&acpi_dma_lock);
> > +
> > +	list_for_each_entry(adma, &acpi_dma_list, dma_controllers) {
> > +		dma_spec->dev = adma->dev;
> > +		chan = adma->acpi_dma_xlate(dma_spec, adma);
> > +		if (chan) {
> > +			mutex_unlock(&acpi_dma_lock);
> > +			return chan;
> > +		}
> > +	}
> > +
> > +	mutex_unlock(&acpi_dma_lock);
> > +	return NULL;
> in this and error handling you are not doing anything different so why code
> duplication?

There is no specific reason for that. We can change it to goto + return
chan in the next revision.

> 
> > +}
> > +EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
> > +
> > +/**
> > + * acpi_dma_request_slave_chan_by_name - Get the DMA slave channel
> > + * @dev:	struct device to get DMA request from
> > + * @name:	represents corresponding FixedDMA descriptor for @dev
> > + *
> > + * In order to support both Device Tree and ACPI in a single driver we
> > + * translate the names "tx" and "rx" here based on the most common case where
> > + * the first FixedDMA descriptor is TX and second is RX.
> > + *
> > + * Returns pointer to appropriate dma channel on success or NULL on error.
> > + */
> > +struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
> > +		const char *name)
> > +{
> > +	size_t index;
> > +
> > +	if (!strcmp(name, "tx"))
> > +		index = 0;
> > +	else if (!strcmp(name, "rx"))
> > +		index = 1;
> > +	else
> > +		return NULL;
> > +
> > +	return acpi_dma_request_slave_chan_by_index(dev, index);
> are you going to a have a different descriptor for tx and rx? Is index only for
> tx = 0 and rx =1 always. How would a controller be represented which has 8
> channels, each channel can do DMA in either direction

This is a special case so that we can cope with
dma_request_slave_channel().

But this has nothing to do with the controller. This is a client API and
the FixedDMA descriptors are only associated with clients.

For example on Lynxpoint we have a DMA controller you describe above and we
can use dma_request_slave_channel() with that for each client device
(HSUART, SPI and I2C) without problems.

> > +}
> > +EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);
> > +
> > +/**
> > + * acpi_dma_simple_xlate - Simple ACPI DMA engine translation helper
> > + * @dma_spec: pointer to ACPI DMA specifier
> > + * @adma: pointer to ACPI DMA controller data
> > + *
> > + * A simple translation function for ACPI based devices. Passes &struct
> > + * dma_spec to the DMA controller driver provided filter function. Returns
> > + * pointer to the channel if found or %NULL otherwise.
> > + */
> > +struct dma_chan *acpi_dma_simple_xlate(struct acpi_dma_spec *dma_spec,
> > +		struct acpi_dma *adma)
> > +{
> > +	struct acpi_dma_filter_info *info = adma->data;
> what is the purpose of filter here?

It is pretty much the same as with DeviceTree version. There is an example
of that in the dw_dmac ACPI support patch include in this series.

Basically the controller driver passes the filter (well the
acpi_dma_filter_info) structure to acpi_dma_controller_register(). It can
then do the requred setup for the channel once it is found.

> > +
> > +	if (!info || !info->filter_fn)
> > +		return NULL;
> > +
> > +	return dma_request_channel(info->dma_cap, info->filter_fn, dma_spec);
> > +}
> > +EXPORT_SYMBOL_GPL(acpi_dma_simple_xlate);
> > diff --git a/include/linux/acpi_dma.h b/include/linux/acpi_dma.h
> > new file mode 100644
> > index 0000000..d09deab
> > --- /dev/null
> > +++ b/include/linux/acpi_dma.h
> > @@ -0,0 +1,116 @@
> > +/*
> > + * ACPI helpers for DMA request / controller
> > + *
> > + * Based on of_dma.h
> > + *
> > + * Copyright (C) 2013, Intel Corporation
> > + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef __LINUX_ACPI_DMA_H
> > +#define __LINUX_ACPI_DMA_H
> > +
> > +#include <linux/list.h>
> > +#include <linux/device.h>
> > +#include <linux/dmaengine.h>
> > +
> > +/**
> > + * struct acpi_dma_spec - slave device DMA resources
> > + * @chan_id:	channel unique id
> > + * @slave_id:	request line unique id
> > + * @dev:	struct device of the DMA controller to be used in the filter
> > + *		function
> > + */
> > +struct acpi_dma_spec {
> > +	int		chan_id;
> > +	int		slave_id;
> > +	struct device	*dev;
> > +};
> is this the representation of Fixed DMA, if so why have you omitted transfer widths?
> I can see obvious benefits of using that 

It is, yes. But we haven't been using the widths with our hardware. BIOS
seems to set it always 32-bit even though we should access the peripheral
FIFO using 8-bit transfers for example.

The client never sees these when it uses dma_request_slave_channel() API.

Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources

[Mika Westerberg]

On Sat, Mar 30, 2013 at 03:03:50AM +0530, Vinod Koul wrote:
> On Wed, Mar 27, 2013 at 10:57:59AM +0200, Andy Shevchenko wrote:
> > Since we have CSRT only to get additional DMA controller resources, let's get
> > rid of drivers/acpi/csrt.c and move its logic inside ACPI DMA helpers code.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> for such a patch git format-patch -M is your friend. It generates patch to show
> file movement. It helps review greatly if you just move the file and then do
> additions on second patch, as diffstat tells me some changes have been done.

OK, thanks for the tip. We will use -M with the next revision.

> > @@ -23,6 +25,117 @@ static LIST_HEAD(acpi_dma_list);
> >  static DEFINE_MUTEX(acpi_dma_lock);
> >  
> >  /**
> > + * acpi_dma_parse_resource_group - match device and parse resource group
> > + * @grp:	CSRT resource group
> > + * @adev:	ACPI device to match with
> > + * @adma:	struct acpi_dma of the given DMA controller
> > + *
> > + * Returns 1 on success, 0 when no information is available, or appropriate
> > + * errno value on error.
> > + *
> > + * In order to match a device from DSDT table to the corresponding CSRT device
> > + * we use MMIO address and IRQ.
> > + */
> >  
> > +/**
> > + * acpi_dma_update_dma_spec - prepare dma specifier to pass to translation function
> > + * @adma:	struct acpi_dma of DMA controller
> > + * @dma_spec:	dma specifier to update
> > + *
> > + * Returns 0, if no information is avaiable, -1 on mismatch, and 1 otherwise.
> > + *
> > + * Accordingly to ACPI 5.0 Specification Table 6-170 "Fixed DMA Resource
> > + * Descriptor":
> > + *	DMA Request Line bits is a platform-relative number uniquely
> > + *	identifying the request line assigned. Request line-to-Controller
> > + *	mapping is done in a controller-specific OS driver.
> > + * That's why we can safely adjust slave_id when the appropriate controller is
> > + * found.
> > + */
> > +static int acpi_dma_update_dma_spec(struct acpi_dma *adma,
> > +		struct acpi_dma_spec *dma_spec)
> > +{
> > +	/* Set link to the DMA controller device */
> > +	dma_spec->dev = adma->dev;
> > +
> > +	/* Check if the request line range is available */
> > +	if (adma->base_request_line == 0 && adma->end_request_line == 0)
> > +		return 0;
> > +
> > +	/* Check if slave_id falls to the range */
> > +	if (dma_spec->slave_id < adma->base_request_line ||
> > +	    dma_spec->slave_id > adma->end_request_line)
> > +		return -1;
> > +
> > +	/*
> > +	 * Here we adjust slave_id. It should be a relative number to the base
> > +	 * request line.
> > +	 */
> > +	dma_spec->slave_id -= adma->base_request_line;
> where are you getting the base_request_line, i didnt see anything for this in
> ACPI spec?

It comes from the CSRT table. In this same patch there is a function
acpi_dma_parse_resource_group() that extracts it.

If you check the kerneldoc of this function (acpi_dma_update_dma_spec()) it
says this:

Accordingly to ACPI 5.0 Specification Table 6-170 "Fixed DMA Resource
Descriptor":
	DMA Request Line bits is a platform-relative number uniquely
	identifying the request line assigned. Request line-to-Controller
	mapping is done in a controller-specific OS driver.

So this patch series is actually the "controller-specific OS driver". And
we use CSRT table to extract the request line range for a given controller.

> > +	return 1;
> > +}
> > +
> >  struct acpi_dma_parser_data {
> >  	struct acpi_dma_spec dma_spec;
> >  	size_t index;
> > @@ -193,6 +347,7 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
> >  	struct acpi_device *adev;
> >  	struct acpi_dma *adma;
> >  	struct dma_chan *chan;
> > +	int found;
> >  
> >  	/* Check if the device was enumerated by ACPI */
> >  	if (!dev || !ACPI_HANDLE(dev))
> > @@ -219,9 +374,20 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
> >  	mutex_lock(&acpi_dma_lock);
> >  
> >  	list_for_each_entry(adma, &acpi_dma_list, dma_controllers) {
> > -		dma_spec->dev = adma->dev;
> > +		/*
> > +		 * We are not going to call translation function if slave_id
> > +		 * doesn't fall to the request range.
> > +		 */
> > +		found = acpi_dma_update_dma_spec(adma, dma_spec);
> > +		if (found < 0)
> > +			continue;
> >  		chan = adma->acpi_dma_xlate(dma_spec, adma);
> > -		if (chan) {
> > +		/*
> > +		 * Try to get a channel only from the DMA controller that
> > +		 * matches the slave_id. See acpi_dma_update_dma_spec()
> > +		 * description for the details.
> > +		 */
> > +		if (found > 0 || chan) {
> >  			mutex_unlock(&acpi_dma_lock);
> >  			return chan;
> >  		}
> > diff --git a/include/linux/acpi_dma.h b/include/linux/acpi_dma.h
> > index d09deab..fb02980 100644
> > --- a/include/linux/acpi_dma.h
> > +++ b/include/linux/acpi_dma.h
> > @@ -37,6 +37,8 @@ struct acpi_dma_spec {
> >   * @dev:		struct device of this controller
> >   * @acpi_dma_xlate:	callback function to find a suitable channel
> >   * @data:		private data used by a callback function
> > + * @base_request_line:	first supported request line (CSRT)
> > + * @end_request_line:	last supported request line (CSRT)
> okay here it is, can you add the width here as well

You mean the width from FixedDMA descriptor?

If yes, then I think it does not belong here. This stuff is for controllers
and FixedDMA belongs to the clients. It can be differrent for each FixedDMA
descriptor for each client (although we have only seen 32-bit widhts
currently).

> >   */
> >  struct acpi_dma {
> >  	struct list_head	dma_controllers;
> > @@ -44,6 +46,8 @@ struct acpi_dma {
> >  	struct dma_chan		*(*acpi_dma_xlate)
> >  				(struct acpi_dma_spec *, struct acpi_dma *);
> >  	void			*data;
> > +	unsigned short		base_request_line;
> > +	unsigned short		end_request_line;
> how do you define these two?

It is the range of request lines assigned for each controller, if that is
what you asked. In ACPI 5 it is the request line in FixedDMA descriptor
that is used to locate the right controller. The request line should be
unique accross the platform.

> >  };
> >  
> >  /* Used with acpi_dma_simple_xlate() */
> > -- 
> > 1.8.2.rc0.22.gb3600c3
> > 

Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources

[Andy Shevchenko]

On Sat, 2013-03-30 at 03:03 +0530, Vinod Koul wrote: 
> On Wed, Mar 27, 2013 at 10:57:59AM +0200, Andy Shevchenko wrote:
> > Since we have CSRT only to get additional DMA controller resources, let's get
> > rid of drivers/acpi/csrt.c and move its logic inside ACPI DMA helpers code.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> for such a patch git format-patch -M is your friend. It generates patch to show
> file movement. It helps review greatly if you just move the file and then do
> additions on second patch, as diffstat tells me some changes have been done.

It obviously will not help. We are not creating new file, but moving
(quite partially) contents from one file to the _existing_ one.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy