Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources
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 > > Signed-off-by: Mika Westerberg > > Acked-by: Rafael J. Wysocki > 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 Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources
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.shevche...@linux.intel.com Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com Acked-by: Rafael J. Wysocki rafael.j.wyso...@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.shevche...@linux.intel.com Intel Finland Oy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources
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
> > Signed-off-by: Mika Westerberg
> > Acked-by: Rafael J. Wysocki
> 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(_dma_lock);
> >
> > list_for_each_entry(adma, _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(_dma_lock);
> > return chan;
> > }
> >
Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources
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.shevche...@linux.intel.com
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Acked-by: Rafael J. Wysocki rafael.j.wyso...@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
+++
Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources
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
> Signed-off-by: Mika Westerberg
> Acked-by: Rafael J. Wysocki
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(_dma_lock);
>
> list_for_each_entry(adma, _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(_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_headdma_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
Re: [PATCH 3/6] dma: acpi-dma: parse CSRT to extract additional resources
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.shevche...@linux.intel.com
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Acked-by: Rafael J. Wysocki rafael.j.wyso...@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_headdma_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;
+
