Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 << x)'s

2013-04-10 Thread Arnd Bergmann
On Wednesday 10 April 2013, Lee Jones wrote:
> On Tue, 09 Apr 2013, Arnd Bergmann wrote:

> > Yes, good point. The macro is only used in one place, to compare
> > two compile-time constant values, but we should define macros in
> > drivers that are already provided by the kernel.
> 
> Okay, it looks like the generic implementation has brackets in it,
> which isn't allowed for this particular usecase.

Ah, right.

> I'll fix this one instead.

Probably not worth then. You could open-code the comparison where
it is used and remove the macro if you want, but I probably
would not bother.

Arnd

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


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 << x)'s

2013-04-10 Thread Lee Jones
On Tue, 09 Apr 2013, Arnd Bergmann wrote:

> On Tuesday 09 April 2013, Harvey Harrison wrote:
> > 
> > On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones  wrote:
> > >
> > > The aim is to make the code that little more readable.
> > >
> > > Signed-off-by: Lee Jones 
> > > ---
> > 
> > >
> > >  #define MAX(a, b) (((a) < (b)) ? (b) : (a))
> > 
> > Not part of your patch, but probably a good idea to switch to the
> > generic MAX macro, this
> > one is evaluating its args twice.
> 
> Yes, good point. The macro is only used in one place, to compare
> two compile-time constant values, but we should define macros in
> drivers that are already provided by the kernel.

Okay, it looks like the generic implementation has brackets in it,
which isn't allowed for this particular usecase. I'll fix this one
instead.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

2013-04-10 Thread Lee Jones
On Tue, 09 Apr 2013, Arnd Bergmann wrote:

 On Tuesday 09 April 2013, Harvey Harrison wrote:
  
  On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones lee.jo...@linaro.org wrote:
  
   The aim is to make the code that little more readable.
  
   Signed-off-by: Lee Jones lee.jo...@linaro.org
   ---
  
  
#define MAX(a, b) (((a)  (b)) ? (b) : (a))
  
  Not part of your patch, but probably a good idea to switch to the
  generic MAX macro, this
  one is evaluating its args twice.
 
 Yes, good point. The macro is only used in one place, to compare
 two compile-time constant values, but we should define macros in
 drivers that are already provided by the kernel.

Okay, it looks like the generic implementation has brackets in it,
which isn't allowed for this particular usecase. I'll fix this one
instead.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

2013-04-10 Thread Arnd Bergmann
On Wednesday 10 April 2013, Lee Jones wrote:
 On Tue, 09 Apr 2013, Arnd Bergmann wrote:

  Yes, good point. The macro is only used in one place, to compare
  two compile-time constant values, but we should define macros in
  drivers that are already provided by the kernel.
 
 Okay, it looks like the generic implementation has brackets in it,
 which isn't allowed for this particular usecase.

Ah, right.

 I'll fix this one instead.

Probably not worth then. You could open-code the comparison where
it is used and remove the macro if you want, but I probably
would not bother.

Arnd

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


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 << x)'s

2013-04-09 Thread Arnd Bergmann
On Tuesday 09 April 2013, Harvey Harrison wrote:
> 
> On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones  wrote:
> >
> > The aim is to make the code that little more readable.
> >
> > Signed-off-by: Lee Jones 
> > ---
> 
> >
> >  #define MAX(a, b) (((a) < (b)) ? (b) : (a))
> 
> Not part of your patch, but probably a good idea to switch to the
> generic MAX macro, this
> one is evaluating its args twice.

Yes, good point. The macro is only used in one place, to compare
two compile-time constant values, but we should define macros in
drivers that are already provided by the kernel.

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


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 << x)'s

2013-04-09 Thread Lee Jones
On Tue, 09 Apr 2013, Harvey Harrison wrote:

> On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones  wrote:
> >
> > The aim is to make the code that little more readable.
> >
> > Signed-off-by: Lee Jones 
> > ---
> 
> >
> >  #define MAX(a, b) (((a) < (b)) ? (b) : (a))
> 
> Not part of your patch, but probably a good idea to switch to the
> generic MAX macro, this
> one is evaluating its args twice.

Sure, thanks for the heads-up.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 << x)'s

2013-04-09 Thread Harvey Harrison
On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones  wrote:
>
> The aim is to make the code that little more readable.
>
> Signed-off-by: Lee Jones 
> ---

>
>  #define MAX(a, b) (((a) < (b)) ? (b) : (a))

