All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width
@ 2013-01-14 14:20 Andy Shevchenko
  2013-01-15  1:54 ` Viresh Kumar
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2013-01-14 14:20 UTC (permalink / raw)
  To: Viresh Kumar, Vinod Koul, linux-kernel, spear-devel; +Cc: Andy Shevchenko

The driver's default assumes that hardware has two AHB masters which might be
not true. In such rare cases we have not to exceed a number of the AHB masters
present in the hardware. Thus, the AHB master with highest possible number will
be used to retrieve the data witdh value.

The patch also changes the logic a bit when the master parameter of the
dwc_get_data_width() is out of range. Now the function will return a value
related to the first AHB master.

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

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 30ff2a4..b2d564a 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -80,13 +80,14 @@ static inline unsigned int dwc_get_data_width(struct dma_chan *chan, int master)
 {
 	struct dw_dma *dw = to_dw_dma(chan->device);
 	struct dw_dma_slave *dws = chan->private;
+	unsigned int m = 0;
 
 	if (master == SRC_MASTER)
-		return dw->data_width[dwc_get_sms(dws)];
+		m = dwc_get_sms(dws);
 	else if (master == DST_MASTER)
-		return dw->data_width[dwc_get_dms(dws)];
+		m = dwc_get_dms(dws);
 
-	return 0;
+	return dw->data_width[min_t(unsigned int, dw->nr_masters - 1, m)];
 }
 
 /*----------------------------------------------------------------------*/
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width
  2013-01-14 14:20 [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width Andy Shevchenko
@ 2013-01-15  1:54 ` Viresh Kumar
  2013-01-15  7:05   ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2013-01-15  1:54 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Vinod Koul, linux-kernel, spear-devel

On Mon, Jan 14, 2013 at 7:50 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> The driver's default assumes that hardware has two AHB masters which might be
> not true. In such rare cases we have not to exceed a number of the AHB masters
> present in the hardware. Thus, the AHB master with highest possible number will
> be used to retrieve the data witdh value.
>
> The patch also changes the logic a bit when the master parameter of the
> dwc_get_data_width() is out of range. Now the function will return a value
> related to the first AHB master.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/dma/dw_dmac.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 30ff2a4..b2d564a 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -80,13 +80,14 @@ static inline unsigned int dwc_get_data_width(struct dma_chan *chan, int master)
>  {
>         struct dw_dma *dw = to_dw_dma(chan->device);
>         struct dw_dma_slave *dws = chan->private;
> +       unsigned int m = 0;
>
>         if (master == SRC_MASTER)
> -               return dw->data_width[dwc_get_sms(dws)];
> +               m = dwc_get_sms(dws);
>         else if (master == DST_MASTER)
> -               return dw->data_width[dwc_get_dms(dws)];
> +               m = dwc_get_dms(dws);
>
> -       return 0;
> +       return dw->data_width[min_t(unsigned int, dw->nr_masters - 1, m)];

The details of the patch are fine, but i didn't get how do you get
master 1 selected
on a system where you have only 1 master?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width
  2013-01-15  1:54 ` Viresh Kumar
@ 2013-01-15  7:05   ` Andy Shevchenko
  2013-01-15  9:01     ` Viresh Kumar
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2013-01-15  7:05 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Andy Shevchenko, Vinod Koul, linux-kernel, spear-devel

On Tue, Jan 15, 2013 at 3:54 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On Mon, Jan 14, 2013 at 7:50 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> The driver's default assumes that hardware has two AHB masters which might be
>> not true. In such rare cases we have not to exceed a number of the AHB masters
>> present in the hardware. Thus, the AHB master with highest possible number will
>> be used to retrieve the data witdh value.
>>
>> The patch also changes the logic a bit when the master parameter of the
>> dwc_get_data_width() is out of range. Now the function will return a value
>> related to the first AHB master.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/dma/dw_dmac.c |    7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index 30ff2a4..b2d564a 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -80,13 +80,14 @@ static inline unsigned int dwc_get_data_width(struct dma_chan *chan, int master)
>>  {
>>         struct dw_dma *dw = to_dw_dma(chan->device);
>>         struct dw_dma_slave *dws = chan->private;
>> +       unsigned int m = 0;
>>
>>         if (master == SRC_MASTER)
>> -               return dw->data_width[dwc_get_sms(dws)];
>> +               m = dwc_get_sms(dws);
>>         else if (master == DST_MASTER)
>> -               return dw->data_width[dwc_get_dms(dws)];
>> +               m = dwc_get_dms(dws);
>>
>> -       return 0;
>> +       return dw->data_width[min_t(unsigned int, dw->nr_masters - 1, m)];
>
> The details of the patch are fine, but i didn't get how do you get
> master 1 selected
> on a system where you have only 1 master?

min_t(..., nr_masters - 1, m) defines this. m is returned value with
default settings, let's say 1. In that case if nr_masters is also 1
the min_t(..., 1-1, 1) returns 0.

The value of nr_masters itself is come from autoconfig or platform data.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width
  2013-01-15  7:05   ` Andy Shevchenko
@ 2013-01-15  9:01     ` Viresh Kumar
  2013-01-15  9:08       ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2013-01-15  9:01 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Andy Shevchenko, Vinod Koul, linux-kernel, spear-devel

On Tue, Jan 15, 2013 at 12:35 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>> The details of the patch are fine, but i didn't get how do you get
>> master 1 selected
>> on a system where you have only 1 master?
>
> min_t(..., nr_masters - 1, m) defines this. m is returned value with
> default settings, let's say 1. In that case if nr_masters is also 1
> the min_t(..., 1-1, 1) returns 0.
>
> The value of nr_masters itself is come from autoconfig or platform data.

:)
You didn't get me. How this code would work is pretty easy to understand.
What i wanted to know is, the master information is coming from slave drivers
and they must select master zero always. How would master 1 be selected ever
in this driver.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width
  2013-01-15  9:01     ` Viresh Kumar
@ 2013-01-15  9:08       ` Andy Shevchenko
  2013-01-15  9:09         ` Viresh Kumar
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2013-01-15  9:08 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Andy Shevchenko, Vinod Koul, linux-kernel, spear-devel

On Tue, Jan 15, 2013 at 11:01 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On Tue, Jan 15, 2013 at 12:35 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>>> The details of the patch are fine, but i didn't get how do you get
>>> master 1 selected
>>> on a system where you have only 1 master?
>>
>> min_t(..., nr_masters - 1, m) defines this. m is returned value with
>> default settings, let's say 1. In that case if nr_masters is also 1
>> the min_t(..., 1-1, 1) returns 0.
>>
>> The value of nr_masters itself is come from autoconfig or platform data.
>
> :)
> You didn't get me. How this code would work is pretty easy to understand.
> What i wanted to know is, the master information is coming from slave drivers
> and they must select master zero always. How would master 1 be selected ever
> in this driver.

