All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
@ 2016-02-08 14:18 ` Robert Jarzmik
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2016-02-08 14:18 UTC (permalink / raw)
  To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Vinod Koul
  Cc: linux-arm-kernel, dmaengine, linux-kernel

The current number of requestor lines is limited to 31. This was an
error of a previous commit, as this number is platform dependent, and is
actually :
 - for pxa25x: 40 requestor lines
 - for pxa27x: 74 requestor lines
 - for pxa3xx: 100 requestor lines

As the driver doesn't need to know the exact number, but only an
absolute maximum value above which is it not using any flow control, set
the maximum to 0x8000 requestor lines, which should never be reached.

The previous testing did not reveal the faulty constant as on pxa[23]xx
platforms, only camera, MSL and USB are above requestor 32, and in these
only the camera has a driver using dma.

Fixes: e87ffbdf0697 ("dmaengine: pxa_dma: fix the no-requestor case")
Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/dma/pxa_dma.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index ca6c088edc8a..817a2bdc423d 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -53,6 +53,7 @@
 
 #define DRCMR_MAPVLD	BIT(7)	/* Map Valid (read / write) */
 #define DRCMR_CHLNUM	0x1f	/* mask for Channel Number (read / write) */
+#define DRCMR_MAXREQST	0x7fff	/* Maximum requestor line */
 
 #define DDADR_DESCADDR	0xfffffff0	/* Address of next descriptor (mask) */
 #define DDADR_STOP	BIT(0)	/* Stop (read / write) */
@@ -473,7 +474,7 @@ static void pxad_free_phy(struct pxad_chan *chan)
 		return;
 
 	/* clear the channel mapping in DRCMR */
-	if (chan->drcmr <= DRCMR_CHLNUM) {
+	if (chan->drcmr <= DRCMR_MAXREQST) {
 		reg = pxad_drcmr(chan->drcmr);
 		writel_relaxed(0, chan->phy->base + reg);
 	}
@@ -518,7 +519,7 @@ static void phy_enable(struct pxad_phy *phy, bool misaligned)
 		"%s(); phy=%p(%d) misaligned=%d\n", __func__,
 		phy, phy->idx, misaligned);
 
-	if (phy->vchan->drcmr <= DRCMR_CHLNUM) {
+	if (phy->vchan->drcmr <= DRCMR_MAXREQST) {
 		reg = pxad_drcmr(phy->vchan->drcmr);
 		writel_relaxed(DRCMR_MAPVLD | phy->idx, phy->base + reg);
 	}
@@ -922,7 +923,7 @@ static void pxad_get_config(struct pxad_chan *chan,
 		dev_addr = chan->cfg.src_addr;
 		*dev_src = dev_addr;
 		*dcmd |= PXA_DCMD_INCTRGADDR;
-		if (chan->drcmr <= DRCMR_CHLNUM)
+		if (chan->drcmr <= DRCMR_MAXREQST)
 			*dcmd |= PXA_DCMD_FLOWSRC;
 	}
 	if (dir == DMA_MEM_TO_DEV) {
@@ -931,7 +932,7 @@ static void pxad_get_config(struct pxad_chan *chan,
 		dev_addr = chan->cfg.dst_addr;
 		*dev_dst = dev_addr;
 		*dcmd |= PXA_DCMD_INCSRCADDR;
-		if (chan->drcmr <= DRCMR_CHLNUM)
+		if (chan->drcmr <= DRCMR_MAXREQST)
 			*dcmd |= PXA_DCMD_FLOWTRG;
 	}
 	if (dir == DMA_MEM_TO_MEM)
-- 
2.1.4

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

* [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
@ 2016-02-08 14:18 ` Robert Jarzmik
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2016-02-08 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

The current number of requestor lines is limited to 31. This was an
error of a previous commit, as this number is platform dependent, and is
actually :
 - for pxa25x: 40 requestor lines
 - for pxa27x: 74 requestor lines
 - for pxa3xx: 100 requestor lines

As the driver doesn't need to know the exact number, but only an
absolute maximum value above which is it not using any flow control, set
the maximum to 0x8000 requestor lines, which should never be reached.

The previous testing did not reveal the faulty constant as on pxa[23]xx
platforms, only camera, MSL and USB are above requestor 32, and in these
only the camera has a driver using dma.