Not part of your patch, but probably a good idea to switch to the
generic MAX macro, this
one is evaluating its args twice.

Cheers,

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


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 << x)'s

2013-04-09 Thread Arnd Bergmann
On Tuesday 09 April 2013, Lee Jones wrote:
> The aim is to make the code that little more readable.
> 
> Signed-off-by: Lee Jones 

I don't find the new version any more or less readable than the old one,
but I have no objections if other people think it's a good idea.

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


[PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 << x)'s

2013-04-09 Thread Lee Jones
The aim is to make the code that little more readable.

Signed-off-by: Lee Jones 
---
 drivers/dma/ste_dma40.c |   34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index c14db3e..7b96e75 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -49,9 +49,9 @@
 #define MAX_LCLA_ALLOC_ATTEMPTS 256
 
 /* Bit markings for allocation map */
-#define D40_ALLOC_FREE (1 << 31)
-#define D40_ALLOC_PHY  (1 << 30)
-#define D40_ALLOC_LOG_FREE 0
+#define D40_ALLOC_FREE BIT(31)
+#define D40_ALLOC_PHY  BIT(30)
+#define D40_ALLOC_LOG_FREE BIT(0)
 
 #define MAX(a, b) (((a) < (b)) ? (b) : (a))
 
@@ -993,12 +993,12 @@ static int d40_size_2_dmalen(int size, u32 data_width1, 
u32 data_width2)
int dmalen;
u32 max_w = max(data_width1, data_width2);
u32 min_w = min(data_width1, data_width2);
-   u32 seg_max = ALIGN(STEDMA40_MAX_SEG_SIZE << min_w, 1 << max_w);
+   u32 seg_max = ALIGN(STEDMA40_MAX_SEG_SIZE << min_w, BIT(max_w));
 
if (seg_max > STEDMA40_MAX_SEG_SIZE)
-   seg_max -= (1 << max_w);
+   seg_max -= BIT(max_w);
 
-   if (!IS_ALIGNED(size, 1 << max_w))
+   if (!IS_ALIGNED(size, BIT(max_w)))
return -EINVAL;
 
if (size <= seg_max)
@@ -1448,7 +1448,7 @@ static u32 d40_residue(struct d40_chan *d40c)
  >> D40_SREG_ELEM_PHY_ECNT_POS;
}
 
-   return num_elt * (1 << d40c->dma_cfg.dst_info.data_width);
+   return num_elt * BIT(d40c->dma_cfg.dst_info.data_width);
 }
 
 static bool d40_tx_is_linked(struct d40_chan *d40c)
@@ -1722,7 +1722,7 @@ static irqreturn_t d40_handle_interrupt(int irq, void 
*data)
}
 
/* ACK interrupt */
-   writel(1 << idx, base->virtbase + il[row].clr);
+   writel(BIT(idx), base->virtbase + il[row].clr);
 
spin_lock(>lock);
 
@@ -1804,9 +1804,9 @@ static int d40_validate_conf(struct d40_chan *d40c,
}
 
