diff mbox

[v3,4/5] dmaengine: sun6i: Simplify some macros

Message ID a25c76bc3c63afc311069366af4970b909d356ef.1458311540.git.moinejf@free.fr (mailing list archive)
State New, archived
Headers show

Commit Message

Jean-Francois Moine March 18, 2016, 2:21 p.m. UTC
As the max burst and bus width values are now checked, there is
no reason to check them again when setting the hardware registers.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Maxime Ripard March 21, 2016, 7:48 a.m. UTC | #1
On Fri, Mar 18, 2016 at 03:21:11PM +0100, Jean-Francois Moine wrote:
> As the max burst and bus width values are now checked, there is
> no reason to check them again when setting the hardware registers.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
>  drivers/dma/sun6i-dma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 3f73123..643ba4c 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -65,8 +65,8 @@
>  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
>  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
>  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> -#define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
> +#define DMA_CHAN_CFG_SRC_BURST(x)	((x) << 7)
> +#define DMA_CHAN_CFG_SRC_WIDTH(x)	((x) << 9)

Well, you can easily end up with values higher than that. I still
believe that making sure you only always write what is expected is a
good thing.

Maxime
Jean-Francois Moine March 21, 2016, 2:40 p.m. UTC | #2
On Mon, 21 Mar 2016 08:48:43 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Fri, Mar 18, 2016 at 03:21:11PM +0100, Jean-Francois Moine wrote:
> > As the max burst and bus width values are now checked, there is
> > no reason to check them again when setting the hardware registers.
> > 
> > Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> > ---
> >  drivers/dma/sun6i-dma.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 3f73123..643ba4c 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -65,8 +65,8 @@
> >  #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
> >  #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
> >  #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
> > -#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
> > -#define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
> > +#define DMA_CHAN_CFG_SRC_BURST(x)	((x) << 7)
> > +#define DMA_CHAN_CFG_SRC_WIDTH(x)	((x) << 9)
> 
> Well, you can easily end up with values higher than that. I still
> believe that making sure you only always write what is expected is a
> good thing.

How the values could be higher? They are locally generated in
convert_burst() and convert_buswidth() from checked client values.
diff mbox

Patch

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 3f73123..643ba4c 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -65,8 +65,8 @@ 
 #define DMA_CHAN_CFG_SRC_DRQ(x)		((x) & 0x1f)
 #define DMA_CHAN_CFG_SRC_IO_MODE	BIT(5)
 #define DMA_CHAN_CFG_SRC_LINEAR_MODE	(0 << 5)
-#define DMA_CHAN_CFG_SRC_BURST(x)	(((x) & 0x3) << 7)
-#define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
+#define DMA_CHAN_CFG_SRC_BURST(x)	((x) << 7)
+#define DMA_CHAN_CFG_SRC_WIDTH(x)	((x) << 9)
 
 #define DMA_CHAN_CFG_DST_DRQ(x)		(DMA_CHAN_CFG_SRC_DRQ(x) << 16)
 #define DMA_CHAN_CFG_DST_IO_MODE	(DMA_CHAN_CFG_SRC_IO_MODE << 16)