[PATCH] dw_dmac: adjust slave_id accordingly to request line base

[PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Andy Shevchenko]

On some hardware configurations we have got the request line with the offset.
The patch introduces convert_slave_id() helper for that cases. The request line
base is got from the platform device resources provided by the IORESOURCE_DMA
type.

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      |   13 +++++++++++++
 drivers/dma/dw_dmac_regs.h |    1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 3155c11..8484ca1 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -995,6 +995,13 @@ 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)
 {
@@ -1009,6 +1016,7 @@ 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;
 }
@@ -1717,6 +1725,11 @@ static int dw_probe(struct platform_device *pdev)
 		memcpy(dw->data_width, pdata->data_width, 4);
 	}
 
+	/* Get the base request line if set */
+	io = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+	if (io)
+		dw->request_line_base = (unsigned int)io->start;
+
 	/* Calculate all channel mask before DMA setup */
 	dw->all_chan_mask = (1 << nr_channels) - 1;
 
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index 88dd8eb..3eeb766 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -248,6 +248,7 @@ 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.7.10.4

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Viresh Kumar]

Next time, please direct these mails to my Linaro id :)

On Mon, Jan 28, 2013 at 4:34 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On some hardware configurations we have got the request line with the offset.
> The patch introduces convert_slave_id() helper for that cases. The request line
> base is got from the platform device resources provided by the IORESOURCE_DMA
> type.

@Vinod: Is IORESOURCE_DMA suitable for this purpose?

> 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      |   13 +++++++++++++
>  drivers/dma/dw_dmac_regs.h |    1 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 3155c11..8484ca1 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -995,6 +995,13 @@ 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)
>  {
> @@ -1009,6 +1016,7 @@ 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;
>  }
> @@ -1717,6 +1725,11 @@ static int dw_probe(struct platform_device *pdev)
>                 memcpy(dw->data_width, pdata->data_width, 4);
>         }
>
> +       /* Get the base request line if set */
> +       io = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> +       if (io)
> +               dw->request_line_base = (unsigned int)io->start;
> +

How will it work in case of DT?

>         /* Calculate all channel mask before DMA setup */
>         dw->all_chan_mask = (1 << nr_channels) - 1;
>
> diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
> index 88dd8eb..3eeb766 100644
> --- a/drivers/dma/dw_dmac_regs.h
> +++ b/drivers/dma/dw_dmac_regs.h
> @@ -248,6 +248,7 @@ 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];
>  };

--
viresh

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Mika Westerberg]

On Tue, Jan 29, 2013 at 10:29:43AM +0530, Viresh Kumar wrote:
> Next time, please direct these mails to my Linaro id :)
> 
> On Mon, Jan 28, 2013 at 4:34 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On some hardware configurations we have got the request line with the offset.
> > The patch introduces convert_slave_id() helper for that cases. The request line
> > base is got from the platform device resources provided by the IORESOURCE_DMA
> > type.
> 
> @Vinod: Is IORESOURCE_DMA suitable for this purpose?

We had a discusssion about this with Andy as well. The thing is that there
is no way in current resource to pass DMA request line numbers supported by
the controller to the driver in a generic way. We on the other hand have to
deal this somehow as we have a shared DMA controller on Lynxpoint where the
offset will start from 16 (but it might be something else as well).

Is there something which limits the usage of IORESOUCE_DMA to be only
usable for ISA DMA channels?

We have made supporting code that currently sits in linux-pm/linux-next
tree that parses an ACPI CSRT table and creates the platform devices with
suitable resources here: 13176bbf183c8 (ACPI: add support for CSRT 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      |   13 +++++++++++++
> >  drivers/dma/dw_dmac_regs.h |    1 +
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> > index 3155c11..8484ca1 100644
> > --- a/drivers/dma/dw_dmac.c
> > +++ b/drivers/dma/dw_dmac.c
> > @@ -995,6 +995,13 @@ 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)
> >  {
> > @@ -1009,6 +1016,7 @@ 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;
> >  }
> > @@ -1717,6 +1725,11 @@ static int dw_probe(struct platform_device *pdev)
> >                 memcpy(dw->data_width, pdata->data_width, 4);
> >         }
> >
> > +       /* Get the base request line if set */
> > +       io = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> > +       if (io)
> > +               dw->request_line_base = (unsigned int)io->start;
> > +
> 
> How will it work in case of DT?

