* [PATCH v1 2/4] dmaengine: Use negative condition for better readability
2020-02-26 10:18 [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only Andy Shevchenko
@ 2020-02-26 10:18 ` Andy Shevchenko
2020-02-26 12:36 ` Peter Ujfalusi
2020-02-26 10:18 ` [PATCH v1 3/4] dmaengine: Drop redundant 'else' keyword Andy Shevchenko
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-26 10:18 UTC (permalink / raw)
To: Vinod Koul, dmaengine, Dan Williams, Peter Ujfalusi; +Cc: Andy Shevchenko
When negative condition is in use we may decrease indentation level
and make the main part of logic better visible.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/dmaengine.h | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9f3f5582816a..ae56a91c2a05 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -618,10 +618,11 @@ static inline void dmaengine_unmap_put(struct dmaengine_unmap_data *unmap)
static inline void dma_descriptor_unmap(struct dma_async_tx_descriptor *tx)
{
- if (tx->unmap) {
- dmaengine_unmap_put(tx->unmap);
- tx->unmap = NULL;
- }
+ if (!tx->unmap)
+ return;
+
+ dmaengine_unmap_put(tx->unmap);
+ tx->unmap = NULL;
}
#ifndef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
@@ -1408,11 +1409,12 @@ static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
static inline void
dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
{
- if (st) {
- st->last = last;
- st->used = used;
- st->residue = residue;
- }
+ if (!st)
+ return;
+
+ st->last = last;
+ st->used = used;
+ st->residue = residue;
}
#ifdef CONFIG_DMA_ENGINE
@@ -1489,12 +1491,11 @@ static inline int dmaengine_desc_set_reuse(struct dma_async_tx_descriptor *tx)
if (ret)
return ret;
- if (caps.descriptor_reuse) {
- tx->flags |= DMA_CTRL_REUSE;
- return 0;
- } else {
+ if (!caps.descriptor_reuse)
return -EPERM;
- }
+
+ tx->flags |= DMA_CTRL_REUSE;
+ return 0;
}
static inline void dmaengine_desc_clear_reuse(struct dma_async_tx_descriptor *tx)
@@ -1510,10 +1511,10 @@ static inline bool dmaengine_desc_test_reuse(struct dma_async_tx_descriptor *tx)
static inline int dmaengine_desc_free(struct dma_async_tx_descriptor *desc)
{
/* this is supported for reusable desc, so check that */
- if (dmaengine_desc_test_reuse(desc))
- return desc->desc_free(desc);
- else
+ if (!dmaengine_desc_test_reuse(desc))
return -EPERM;
+
+ return desc->desc_free(desc);
}
/* --- DMA device --- */
--
2.25.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 2/4] dmaengine: Use negative condition for better readability
2020-02-26 10:18 ` [PATCH v1 2/4] dmaengine: Use negative condition for better readability Andy Shevchenko
@ 2020-02-26 12:36 ` Peter Ujfalusi
0 siblings, 0 replies; 10+ messages in thread
From: Peter Ujfalusi @ 2020-02-26 12:36 UTC (permalink / raw)
To: Andy Shevchenko, Vinod Koul, dmaengine, Dan Williams
Hi Andy,
On 2/26/20 12:18 PM, Andy Shevchenko wrote:
> When negative condition is in use we may decrease indentation level
> and make the main part of logic better visible.
It makes the code a bit nicer, I agree.
Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> include/linux/dmaengine.h | 35 ++++++++++++++++++-----------------
> 1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 9f3f5582816a..ae56a91c2a05 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -618,10 +618,11 @@ static inline void dmaengine_unmap_put(struct dmaengine_unmap_data *unmap)
>
> static inline void dma_descriptor_unmap(struct dma_async_tx_descriptor *tx)
> {
> - if (tx->unmap) {
> - dmaengine_unmap_put(tx->unmap);
> - tx->unmap = NULL;
> - }
> + if (!tx->unmap)
> + return;
> +
> + dmaengine_unmap_put(tx->unmap);
> + tx->unmap = NULL;
> }
>
> #ifndef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
> @@ -1408,11 +1409,12 @@ static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
> static inline void
> dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
> {
> - if (st) {
> - st->last = last;
> - st->used = used;
> - st->residue = residue;
> - }
> + if (!st)
> + return;
> +
> + st->last = last;
> + st->used = used;
> + st->residue = residue;
> }
>
> #ifdef CONFIG_DMA_ENGINE
> @@ -1489,12 +1491,11 @@ static inline int dmaengine_desc_set_reuse(struct dma_async_tx_descriptor *tx)
> if (ret)
> return ret;
>
> - if (caps.descriptor_reuse) {
> - tx->flags |= DMA_CTRL_REUSE;
> - return 0;
> - } else {
> + if (!caps.descriptor_reuse)
> return -EPERM;
> - }
> +
> + tx->flags |= DMA_CTRL_REUSE;
> + return 0;
> }
>
> static inline void dmaengine_desc_clear_reuse(struct dma_async_tx_descriptor *tx)
> @@ -1510,10 +1511,10 @@ static inline bool dmaengine_desc_test_reuse(struct dma_async_tx_descriptor *tx)
> static inline int dmaengine_desc_free(struct dma_async_tx_descriptor *desc)
> {
> /* this is supported for reusable desc, so check that */
> - if (dmaengine_desc_test_reuse(desc))
> - return desc->desc_free(desc);
> - else
> + if (!dmaengine_desc_test_reuse(desc))
> return -EPERM;
> +
> + return desc->desc_free(desc);
> }
>
> /* --- DMA device --- */
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 3/4] dmaengine: Drop redundant 'else' keyword
2020-02-26 10:18 [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only Andy Shevchenko
2020-02-26 10:18 ` [PATCH v1 2/4] dmaengine: Use negative condition for better readability Andy Shevchenko
@ 2020-02-26 10:18 ` Andy Shevchenko
2020-02-26 12:37 ` Peter Ujfalusi
2020-02-26 10:18 ` [PATCH v1 4/4] dmaengine: consistently return string literal from switch-case Andy Shevchenko
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-26 10:18 UTC (permalink / raw)
To: Vinod Koul, dmaengine, Dan Williams, Peter Ujfalusi; +Cc: Andy Shevchenko
It's obvious that 'else' keyword is redundant in the code like
if (foo)
return bar;
else if (baz)
...
Drop it for good.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/dmaengine.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index ae56a91c2a05..1bb5477ef7ec 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1230,9 +1230,9 @@ static inline int dma_maxpq(struct dma_device *dma, enum dma_ctrl_flags flags)
{
if (dma_dev_has_pq_continue(dma) || !dmaf_continue(flags))
return dma_dev_to_maxpq(dma);
- else if (dmaf_p_disabled_continue(flags))
+ if (dmaf_p_disabled_continue(flags))
return dma_dev_to_maxpq(dma) - 1;
- else if (dmaf_continue(flags))
+ if (dmaf_continue(flags))
return dma_dev_to_maxpq(dma) - 3;
BUG();
}
@@ -1243,7 +1243,7 @@ static inline size_t dmaengine_get_icg(bool inc, bool sgl, size_t icg,
if (inc) {
if (dir_icg)
return dir_icg;
- else if (sgl)
+ if (sgl)
return icg;
}
--
2.25.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 3/4] dmaengine: Drop redundant 'else' keyword
2020-02-26 10:18 ` [PATCH v1 3/4] dmaengine: Drop redundant 'else' keyword Andy Shevchenko
@ 2020-02-26 12:37 ` Peter Ujfalusi
2020-02-26 13:44 ` Andy Shevchenko
0 siblings, 1 reply; 10+ messages in thread
From: Peter Ujfalusi @ 2020-02-26 12:37 UTC (permalink / raw)
To: Andy Shevchenko, Vinod Koul, dmaengine, Dan Williams
Hi Andy,
On 2/26/20 12:18 PM, Andy Shevchenko wrote:
> It's obvious that 'else' keyword is redundant in the code like
>
> if (foo)
> return bar;
> else if (baz)
> ...
>
> Drop it for good.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> include/linux/dmaengine.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index ae56a91c2a05..1bb5477ef7ec 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1230,9 +1230,9 @@ static inline int dma_maxpq(struct dma_device *dma, enum dma_ctrl_flags flags)
> {
> if (dma_dev_has_pq_continue(dma) || !dmaf_continue(flags))
> return dma_dev_to_maxpq(dma);
> - else if (dmaf_p_disabled_continue(flags))
> + if (dmaf_p_disabled_continue(flags))
I would add blank line in between the - new - if cases for better
readability.
> return dma_dev_to_maxpq(dma) - 1;
> - else if (dmaf_continue(flags))
> + if (dmaf_continue(flags))
> return dma_dev_to_maxpq(dma) - 3;
> BUG();
> }
> @@ -1243,7 +1243,7 @@ static inline size_t dmaengine_get_icg(bool inc, bool sgl, size_t icg,
> if (inc) {
> if (dir_icg)
> return dir_icg;
> - else if (sgl)
> + if (sgl)
> return icg;
> }
>
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1 3/4] dmaengine: Drop redundant 'else' keyword
2020-02-26 12:37 ` Peter Ujfalusi
@ 2020-02-26 13:44 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-26 13:44 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: Vinod Koul, dmaengine, Dan Williams
On Wed, Feb 26, 2020 at 02:37:27PM +0200, Peter Ujfalusi wrote:
> On 2/26/20 12:18 PM, Andy Shevchenko wrote:
> > It's obvious that 'else' keyword is redundant in the code like
> >
> > if (foo)
> > return bar;
> > else if (baz)
> > ...
> >
> > Drop it for good.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > include/linux/dmaengine.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index ae56a91c2a05..1bb5477ef7ec 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -1230,9 +1230,9 @@ static inline int dma_maxpq(struct dma_device *dma, enum dma_ctrl_flags flags)
> > {
> > if (dma_dev_has_pq_continue(dma) || !dmaf_continue(flags))
> > return dma_dev_to_maxpq(dma);
> > - else if (dmaf_p_disabled_continue(flags))
> > + if (dmaf_p_disabled_continue(flags))
>
> I would add blank line in between the - new - if cases for better
> readability.
Thank you for review and comment.
Here I have opposite opinion, but let Vinod and Dan, as maintainers, to decide.
I'll be not against it if it's preferred way.
> > return dma_dev_to_maxpq(dma) - 1;
> > - else if (dmaf_continue(flags))
> > + if (dmaf_continue(flags))
> > return dma_dev_to_maxpq(dma) - 3;
> > BUG();
> > }
> > @@ -1243,7 +1243,7 @@ static inline size_t dmaengine_get_icg(bool inc, bool sgl, size_t icg,
> > if (inc) {
> > if (dir_icg)
> > return dir_icg;
> > - else if (sgl)
> > + if (sgl)
> > return icg;
> > }
> >
> >
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 4/4] dmaengine: consistently return string literal from switch-case
2020-02-26 10:18 [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only Andy Shevchenko
2020-02-26 10:18 ` [PATCH v1 2/4] dmaengine: Use negative condition for better readability Andy Shevchenko
2020-02-26 10:18 ` [PATCH v1 3/4] dmaengine: Drop redundant 'else' keyword Andy Shevchenko
@ 2020-02-26 10:18 ` Andy Shevchenko
2020-02-26 12:35 ` Peter Ujfalusi
2020-02-26 12:35 ` [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only Peter Ujfalusi
2020-03-02 7:18 ` Vinod Koul
4 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2020-02-26 10:18 UTC (permalink / raw)
To: Vinod Koul, dmaengine, Dan Williams, Peter Ujfalusi; +Cc: Andy Shevchenko
There is no need to have 'break;' statement in the default case followed by
return certain string literal when all other cases have returned the string
literals. So, refactor it accordingly.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/dmaengine.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 1bb5477ef7ec..d3672f065a64 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1560,9 +1560,7 @@ dmaengine_get_direction_text(enum dma_transfer_direction dir)
case DMA_DEV_TO_DEV:
return "DEV_TO_DEV";
default:
- break;
+ return "invalid";
}
-
- return "invalid";
}
#endif /* DMAENGINE_H */
--
2.25.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1 4/4] dmaengine: consistently return string literal from switch-case
2020-02-26 10:18 ` [PATCH v1 4/4] dmaengine: consistently return string literal from switch-case Andy Shevchenko
@ 2020-02-26 12:35 ` Peter Ujfalusi
0 siblings, 0 replies; 10+ messages in thread
From: Peter Ujfalusi @ 2020-02-26 12:35 UTC (permalink / raw)
To: Andy Shevchenko, Vinod Koul, dmaengine, Dan Williams
Hi Andy,
On 2/26/20 12:18 PM, Andy Shevchenko wrote:
> There is no need to have 'break;' statement in the default case followed by
> return certain string literal when all other cases have returned the string
> literals. So, refactor it accordingly.
Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> include/linux/dmaengine.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 1bb5477ef7ec..d3672f065a64 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1560,9 +1560,7 @@ dmaengine_get_direction_text(enum dma_transfer_direction dir)
> case DMA_DEV_TO_DEV:
> return "DEV_TO_DEV";
> default:
> - break;
> + return "invalid";
> }
> -
> - return "invalid";
> }
> #endif /* DMAENGINE_H */
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only
2020-02-26 10:18 [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only Andy Shevchenko
` (2 preceding siblings ...)
2020-02-26 10:18 ` [PATCH v1 4/4] dmaengine: consistently return string literal from switch-case Andy Shevchenko
@ 2020-02-26 12:35 ` Peter Ujfalusi
2020-03-02 7:18 ` Vinod Koul
4 siblings, 0 replies; 10+ messages in thread
From: Peter Ujfalusi @ 2020-02-26 12:35 UTC (permalink / raw)
To: Andy Shevchenko, Vinod Koul, dmaengine, Dan Williams
On 2/26/20 12:18 PM, Andy Shevchenko wrote:
> There is no need to have branch and temporary variable in the function.
> Simple convert it to be a set of bit and arithmetic operations.
Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> include/linux/dmaengine.h | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 64461fc64e1b..9f3f5582816a 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1155,14 +1155,7 @@ static inline dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc
> static inline bool dmaengine_check_align(enum dmaengine_alignment align,
> size_t off1, size_t off2, size_t len)
> {
> - size_t mask;
> -
> - if (!align)
> - return true;
> - mask = (1 << align) - 1;
> - if (mask & (off1 | off2 | len))
> - return false;
> - return true;
> + return !(((1 << align) - 1) & (off1 | off2 | len));
> }
>
> static inline bool is_dma_copy_aligned(struct dma_device *dev, size_t off1,
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only
2020-02-26 10:18 [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only Andy Shevchenko
` (3 preceding siblings ...)
2020-02-26 12:35 ` [PATCH v1 1/4] dmaengine: Refactor dmaengine_check_align() to be bit operations only Peter Ujfalusi
@ 2020-03-02 7:18 ` Vinod Koul
4 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2020-03-02 7:18 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: dmaengine, Dan Williams, Peter Ujfalusi
On 26-02-20, 12:18, Andy Shevchenko wrote:
> There is no need to have branch and temporary variable in the function.
> Simple convert it to be a set of bit and arithmetic operations.
Applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 10+ messages in thread