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-06-11 Thread Christopher Li 
On Sun, Apr 26, 2015 at 10:20 PM, Fengguang Wu fengguang...@intel.com 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-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 fengguang...@intel.com 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-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-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 rs...@apm.com);
   -MODULE_AUTHOR(Loc Ho l...@apm.com);
   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 rs...@apm.com);
  -MODULE_AUTHOR(Loc Ho l...@apm.com);
  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-19 Thread Rameshwar Sahu 
Hi Vinod,

On Fri, Apr 17, 2015 at 11:59 PM, Vinod Koul vinod.k...@intel.com 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 rs...@apm.com);
 -MODULE_AUTHOR(Loc Ho l...@apm.com);
 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 >m1;
> >> + case 1:
> >> + return >m0;
> >> + case 2:
> >> + return >m3;
> >> + case 3:
> >> + return >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) ? >m1 :
> >>xgene_dma_lookup_ext8(desc2, i - 1),
> >>, [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(>m1, , [0]);
> > xgene_dma_set_src_buffer(>m0, , [1]);
> > xgene_dma_set_src_buffer(>m3, , [2]);
> > xgene_dma_set_src_buffer(>m2, , [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 >m1;
>> + case 1:
>> + return >m0;
>> + case 2:
>> + return >m3;
>> + case 3:
>> + return >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) ? >m1 :
>>xgene_dma_lookup_ext8(desc2, i - 1),
>>, [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(>m1, , [0]);
> xgene_dma_set_src_buffer(>m0, , [1]);
> xgene_dma_set_src_buffer(>m3, , [2]);
> xgene_dma_set_src_buffer(>m2, , [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 >m1;
> + case 1:
> + return >m0;
> + case 2:
> + return >m3;
> + case 3:
> + return >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) ? >m1 :
>xgene_dma_lookup_ext8(desc2, i - 1),
>, [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(>m1, , [0]);
xgene_dma_set_src_buffer(>m0, , [1]);
xgene_dma_set_src_buffer(>m3, , [2]);
xgene_dma_set_src_buffer(>m2, , [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/

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 fengguang...@intel.com
 Signed-off-by: Rameshwar Prasad Sahu rs...@apm.com

Looks good now,

Reviewed-by: Arnd Bergmann a...@arndb.de

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/

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 a...@arndb.de 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 fengguang...@intel.com
 Signed-off-by: Rameshwar Prasad Sahu rs...@apm.com

 Looks good now,

 Reviewed-by: Arnd Bergmann a...@arndb.de

 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 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 rs...@apm.com);
 -MODULE_AUTHOR(Loc Ho l...@apm.com);
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/

[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) :

[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 fengguang...@intel.com
Signed-off-by: Rameshwar Prasad Sahu rs...@apm.com
---
 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