Can't the DT version do the same thing and pass IORESOURCE_DMA to the
driver? Or we can check dev->of_node and parse it directly from DT instead
of resource.

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Viresh Kumar]

On Tue, Jan 29, 2013 at 10:52 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> We had a discusssion about this with Andy as well. The thing is that there
> is no way in current resource to pass DMA request line numbers supported by
> the controller to the driver in a generic way. We on the other hand have to
> deal this somehow as we have a shared DMA controller on Lynxpoint where the
> offset will start from 16 (but it might be something else as well).
>
> Is there something which limits the usage of IORESOUCE_DMA to be only
> usable for ISA DMA channels?

Andy,

Why do we need this request_line_base? Why not program the actual value
directly into the slave structure (base + offset)?

In case it is required, add it in platform data field or can add DT
binding for it.

--
viresh

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Andy Shevchenko]

On Tue, 2013-01-29 at 20:57 +0530, Viresh Kumar wrote: 
> On Tue, Jan 29, 2013 at 10:52 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > We had a discusssion about this with Andy as well. The thing is that there
> > is no way in current resource to pass DMA request line numbers supported by
> > the controller to the driver in a generic way. We on the other hand have to
> > deal this somehow as we have a shared DMA controller on Lynxpoint where the
> > offset will start from 16 (but it might be something else as well).
> >
> > Is there something which limits the usage of IORESOUCE_DMA to be only
> > usable for ISA DMA channels?
> 
> Andy,
> 
> Why do we need this request_line_base? Why not program the actual value
> directly into the slave structure (base + offset)?

Slave device exactly has knowledge about this number slave_id = (base
+offset). However...

> In case it is required, add it in platform data field or can add DT
> binding for it.

We get a device from the ACPI CSRT table. During enumeration we don't
know what kind of the device it is. From the enumeration point of view
each device enumerated from CSRT is a platform device (see
drivers/acpi/csrt.c for the details).

That's why we have to pass this info somehow to the DMAC driver. And
like Mika already mentioned we have no other generic way except
IORESOURCE_DMA. If you have something better in mind, please share. We
could adopt our code then.


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

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Viresh Kumar]

On Wed, Jan 30, 2013 at 1:02 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> We get a device from the ACPI CSRT table. During enumeration we don't
> know what kind of the device it is. From the enumeration point of view
> each device enumerated from CSRT is a platform device (see
> drivers/acpi/csrt.c for the details).
>
> That's why we have to pass this info somehow to the DMAC driver. And
> like Mika already mentioned we have no other generic way except
> IORESOURCE_DMA. If you have something better in mind, please share. We
> could adopt our code then.

You don't need this in DT case too? Or only for ACPI case?

I think "Yes" is the answer of both the questions. If so, then you
can keep it this way or add another variable in platform data.

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Andy Shevchenko]

On Wed, 2013-01-30 at 13:31 +0530, Viresh Kumar wrote: 
> On Wed, Jan 30, 2013 at 1:02 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > We get a device from the ACPI CSRT table. During enumeration we don't
> > know what kind of the device it is. From the enumeration point of view
> > each device enumerated from CSRT is a platform device (see
> > drivers/acpi/csrt.c for the details).
> >
> > That's why we have to pass this info somehow to the DMAC driver. And
> > like Mika already mentioned we have no other generic way except
> > IORESOURCE_DMA. If you have something better in mind, please share. We
> > could adopt our code then.
> 
> You don't need this in DT case too? 

I think no. It seems the DT has a proper backlink from the slave device
to the dmac ("dma-names" property if I understood it correctly).

