Re: [PATCH v3] dmaengine: xgene-dma: Fix sparse wannings and coccinelle warnings
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
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
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
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
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
> /* 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
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
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
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
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) :