diff mbox

[v2,2/2] dma: mv_xor: Fix mis-usage of mmio 'base' and 'high_base' registers

Message ID 1383145303-7207-2-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive)
State Accepted
Commit 82a1402eaee5dab1f3ab2d5aa4c316451374c5af
Delegated to: Dan Williams
Headers show

Commit Message

Ezequiel Garcia Oct. 30, 2013, 3:01 p.m. UTC 
Despite requesting two memory resources, called 'base' and 'high_base', the
driver uses explicitly only the former. The latter is being used implicitly
by addressing at offset +0x200, which in practice accesses high_base.

In other words, the current driver breaks if the second memory resource
is ever place at an offset different from +0x200.

This patch fixes the above by defining the registers with the offset from
high_base, and use high_base explicitly where appropriate.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
v1-v2:
  * Re-write the commit message to make it clear this is
    a fix and not a cosmetic clean-up.

 drivers/dma/mv_xor.c |  3 ++-
 drivers/dma/mv_xor.h | 25 +++++++++++++------------
 2 files changed, 15 insertions(+), 13 deletions(-)

Comments

Ezequiel Garcia Nov. 5, 2013, 2:27 p.m. UTC | #1 
On Wed, Oct 30, 2013 at 12:01:43PM -0300, Ezequiel Garcia wrote:
> Despite requesting two memory resources, called 'base' and 'high_base', the
> driver uses explicitly only the former. The latter is being used implicitly
> by addressing at offset +0x200, which in practice accesses high_base.
> 
> In other words, the current driver breaks if the second memory resource
> is ever place at an offset different from +0x200.
> 
> This patch fixes the above by defining the registers with the offset from
> high_base, and use high_base explicitly where appropriate.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> v1-v2:
>   * Re-write the commit message to make it clear this is
>     a fix and not a cosmetic clean-up.
> 
>  drivers/dma/mv_xor.c |  3 ++-
>  drivers/dma/mv_xor.h | 25 +++++++++++++------------
>  2 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
> index d00d58e..c7da545 100644
> --- a/drivers/dma/mv_xor.c
> +++ b/drivers/dma/mv_xor.c
> @@ -1076,6 +1076,7 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
>  	}
>  
>  	mv_chan->mmr_base = xordev->xor_base;
> +	mv_chan->mmr_high_base = xordev->xor_high_base;
>  	tasklet_init(&mv_chan->irq_tasklet, mv_xor_tasklet, (unsigned long)
>  		     mv_chan);
>  
> @@ -1134,7 +1135,7 @@ static void
>  mv_xor_conf_mbus_windows(struct mv_xor_device *xordev,
>  			 const struct mbus_dram_target_info *dram)
>  {
> -	void __iomem *base = xordev->xor_base;
> +	void __iomem *base = xordev->xor_high_base;
>  	u32 win_enable = 0;
>  	int i;
>  
> diff --git a/drivers/dma/mv_xor.h b/drivers/dma/mv_xor.h
> index 06b067f..d074922 100644
> --- a/drivers/dma/mv_xor.h
> +++ b/drivers/dma/mv_xor.h
> @@ -34,13 +34,13 @@
>  #define XOR_OPERATION_MODE_MEMCPY	2
>  #define XOR_DESCRIPTOR_SWAP		BIT(14)
>  
> -#define XOR_CURR_DESC(chan)	(chan->mmr_base + 0x210 + (chan->idx * 4))
> -#define XOR_NEXT_DESC(chan)	(chan->mmr_base + 0x200 + (chan->idx * 4))
> -#define XOR_BYTE_COUNT(chan)	(chan->mmr_base + 0x220 + (chan->idx * 4))
> -#define XOR_DEST_POINTER(chan)	(chan->mmr_base + 0x2B0 + (chan->idx * 4))
> -#define XOR_BLOCK_SIZE(chan)	(chan->mmr_base + 0x2C0 + (chan->idx * 4))
> -#define XOR_INIT_VALUE_LOW(chan)	(chan->mmr_base + 0x2E0)
> -#define XOR_INIT_VALUE_HIGH(chan)	(chan->mmr_base + 0x2E4)
> +#define XOR_CURR_DESC(chan)	(chan->mmr_high_base + 0x10 + (chan->idx * 4))
> +#define XOR_NEXT_DESC(chan)	(chan->mmr_high_base + 0x00 + (chan->idx * 4))
> +#define XOR_BYTE_COUNT(chan)	(chan->mmr_high_base + 0x20 + (chan->idx * 4))
> +#define XOR_DEST_POINTER(chan)	(chan->mmr_high_base + 0xB0 + (chan->idx * 4))
> +#define XOR_BLOCK_SIZE(chan)	(chan->mmr_high_base + 0xC0 + (chan->idx * 4))
> +#define XOR_INIT_VALUE_LOW(chan)	(chan->mmr_high_base + 0xE0)
> +#define XOR_INIT_VALUE_HIGH(chan)	(chan->mmr_high_base + 0xE4)
>  
>  #define XOR_CONFIG(chan)	(chan->mmr_base + 0x10 + (chan->idx * 4))
>  #define XOR_ACTIVATION(chan)	(chan->mmr_base + 0x20 + (chan->idx * 4))
> @@ -50,11 +50,11 @@
>  #define XOR_ERROR_ADDR(chan)	(chan->mmr_base + 0x60)
>  #define XOR_INTR_MASK_VALUE	0x3F5
>  
> -#define WINDOW_BASE(w)		(0x250 + ((w) << 2))
> -#define WINDOW_SIZE(w)		(0x270 + ((w) << 2))
> -#define WINDOW_REMAP_HIGH(w)	(0x290 + ((w) << 2))
> -#define WINDOW_BAR_ENABLE(chan)	(0x240 + ((chan) << 2))
> -#define WINDOW_OVERRIDE_CTRL(chan)	(0x2A0 + ((chan) << 2))
> +#define WINDOW_BASE(w)		(0x50 + ((w) << 2))
> +#define WINDOW_SIZE(w)		(0x70 + ((w) << 2))
> +#define WINDOW_REMAP_HIGH(w)	(0x90 + ((w) << 2))
> +#define WINDOW_BAR_ENABLE(chan)	(0x40 + ((chan) << 2))
> +#define WINDOW_OVERRIDE_CTRL(chan)	(0xA0 + ((chan) << 2))
>  
>  struct mv_xor_device {
>  	void __iomem	     *xor_base;
> @@ -82,6 +82,7 @@ struct mv_xor_chan {
>  	int			pending;
>  	spinlock_t		lock; /* protects the descriptor slot pool */
>  	void __iomem		*mmr_base;
> +	void __iomem		*mmr_high_base;
>  	unsigned int		idx;
>  	int                     irq;
>  	enum dma_transaction_type	current_type;

Any comments on this two?
Dan Williams Nov. 7, 2013, 7:32 p.m. UTC | #2 
On Tue, Nov 5, 2013 at 6:27 AM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> On Wed, Oct 30, 2013 at 12:01:43PM -0300, Ezequiel Garcia wrote:
>
> Any comments on this two?
>

Look fine to me I'll queue them up.
--
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
diff mbox

Patch

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index d00d58e..c7da545 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1076,6 +1076,7 @@  mv_xor_channel_add(struct mv_xor_device *xordev,
 	}
 
 	mv_chan->mmr_base = xordev->xor_base;
+	mv_chan->mmr_high_base = xordev->xor_high_base;
 	tasklet_init(&mv_chan->irq_tasklet, mv_xor_tasklet, (unsigned long)
 		     mv_chan);
 
@@ -1134,7 +1135,7 @@  static void
 mv_xor_conf_mbus_windows(struct mv_xor_device *xordev,
 			 const struct mbus_dram_target_info *dram)
 {
-	void __iomem *base = xordev->xor_base;
+	void __iomem *base = xordev->xor_high_base;
 	u32 win_enable = 0;
 	int i;
 
diff --git a/drivers/dma/mv_xor.h b/drivers/dma/mv_xor.h
index 06b067f..d074922 100644
--- a/drivers/dma/mv_xor.h
+++ b/drivers/dma/mv_xor.h
@@ -34,13 +34,13 @@ 
 #define XOR_OPERATION_MODE_MEMCPY	2
 #define XOR_DESCRIPTOR_SWAP		BIT(14)
 
-#define XOR_CURR_DESC(chan)	(chan->mmr_base + 0x210 + (chan->idx * 4))
-#define XOR_NEXT_DESC(chan)	(chan->mmr_base + 0x200 + (chan->idx * 4))
-#define XOR_BYTE_COUNT(chan)	(chan->mmr_base + 0x220 + (chan->idx * 4))
-#define XOR_DEST_POINTER(chan)	(chan->mmr_base + 0x2B0 + (chan->idx * 4))
-#define XOR_BLOCK_SIZE(chan)	(chan->mmr_base + 0x2C0 + (chan->idx * 4))
-#define XOR_INIT_VALUE_LOW(chan)	(chan->mmr_base + 0x2E0)
-#define XOR_INIT_VALUE_HIGH(chan)	(chan->mmr_base + 0x2E4)
+#define XOR_CURR_DESC(chan)	(chan->mmr_high_base + 0x10 + (chan->idx * 4))
+#define XOR_NEXT_DESC(chan)	(chan->mmr_high_base + 0x00 + (chan->idx * 4))
+#define XOR_BYTE_COUNT(chan)	(chan->mmr_high_base + 0x20 + (chan->idx * 4))
+#define XOR_DEST_POINTER(chan)	(chan->mmr_high_base + 0xB0 + (chan->idx * 4))
+#define XOR_BLOCK_SIZE(chan)	(chan->mmr_high_base + 0xC0 + (chan->idx * 4))
+#define XOR_INIT_VALUE_LOW(chan)	(chan->mmr_high_base + 0xE0)
+#define XOR_INIT_VALUE_HIGH(chan)	(chan->mmr_high_base + 0xE4)
 
 #define XOR_CONFIG(chan)	(chan->mmr_base + 0x10 + (chan->idx * 4))
 #define XOR_ACTIVATION(chan)	(chan->mmr_base + 0x20 + (chan->idx * 4))
@@ -50,11 +50,11 @@ 
 #define XOR_ERROR_ADDR(chan)	(chan->mmr_base + 0x60)
 #define XOR_INTR_MASK_VALUE	0x3F5
 
-#define WINDOW_BASE(w)		(0x250 + ((w) << 2))
-#define WINDOW_SIZE(w)		(0x270 + ((w) << 2))
-#define WINDOW_REMAP_HIGH(w)	(0x290 + ((w) << 2))
-#define WINDOW_BAR_ENABLE(chan)	(0x240 + ((chan) << 2))
-#define WINDOW_OVERRIDE_CTRL(chan)	(0x2A0 + ((chan) << 2))
+#define WINDOW_BASE(w)		(0x50 + ((w) << 2))
+#define WINDOW_SIZE(w)		(0x70 + ((w) << 2))
+#define WINDOW_REMAP_HIGH(w)	(0x90 + ((w) << 2))
+#define WINDOW_BAR_ENABLE(chan)	(0x40 + ((chan) << 2))
+#define WINDOW_OVERRIDE_CTRL(chan)	(0xA0 + ((chan) << 2))
 
 struct mv_xor_device {
 	void __iomem	     *xor_base;
@@ -82,6 +82,7 @@  struct mv_xor_chan {
 	int			pending;
 	spinlock_t		lock; /* protects the descriptor slot pool */
 	void __iomem		*mmr_base;
+	void __iomem		*mmr_high_base;
 	unsigned int		idx;
 	int                     irq;
 	enum dma_transaction_type	current_type;