> Or only for ACPI case?

ACPI 5 case only until now.
We would like to initiate the proposal to the ACPI specification
team/whoever to have this part designed better in next versions.

> I think "Yes" is the answer of both the questions. If so, then you
> can keep it this way or add another variable in platform data.

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

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Shevchenko, Andriy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2100 bytes --]

On Wed, 2013-01-30 at 10:07 +0200, Andy Shevchenko wrote: 
> On Wed, 2013-01-30 at 13:31 +0530, Viresh Kumar wrote: 
> > On Wed, Jan 30, 2013 at 1:02 PM, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > We get a device from the ACPI CSRT table. During enumeration we don't
> > > know what kind of the device it is. From the enumeration point of view
> > > each device enumerated from CSRT is a platform device (see
> > > drivers/acpi/csrt.c for the details).
> > >
> > > That's why we have to pass this info somehow to the DMAC driver. And
> > > like Mika already mentioned we have no other generic way except
> > > IORESOURCE_DMA. If you have something better in mind, please share. We
> > > could adopt our code then.
> > 
> > You don't need this in DT case too? 
> 
> I think no. It seems the DT has a proper backlink from the slave device
> to the dmac ("dma-names" property if I understood it correctly).

Actually I meant "dmas" property in the brackets.

> > Or only for ACPI case?
> 
> ACPI 5 case only until now.
> We would like to initiate the proposal to the ACPI specification
> team/whoever to have this part designed better in next versions.
> 
> > I think "Yes" is the answer of both the questions. If so, then you
> > can keep it this way or add another variable in platform data.

So, are you fine with it? Could you please Ack it if there is neither
objections nor more comments.

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Viresh Kumar]

On Mon, Jan 28, 2013 at 4:34 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On some hardware configurations we have got the request line with the offset.
> The patch introduces convert_slave_id() helper for that cases. The request line
> base is got from the platform device resources provided by the IORESOURCE_DMA
> type.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Vinod Koul]

On Tue, Jan 29, 2013 at 10:29:43AM +0530, Viresh Kumar wrote:
> Next time, please direct these mails to my Linaro id :)
> 
> On Mon, Jan 28, 2013 at 4:34 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On some hardware configurations we have got the request line with the offset.
> > The patch introduces convert_slave_id() helper for that cases. The request line
> > base is got from the platform device resources provided by the IORESOURCE_DMA
> > type.
> 
> @Vinod: Is IORESOURCE_DMA suitable for this purpose?
Looks unlikely...

But why do we need this in first place?

I know this is due to common platform probe. Clearly one size not fitting all.
If you had dedicated PCI probe you wouldnt worrry right?

--
~Vinod
> 
> > 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      |   13 +++++++++++++
> >  drivers/dma/dw_dmac_regs.h |    1 +
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> > index 3155c11..8484ca1 100644
> > --- a/drivers/dma/dw_dmac.c
> > +++ b/drivers/dma/dw_dmac.c
> > @@ -995,6 +995,13 @@ 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)
> >  {
> > @@ -1009,6 +1016,7 @@ 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;
> >  }
> > @@ -1717,6 +1725,11 @@ static int dw_probe(struct platform_device *pdev)
> >                 memcpy(dw->data_width, pdata->data_width, 4);
> >         }
> >
> > +       /* Get the base request line if set */
> > +       io = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> > +       if (io)
> > +               dw->request_line_base = (unsigned int)io->start;
> > +
> 
> How will it work in case of DT?
> 
> >         /* Calculate all channel mask before DMA setup */
> >         dw->all_chan_mask = (1 << nr_channels) - 1;
> >
> > diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
> > index 88dd8eb..3eeb766 100644
> > --- a/drivers/dma/dw_dmac_regs.h
> > +++ b/drivers/dma/dw_dmac_regs.h
> > @@ -248,6 +248,7 @@ 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];
> >  };
> 
> --
> viresh

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Vinod Koul]