You forgot the mem2mem type of transfers where we have no information
about masters except default settings. Otherwise there is the case of
custom slave config absence when we supply only request line via
slave_id.

In the rest you are right.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width
  2013-01-15  9:08       ` Andy Shevchenko
@ 2013-01-15  9:09         ` Viresh Kumar
  2013-01-15  9:36           ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2013-01-15  9:09 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Andy Shevchenko, Vinod Koul, linux-kernel, spear-devel

On 15 January 2013 14:38, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> You forgot the mem2mem type of transfers where we have no information
> about masters except default settings. Otherwise there is the case of
> custom slave config absence when we supply only request line via
> slave_id.

Okay, but even for mem2mem you have just got the right bus width by
selecting same master. But are you programming same masters in control
register? I can't see that :(

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width
  2013-01-15  9:09         ` Viresh Kumar
@ 2013-01-15  9:36           ` Andy Shevchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2013-01-15  9:36 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Andy Shevchenko, Vinod Koul, linux-kernel, spear-devel

On Tue, Jan 15, 2013 at 11:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 15 January 2013 14:38, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> You forgot the mem2mem type of transfers where we have no information
>> about masters except default settings. Otherwise there is the case of
>> custom slave config absence when we supply only request line via
>> slave_id.
>
> Okay, but even for mem2mem you have just got the right bus width by
> selecting same master. But are you programming same masters in control
> register? I can't see that :(

Oops. Seems this patch does not cover the CTL lo register value.
Thank you for pointing this out. I will think how to make the fix better.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-01-15  9:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-14 14:20 [PATCH] dw_dmac: don't exceed AHB master number in dwc_get_data_width Andy Shevchenko
2013-01-15  1:54 ` Viresh Kumar
2013-01-15  7:05   ` Andy Shevchenko
2013-01-15  9:01     ` Viresh Kumar
2013-01-15  9:08       ` Andy Shevchenko
2013-01-15  9:09         ` Viresh Kumar
2013-01-15  9:36           ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.