if (d40_psize_2_burst_size(is_log, conf->src_info.psize) *
-   (1 << conf->src_info.data_width) !=
+   BIT(conf->src_info.data_width) !=
d40_psize_2_burst_size(is_log, conf->dst_info.psize) *
-   (1 << conf->dst_info.data_width)) {
+   BIT(conf->dst_info.data_width)) {
/*
 * The DMAC hardware only supports
 * src (burst x width) == dst (burst x width)
@@ -1848,8 +1848,8 @@ static bool d40_alloc_mask_set(struct d40_phy_res *phy,
if (phy->allocated_src == D40_ALLOC_FREE)
phy->allocated_src = D40_ALLOC_LOG_FREE;
 
-   if (!(phy->allocated_src & (1 << log_event_line))) {
-   phy->allocated_src |= 1 << log_event_line;
+   if (!(phy->allocated_src & BIT(log_event_line))) {
+   phy->allocated_src |= BIT(log_event_line);
goto found;
} else
goto not_found;
@@ -1860,8 +1860,8 @@ static bool d40_alloc_mask_set(struct d40_phy_res *phy,
if (phy->allocated_dst == D40_ALLOC_FREE)
phy->allocated_dst = D40_ALLOC_LOG_FREE;
 
-   if (!(phy->allocated_dst & (1 << log_event_line))) {
-   phy->allocated_dst |= 1 << log_event_line;
+   if (!(phy->allocated_dst & BIT(log_event_line))) {
+   phy->allocated_dst |= BIT(log_event_line);
goto found;
} else
goto not_found;
@@ -1891,11 +1891,11 @@ static bool d40_alloc_mask_free(struct d40_phy_res 
*phy, bool is_src,
 
/* Logical channel */
if (is_src) {
-   phy->allocated_src &= ~(1 << log_event_line);
+   phy->allocated_src &= ~BIT(log_event_line);
if (phy->allocated_src == D40_ALLOC_LOG_FREE)
phy->allocated_src = D40_ALLOC_FREE;
} else {
-   phy->allocated_dst &= ~(1 << log_event_line);
+   phy->allocated_dst &= ~BIT(log_event_line);
if (phy->allocated_dst == D40_ALLOC_LOG_FREE)
phy->allocated_dst = D40_ALLOC_FREE;
}
@@ -2394,7 +2394,7 @@ static void __d40_set_prio_rt(struct d40_chan *d40c, int 
dev_type, bool src)
u32 rtreg;
u32 event = D40_TYPE_TO_EVENT(dev_type);
u32 group = D40_TYPE_TO_GROUP(dev_type);
-   u32 bit = 1 << event;
+   u32 bit = BIT(event);
u32 prioreg;
struct d40_gen_dmac *dmac = >base->gen_dmac;
 
-- 
1.7.10.4

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


[PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

2013-04-09 Thread Lee Jones
The aim is to make the code that little more readable.

Signed-off-by: Lee Jones lee.jo...@linaro.org
---
 drivers/dma/ste_dma40.c |   34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index c14db3e..7b96e75 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -49,9 +49,9 @@
 #define MAX_LCLA_ALLOC_ATTEMPTS 256
 
 /* Bit markings for allocation map */
-#define D40_ALLOC_FREE (1  31)
-#define D40_ALLOC_PHY  (1  30)
-#define D40_ALLOC_LOG_FREE 0
+#define D40_ALLOC_FREE BIT(31)
+#define D40_ALLOC_PHY  BIT(30)
+#define D40_ALLOC_LOG_FREE BIT(0)
 
 #define MAX(a, b) (((a)  (b)) ? (b) : (a))
 
@@ -993,12 +993,12 @@ static int d40_size_2_dmalen(int size, u32 data_width1, 
u32 data_width2)
int dmalen;
u32 max_w = max(data_width1, data_width2);
u32 min_w = min(data_width1, data_width2);
-   u32 seg_max = ALIGN(STEDMA40_MAX_SEG_SIZE  min_w, 1  max_w);
+   u32 seg_max = ALIGN(STEDMA40_MAX_SEG_SIZE  min_w, BIT(max_w));
 
if (seg_max  STEDMA40_MAX_SEG_SIZE)
-   seg_max -= (1  max_w);
+   seg_max -= BIT(max_w);
 
-   if (!IS_ALIGNED(size, 1  max_w))
+   if (!IS_ALIGNED(size, BIT(max_w)))
return -EINVAL;
 
if (size = seg_max)
@@ -1448,7 +1448,7 @@ static u32 d40_residue(struct d40_chan *d40c)
   D40_SREG_ELEM_PHY_ECNT_POS;
}
 
-   return num_elt * (1  d40c-dma_cfg.dst_info.data_width);
+   return num_elt * BIT(d40c-dma_cfg.dst_info.data_width);
 }
 
 static bool d40_tx_is_linked(struct d40_chan *d40c)
@@ -1722,7 +1722,7 @@ static irqreturn_t d40_handle_interrupt(int irq, void 
*data)
}
 
/* ACK interrupt */
-   writel(1  idx, base-virtbase + il[row].clr);
+   writel(BIT(idx), base-virtbase + il[row].clr);
 
spin_lock(d40c-lock);
 
@@ -1804,9 +1804,9 @@ static int d40_validate_conf(struct d40_chan *d40c,
}
 