On Wed, Jan 30, 2013 at 09:32:56AM +0200, Andy Shevchenko wrote:
> On Tue, 2013-01-29 at 20:57 +0530, Viresh Kumar wrote: 
> > On Tue, Jan 29, 2013 at 10:52 AM, Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > > We had a discusssion about this with Andy as well. The thing is that there
> > > is no way in current resource to pass DMA request line numbers supported by
> > > the controller to the driver in a generic way. We on the other hand have to
> > > deal this somehow as we have a shared DMA controller on Lynxpoint where the
> > > offset will start from 16 (but it might be something else as well).
> > >
> > > Is there something which limits the usage of IORESOUCE_DMA to be only
> > > usable for ISA DMA channels?
> > 
> > Andy,
> > 
> > Why do we need this request_line_base? Why not program the actual value
> > directly into the slave structure (base + offset)?
> 
> Slave device exactly has knowledge about this number slave_id = (base
> +offset). However...
Btw request line needs to be programmed by client in slave_id field.
So why exactly is this required for dma driver?

--
~Vinod
> > In case it is required, add it in platform data field or can add DT
> > binding for it.
> 
> We get a device from the ACPI CSRT table. During enumeration we don't
> know what kind of the device it is. From the enumeration point of view
> each device enumerated from CSRT is a platform device (see
> drivers/acpi/csrt.c for the details).
> 
> That's why we have to pass this info somehow to the DMAC driver. And
> like Mika already mentioned we have no other generic way except
> IORESOURCE_DMA. If you have something better in mind, please share. We
> could adopt our code then.
> 
> 
> -- 
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Andy Shevchenko]

On Tue, Feb 12, 2013 at 5:56 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Jan 30, 2013 at 09:32:56AM +0200, Andy Shevchenko wrote:
>> On Tue, 2013-01-29 at 20:57 +0530, Viresh Kumar wrote:
>> > On Tue, Jan 29, 2013 at 10:52 AM, Mika Westerberg
>> > <mika.westerberg@linux.intel.com> wrote:
>> > > We had a discusssion about this with Andy as well. The thing is that there
>> > > is no way in current resource to pass DMA request line numbers supported by
>> > > the controller to the driver in a generic way. We on the other hand have to
>> > > deal this somehow as we have a shared DMA controller on Lynxpoint where the
>> > > offset will start from 16 (but it might be something else as well).
>> > >
>> > > Is there something which limits the usage of IORESOUCE_DMA to be only
>> > > usable for ISA DMA channels?
>> >
>> > Andy,
>> >
>> > Why do we need this request_line_base? Why not program the actual value
>> > directly into the slave structure (base + offset)?
>>
>> Slave device exactly has knowledge about this number slave_id = (base
>> +offset). However...
> Btw request line needs to be programmed by client in slave_id field.
> So why exactly is this required for dma driver?

The slave device driver doesn't know the DMAC limitations and won't
know them. So, it gets request line from the ACPI resource and passes
it to the DMAC. However, the request line is actualy base + offset.
Slave driver has no clue about this and especially about base. The
base is a property of the DMAC.

>> > In case it is required, add it in platform data field or can add DT
>> > binding for it.
>>
>> We get a device from the ACPI CSRT table. During enumeration we don't
>> know what kind of the device it is. From the enumeration point of view
>> each device enumerated from CSRT is a platform device (see
>> drivers/acpi/csrt.c for the details).
>>
>> That's why we have to pass this info somehow to the DMAC driver. And
>> like Mika already mentioned we have no other generic way except
>> IORESOURCE_DMA. If you have something better in mind, please share. We
>> could adopt our code then.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Mika Westerberg]

On Tue, Feb 12, 2013 at 07:53:34AM -0800, Vinod Koul wrote:
> On Tue, Jan 29, 2013 at 10:29:43AM +0530, Viresh Kumar wrote:
> > Next time, please direct these mails to my Linaro id :)
> > 
> > On Mon, Jan 28, 2013 at 4:34 PM, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On some hardware configurations we have got the request line with the offset.
> > > The patch introduces convert_slave_id() helper for that cases. The request line
> > > base is got from the platform device resources provided by the IORESOURCE_DMA
> > > type.
> > 
> > @Vinod: Is IORESOURCE_DMA suitable for this purpose?
> Looks unlikely...
> 
> But why do we need this in first place?
> 
> I know this is due to common platform probe. Clearly one size not fitting all.
> If you had dedicated PCI probe you wouldnt worrry right?

This is all related to new ACPI 5.0 features for devices enumerated from
ACPI namespace, such as SPI controller.

ACPI 5.0 allows BIOS to specify DMA request lines and channels used by a
slave device with FixedDMA() descriptors, such as:

	FixedDMA (0x0010, 0x0000, Width32bit,) // TX
	FixedDMA (0x0011, 0x0001, Width32bit,) // RX

The first field (0x10, 0x11) is the request line assigned to the device. In
this particular case it is the SPI controller.

Now there is only one DMA controller in this platform but the request lines
still start from 0x10 (16).

Then there is another ACPI table called CSRT (Core System Resources Table)
that assings request line range to the DMA controller itself. This range
starts from 0x10 (16) for this platform.

So what we do is that we parse this CSRT table, create platform device for
the DMA controller of found and pass the request line range as
IORESOURCE_DMA resource with the platform device.

In this patch then Andy uses that resource to map the FixedDMA() request
line to the actual hardware request line, which is for the SPI case:

	0x10 -> 0
	0x11 -> 1

Unfortunately we didn't find any better way to pass this information than
IORESOURCE_DMA. However, if you have better solution we certainly can
change that :)

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Vinod Koul]

On Tue, Feb 12, 2013 at 06:43:51PM +0200, Mika Westerberg wrote:
> On Tue, Feb 12, 2013 at 07:53:34AM -0800, Vinod Koul wrote:
> > On Tue, Jan 29, 2013 at 10:29:43AM +0530, Viresh Kumar wrote:
> > > Next time, please direct these mails to my Linaro id :)
> > > 
> > > On Mon, Jan 28, 2013 at 4:34 PM, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On some hardware configurations we have got the request line with the offset.
> > > > The patch introduces convert_slave_id() helper for that cases. The request line
> > > > base is got from the platform device resources provided by the IORESOURCE_DMA
> > > > type.
> > > 
> > > @Vinod: Is IORESOURCE_DMA suitable for this purpose?
> > Looks unlikely...
> > 
> > But why do we need this in first place?
> > 
> > I know this is due to common platform probe. Clearly one size not fitting all.
> > If you had dedicated PCI probe you wouldnt worrry right?
> 
> This is all related to new ACPI 5.0 features for devices enumerated from
> ACPI namespace, such as SPI controller.
> 
> ACPI 5.0 allows BIOS to specify DMA request lines and channels used by a
> slave device with FixedDMA() descriptors, such as:
> 
> 	FixedDMA (0x0010, 0x0000, Width32bit,) // TX
> 	FixedDMA (0x0011, 0x0001, Width32bit,) // RX
> 
> The first field (0x10, 0x11) is the request line assigned to the device. In
> this particular case it is the SPI controller.
> 
> Now there is only one DMA controller in this platform but the request lines
> still start from 0x10 (16).
> 
> Then there is another ACPI table called CSRT (Core System Resources Table)
> that assings request line range to the DMA controller itself. This range
> starts from 0x10 (16) for this platform.
> 
> So what we do is that we parse this CSRT table, create platform device for
> the DMA controller of found and pass the request line range as
> IORESOURCE_DMA resource with the platform device.
Okay, who parses the CSRT table and how is this presented to OS.

Can we do this in platform data and complete the base calculation.

