* [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID
@ 2013-05-15 13:27 Laurent Pinchart
2013-05-15 13:39 ` Lars-Peter Clausen
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2013-05-15 13:27 UTC (permalink / raw)
To: linux-kernel
Cc: Vinod Koul, Dan Williams, Lars-Peter Clausen, Russell King,
Peter Ujfalusi
When translating a DT DMA channel specifier, the most common use case is
to match the DMA channel based on the channel DMA engine and channel ID.
Modify the of_dma_simple_xlate() function to do so, simplifying the API
for DMA engine drivers.
There is no need to check the DMA cells count in the filter function as
the check is already performed by the caller in of_dma_get_controller().
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/dma/of-dma.c | 39 +++++++++++++++++++++++++++++++--------
drivers/dma/omap-dma.c | 8 +-------
include/linux/of_dma.h | 5 -----
3 files changed, 32 insertions(+), 20 deletions(-)
I've implemented this function while working on a DMA engine driver, and
realized that it wasn't specific to my driver at all. As I'm new to the DMA
engine API, I'd appreciate feedback on whether this change makes sense, or if
I should instead keep the existing of_dma_simple_xlate() function and create a
separate simple xlate helper.
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 7aa0864..702a45d 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -202,6 +202,27 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
return NULL;
}
+struct of_dma_filter_args {
+ struct dma_device *device;
+ unsigned int chan_id;
+};
+
+/**
+ * of_dma_simple_xlate_filter - Channel filter function for of_dma_simple_xlate
+ * @dma_chan: pointer to candidate DMA channel
+ * @param: pointer to filter parameter
+ *
+ * Filter out candidate DMA channels that don't match the channel ID or DMA
+ * engine device passed as arguments.
+ */
+static bool of_dma_simple_xlate_filter(struct dma_chan *chan, void *param)
+{
+ struct of_dma_filter_args *fargs = param;
+
+ return (chan->device == fargs->device) &&
+ (chan->chan_id == fargs->chan_id);
+}
+
/**
* of_dma_simple_xlate - Simple DMA engine translation function
* @dma_spec: pointer to DMA specifier as found in the device tree
@@ -216,16 +237,18 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma)
{
- int count = dma_spec->args_count;
- struct of_dma_filter_info *info = ofdma->of_dma_data;
+ struct dma_device *dev = ofdma->of_dma_data;
+ struct of_dma_filter_args fargs;
+ dma_cap_mask_t cap;
- if (!info || !info->filter_fn)
- return NULL;
+ /* There's no need to specify matching caps as we're going to match a
+ * specific DMA channel anyway.
+ */
+ dma_cap_zero(cap);
- if (count != 1)
- return NULL;
+ fargs.device = dev;
+ fargs.chan_id = dma_spec->args[0];
- return dma_request_channel(info->dma_cap, info->filter_fn,
- &dma_spec->args[0]);
+ return dma_request_channel(cap, of_dma_simple_xlate_filter, &fargs);
}
EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index ec3fc4f..7e7ff06 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -69,10 +69,6 @@ static const unsigned es_bytes[] = {
[OMAP_DMA_DATA_TYPE_S32] = 4,
};
-static struct of_dma_filter_info omap_dma_info = {
- .filter_fn = omap_dma_filter_fn,
-};
-
static inline struct omap_dmadev *to_omap_dma_dev(struct dma_device *d)
{
return container_of(d, struct omap_dmadev, ddev);
@@ -641,11 +637,9 @@ static int omap_dma_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, od);
if (pdev->dev.of_node) {
- omap_dma_info.dma_cap = od->ddev.cap_mask;
-
/* Device-tree DMA controller registration */
rc = of_dma_controller_register(pdev->dev.of_node,
- of_dma_simple_xlate, &omap_dma_info);
+ of_dma_simple_xlate, &od->ddev);
if (rc) {
pr_warn("OMAP-DMA: failed to register DMA controller\n");
dma_async_device_unregister(&od->ddev);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index 364dda7..3c0d9ac 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -27,11 +27,6 @@ struct of_dma {
void *of_dma_data;
};
-struct of_dma_filter_info {
- dma_cap_mask_t dma_cap;
- dma_filter_fn filter_fn;
-};
-
#ifdef CONFIG_OF
extern int of_dma_controller_register(struct device_node *np,
struct dma_chan *(*of_dma_xlate)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID
2013-05-15 13:27 [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID Laurent Pinchart
@ 2013-05-15 13:39 ` Lars-Peter Clausen
2013-05-15 13:55 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-05-15 13:39 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-kernel, Vinod Koul, Dan Williams, Russell King,
Peter Ujfalusi
On 05/15/2013 03:27 PM, Laurent Pinchart wrote:
> When translating a DT DMA channel specifier, the most common use case is
> to match the DMA channel based on the channel DMA engine and channel ID.
> Modify the of_dma_simple_xlate() function to do so, simplifying the API
> for DMA engine drivers.
>
> There is no need to check the DMA cells count in the filter function as
> the check is already performed by the caller in of_dma_get_controller().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Hi,
I've submitted a very similar patch some time ago, see
https://lkml.org/lkml/2013/3/25/250
- Lars
> ---
> drivers/dma/of-dma.c | 39 +++++++++++++++++++++++++++++++--------
> drivers/dma/omap-dma.c | 8 +-------
> include/linux/of_dma.h | 5 -----
> 3 files changed, 32 insertions(+), 20 deletions(-)
>
> I've implemented this function while working on a DMA engine driver, and
> realized that it wasn't specific to my driver at all. As I'm new to the DMA
> engine API, I'd appreciate feedback on whether this change makes sense, or if
> I should instead keep the existing of_dma_simple_xlate() function and create a
> separate simple xlate helper.
>
> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> index 7aa0864..702a45d 100644
> --- a/drivers/dma/of-dma.c
> +++ b/drivers/dma/of-dma.c
> @@ -202,6 +202,27 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> return NULL;
> }
>
> +struct of_dma_filter_args {
> + struct dma_device *device;
> + unsigned int chan_id;
> +};
> +
> +/**
> + * of_dma_simple_xlate_filter - Channel filter function for of_dma_simple_xlate
> + * @dma_chan: pointer to candidate DMA channel
> + * @param: pointer to filter parameter
> + *
> + * Filter out candidate DMA channels that don't match the channel ID or DMA
> + * engine device passed as arguments.
> + */
> +static bool of_dma_simple_xlate_filter(struct dma_chan *chan, void *param)
> +{
> + struct of_dma_filter_args *fargs = param;
> +
> + return (chan->device == fargs->device) &&
> + (chan->chan_id == fargs->chan_id);
> +}
> +
> /**
> * of_dma_simple_xlate - Simple DMA engine translation function
> * @dma_spec: pointer to DMA specifier as found in the device tree
> @@ -216,16 +237,18 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
> struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
> struct of_dma *ofdma)
> {
> - int count = dma_spec->args_count;
> - struct of_dma_filter_info *info = ofdma->of_dma_data;
> + struct dma_device *dev = ofdma->of_dma_data;
> + struct of_dma_filter_args fargs;
> + dma_cap_mask_t cap;
>
> - if (!info || !info->filter_fn)
> - return NULL;
> + /* There's no need to specify matching caps as we're going to match a
> + * specific DMA channel anyway.
> + */
> + dma_cap_zero(cap);
>
> - if (count != 1)
> - return NULL;
> + fargs.device = dev;
> + fargs.chan_id = dma_spec->args[0];
>
> - return dma_request_channel(info->dma_cap, info->filter_fn,
> - &dma_spec->args[0]);
> + return dma_request_channel(cap, of_dma_simple_xlate_filter, &fargs);
> }
> EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index ec3fc4f..7e7ff06 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -69,10 +69,6 @@ static const unsigned es_bytes[] = {
> [OMAP_DMA_DATA_TYPE_S32] = 4,
> };
>
> -static struct of_dma_filter_info omap_dma_info = {
> - .filter_fn = omap_dma_filter_fn,
> -};
> -
> static inline struct omap_dmadev *to_omap_dma_dev(struct dma_device *d)
> {
> return container_of(d, struct omap_dmadev, ddev);
> @@ -641,11 +637,9 @@ static int omap_dma_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, od);
>
> if (pdev->dev.of_node) {
> - omap_dma_info.dma_cap = od->ddev.cap_mask;
> -
> /* Device-tree DMA controller registration */
> rc = of_dma_controller_register(pdev->dev.of_node,
> - of_dma_simple_xlate, &omap_dma_info);
> + of_dma_simple_xlate, &od->ddev);
> if (rc) {
> pr_warn("OMAP-DMA: failed to register DMA controller\n");
> dma_async_device_unregister(&od->ddev);
> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> index 364dda7..3c0d9ac 100644
> --- a/include/linux/of_dma.h
> +++ b/include/linux/of_dma.h
> @@ -27,11 +27,6 @@ struct of_dma {
> void *of_dma_data;
> };
>
> -struct of_dma_filter_info {
> - dma_cap_mask_t dma_cap;
> - dma_filter_fn filter_fn;
> -};
> -
> #ifdef CONFIG_OF
> extern int of_dma_controller_register(struct device_node *np,
> struct dma_chan *(*of_dma_xlate)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID
2013-05-15 13:39 ` Lars-Peter Clausen
@ 2013-05-15 13:55 ` Laurent Pinchart
2013-05-15 14:52 ` Lars-Peter Clausen
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2013-05-15 13:55 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: linux-kernel, Vinod Koul, Dan Williams, Russell King,
Peter Ujfalusi, linux-arm-kernel
Hi Lars-Peter,
(CC'ing LAKML, I had forgotten to do so when I sent the patch)
On Wednesday 15 May 2013 15:39:09 Lars-Peter Clausen wrote:
> On 05/15/2013 03:27 PM, Laurent Pinchart wrote:
> > When translating a DT DMA channel specifier, the most common use case is
> > to match the DMA channel based on the channel DMA engine and channel ID.
> > Modify the of_dma_simple_xlate() function to do so, simplifying the API
> > for DMA engine drivers.
> >
> > There is no need to check the DMA cells count in the filter function as
> > the check is already performed by the caller in of_dma_get_controller().
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Hi,
>
> I've submitted a very similar patch some time ago, see
> https://lkml.org/lkml/2013/3/25/250
Thanks. So my patch makes at least some sense :-)
I had the impression that this is what omap-dma needs as well, hence the
modification to the existing xlate function. I'm fine with a separate function
as well if the current code covers different use cases.
> > ---
> >
> > drivers/dma/of-dma.c | 39 +++++++++++++++++++++++++++++++--------
> > drivers/dma/omap-dma.c | 8 +-------
> > include/linux/of_dma.h | 5 -----
> > 3 files changed, 32 insertions(+), 20 deletions(-)
> >
> > I've implemented this function while working on a DMA engine driver, and
> > realized that it wasn't specific to my driver at all. As I'm new to the
> > DMA engine API, I'd appreciate feedback on whether this change makes
> > sense, or if I should instead keep the existing of_dma_simple_xlate()
> > function and create a separate simple xlate helper.
> >
> > diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> > index 7aa0864..702a45d 100644
> > --- a/drivers/dma/of-dma.c
> > +++ b/drivers/dma/of-dma.c
> > @@ -202,6 +202,27 @@ struct dma_chan *of_dma_request_slave_channel(struct
> > device_node *np,
> > return NULL;
> > }
> >
> > +struct of_dma_filter_args {
> > + struct dma_device *device;
> > + unsigned int chan_id;
> > +};
> > +
> > +/**
> > + * of_dma_simple_xlate_filter - Channel filter function for
> > of_dma_simple_xlate + * @dma_chan: pointer to candidate DMA channel
> > + * @param: pointer to filter parameter
> > + *
> > + * Filter out candidate DMA channels that don't match the channel ID or
> > + * DMA engine device passed as arguments.
> > + */
> > +static bool of_dma_simple_xlate_filter(struct dma_chan *chan, void
> > *param)
> > +{
> > + struct of_dma_filter_args *fargs = param;
> > +
> > + return (chan->device == fargs->device) &&
> > + (chan->chan_id == fargs->chan_id);
> > +}
> > +
> > /**
> > * of_dma_simple_xlate - Simple DMA engine translation function
> > * @dma_spec: pointer to DMA specifier as found in the device tree
> > @@ -216,16 +237,18 @@ struct dma_chan *of_dma_request_slave_channel(struct
> > device_node *np,>
> > struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
> > struct of_dma *ofdma)
> > {
> > - int count = dma_spec->args_count;
> > - struct of_dma_filter_info *info = ofdma->of_dma_data;
> > + struct dma_device *dev = ofdma->of_dma_data;
> > + struct of_dma_filter_args fargs;
> > + dma_cap_mask_t cap;
> >
> > - if (!info || !info->filter_fn)
> > - return NULL;
> > + /* There's no need to specify matching caps as we're going to match a
> > + * specific DMA channel anyway.
> > + */
> > + dma_cap_zero(cap);
> >
> > - if (count != 1)
> > - return NULL;
> > + fargs.device = dev;
> > + fargs.chan_id = dma_spec->args[0];
> >
> > - return dma_request_channel(info->dma_cap, info->filter_fn,
> > - &dma_spec->args[0]);
> > + return dma_request_channel(cap, of_dma_simple_xlate_filter, &fargs);
> > }
> > EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
> > diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> > index ec3fc4f..7e7ff06 100644
> > --- a/drivers/dma/omap-dma.c
> > +++ b/drivers/dma/omap-dma.c
> > @@ -69,10 +69,6 @@ static const unsigned es_bytes[] = {
> > [OMAP_DMA_DATA_TYPE_S32] = 4,
> > };
> >
> > -static struct of_dma_filter_info omap_dma_info = {
> > - .filter_fn = omap_dma_filter_fn,
> > -};
> > -
> > static inline struct omap_dmadev *to_omap_dma_dev(struct dma_device *d)
> > {
> > return container_of(d, struct omap_dmadev, ddev);
> > @@ -641,11 +637,9 @@ static int omap_dma_probe(struct platform_device
> > *pdev)>
> > platform_set_drvdata(pdev, od);
> > if (pdev->dev.of_node) {
> > - omap_dma_info.dma_cap = od->ddev.cap_mask;
> > -
> > /* Device-tree DMA controller registration */
> > rc = of_dma_controller_register(pdev->dev.of_node,
> > - of_dma_simple_xlate, &omap_dma_info);
> > + of_dma_simple_xlate, &od->ddev);
> > if (rc) {
> > pr_warn("OMAP-DMA: failed to register DMA controller\n");
> > dma_async_device_unregister(&od->ddev);
> > diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> > index 364dda7..3c0d9ac 100644
> > --- a/include/linux/of_dma.h
> > +++ b/include/linux/of_dma.h
> > @@ -27,11 +27,6 @@ struct of_dma {
> > void *of_dma_data;
> > };
> >
> > -struct of_dma_filter_info {
> > - dma_cap_mask_t dma_cap;
> > - dma_filter_fn filter_fn;
> > -};
> > -
> >
> > #ifdef CONFIG_OF
> > extern int of_dma_controller_register(struct device_node *np,
> > struct dma_chan *(*of_dma_xlate)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID
2013-05-15 13:55 ` Laurent Pinchart
@ 2013-05-15 14:52 ` Lars-Peter Clausen
2013-05-16 11:30 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-05-15 14:52 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-kernel, Vinod Koul, Dan Williams, Russell King,
Peter Ujfalusi, linux-arm-kernel
On 05/15/2013 03:55 PM, Laurent Pinchart wrote:
> Hi Lars-Peter,
>
> (CC'ing LAKML, I had forgotten to do so when I sent the patch)
>
> On Wednesday 15 May 2013 15:39:09 Lars-Peter Clausen wrote:
>> On 05/15/2013 03:27 PM, Laurent Pinchart wrote:
>>> When translating a DT DMA channel specifier, the most common use case is
>>> to match the DMA channel based on the channel DMA engine and channel ID.
>>> Modify the of_dma_simple_xlate() function to do so, simplifying the API
>>> for DMA engine drivers.
>>>
>>> There is no need to check the DMA cells count in the filter function as
>>> the check is already performed by the caller in of_dma_get_controller().
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Hi,
>>
>> I've submitted a very similar patch some time ago, see
>> https://lkml.org/lkml/2013/3/25/250
>
> Thanks. So my patch makes at least some sense :-)
>
> I had the impression that this is what omap-dma needs as well, hence the
> modification to the existing xlate function. I'm fine with a separate function
> as well if the current code covers different use cases.
My first attempt was to modify simple_xlate function, but I think it didn't
work for all users (some of which aren't applied yet), so I added a separate
function.
>
>>> ---
>>>
>>> drivers/dma/of-dma.c | 39 +++++++++++++++++++++++++++++++--------
>>> drivers/dma/omap-dma.c | 8 +-------
>>> include/linux/of_dma.h | 5 -----
>>> 3 files changed, 32 insertions(+), 20 deletions(-)
>>>
>>> I've implemented this function while working on a DMA engine driver, and
>>> realized that it wasn't specific to my driver at all. As I'm new to the
>>> DMA engine API, I'd appreciate feedback on whether this change makes
>>> sense, or if I should instead keep the existing of_dma_simple_xlate()
>>> function and create a separate simple xlate helper.
>>>
>>> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
>>> index 7aa0864..702a45d 100644
>>> --- a/drivers/dma/of-dma.c
>>> +++ b/drivers/dma/of-dma.c
>>> @@ -202,6 +202,27 @@ struct dma_chan *of_dma_request_slave_channel(struct
>>> device_node *np,
>>> return NULL;
>>> }
>>>
>>> +struct of_dma_filter_args {
>>> + struct dma_device *device;
>>> + unsigned int chan_id;
>>> +};
>>> +
>>> +/**
>>> + * of_dma_simple_xlate_filter - Channel filter function for
>>> of_dma_simple_xlate + * @dma_chan: pointer to candidate DMA channel
>>> + * @param: pointer to filter parameter
>>> + *
>>> + * Filter out candidate DMA channels that don't match the channel ID or
>>> + * DMA engine device passed as arguments.
>>> + */
>>> +static bool of_dma_simple_xlate_filter(struct dma_chan *chan, void
>>> *param)
>>> +{
>>> + struct of_dma_filter_args *fargs = param;
>>> +
>>> + return (chan->device == fargs->device) &&
>>> + (chan->chan_id == fargs->chan_id);
>>> +}
>>> +
>>> /**
>>> * of_dma_simple_xlate - Simple DMA engine translation function
>>> * @dma_spec: pointer to DMA specifier as found in the device tree
>>> @@ -216,16 +237,18 @@ struct dma_chan *of_dma_request_slave_channel(struct
>>> device_node *np,>
>>> struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>>> struct of_dma *ofdma)
>>> {
>>> - int count = dma_spec->args_count;
>>> - struct of_dma_filter_info *info = ofdma->of_dma_data;
>>> + struct dma_device *dev = ofdma->of_dma_data;
>>> + struct of_dma_filter_args fargs;
>>> + dma_cap_mask_t cap;
>>>
>>> - if (!info || !info->filter_fn)
>>> - return NULL;
>>> + /* There's no need to specify matching caps as we're going to match a
>>> + * specific DMA channel anyway.
>>> + */
>>> + dma_cap_zero(cap);
>>>
>>> - if (count != 1)
>>> - return NULL;
>>> + fargs.device = dev;
>>> + fargs.chan_id = dma_spec->args[0];
>>>
>>> - return dma_request_channel(info->dma_cap, info->filter_fn,
>>> - &dma_spec->args[0]);
>>> + return dma_request_channel(cap, of_dma_simple_xlate_filter, &fargs);
>>> }
>>> EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
>>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>>> index ec3fc4f..7e7ff06 100644
>>> --- a/drivers/dma/omap-dma.c
>>> +++ b/drivers/dma/omap-dma.c
>>> @@ -69,10 +69,6 @@ static const unsigned es_bytes[] = {
>>> [OMAP_DMA_DATA_TYPE_S32] = 4,
>>> };
>>>
>>> -static struct of_dma_filter_info omap_dma_info = {
>>> - .filter_fn = omap_dma_filter_fn,
>>> -};
>>> -
>>> static inline struct omap_dmadev *to_omap_dma_dev(struct dma_device *d)
>>> {
>>> return container_of(d, struct omap_dmadev, ddev);
>>> @@ -641,11 +637,9 @@ static int omap_dma_probe(struct platform_device
>>> *pdev)>
>>> platform_set_drvdata(pdev, od);
>>> if (pdev->dev.of_node) {
>>> - omap_dma_info.dma_cap = od->ddev.cap_mask;
>>> -
>>> /* Device-tree DMA controller registration */
>>> rc = of_dma_controller_register(pdev->dev.of_node,
>>> - of_dma_simple_xlate, &omap_dma_info);
>>> + of_dma_simple_xlate, &od->ddev);
>>> if (rc) {
>>> pr_warn("OMAP-DMA: failed to register DMA controller\n");
>>> dma_async_device_unregister(&od->ddev);
>>> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
>>> index 364dda7..3c0d9ac 100644
>>> --- a/include/linux/of_dma.h
>>> +++ b/include/linux/of_dma.h
>>> @@ -27,11 +27,6 @@ struct of_dma {
>>> void *of_dma_data;
>>> };
>>>
>>> -struct of_dma_filter_info {
>>> - dma_cap_mask_t dma_cap;
>>> - dma_filter_fn filter_fn;
>>> -};
>>> -
>>>
>>> #ifdef CONFIG_OF
>>> extern int of_dma_controller_register(struct device_node *np,
>>> struct dma_chan *(*of_dma_xlate)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID
2013-05-15 14:52 ` Lars-Peter Clausen
@ 2013-05-16 11:30 ` Laurent Pinchart
2013-05-16 11:38 ` Lars-Peter Clausen
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2013-05-16 11:30 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: linux-kernel, Vinod Koul, Dan Williams, Russell King,
Peter Ujfalusi, linux-arm-kernel
Hi Lars-Peter,
On Wednesday 15 May 2013 16:52:03 Lars-Peter Clausen wrote:
> On 05/15/2013 03:55 PM, Laurent Pinchart wrote:
> > On Wednesday 15 May 2013 15:39:09 Lars-Peter Clausen wrote:
> >> On 05/15/2013 03:27 PM, Laurent Pinchart wrote:
> >>> When translating a DT DMA channel specifier, the most common use case is
> >>> to match the DMA channel based on the channel DMA engine and channel ID.
> >>> Modify the of_dma_simple_xlate() function to do so, simplifying the API
> >>> for DMA engine drivers.
> >>>
> >>> There is no need to check the DMA cells count in the filter function as
> >>> the check is already performed by the caller in of_dma_get_controller().
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> Hi,
> >>
> >> I've submitted a very similar patch some time ago, see
> >> https://lkml.org/lkml/2013/3/25/250
> >
> > Thanks. So my patch makes at least some sense :-)
> >
> > I had the impression that this is what omap-dma needs as well, hence the
> > modification to the existing xlate function. I'm fine with a separate
> > function as well if the current code covers different use cases.
>
> My first attempt was to modify simple_xlate function, but I think it didn't
> work for all users (some of which aren't applied yet), so I added a separate
> function.
OK. Do you plan to push your patch to mainline ? You can add my
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> ---
> >>>
> >>> drivers/dma/of-dma.c | 39 +++++++++++++++++++++++++++++++--------
> >>> drivers/dma/omap-dma.c | 8 +-------
> >>> include/linux/of_dma.h | 5 -----
> >>> 3 files changed, 32 insertions(+), 20 deletions(-)
> >>>
> >>> I've implemented this function while working on a DMA engine driver, and
> >>> realized that it wasn't specific to my driver at all. As I'm new to the
> >>> DMA engine API, I'd appreciate feedback on whether this change makes
> >>> sense, or if I should instead keep the existing of_dma_simple_xlate()
> >>> function and create a separate simple xlate helper.
> >>>
> >>> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> >>> index 7aa0864..702a45d 100644
> >>> --- a/drivers/dma/of-dma.c
> >>> +++ b/drivers/dma/of-dma.c
> >>> @@ -202,6 +202,27 @@ struct dma_chan
> >>> *of_dma_request_slave_channel(struct
> >>> device_node *np,
> >>>
> >>> return NULL;
> >>>
> >>> }
> >>>
> >>> +struct of_dma_filter_args {
> >>> + struct dma_device *device;
> >>> + unsigned int chan_id;
> >>> +};
> >>> +
> >>> +/**
> >>> + * of_dma_simple_xlate_filter - Channel filter function for
> >>> of_dma_simple_xlate + * @dma_chan: pointer to candidate DMA channel
> >>> + * @param: pointer to filter parameter
> >>> + *
> >>> + * Filter out candidate DMA channels that don't match the channel ID or
> >>> + * DMA engine device passed as arguments.
> >>> + */
> >>> +static bool of_dma_simple_xlate_filter(struct dma_chan *chan, void
> >>> *param)
> >>> +{
> >>> + struct of_dma_filter_args *fargs = param;
> >>> +
> >>> + return (chan->device == fargs->device) &&
> >>> + (chan->chan_id == fargs->chan_id);
> >>> +}
> >>> +
> >>>
> >>> /**
> >>>
> >>> * of_dma_simple_xlate - Simple DMA engine translation function
> >>> * @dma_spec: pointer to DMA specifier as found in the device tree
> >>>
> >>> @@ -216,16 +237,18 @@ struct dma_chan
> >>> *of_dma_request_slave_channel(struct
> >>> device_node *np,>
> >>>
> >>> struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
> >>>
> >>> struct of_dma *ofdma)
> >>>
> >>> {
> >>>
> >>> - int count = dma_spec->args_count;
> >>> - struct of_dma_filter_info *info = ofdma->of_dma_data;
> >>> + struct dma_device *dev = ofdma->of_dma_data;
> >>> + struct of_dma_filter_args fargs;
> >>> + dma_cap_mask_t cap;
> >>>
> >>> - if (!info || !info->filter_fn)
> >>> - return NULL;
> >>> + /* There's no need to specify matching caps as we're going to match a
> >>> + * specific DMA channel anyway.
> >>> + */
> >>> + dma_cap_zero(cap);
> >>>
> >>> - if (count != 1)
> >>> - return NULL;
> >>> + fargs.device = dev;
> >>> + fargs.chan_id = dma_spec->args[0];
> >>>
> >>> - return dma_request_channel(info->dma_cap, info->filter_fn,
> >>> - &dma_spec->args[0]);
> >>> + return dma_request_channel(cap, of_dma_simple_xlate_filter, &fargs);
> >>>
> >>> }
> >>> EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
> >>>
> >>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> >>> index ec3fc4f..7e7ff06 100644
> >>> --- a/drivers/dma/omap-dma.c
> >>> +++ b/drivers/dma/omap-dma.c
> >>> @@ -69,10 +69,6 @@ static const unsigned es_bytes[] = {
> >>>
> >>> [OMAP_DMA_DATA_TYPE_S32] = 4,
> >>>
> >>> };
> >>>
> >>> -static struct of_dma_filter_info omap_dma_info = {
> >>> - .filter_fn = omap_dma_filter_fn,
> >>> -};
> >>> -
> >>>
> >>> static inline struct omap_dmadev *to_omap_dma_dev(struct dma_device *d)
> >>> {
> >>>
> >>> return container_of(d, struct omap_dmadev, ddev);
> >>>
> >>> @@ -641,11 +637,9 @@ static int omap_dma_probe(struct platform_device
> >>> *pdev)>
> >>>
> >>> platform_set_drvdata(pdev, od);
> >>> if (pdev->dev.of_node) {
> >>>
> >>> - omap_dma_info.dma_cap = od->ddev.cap_mask;
> >>> -
> >>>
> >>> /* Device-tree DMA controller registration */
> >>> rc = of_dma_controller_register(pdev->dev.of_node,
> >>>
> >>> - of_dma_simple_xlate, &omap_dma_info);
> >>> + of_dma_simple_xlate, &od->ddev);
> >>>
> >>> if (rc) {
> >>>
> >>> pr_warn("OMAP-DMA: failed to register DMA controller\n");
> >>> dma_async_device_unregister(&od->ddev);
> >>>
> >>> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> >>> index 364dda7..3c0d9ac 100644
> >>> --- a/include/linux/of_dma.h
> >>> +++ b/include/linux/of_dma.h
> >>> @@ -27,11 +27,6 @@ struct of_dma {
> >>>
> >>> void *of_dma_data;
> >>>
> >>> };
> >>>
> >>> -struct of_dma_filter_info {
> >>> - dma_cap_mask_t dma_cap;
> >>> - dma_filter_fn filter_fn;
> >>> -};
> >>> -
> >>>
> >>> #ifdef CONFIG_OF
> >>> extern int of_dma_controller_register(struct device_node *np,
> >>>
> >>> struct dma_chan *(*of_dma_xlate)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID
2013-05-16 11:30 ` Laurent Pinchart
@ 2013-05-16 11:38 ` Lars-Peter Clausen
0 siblings, 0 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2013-05-16 11:38 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-kernel, Vinod Koul, Dan Williams, Russell King,
Peter Ujfalusi, linux-arm-kernel
On 05/16/2013 01:30 PM, Laurent Pinchart wrote:
> Hi Lars-Peter,
>
> On Wednesday 15 May 2013 16:52:03 Lars-Peter Clausen wrote:
>> On 05/15/2013 03:55 PM, Laurent Pinchart wrote:
>>> On Wednesday 15 May 2013 15:39:09 Lars-Peter Clausen wrote:
>>>> On 05/15/2013 03:27 PM, Laurent Pinchart wrote:
>>>>> When translating a DT DMA channel specifier, the most common use case is
>>>>> to match the DMA channel based on the channel DMA engine and channel ID.
>>>>> Modify the of_dma_simple_xlate() function to do so, simplifying the API
>>>>> for DMA engine drivers.
>>>>>
>>>>> There is no need to check the DMA cells count in the filter function as
>>>>> the check is already performed by the caller in of_dma_get_controller().
>>>>>
>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>
>>>> Hi,
>>>>
>>>> I've submitted a very similar patch some time ago, see
>>>> https://lkml.org/lkml/2013/3/25/250
>>>
>>> Thanks. So my patch makes at least some sense :-)
>>>
>>> I had the impression that this is what omap-dma needs as well, hence the
>>> modification to the existing xlate function. I'm fine with a separate
>>> function as well if the current code covers different use cases.
>>
>> My first attempt was to modify simple_xlate function, but I think it didn't
>> work for all users (some of which aren't applied yet), so I added a separate
>> function.
>
> OK. Do you plan to push your patch to mainline ? You can add my
>
Yes, I already submitted it for mainline inclusion. Waiting for Vinod to
either apply or comment on it.
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-16 11:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-15 13:27 [RFC/PATCH] dma: of: Make of_dma_simple_xlate match on DMA device and channel ID Laurent Pinchart
2013-05-15 13:39 ` Lars-Peter Clausen
2013-05-15 13:55 ` Laurent Pinchart
2013-05-15 14:52 ` Lars-Peter Clausen
2013-05-16 11:30 ` Laurent Pinchart
2013-05-16 11:38 ` Lars-Peter Clausen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox