Hi Russell,
After merging the arm tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:
In file included from include/linux/bitops.h:36:0,
from include/linux/bitmap.h:7,
from drivers/dma/sun4i-dma.c:11:
drivers/dma/sun4i-dma.c: In function 'find_and_use_pchan':
include/linux/bitops.h:56:34: error: passing argument 1 of '_find_next_zero_bit_le' from incompatible pointer type [-Werror=incompatible-pointer-types]
for ((bit) = find_next_zero_bit((addr), (size), (bit)); \
^
arch/arm/include/asm/bitops.h:200:61: note: in definition of macro 'find_next_zero_bit'
#define find_next_zero_bit(p,sz,off) _find_next_zero_bit_le(p,sz,off)
^
drivers/dma/sun4i-dma.c:241:2: note: in expansion of macro 'for_each_clear_bit_from'
for_each_clear_bit_from(i, &priv->pchans_used, max) {
^
arch/arm/include/asm/bitops.h:163:12: note: expected 'const long unsigned int *' but argument is of type 'long unsigned int (*)[1]'
extern int _find_next_zero_bit_le(const unsigned long *p, int size, int offset);
^
include/linux/bitops.h:58:34: error: passing argument 1 of '_find_next_zero_bit_le' from incompatible pointer type [-Werror=incompatible-pointer-types]
(bit) = find_next_zero_bit((addr), (size), (bit) + 1))
^
arch/arm/include/asm/bitops.h:200:61: note: in definition of macro 'find_next_zero_bit'
#define find_next_zero_bit(p,sz,off) _find_next_zero_bit_le(p,sz,off)
^
drivers/dma/sun4i-dma.c:241:2: note: in expansion of macro 'for_each_clear_bit_from'
for_each_clear_bit_from(i, &priv->pchans_used, max) {
^
arch/arm/include/asm/bitops.h:163:12: note: expected 'const long unsigned int *' but argument is of type 'long unsigned int (*)[1]'
extern int _find_next_zero_bit_le(const unsigned long *p, int size, int offset);
^
Caused (or exposed) by commit
c4f8ff16b46b ("ARM: 8669/1: bitops: Align prototypes to generic API")
I have used the arm tree from next-20170420 for today.
--
Cheers,
Stephen Rothwell
On 21/04/2017 00:40, Stephen Rothwell wrote:
> After merging the arm tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
>
> In file included from include/linux/bitops.h:36:0,
> from include/linux/bitmap.h:7,
> from drivers/dma/sun4i-dma.c:11:
> drivers/dma/sun4i-dma.c: In function 'find_and_use_pchan':
> include/linux/bitops.h:56:34: error:
> passing argument 1 of '_find_next_zero_bit_le' from incompatible pointer type [-Werror=incompatible-pointer-types]
> for ((bit) = find_next_zero_bit((addr), (size), (bit)); \
> ^
> arch/arm/include/asm/bitops.h:200:61: note: in definition of macro 'find_next_zero_bit'
> #define find_next_zero_bit(p,sz,off) _find_next_zero_bit_le(p,sz,off)
> ^
> drivers/dma/sun4i-dma.c:241:2: note: in expansion of macro 'for_each_clear_bit_from'
> for_each_clear_bit_from(i, &priv->pchans_used, max) {
> ^
> arch/arm/include/asm/bitops.h:163:12: note:
> expected 'const long unsigned int *' but argument is of type 'long unsigned int (*)[1]'
> extern int _find_next_zero_bit_le(const unsigned long *p, int size, int offset);
> ^
> [...]
>
> Caused (or exposed) by commit
>
> c4f8ff16b46b ("ARM: 8669/1: bitops: Align prototypes to generic API")
>
> I have used the arm tree from next-20170420 for today.
Weird that I didn't catch this when I ran make allyesconfig.
https://www.spinics.net/lists/arm-kernel/msg573736.html
Anyway, the fix is trivial.
The "pchans_used" field is an unsigned long array.
for_each_clear_bit_from() expects an unsigned long pointer,
not an array address.
I'll send a patch to the drivers/dma maintainers.
$ make C=2 drivers/dma/sun4i-dma.o
CHECK drivers/dma/sun4i-dma.c
CC drivers/dma/sun4i-dma.o
Regards.
The "pchans_used" field is an unsigned long array.
for_each_clear_bit_from() expects an unsigned long pointer,
not an array address.
$ make C=2 drivers/dma/sun4i-dma.o
CHECK drivers/dma/sun4i-dma.c
drivers/dma/sun4i-dma.c:241:9: warning: incorrect type in argument 1 (different base types)
drivers/dma/sun4i-dma.c:241:9: expected unsigned long const *p
drivers/dma/sun4i-dma.c:241:9: got unsigned long ( *<noident> )[1]
Signed-off-by: Mason <[email protected]>
---
drivers/dma/sun4i-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
index 57aa227bfadb..f4ed3f17607c 100644
--- a/drivers/dma/sun4i-dma.c
+++ b/drivers/dma/sun4i-dma.c
@@ -238,7 +238,7 @@ static struct sun4i_dma_pchan *find_and_use_pchan(struct sun4i_dma_dev *priv,
}
spin_lock_irqsave(&priv->lock, flags);
- for_each_clear_bit_from(i, &priv->pchans_used, max) {
+ for_each_clear_bit_from(i, priv->pchans_used, max) {
pchan = &pchans[i];
pchan->vchan = vchan;
set_bit(i, priv->pchans_used);
--
3.14159
Hi Mason,
On Fri, 21 Apr 2017 09:58:58 +0200 Mason <[email protected]> wrote:
>
> Anyway, the fix is trivial.
>
> The "pchans_used" field is an unsigned long array.
> for_each_clear_bit_from() expects an unsigned long pointer,
> not an array address.
>
> I'll send a patch to the drivers/dma maintainers.
The fix really needs to go into the arm tree (as well?) since that is
the tree that has the patch that causes the build to break (even if the
actual bug was preexisting).
--
Cheers,
Stephen Rothwell
On Fri, Apr 21, 2017 at 10:06:10AM +0200, Mason wrote:
> The "pchans_used" field is an unsigned long array.
>
> for_each_clear_bit_from() expects an unsigned long pointer,
> not an array address.
>
> $ make C=2 drivers/dma/sun4i-dma.o
> CHECK drivers/dma/sun4i-dma.c
> drivers/dma/sun4i-dma.c:241:9: warning: incorrect type in argument 1 (different base types)
> drivers/dma/sun4i-dma.c:241:9: expected unsigned long const *p
> drivers/dma/sun4i-dma.c:241:9: got unsigned long ( *<noident> )[1]
The patch looks good...
> Signed-off-by: Mason <[email protected]>
However this doesn't.
See https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On 21/04/2017 10:12, Stephen Rothwell wrote:
> Mason wrote:
>
>> Anyway, the fix is trivial.
>>
>> The "pchans_used" field is an unsigned long array.
>> for_each_clear_bit_from() expects an unsigned long pointer,
>> not an array address.
>>
>> I'll send a patch to the drivers/dma maintainers.
>
> The fix really needs to go into the arm tree (as well?) since that is
> the tree that has the patch that causes the build to break (even if the
> actual bug was preexisting).
Hello Stephen,
Since it's a trivial patch, and since Vinod is on vacation
until Monday, I suppose Russell could push the patch through
his own tree? (Maybe after an ACK from a sunxi maintainer.)
I am currently building an allyesconfig next-20170420 kernel.
Considering the speed of this system, this will take a while.
Regards.
From: Marc Gonzalez <[email protected]>
The "pchans_used" field is an unsigned long array.
for_each_clear_bit_from() expects an unsigned long pointer,
not an array address.
$ make C=2 drivers/dma/sun4i-dma.o
CHECK drivers/dma/sun4i-dma.c
drivers/dma/sun4i-dma.c:241:9: warning: incorrect type in argument 1 (different base types)
drivers/dma/sun4i-dma.c:241:9: expected unsigned long const *p
drivers/dma/sun4i-dma.c:241:9: got unsigned long ( *<noident> )[1]
Signed-off-by: Marc Gonzalez <[email protected]>
---
drivers/dma/sun4i-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
index 57aa227bfadb..f4ed3f17607c 100644
--- a/drivers/dma/sun4i-dma.c
+++ b/drivers/dma/sun4i-dma.c
@@ -238,7 +238,7 @@ static struct sun4i_dma_pchan *find_and_use_pchan(struct sun4i_dma_dev *priv,
}
spin_lock_irqsave(&priv->lock, flags);
- for_each_clear_bit_from(i, &priv->pchans_used, max) {
+ for_each_clear_bit_from(i, priv->pchans_used, max) {
pchan = &pchans[i];
pchan->vchan = vchan;
set_bit(i, priv->pchans_used);
--
3.14159
On 21/04/2017 09:58, Mason wrote:
> Weird that I didn't catch this when I ran make allyesconfig.
Doh! make allyesconfig builds for BE systems.
CONFIG_CPU_BIG_ENDIAN=y
CONFIG_CPU_ENDIAN_BE8=y
But the patch I originally tested with only updated the LE bitops.
With the complete patch, I didn't see any build issues, other than
drivers/dma/sun4i-dma.c
Regards.
On Fri, Apr 21, 2017 at 10:43:20AM +0200, Mason wrote:
> From: Marc Gonzalez <[email protected]>
>
> The "pchans_used" field is an unsigned long array.
>
> for_each_clear_bit_from() expects an unsigned long pointer,
> not an array address.
>
> $ make C=2 drivers/dma/sun4i-dma.o
> CHECK drivers/dma/sun4i-dma.c
> drivers/dma/sun4i-dma.c:241:9: warning: incorrect type in argument 1 (different base types)
> drivers/dma/sun4i-dma.c:241:9: expected unsigned long const *p
> drivers/dma/sun4i-dma.c:241:9: got unsigned long ( *<noident> )[1]
>
> Signed-off-by: Marc Gonzalez <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Fri, Apr 21, 2017 at 06:12:30PM +1000, Stephen Rothwell wrote:
> Hi Mason,
>
> On Fri, 21 Apr 2017 09:58:58 +0200 Mason <[email protected]> wrote:
> >
> > Anyway, the fix is trivial.
> >
> > The "pchans_used" field is an unsigned long array.
> > for_each_clear_bit_from() expects an unsigned long pointer,
> > not an array address.
> >
> > I'll send a patch to the drivers/dma maintainers.
>
> The fix really needs to go into the arm tree (as well?) since that is
> the tree that has the patch that causes the build to break (even if the
> actual bug was preexisting).
Or I drop the offending patch (done) and we get the DMA subsystem fixed
first. Given how long it's been this way, I doubt there's any hurry to
get this change in for the next merge window.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
On 22/04/2017 01:43, Russell King - ARM Linux wrote:
> Or I drop the offending patch (done) and we get the DMA subsystem fixed
> first. Given how long it's been this way, I doubt there's any hurry to
> get this change in for the next merge window.
Your solution makes sense.
Vinod, could you apply [PATCH v2] dmaengine: sun4i: fix invalid argument
to your tree when you have the time?
Regards.
On Sat, Apr 22, 2017 at 10:41:37AM +0200, Mason wrote:
> On 22/04/2017 01:43, Russell King - ARM Linux wrote:
>
> > Or I drop the offending patch (done) and we get the DMA subsystem fixed
> > first. Given how long it's been this way, I doubt there's any hurry to
> > get this change in for the next merge window.
>
> Your solution makes sense.
>
> Vinod, could you apply [PATCH v2] dmaengine: sun4i: fix invalid argument
> to your tree when you have the time?
Done now..
--
~Vinod
From: Marc Gonzalez <[email protected]>
include/asm-generic/bitops/find.h declares:
extern unsigned long
find_first_zero_bit(const unsigned long *addr, unsigned long size);
while arch/arm/include/asm/bitops.h declares:
#define find_first_zero_bit(p,sz) _find_first_zero_bit_le(p,sz)
extern int _find_first_zero_bit_le(const void * p, unsigned size);
Align the arm prototypes to the generic API, to have gcc report
inadequate arguments, such as pointer to u32.
Signed-off-by: Marc Gonzalez <[email protected]>
---
The patch fixing drivers/dma/sun4i-dma.c
("dmaengine: sun4i: fix invalid argument")
has landed on linux-next (thanks Vinod)
as 57192245bc074710ea1a128d39ecc429455ac815
---
arch/arm/include/asm/bitops.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h
index e943e6cee254..f308c8c40cb9 100644
--- a/arch/arm/include/asm/bitops.h
+++ b/arch/arm/include/asm/bitops.h
@@ -159,16 +159,16 @@ extern int _test_and_change_bit(int nr, volatile unsigned long * p);
/*
* Little endian assembly bitops. nr = 0 -> byte 0 bit 0.
*/
-extern int _find_first_zero_bit_le(const void * p, unsigned size);
-extern int _find_next_zero_bit_le(const void * p, int size, int offset);
+extern int _find_first_zero_bit_le(const unsigned long *p, unsigned size);
+extern int _find_next_zero_bit_le(const unsigned long *p, int size, int offset);
extern int _find_first_bit_le(const unsigned long *p, unsigned size);
extern int _find_next_bit_le(const unsigned long *p, int size, int offset);
/*
* Big endian assembly bitops. nr = 0 -> byte 3 bit 0.
*/
-extern int _find_first_zero_bit_be(const void * p, unsigned size);
-extern int _find_next_zero_bit_be(const void * p, int size, int offset);
+extern int _find_first_zero_bit_be(const unsigned long *p, unsigned size);
+extern int _find_next_zero_bit_be(const unsigned long *p, int size, int offset);
extern int _find_first_bit_be(const unsigned long *p, unsigned size);
extern int _find_next_bit_be(const unsigned long *p, int size, int offset);
--
2.11.0
On 22/04/2017 01:43, Russell King - ARM Linux wrote:
> On Fri, Apr 21, 2017 at 06:12:30PM +1000, Stephen Rothwell wrote:
>> Hi Mason,
>>
>> On Fri, 21 Apr 2017 09:58:58 +0200 Mason <[email protected]> wrote:
>>>
>>> Anyway, the fix is trivial.
>>>
>>> The "pchans_used" field is an unsigned long array.
>>> for_each_clear_bit_from() expects an unsigned long pointer,
>>> not an array address.
>>>
>>> I'll send a patch to the drivers/dma maintainers.
>>
>> The fix really needs to go into the arm tree (as well?) since that is
>> the tree that has the patch that causes the build to break (even if the
>> actual bug was preexisting).
>
> Or I drop the offending patch (done) and we get the DMA subsystem fixed
> first. Given how long it's been this way, I doubt there's any hurry to
> get this change in for the next merge window.
Hello Russell,
Is it safe to apply the patch now?
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8679/1
Regards.