In ACPI 5, is DMA a PCI device or platfrom device?
--
~Vinod
> 
> In this patch then Andy uses that resource to map the FixedDMA() request
> line to the actual hardware request line, which is for the SPI case:
> 
> 	0x10 -> 0
> 	0x11 -> 1
> 
> Unfortunately we didn't find any better way to pass this information than
> IORESOURCE_DMA. However, if you have better solution we certainly can
> change that :)

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Mika Westerberg]

On Tue, Feb 12, 2013 at 09:34:35AM -0800, Vinod Koul wrote:
> On Tue, Feb 12, 2013 at 06:43:51PM +0200, Mika Westerberg wrote:
> > On Tue, Feb 12, 2013 at 07:53:34AM -0800, Vinod Koul wrote:
> > > On Tue, Jan 29, 2013 at 10:29:43AM +0530, Viresh Kumar wrote:
> > > > Next time, please direct these mails to my Linaro id :)
> > > > 
> > > > On Mon, Jan 28, 2013 at 4:34 PM, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On some hardware configurations we have got the request line with the offset.
> > > > > The patch introduces convert_slave_id() helper for that cases. The request line
> > > > > base is got from the platform device resources provided by the IORESOURCE_DMA
> > > > > type.
> > > > 
> > > > @Vinod: Is IORESOURCE_DMA suitable for this purpose?
> > > Looks unlikely...
> > > 
> > > But why do we need this in first place?
> > > 
> > > I know this is due to common platform probe. Clearly one size not fitting all.
> > > If you had dedicated PCI probe you wouldnt worrry right?
> > 
> > This is all related to new ACPI 5.0 features for devices enumerated from
> > ACPI namespace, such as SPI controller.
> > 
> > ACPI 5.0 allows BIOS to specify DMA request lines and channels used by a
> > slave device with FixedDMA() descriptors, such as:
> > 
> > 	FixedDMA (0x0010, 0x0000, Width32bit,) // TX
> > 	FixedDMA (0x0011, 0x0001, Width32bit,) // RX
> > 
> > The first field (0x10, 0x11) is the request line assigned to the device. In
> > this particular case it is the SPI controller.
> > 
> > Now there is only one DMA controller in this platform but the request lines
> > still start from 0x10 (16).
> > 
> > Then there is another ACPI table called CSRT (Core System Resources Table)
> > that assings request line range to the DMA controller itself. This range
> > starts from 0x10 (16) for this platform.
> > 
> > So what we do is that we parse this CSRT table, create platform device for
> > the DMA controller of found and pass the request line range as
> > IORESOURCE_DMA resource with the platform device.
> Okay, who parses the CSRT table and how is this presented to OS.

ACPI code, found under drivers/acpi/csrt.c handles this. Note that this
code is heading 3.9 so it is not in mainline yet but available in
linux-pm.git/linux-next branch. I can dig the commit id if you are
interested.

> Can we do this in platform data and complete the base calculation.

Do you mean platform data for the dmac driver?

The CSRT parser code can't know what the platform data requirements for the
particular dmac driver are. It only creates the platform devices and adds
MMIO, IRQ and possible DMA resources. It is up to the (dmac) driver to use
this information for whatever purposes in needs.

> In ACPI 5, is DMA a PCI device or platfrom device?

It is platform device.

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Vinod Koul]

On Tue, Feb 12, 2013 at 09:02:36PM +0200, Mika Westerberg wrote:
> > Okay, who parses the CSRT table and how is this presented to OS.
> 
> ACPI code, found under drivers/acpi/csrt.c handles this. Note that this
> code is heading 3.9 so it is not in mainline yet but available in
> linux-pm.git/linux-next branch. I can dig the commit id if you are
> interested.
> 
> > Can we do this in platform data and complete the base calculation.
> 
> Do you mean platform data for the dmac driver?
> 
> The CSRT parser code can't know what the platform data requirements for the
> particular dmac driver are. It only creates the platform devices and adds
> MMIO, IRQ and possible DMA resources. It is up to the (dmac) driver to use
> this information for whatever purposes in needs.
Not in the CSRT parser. Once your plaform devices are created, you can add your
own board handler for each platform device. This board code, using the
data availble from CSRT can go ahead and do the translation and present the
actual addresses to dmac.
Would this be doable with ACPI5.