Fixes: e87ffbdf0697 ("dmaengine: pxa_dma: fix the no-requestor case")
Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/dma/pxa_dma.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index ca6c088edc8a..817a2bdc423d 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -53,6 +53,7 @@
 
 #define DRCMR_MAPVLD	BIT(7)	/* Map Valid (read / write) */
 #define DRCMR_CHLNUM	0x1f	/* mask for Channel Number (read / write) */
+#define DRCMR_MAXREQST	0x7fff	/* Maximum requestor line */
 
 #define DDADR_DESCADDR	0xfffffff0	/* Address of next descriptor (mask) */
 #define DDADR_STOP	BIT(0)	/* Stop (read / write) */
@@ -473,7 +474,7 @@ static void pxad_free_phy(struct pxad_chan *chan)
 		return;
 
 	/* clear the channel mapping in DRCMR */
-	if (chan->drcmr <= DRCMR_CHLNUM) {
+	if (chan->drcmr <= DRCMR_MAXREQST) {
 		reg = pxad_drcmr(chan->drcmr);
 		writel_relaxed(0, chan->phy->base + reg);
 	}
@@ -518,7 +519,7 @@ static void phy_enable(struct pxad_phy *phy, bool misaligned)
 		"%s(); phy=%p(%d) misaligned=%d\n", __func__,
 		phy, phy->idx, misaligned);
 