if (d40_psize_2_burst_size(is_log, conf-src_info.psize) *
-   (1  conf-src_info.data_width) !=
+   BIT(conf-src_info.data_width) !=
d40_psize_2_burst_size(is_log, conf-dst_info.psize) *
-   (1  conf-dst_info.data_width)) {
+   BIT(conf-dst_info.data_width)) {
/*
 * The DMAC hardware only supports
 * src (burst x width) == dst (burst x width)
@@ -1848,8 +1848,8 @@ static bool d40_alloc_mask_set(struct d40_phy_res *phy,
if (phy-allocated_src == D40_ALLOC_FREE)
phy-allocated_src = D40_ALLOC_LOG_FREE;
 
-   if (!(phy-allocated_src  (1  log_event_line))) {
-   phy-allocated_src |= 1  log_event_line;
+   if (!(phy-allocated_src  BIT(log_event_line))) {
+   phy-allocated_src |= BIT(log_event_line);
goto found;
} else
goto not_found;
@@ -1860,8 +1860,8 @@ static bool d40_alloc_mask_set(struct d40_phy_res *phy,
if (phy-allocated_dst == D40_ALLOC_FREE)
phy-allocated_dst = D40_ALLOC_LOG_FREE;
 
-   if (!(phy-allocated_dst  (1  log_event_line))) {
-   phy-allocated_dst |= 1  log_event_line;
+   if (!(phy-allocated_dst  BIT(log_event_line))) {
+   phy-allocated_dst |= BIT(log_event_line);
goto found;
} else
goto not_found;
@@ -1891,11 +1891,11 @@ static bool d40_alloc_mask_free(struct d40_phy_res 
*phy, bool is_src,
 
/* Logical channel */
if (is_src) {
-   phy-allocated_src = ~(1  log_event_line);
+   phy-allocated_src = ~BIT(log_event_line);
if (phy-allocated_src == D40_ALLOC_LOG_FREE)
phy-allocated_src = D40_ALLOC_FREE;
} else {
-   phy-allocated_dst = ~(1  log_event_line);
+   phy-allocated_dst = ~BIT(log_event_line);
if (phy-allocated_dst == D40_ALLOC_LOG_FREE)
phy-allocated_dst = D40_ALLOC_FREE;
}
@@ -2394,7 +2394,7 @@ static void __d40_set_prio_rt(struct d40_chan *d40c, int 
dev_type, bool src)
u32 rtreg;
u32 event = D40_TYPE_TO_EVENT(dev_type);
u32 group = D40_TYPE_TO_GROUP(dev_type);
-   u32 bit = 1  event;
+   u32 bit = BIT(event);
u32 prioreg;
struct d40_gen_dmac *dmac = d40c-base-gen_dmac;
 
-- 
1.7.10.4

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


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

2013-04-09 Thread Arnd Bergmann
On Tuesday 09 April 2013, Lee Jones wrote:
 The aim is to make the code that little more readable.
 
 Signed-off-by: Lee Jones lee.jo...@linaro.org

I don't find the new version any more or less readable than the old one,
but I have no objections if other people think it's a good idea.

Acked-by: Arnd Bergmann a...@arndb.de
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

2013-04-09 Thread Harvey Harrison
On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones lee.jo...@linaro.org wrote:

 The aim is to make the code that little more readable.

 Signed-off-by: Lee Jones lee.jo...@linaro.org
 ---


  #define MAX(a, b) (((a)  (b)) ? (b) : (a))

Not part of your patch, but probably a good idea to switch to the
generic MAX macro, this
one is evaluating its args twice.

Cheers,

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


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

2013-04-09 Thread Lee Jones
On Tue, 09 Apr 2013, Harvey Harrison wrote:

 On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones lee.jo...@linaro.org wrote:
 
  The aim is to make the code that little more readable.
 
  Signed-off-by: Lee Jones lee.jo...@linaro.org
  ---
 
 
   #define MAX(a, b) (((a)  (b)) ? (b) : (a))
 
 Not part of your patch, but probably a good idea to switch to the
 generic MAX macro, this
 one is evaluating its args twice.

Sure, thanks for the heads-up.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 7/8] dmaengine: ste_dma40: Use the BIT macro to replace ugly '(1 x)'s

2013-04-09 Thread Arnd Bergmann
On Tuesday 09 April 2013, Harvey Harrison wrote:
 
 On Tue, Apr 9, 2013 at 11:39 AM, Lee Jones lee.jo...@linaro.org wrote:
 
  The aim is to make the code that little more readable.
 
  Signed-off-by: Lee Jones lee.jo...@linaro.org
  ---
 
 
   #define MAX(a, b) (((a)  (b)) ? (b) : (a))
 
 Not part of your patch, but probably a good idea to switch to the
 generic MAX macro, this
 one is evaluating its args twice.

Yes, good point. The macro is only used in one place, to compare
two compile-time constant values, but we should define macros in
drivers that are already provided by the kernel.

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