Somehow I dont like the idea of DMA resource for request lines. DMA resources
have specific meaning, perhpas IO resource maybe better idea.

--
~Vinod

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Mika Westerberg]

On Thu, Feb 14, 2013 at 03:27:38PM +0530, Vinod Koul wrote:
> Not in the CSRT parser. Once your plaform devices are created, you can add your
> own board handler for each platform device. This board code, using the
> data availble from CSRT can go ahead and do the translation and present the
> actual addresses to dmac.
> Would this be doable with ACPI5.

I'm not sure what you mean by own board handler?

AFAIK, the only platform specific code is under arch/x86/platform/* but we
don't have such thing for Lynxpoint and it should not be needed as we can
enumerate the devices from ACPI.

> Somehow I dont like the idea of DMA resource for request lines. DMA
> resources have specific meaning, perhpas IO resource maybe better idea.

Well, it's a DMA request line so we thought that IORESOURCE_DMA would suit
fine there and the drivers that use these are well aware of their usage.
Not sure about IORESOURCE_IO, it has also specific meaning and I don't
think DMA request lines fit there either :-(

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Andy Shevchenko]

On Mon, 2013-01-28 at 13:04 +0200, Andy Shevchenko wrote: 
> On some hardware configurations we have got the request line with the offset.
> The patch introduces convert_slave_id() helper for that cases. The request line
> base is got from the platform device resources provided by the IORESOURCE_DMA
> type.

Hi, Vinod.

v3.8 is just out, and we have not got any proposal from anyone how to
make our approach better. So, I'm just wondering about current state.

Rafael, could you share your opinion about our case with CSRT and
request line base (perhaps you need to go through the entire thread)?

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

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Rafael J. Wysocki]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8"; format="flowed", Size: 1491 bytes --]

On 2/19/2013 10:02 AM, Andy Shevchenko wrote:
> On Mon, 2013-01-28 at 13:04 +0200, Andy Shevchenko wrote:
>> On some hardware configurations we have got the request line with the offset.
>> The patch introduces convert_slave_id() helper for that cases. The request line
>> base is got from the platform device resources provided by the IORESOURCE_DMA
>> type.
> Hi, Vinod.
>
> v3.8 is just out, and we have not got any proposal from anyone how to
> make our approach better. So, I'm just wondering about current state.
>
> Rafael, could you share your opinion about our case with CSRT and
> request line base (perhaps you need to go through the entire thread)?
>

Well, I can do that, but Mika is the author of the patches, so he should 
know. :-)

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] dw_dmac: adjust slave_id accordingly to request line base

[Vinod Koul]

On Tue, Feb 19, 2013 at 11:02:24AM +0200, Andy Shevchenko wrote:
> On Mon, 2013-01-28 at 13:04 +0200, Andy Shevchenko wrote: 
> > On some hardware configurations we have got the request line with the offset.
> > The patch introduces convert_slave_id() helper for that cases. The request line
> > base is got from the platform device resources provided by the IORESOURCE_DMA
> > type.
> 
> Hi, Vinod.
> 
> v3.8 is just out, and we have not got any proposal from anyone how to
> make our approach better. So, I'm just wondering about current state.
I was trying to go thru ACPI 5 and see a solution, i think we 
- use IORESOURCE_IO, but they have a specfic meaning
- accpet this patch with IORESOURCE_DMA, but again this has specfic dma meaning
  too. Just becoz you are in slave-dma driver, doesn't justify use of
  IORESOURCE_DMA as well
- use platform_data (somehow get this from CSRT into device parsing logic of
  ACPI where you create your platform device and add this in platform data

> Rafael, could you share your opinion about our case with CSRT and
> request line base (perhaps you need to go through the entire thread)?
Sure, do we have anything better than above?

--
~Vinod