-	if (phy->vchan->drcmr <= DRCMR_CHLNUM) {
+	if (phy->vchan->drcmr <= DRCMR_MAXREQST) {
 		reg = pxad_drcmr(phy->vchan->drcmr);
 		writel_relaxed(DRCMR_MAPVLD | phy->idx, phy->base + reg);
 	}
@@ -922,7 +923,7 @@ static void pxad_get_config(struct pxad_chan *chan,
 		dev_addr = chan->cfg.src_addr;
 		*dev_src = dev_addr;
 		*dcmd |= PXA_DCMD_INCTRGADDR;
-		if (chan->drcmr <= DRCMR_CHLNUM)
+		if (chan->drcmr <= DRCMR_MAXREQST)
 			*dcmd |= PXA_DCMD_FLOWSRC;
 	}
 	if (dir == DMA_MEM_TO_DEV) {
@@ -931,7 +932,7 @@ static void pxad_get_config(struct pxad_chan *chan,
 		dev_addr = chan->cfg.dst_addr;
 		*dev_dst = dev_addr;
 		*dcmd |= PXA_DCMD_INCSRCADDR;
-		if (chan->drcmr <= DRCMR_CHLNUM)
+		if (chan->drcmr <= DRCMR_MAXREQST)
 			*dcmd |= PXA_DCMD_FLOWTRG;
 	}
 	if (dir == DMA_MEM_TO_MEM)
-- 
2.1.4

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

* Re: [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
  2016-02-08 14:18 ` Robert Jarzmik
@ 2016-02-09  3:36   ` Vinod Koul
  -1 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2016-02-09  3:36 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Daniel Mack, Haojian Zhuang, linux-arm-kernel, dmaengine, linux-kernel

On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
> The current number of requestor lines is limited to 31. This was an
> error of a previous commit, as this number is platform dependent, and is
> actually :
>  - for pxa25x: 40 requestor lines
>  - for pxa27x: 74 requestor lines
>  - for pxa3xx: 100 requestor lines
> 
> As the driver doesn't need to know the exact number, but only an

and why would that be a good assumption?

Btw shouldn't this data come from DT?

> absolute maximum value above which is it not using any flow control, set
> the maximum to 0x8000 requestor lines, which should never be reached.
> 
> The previous testing did not reveal the faulty constant as on pxa[23]xx
> platforms, only camera, MSL and USB are above requestor 32, and in these
> only the camera has a driver using dma.
> 
> Fixes: e87ffbdf0697 ("dmaengine: pxa_dma: fix the no-requestor case")
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  drivers/dma/pxa_dma.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index ca6c088edc8a..817a2bdc423d 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -53,6 +53,7 @@
>  
>  #define DRCMR_MAPVLD	BIT(7)	/* Map Valid (read / write) */
>  #define DRCMR_CHLNUM	0x1f	/* mask for Channel Number (read / write) */
> +#define DRCMR_MAXREQST	0x7fff	/* Maximum requestor line */
>  
>  #define DDADR_DESCADDR	0xfffffff0	/* Address of next descriptor (mask) */
>  #define DDADR_STOP	BIT(0)	/* Stop (read / write) */
> @@ -473,7 +474,7 @@ static void pxad_free_phy(struct pxad_chan *chan)
>  		return;
>  
>  	/* clear the channel mapping in DRCMR */
> -	if (chan->drcmr <= DRCMR_CHLNUM) {
> +	if (chan->drcmr <= DRCMR_MAXREQST) {
>  		reg = pxad_drcmr(chan->drcmr);
>  		writel_relaxed(0, chan->phy->base + reg);
>  	}
> @@ -518,7 +519,7 @@ static void phy_enable(struct pxad_phy *phy, bool misaligned)
>  		"%s(); phy=%p(%d) misaligned=%d\n", __func__,
>  		phy, phy->idx, misaligned);
>  
> -	if (phy->vchan->drcmr <= DRCMR_CHLNUM) {
> +	if (phy->vchan->drcmr <= DRCMR_MAXREQST) {
>  		reg = pxad_drcmr(phy->vchan->drcmr);
>  		writel_relaxed(DRCMR_MAPVLD | phy->idx, phy->base + reg);
>  	}
> @@ -922,7 +923,7 @@ static void pxad_get_config(struct pxad_chan *chan,
>  		dev_addr = chan->cfg.src_addr;
>  		*dev_src = dev_addr;
>  		*dcmd |= PXA_DCMD_INCTRGADDR;
> -		if (chan->drcmr <= DRCMR_CHLNUM)
> +		if (chan->drcmr <= DRCMR_MAXREQST)
>  			*dcmd |= PXA_DCMD_FLOWSRC;
>  	}
>  	if (dir == DMA_MEM_TO_DEV) {
> @@ -931,7 +932,7 @@ static void pxad_get_config(struct pxad_chan *chan,
>  		dev_addr = chan->cfg.dst_addr;
>  		*dev_dst = dev_addr;
>  		*dcmd |= PXA_DCMD_INCSRCADDR;
> -		if (chan->drcmr <= DRCMR_CHLNUM)
> +		if (chan->drcmr <= DRCMR_MAXREQST)
>  			*dcmd |= PXA_DCMD_FLOWTRG;
>  	}
>  	if (dir == DMA_MEM_TO_MEM)
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
~Vinod

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

* [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
@ 2016-02-09  3:36   ` Vinod Koul
  0 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2016-02-09  3:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
> The current number of requestor lines is limited to 31. This was an
> error of a previous commit, as this number is platform dependent, and is
> actually :
>  - for pxa25x: 40 requestor lines
>  - for pxa27x: 74 requestor lines
>  - for pxa3xx: 100 requestor lines
> 
> As the driver doesn't need to know the exact number, but only an

and why would that be a good assumption?

Btw shouldn't this data come from DT?

> absolute maximum value above which is it not using any flow control, set
> the maximum to 0x8000 requestor lines, which should never be reached.
> 
> The previous testing did not reveal the faulty constant as on pxa[23]xx
> platforms, only camera, MSL and USB are above requestor 32, and in these
> only the camera has a driver using dma.
> 
> Fixes: e87ffbdf0697 ("dmaengine: pxa_dma: fix the no-requestor case")
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  drivers/dma/pxa_dma.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index ca6c088edc8a..817a2bdc423d 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -53,6 +53,7 @@
>  
>  #define DRCMR_MAPVLD	BIT(7)	/* Map Valid (read / write) */
>  #define DRCMR_CHLNUM	0x1f	/* mask for Channel Number (read / write) */
> +#define DRCMR_MAXREQST	0x7fff	/* Maximum requestor line */
>  
>  #define DDADR_DESCADDR	0xfffffff0	/* Address of next descriptor (mask) */
>  #define DDADR_STOP	BIT(0)	/* Stop (read / write) */
> @@ -473,7 +474,7 @@ static void pxad_free_phy(struct pxad_chan *chan)
>  		return;
>  
>  	/* clear the channel mapping in DRCMR */
> -	if (chan->drcmr <= DRCMR_CHLNUM) {
> +	if (chan->drcmr <= DRCMR_MAXREQST) {
>  		reg = pxad_drcmr(chan->drcmr);
>  		writel_relaxed(0, chan->phy->base + reg);
>  	}
> @@ -518,7 +519,7 @@ static void phy_enable(struct pxad_phy *phy, bool misaligned)
>  		"%s(); phy=%p(%d) misaligned=%d\n", __func__,
>  		phy, phy->idx, misaligned);
>  
> -	if (phy->vchan->drcmr <= DRCMR_CHLNUM) {
> +	if (phy->vchan->drcmr <= DRCMR_MAXREQST) {
>  		reg = pxad_drcmr(phy->vchan->drcmr);
>  		writel_relaxed(DRCMR_MAPVLD | phy->idx, phy->base + reg);
>  	}
> @@ -922,7 +923,7 @@ static void pxad_get_config(struct pxad_chan *chan,
>  		dev_addr = chan->cfg.src_addr;
>  		*dev_src = dev_addr;
>  		*dcmd |= PXA_DCMD_INCTRGADDR;
> -		if (chan->drcmr <= DRCMR_CHLNUM)
> +		if (chan->drcmr <= DRCMR_MAXREQST)
>  			*dcmd |= PXA_DCMD_FLOWSRC;
>  	}
>  	if (dir == DMA_MEM_TO_DEV) {
> @@ -931,7 +932,7 @@ static void pxad_get_config(struct pxad_chan *chan,
>  		dev_addr = chan->cfg.dst_addr;
>  		*dev_dst = dev_addr;
>  		*dcmd |= PXA_DCMD_INCSRCADDR;
> -		if (chan->drcmr <= DRCMR_CHLNUM)
> +		if (chan->drcmr <= DRCMR_MAXREQST)
>  			*dcmd |= PXA_DCMD_FLOWTRG;
>  	}
>  	if (dir == DMA_MEM_TO_MEM)
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
~Vinod

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

* Re: [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
  2016-02-09  3:36   ` Vinod Koul
@ 2016-02-09  7:20     ` Robert Jarzmik
  -1 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2016-02-09  7:20 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Daniel Mack, Haojian Zhuang, linux-arm-kernel, dmaengine, linux-kernel

Vinod Koul <vinod.koul@intel.com> writes:

> On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
>> The current number of requestor lines is limited to 31. This was an
>> error of a previous commit, as this number is platform dependent, and is
>> actually :
>>  - for pxa25x: 40 requestor lines
>>  - for pxa27x: 74 requestor lines
>>  - for pxa3xx: 100 requestor lines
>> 
>> As the driver doesn't need to know the exact number, but only an
>
> and why would that be a good assumption?
Well, the driver doesn't use the exact value. Would that be 128, 1000 or 10000,
it wouldn't change its behavior. The clients either pass -1UL (ie. no flow
control) or a requestor line number.

> Btw shouldn't this data come from DT?
It can if you wish, but in this case I must amend the platform data case too, ie
mmp_dma_platdata. And this has an impact on MMP architecture. I'd rather have
the simpler approach unless you really want the exact number of requestor lines
to be passed in platform_data+DT.

Cheers.

-- 
Robert

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

* [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
@ 2016-02-09  7:20     ` Robert Jarzmik
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2016-02-09  7:20 UTC (permalink / raw)
  To: linux-arm-kernel

Vinod Koul <vinod.koul@intel.com> writes:

> On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
>> The current number of requestor lines is limited to 31. This was an
>> error of a previous commit, as this number is platform dependent, and is
>> actually :
>>  - for pxa25x: 40 requestor lines
>>  - for pxa27x: 74 requestor lines
>>  - for pxa3xx: 100 requestor lines
>> 
>> As the driver doesn't need to know the exact number, but only an
>
> and why would that be a good assumption?
Well, the driver doesn't use the exact value. Would that be 128, 1000 or 10000,
it wouldn't change its behavior. The clients either pass -1UL (ie. no flow
control) or a requestor line number.

> Btw shouldn't this data come from DT?
It can if you wish, but in this case I must amend the platform data case too, ie
mmp_dma_platdata. And this has an impact on MMP architecture. I'd rather have
the simpler approach unless you really want the exact number of requestor lines
to be passed in platform_data+DT.

Cheers.

-- 
Robert

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

* Re: [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
  2016-02-09  7:20     ` Robert Jarzmik
@ 2016-02-09 15:20       ` Vinod Koul
  -1 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2016-02-09 15:20 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: Daniel Mack, Haojian Zhuang, linux-arm-kernel, dmaengine, linux-kernel

On Tue, Feb 09, 2016 at 08:20:05AM +0100, Robert Jarzmik wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
> 
> > On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
> >> The current number of requestor lines is limited to 31. This was an
> >> error of a previous commit, as this number is platform dependent, and is
> >> actually :
> >>  - for pxa25x: 40 requestor lines
> >>  - for pxa27x: 74 requestor lines
> >>  - for pxa3xx: 100 requestor lines
> >> 
> >> As the driver doesn't need to know the exact number, but only an
> >
> > and why would that be a good assumption?
> Well, the driver doesn't use the exact value. Would that be 128, 1000 or 10000,
> it wouldn't change its behavior. The clients either pass -1UL (ie. no flow
> control) or a requestor line number.

Somehow that does not sound right to me. You seem to have no way of knowing
what is the actual max request line for a platforms. What if on pxa25x user
requests more than 40 lines?

> > Btw shouldn't this data come from DT?
> It can if you wish, but in this case I must amend the platform data case too, ie
> mmp_dma_platdata. And this has an impact on MMP architecture. I'd rather have
> the simpler approach unless you really want the exact number of requestor lines
> to be passed in platform_data+DT.

The information that you have X lines on a platform needs to be described
and queried. I see current approach error prone

-- 
~Vinod

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

* [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
@ 2016-02-09 15:20       ` Vinod Koul
  0 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2016-02-09 15:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 09, 2016 at 08:20:05AM +0100, Robert Jarzmik wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
> 
> > On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
> >> The current number of requestor lines is limited to 31. This was an
> >> error of a previous commit, as this number is platform dependent, and is
> >> actually :
> >>  - for pxa25x: 40 requestor lines
> >>  - for pxa27x: 74 requestor lines
> >>  - for pxa3xx: 100 requestor lines
> >> 
> >> As the driver doesn't need to know the exact number, but only an
> >
> > and why would that be a good assumption?
> Well, the driver doesn't use the exact value. Would that be 128, 1000 or 10000,
> it wouldn't change its behavior. The clients either pass -1UL (ie. no flow
> control) or a requestor line number.

Somehow that does not sound right to me. You seem to have no way of knowing
what is the actual max request line for a platforms. What if on pxa25x user
requests more than 40 lines?

> > Btw shouldn't this data come from DT?
> It can if you wish, but in this case I must amend the platform data case too, ie
> mmp_dma_platdata. And this has an impact on MMP architecture. I'd rather have
> the simpler approach unless you really want the exact number of requestor lines
> to be passed in platform_data+DT.

The information that you have X lines on a platform needs to be described
and queried. I see current approach error prone

-- 
~Vinod

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

* Re: [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
  2016-02-09 15:20       ` Vinod Koul
@ 2016-02-09 20:22         ` Robert Jarzmik
  -1 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2016-02-09 20:22 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Daniel Mack, Haojian Zhuang, linux-arm-kernel, dmaengine, linux-kernel

Vinod Koul <vinod.koul@intel.com> writes:

> On Tue, Feb 09, 2016 at 08:20:05AM +0100, Robert Jarzmik wrote:
>> Vinod Koul <vinod.koul@intel.com> writes:
>> 
>> > On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
>> >> The current number of requestor lines is limited to 31. This was an
>> >> error of a previous commit, as this number is platform dependent, and is
>> >> actually :
>> >>  - for pxa25x: 40 requestor lines
>> >>  - for pxa27x: 74 requestor lines
>> >>  - for pxa3xx: 100 requestor lines
>> >> 
>> >> As the driver doesn't need to know the exact number, but only an
>> >
>> > and why would that be a good assumption?
>> Well, the driver doesn't use the exact value. Would that be 128, 1000 or 10000,
>> it wouldn't change its behavior. The clients either pass -1UL (ie. no flow
>> control) or a requestor line number.
>
> Somehow that does not sound right to me. You seem to have no way of knowing
> what is the actual max request line for a platforms. What if on pxa25x user
> requests more than 40 lines?
>
>> > Btw shouldn't this data come from DT?
>> It can if you wish, but in this case I must amend the platform data case too, ie
>> mmp_dma_platdata. And this has an impact on MMP architecture. I'd rather have
>> the simpler approach unless you really want the exact number of requestor lines
>> to be passed in platform_data+DT.
>
> The information that you have X lines on a platform needs to be described
> and queried. I see current approach error prone

Actually I have thought over your point, and you're quite right (this is your
case of more than 40 lines).

The exception I have found is :
 - a device driver is wrongly written
 - it requests the requestor line 0x1000 (below 0x8000)
 - pxa_dma computes the register in pxad_drcmr(0x1000)
   => this gives : base + 0x1000 + 4 * 0x1000 = base + 0x5000
   => this is a not existing iomapped register

So yes, I will respin the patchset, with :
 - one patch adding the number of requestors in mmp_dma.h
 - one patch adding the number of requestors in devicetree description
 - one patch adding the number of requestors in arch/arm/mach-pxa (my tree)
 - one patch adding the use of number of requestors in pxa_dma.c

Cheers.

-- 
Robert

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

* [PATCH] dmaengine: pxa_dma: fix the maximum requestor line
@ 2016-02-09 20:22         ` Robert Jarzmik
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Jarzmik @ 2016-02-09 20:22 UTC (permalink / raw)
  To: linux-arm-kernel

Vinod Koul <vinod.koul@intel.com> writes:

> On Tue, Feb 09, 2016 at 08:20:05AM +0100, Robert Jarzmik wrote:
>> Vinod Koul <vinod.koul@intel.com> writes:
>> 
>> > On Mon, Feb 08, 2016 at 03:18:51PM +0100, Robert Jarzmik wrote:
>> >> The current number of requestor lines is limited to 31. This was an
>> >> error of a previous commit, as this number is platform dependent, and is
>> >> actually :
>> >>  - for pxa25x: 40 requestor lines
>> >>  - for pxa27x: 74 requestor lines
>> >>  - for pxa3xx: 100 requestor lines
>> >> 
>> >> As the driver doesn't need to know the exact number, but only an
>> >
>> > and why would that be a good assumption?
>> Well, the driver doesn't use the exact value. Would that be 128, 1000 or 10000,
>> it wouldn't change its behavior. The clients either pass -1UL (ie. no flow
>> control) or a requestor line number.
>
> Somehow that does not sound right to me. You seem to have no way of knowing
> what is the actual max request line for a platforms. What if on pxa25x user
> requests more than 40 lines?
>
>> > Btw shouldn't this data come from DT?
>> It can if you wish, but in this case I must amend the platform data case too, ie
>> mmp_dma_platdata. And this has an impact on MMP architecture. I'd rather have
>> the simpler approach unless you really want the exact number of requestor lines
>> to be passed in platform_data+DT.
>
> The information that you have X lines on a platform needs to be described
> and queried. I see current approach error prone

Actually I have thought over your point, and you're quite right (this is your
case of more than 40 lines).

The exception I have found is :
 - a device driver is wrongly written
 - it requests the requestor line 0x1000 (below 0x8000)
 - pxa_dma computes the register in pxad_drcmr(0x1000)
   => this gives : base + 0x1000 + 4 * 0x1000 = base + 0x5000
   => this is a not existing iomapped register

So yes, I will respin the patchset, with :
 - one patch adding the number of requestors in mmp_dma.h
 - one patch adding the number of requestors in devicetree description
 - one patch adding the number of requestors in arch/arm/mach-pxa (my tree)
 - one patch adding the use of number of requestors in pxa_dma.c

Cheers.

-- 
Robert

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

end of thread, other threads:[~2016-02-09 20:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-08 14:18 [PATCH] dmaengine: pxa_dma: fix the maximum requestor line Robert Jarzmik
2016-02-08 14:18 ` Robert Jarzmik
2016-02-09  3:36 ` Vinod Koul
2016-02-09  3:36   ` Vinod Koul
2016-02-09  7:20   ` Robert Jarzmik
2016-02-09  7:20     ` Robert Jarzmik
2016-02-09 15:20     ` Vinod Koul
2016-02-09 15:20       ` Vinod Koul
2016-02-09 20:22       ` Robert Jarzmik
2016-02-09 20:22         ` Robert Jarzmik

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.