Re: [PATCH v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-06-11 Thread Fengguang Wu
On Thu, Jun 11, 2015 at 05:36:00PM -0700, Christopher Li wrote:
> n Sun, Apr 26, 2015 at 10:20 PM, Fengguang Wu  wrote:
> >> >
> >> > drivers/dma/xgene-dma.c:2088:1: sparse: symbol
> >> > '__UNIQUE_ID_author__COUNTER__' has multiple initializers (originally
> >> > initialized at drivers/dma/xgene-dma.c:2087)
> >> > So, I kept only one author here.
> >> No that is not right, sparse shouldn't have cribbed here.
> >>
> >> Fengguang can we get the bot to ignore this please
> 
> Sorry for the late reply.
> 
> That looks like the __COUNTER__ macro feature. It has been implemented in
> sparse git tree already.

That's great, I'll fetch the latest sparse code from

git://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git

Thanks,
Fengguang
--
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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-06-11 Thread Christopher Li
On Sun, Apr 26, 2015 at 10:20 PM, Fengguang Wu  wrote:
>> >
>> > drivers/dma/xgene-dma.c:2088:1: sparse: symbol
>> > '__UNIQUE_ID_author__COUNTER__' has multiple initializers (originally
>> > initialized at drivers/dma/xgene-dma.c:2087)
>> > So, I kept only one author here.
>> No that is not right, sparse shouldn't have cribbed here.
>>
>> Fengguang can we get the bot to ignore this please

Sorry for the late reply.

That looks like the __COUNTER__ macro feature. It has been implemented in
sparse git tree already.

Chris
--
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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-26 Thread Fengguang Wu
On Mon, Apr 27, 2015 at 08:43:15AM +0530, Vinod Koul wrote:
> On Mon, Apr 20, 2015 at 08:38:18AM +0530, Rameshwar Sahu wrote:
> > Hi Vinod,
> >> >> @@ -2085,6 +2043,5 @@ module_platform_driver(xgene_dma_driver);
> > >>
> > >>  MODULE_DESCRIPTION("APM X-Gene SoC DMA driver");
> > >>  MODULE_AUTHOR("Rameshwar Prasad Sahu ");
> > >> -MODULE_AUTHOR("Loc Ho ");
> > > And why this?
> > 
> > I saw below warning reported by the kbuild robot test
> > 
> > drivers/dma/xgene-dma.c:2088:1: sparse: symbol
> > '__UNIQUE_ID_author__COUNTER__' has multiple initializers (originally
> > initialized at drivers/dma/xgene-dma.c:2087)
> > So, I kept only one author here.
> No that is not right, sparse shouldn't have cribbed here.
> 
> Fengguang can we get the bot to ignore this please

OK, sorry for the noises! CC sparse maintainer btw.

Thanks,
Fengguang
--
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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-26 Thread Vinod Koul
On Mon, Apr 20, 2015 at 08:38:18AM +0530, Rameshwar Sahu wrote:
> Hi Vinod,
>> >> @@ -2085,6 +2043,5 @@ module_platform_driver(xgene_dma_driver);
> >>
> >>  MODULE_DESCRIPTION("APM X-Gene SoC DMA driver");
> >>  MODULE_AUTHOR("Rameshwar Prasad Sahu ");
> >> -MODULE_AUTHOR("Loc Ho ");
> > And why this?
> 
> I saw below warning reported by the kbuild robot test
> 
> drivers/dma/xgene-dma.c:2088:1: sparse: symbol
> '__UNIQUE_ID_author__COUNTER__' has multiple initializers (originally
> initialized at drivers/dma/xgene-dma.c:2087)
> So, I kept only one author here.
No that is not right, sparse shouldn't have cribbed here.

Fengguang can we get the bot to ignore this please

-- 
~Vinod

--
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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-19 Thread Rameshwar Sahu
Hi Vinod,

On Fri, Apr 17, 2015 at 11:59 PM, Vinod Koul  wrote:
>
>>   /* Get DMA error interrupt */
>> @@ -2076,7 +2035,6 @@ static struct platform_driver xgene_dma_driver = {
>>   .remove = xgene_dma_remove,
>>   .driver = {
>>   .name = "X-Gene-DMA",
>> - .owner = THIS_MODULE,
> I have already applied a patch for this

Okay, I didn't pull latest git

>
>>   .of_match_table = xgene_dma_of_match_ptr,
>>   },
>>  };
>> @@ -2085,6 +2043,5 @@ module_platform_driver(xgene_dma_driver);
>>
>>  MODULE_DESCRIPTION("APM X-Gene SoC DMA driver");
>>  MODULE_AUTHOR("Rameshwar Prasad Sahu ");
>> -MODULE_AUTHOR("Loc Ho ");
> And why this?

I saw below warning reported by the kbuild robot test

drivers/dma/xgene-dma.c:2088:1: sparse: symbol
'__UNIQUE_ID_author__COUNTER__' has multiple initializers (originally
initialized at drivers/dma/xgene-dma.c:2087)
So, I kept only one author here.


>
> Fixes looks good though
>
> --
> ~Vinod
>>  MODULE_LICENSE("GPL");
>>  MODULE_VERSION("1.0");
>> --
>> 1.8.2.1
>>
>
> --
--
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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-17 Thread Vinod Koul

>   /* Get DMA error interrupt */
> @@ -2076,7 +2035,6 @@ static struct platform_driver xgene_dma_driver = {
>   .remove = xgene_dma_remove,
>   .driver = {
>   .name = "X-Gene-DMA",
> - .owner = THIS_MODULE,
I have already applied a patch for this

>   .of_match_table = xgene_dma_of_match_ptr,
>   },
>  };
> @@ -2085,6 +2043,5 @@ module_platform_driver(xgene_dma_driver);
> 
>  MODULE_DESCRIPTION("APM X-Gene SoC DMA driver");
>  MODULE_AUTHOR("Rameshwar Prasad Sahu ");
> -MODULE_AUTHOR("Loc Ho ");
And why this?

Fixes looks good though

-- 
~Vinod
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION("1.0");
> --
> 1.8.2.1
> 

-- 
--
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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-17 Thread Arnd Bergmann
On Friday 17 April 2015 14:41:02 Rameshwar Sahu wrote:
> >>
> >> -static void *xgene_dma_lookup_ext8(u64 *desc, int idx)
> >> +static __le64 *xgene_dma_lookup_ext8(struct xgene_dma_desc_hw *desc, int 
> >> idx)
> >>  {
> >> - return (idx % 2) ? (desc + idx - 1) : (desc + idx + 1);
> >> + switch (idx) {
> >> + case 0:
> >> + return &desc->m1;
> >> + case 1:
> >> + return &desc->m0;
> >> + case 2:
> >> + return &desc->m3;
> >> + case 3:
> >> + return &desc->m2;
> >> + default:
> >> + pr_err("Invalid dma descriptor index\n");
> >> + }
> >> +
> >> + return NULL;
> >>  }
> >>
> > ...
> >
> >>   /* Set 1st to 5th source addresses */
> >>   for (i = 0; i < src_cnt; i++) {
> >>   len = *nbytes;
> >> - xgene_dma_set_src_buffer((i == 0) ? (desc1 + 8) :
> >> + xgene_dma_set_src_buffer((i == 0) ? &desc1->m1 :
> >>xgene_dma_lookup_ext8(desc2, i - 1),
> >>&len, &src[i]);
> >> - XGENE_DMA_DESC_MULTI_SET(desc1, scf[i], i);
> >> + desc1->m2 |= cpu_to_le64((scf[i] << ((i + 1) * 8)));
> >>   }
> >
> > If you just unroll this loop, you get code that is smaller and easier to
> > understand:
> >
> > /* Set 1st to 5th source addresses */
> > xgene_dma_set_src_buffer(&desc1->m1, &len, &src[0]);
> > xgene_dma_set_src_buffer(&desc2->m0, &len, &src[1]);
> > xgene_dma_set_src_buffer(&desc2->m3, &len, &src[2]);
> > xgene_dma_set_src_buffer(&desc2->m2, &len, &src[3]);
> > desc1->m2 |= cpu_to_le64(scf[0] | scf[1] << 8 | scf[2] << 16 | 
> > scf[3] << 24);
> 
> Actually here, in run time src_cnt value can be 2 or 3 upto 5, so we
> can't unroll it.

I see, sorry for misreading the code. The patch is good then as it is.

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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-17 Thread Rameshwar Sahu
Hi Arnd,

On Fri, Apr 17, 2015 at 2:19 PM, Arnd Bergmann  wrote:
> On Friday 17 April 2015 01:01:13 Rameshwar Prasad Sahu wrote:
>> v3 changes:
>>   * Minor changes in length setting in DMA descriptor
>>
>> v2 changes:
>>   * Code cleanup
>>   * Changed way of setting DMA descriptors for big-endian
>>
>> This patch fixes compilation sparse warnings like incorrect type in 
>> assignment
>> (different base types), cast to restricted __le64, symbol
>> '__UNIQUE_ID_author__COUNTER__' has multiple initializers etc and
>> coccinelle warnings (No need to set .owner here. The core will do it.)
>>
>> This patch is based on slave-dma / for-linus branch.
>> (commit: 9f2fd0dfa594d857fbdaeda523ff7a46f16567f5 [26/28]
>>   dmaengine: Add support for APM X-Gene SoC DMA engine driver)
>>
>> Reported-by: kbuild test robot 
>> Signed-off-by: Rameshwar Prasad Sahu 
>
> Looks good now,
>
> Reviewed-by: Arnd Bergmann 
>
> There is one small enhancement that you could still do and I'll shut up after
> that ;-)
>
>>
>> -static void *xgene_dma_lookup_ext8(u64 *desc, int idx)
>> +static __le64 *xgene_dma_lookup_ext8(struct xgene_dma_desc_hw *desc, int 
>> idx)
>>  {
>> - return (idx % 2) ? (desc + idx - 1) : (desc + idx + 1);
>> + switch (idx) {
>> + case 0:
>> + return &desc->m1;
>> + case 1:
>> + return &desc->m0;
>> + case 2:
>> + return &desc->m3;
>> + case 3:
>> + return &desc->m2;
>> + default:
>> + pr_err("Invalid dma descriptor index\n");
>> + }
>> +
>> + return NULL;
>>  }
>>
> ...
>
>>   /* Set 1st to 5th source addresses */
>>   for (i = 0; i < src_cnt; i++) {
>>   len = *nbytes;
>> - xgene_dma_set_src_buffer((i == 0) ? (desc1 + 8) :
>> + xgene_dma_set_src_buffer((i == 0) ? &desc1->m1 :
>>xgene_dma_lookup_ext8(desc2, i - 1),
>>&len, &src[i]);
>> - XGENE_DMA_DESC_MULTI_SET(desc1, scf[i], i);
>> + desc1->m2 |= cpu_to_le64((scf[i] << ((i + 1) * 8)));
>>   }
>
> If you just unroll this loop, you get code that is smaller and easier to
> understand:
>
> /* Set 1st to 5th source addresses */
> xgene_dma_set_src_buffer(&desc1->m1, &len, &src[0]);
> xgene_dma_set_src_buffer(&desc2->m0, &len, &src[1]);
> xgene_dma_set_src_buffer(&desc2->m3, &len, &src[2]);
> xgene_dma_set_src_buffer(&desc2->m2, &len, &src[3]);
> desc1->m2 |= cpu_to_le64(scf[0] | scf[1] << 8 | scf[2] << 16 | scf[3] 
> << 24);

Actually here, in run time src_cnt value can be 2 or 3 upto 5, so we
can't unroll it.


>
> 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 v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-17 Thread Arnd Bergmann
On Friday 17 April 2015 01:01:13 Rameshwar Prasad Sahu wrote:
> v3 changes:
>   * Minor changes in length setting in DMA descriptor
> 
> v2 changes:
>   * Code cleanup
>   * Changed way of setting DMA descriptors for big-endian
> 
> This patch fixes compilation sparse warnings like incorrect type in assignment
> (different base types), cast to restricted __le64, symbol
> '__UNIQUE_ID_author__COUNTER__' has multiple initializers etc and
> coccinelle warnings (No need to set .owner here. The core will do it.)
> 
> This patch is based on slave-dma / for-linus branch.
> (commit: 9f2fd0dfa594d857fbdaeda523ff7a46f16567f5 [26/28]
>   dmaengine: Add support for APM X-Gene SoC DMA engine driver)
> 
> Reported-by: kbuild test robot 
> Signed-off-by: Rameshwar Prasad Sahu 

Looks good now,

Reviewed-by: Arnd Bergmann 

There is one small enhancement that you could still do and I'll shut up after
that ;-)

> 
> -static void *xgene_dma_lookup_ext8(u64 *desc, int idx)
> +static __le64 *xgene_dma_lookup_ext8(struct xgene_dma_desc_hw *desc, int idx)
>  {
> - return (idx % 2) ? (desc + idx - 1) : (desc + idx + 1);
> + switch (idx) {
> + case 0:
> + return &desc->m1;
> + case 1:
> + return &desc->m0;
> + case 2:
> + return &desc->m3;
> + case 3:
> + return &desc->m2;
> + default:
> + pr_err("Invalid dma descriptor index\n");
> + }
> +
> + return NULL;
>  }
> 
...

>   /* Set 1st to 5th source addresses */
>   for (i = 0; i < src_cnt; i++) {
>   len = *nbytes;
> - xgene_dma_set_src_buffer((i == 0) ? (desc1 + 8) :
> + xgene_dma_set_src_buffer((i == 0) ? &desc1->m1 :
>xgene_dma_lookup_ext8(desc2, i - 1),
>&len, &src[i]);
> - XGENE_DMA_DESC_MULTI_SET(desc1, scf[i], i);
> + desc1->m2 |= cpu_to_le64((scf[i] << ((i + 1) * 8)));
>   }

If you just unroll this loop, you get code that is smaller and easier to
understand:

/* Set 1st to 5th source addresses */
xgene_dma_set_src_buffer(&desc1->m1, &len, &src[0]);
xgene_dma_set_src_buffer(&desc2->m0, &len, &src[1]);
xgene_dma_set_src_buffer(&desc2->m3, &len, &src[2]);
xgene_dma_set_src_buffer(&desc2->m2, &len, &src[3]);
desc1->m2 |= cpu_to_le64(scf[0] | scf[1] << 8 | scf[2] << 16 | scf[3] 
<< 24);

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/


[PATCH v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings

2015-04-16 Thread Rameshwar Prasad Sahu
v3 changes:
* Minor changes in length setting in DMA descriptor

v2 changes:
* Code cleanup
* Changed way of setting DMA descriptors for big-endian

This patch fixes compilation sparse warnings like incorrect type in assignment
(different base types), cast to restricted __le64, symbol
'__UNIQUE_ID_author__COUNTER__' has multiple initializers etc and
coccinelle warnings (No need to set .owner here. The core will do it.)

This patch is based on slave-dma / for-linus branch.
(commit: 9f2fd0dfa594d857fbdaeda523ff7a46f16567f5 [26/28]
dmaengine: Add support for APM X-Gene SoC DMA engine driver)

Reported-by: kbuild test robot 
Signed-off-by: Rameshwar Prasad Sahu 
---
 drivers/dma/xgene-dma.c | 191 +++-
 1 file changed, 74 insertions(+), 117 deletions(-)

diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c
index aa61935..722c684 100755
--- a/drivers/dma/xgene-dma.c
+++ b/drivers/dma/xgene-dma.c
@@ -124,32 +124,8 @@
 #define XGENE_DMA_DESC_ELERR_POS   46
 #define XGENE_DMA_DESC_RTYPE_POS   56
 #define XGENE_DMA_DESC_LERR_POS60
-#define XGENE_DMA_DESC_FLYBY_POS   4
 #define XGENE_DMA_DESC_BUFLEN_POS  48
 #define XGENE_DMA_DESC_HOENQ_NUM_POS   48
-
-#define XGENE_DMA_DESC_NV_SET(m)   \
-   (((u64 *)(m))[0] |= XGENE_DMA_DESC_NV_BIT)
-#define XGENE_DMA_DESC_IN_SET(m)   \
-   (((u64 *)(m))[0] |= XGENE_DMA_DESC_IN_BIT)
-#define XGENE_DMA_DESC_RTYPE_SET(m, v) \
-   (((u64 *)(m))[0] |= ((u64)(v) << XGENE_DMA_DESC_RTYPE_POS))
-#define XGENE_DMA_DESC_BUFADDR_SET(m, v)   \
-   (((u64 *)(m))[0] |= (v))
-#define XGENE_DMA_DESC_BUFLEN_SET(m, v)\
-   (((u64 *)(m))[0] |= ((u64)(v) << XGENE_DMA_DESC_BUFLEN_POS))
-#define XGENE_DMA_DESC_C_SET(m)\
-   (((u64 *)(m))[1] |= XGENE_DMA_DESC_C_BIT)
-#define XGENE_DMA_DESC_FLYBY_SET(m, v) \
-   (((u64 *)(m))[2] |= ((v) << XGENE_DMA_DESC_FLYBY_POS))
-#define XGENE_DMA_DESC_MULTI_SET(m, v, i)  \
-   (((u64 *)(m))[2] |= ((u64)(v) << (((i) + 1) * 8)))
-#define XGENE_DMA_DESC_DR_SET(m)   \
-   (((u64 *)(m))[2] |= XGENE_DMA_DESC_DR_BIT)
-#define XGENE_DMA_DESC_DST_ADDR_SET(m, v)  \
-   (((u64 *)(m))[3] |= (v))
-#define XGENE_DMA_DESC_H0ENQ_NUM_SET(m, v) \
-   (((u64 *)(m))[3] |= ((u64)(v) << XGENE_DMA_DESC_HOENQ_NUM_POS))
 #define XGENE_DMA_DESC_ELERR_RD(m) \
(((m) >> XGENE_DMA_DESC_ELERR_POS) & 0x3)
 #define XGENE_DMA_DESC_LERR_RD(m)  \
@@ -158,14 +134,7 @@
(((elerr) << 4) | (lerr))

 /* X-Gene DMA descriptor empty s/w signature */
-#define XGENE_DMA_DESC_EMPTY_INDEX 0
 #define XGENE_DMA_DESC_EMPTY_SIGNATURE ~0ULL
-#define XGENE_DMA_DESC_SET_EMPTY(m)\
-   (((u64 *)(m))[XGENE_DMA_DESC_EMPTY_INDEX] = \
-XGENE_DMA_DESC_EMPTY_SIGNATURE)
-#define XGENE_DMA_DESC_IS_EMPTY(m) \
-   (((u64 *)(m))[XGENE_DMA_DESC_EMPTY_INDEX] ==\
-XGENE_DMA_DESC_EMPTY_SIGNATURE)

 /* X-Gene DMA configurable parameters defines */
 #define XGENE_DMA_RING_NUM 512
@@ -184,7 +153,7 @@
 #define XGENE_DMA_XOR_ALIGNMENT6   /* 64 Bytes */
 #define XGENE_DMA_MAX_XOR_SRC  5
 #define XGENE_DMA_16K_BUFFER_LEN_CODE  0x0
-#define XGENE_DMA_INVALID_LEN_CODE 0x7800
+#define XGENE_DMA_INVALID_LEN_CODE 0x7800ULL

 /* X-Gene DMA descriptor error codes */
 #define ERR_DESC_AXI   0x01
@@ -214,10 +183,10 @@
 #define ERR_DESC_SRC_INT   0xB

 /* X-Gene DMA flyby operation code */
-#define FLYBY_2SRC_XOR 0x8
-#define FLYBY_3SRC_XOR 0x9
-#define FLYBY_4SRC_XOR 0xA
-#define FLYBY_5SRC_XOR 0xB
+#define FLYBY_2SRC_XOR 0x80
+#define FLYBY_3SRC_XOR 0x90
+#define FLYBY_4SRC_XOR 0xA0
+#define FLYBY_5SRC_XOR 0xB0

 /* X-Gene DMA SW descriptor flags */
 #define XGENE_DMA_FLAG_64B_DESCBIT(0)
@@ -238,10 +207,10 @@
dev_err(chan->dev, "%s: " fmt, chan->name, ##arg)

 struct xgene_dma_desc_hw {
-   u64 m0;
-   u64 m1;
-   u64 m2;
-   u64 m3;
+   __le64 m0;
+   __le64 m1;
+   __le64 m2;
+   __le64 m3;
 };

 enum xgene_dma_ring_cfgsize {
@@ -388,18 +357,11 @@ static bool is_pq_enabled(struct xgene_dma *pdma)
return !(val & XGENE_DMA_PQ_DISABLE_MASK);
 }

-static void xgene_dma_cpu_to_le64(u64 *desc, int count)
-{
-   int i;
-
-   for (i = 0; i < count; i++)
-   desc[i] = cpu_to_le64(desc[i]);
-}
-
-static u16 xgene_dma_encode_len(u32 len)
+static u64 xgene_dma_encode_len(size_t len)
 {
return (len < XGENE_DMA_MAX_BYTE_CNT) ?
-   len : XGENE_DMA_16K_BUFFER_LEN_CODE;
+   ((u64)len << XGENE_DMA_DESC_BUFLEN_POS) :