[PATCH/RFC] m68k/bitops: Make bitmap data pointer of atomic ops volatile

[Geert Uytterhoeven]

This fixes a.o.

drivers/ide/ide-io.c: In function =E2=80=98ide_lock_host=E2=80=99:
drivers/ide/ide-io.c:415: warning: passing argument 2 of =E2=80=98__con=
stant_test_and_set_bit=E2=80=99 discards qualifiers from pointer target=
type
drivers/ide/ide-io.c:415: warning: passing argument 2 of =E2=80=98__gen=
eric_test_and_set_bit=E2=80=99 discards qualifiers from pointer target =
type

Suggested-by: Ben Hutchings <***@decadent.org.uk>
Signed-off-by: Geert Uytterhoeven <***@linux-m68k.org>
---
I'm not 100% sure whether all of these should be volatile.
We're only getting compiler warnings for calls to test_and_set_bit().

arch/m68k/include/asm/bitops_mm.h | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/m68k/include/asm/bitops_mm.h b/arch/m68k/include/asm/=
bitops_mm.h
index 89cf5b8..2d31d5f 100644
--- a/arch/m68k/include/asm/bitops_mm.h
+++ b/arch/m68k/include/asm/bitops_mm.h
@@ -27,7 +27,8 @@
=20
#define __test_and_set_bit(nr,vaddr) test_and_set_bit(nr,vaddr)
=20
-static inline int __constant_test_and_set_bit(int nr, unsigned long *v=
addr)
+static inline int __constant_test_and_set_bit(int nr,
+ volatile unsigned long *vaddr)
{
char *p =3D (char *)vaddr + (nr ^ 31) / 8;
char retval;
@@ -39,7 +40,8 @@ static inline int __constant_test_and_set_bit(int nr,=
unsigned long *vaddr)
return retval;
}
=20
-static inline int __generic_test_and_set_bit(int nr, unsigned long *va=
ddr)
+static inline int __generic_test_and_set_bit(int nr,
+ volatile unsigned long *vaddr)
{
char retval;
=20
@@ -76,7 +78,8 @@ static inline void __generic_set_bit(int nr, volatile=
unsigned long *vaddr)
=20
#define __test_and_clear_bit(nr,vaddr) test_and_clear_bit(nr,vaddr)
=20
-static inline int __constant_test_and_clear_bit(int nr, unsigned long =
*vaddr)
+static inline int __constant_test_and_clear_bit(int nr,
+ volatile unsigned long *vaddr)
{
char *p =3D (char *)vaddr + (nr ^ 31) / 8;
char retval;
@@ -88,7 +91,8 @@ static inline int __constant_test_and_clear_bit(int n=
r, unsigned long *vaddr)
return retval;
}
=20
-static inline int __generic_test_and_clear_bit(int nr, unsigned long *=
vaddr)
+static inline int __generic_test_and_clear_bit(int nr,
+ volatile unsigned long *vaddr)
{
char retval;
=20
@@ -131,7 +135,8 @@ static inline void __generic_clear_bit(int nr, vola=
tile unsigned long *vaddr)
#define __test_and_change_bit(nr,vaddr) test_and_change_bit(nr,vaddr)
#define __change_bit(nr,vaddr) change_bit(nr,vaddr)
=20
-static inline int __constant_test_and_change_bit(int nr, unsigned long=
*vaddr)
+static inline int __constant_test_and_change_bit(int nr,
+ volatile unsigned long *vaddr)
{
char *p =3D (char *)vaddr + (nr ^ 31) / 8;
char retval;
@@ -143,7 +148,8 @@ static inline int __constant_test_and_change_bit(in=
t nr, unsigned long *vaddr)
return retval;
}
=20
-static inline int __generic_test_and_change_bit(int nr, unsigned long =
*vaddr)
+static inline int __generic_test_and_change_bit(int nr,
+ volatile unsigned long *vaddr)
{
char retval;
=20
@@ -158,14 +164,14 @@ static inline int __generic_test_and_change_bit(i=
nt nr, unsigned long *vaddr)
__constant_change_bit(nr, vaddr) : \
__generic_change_bit(nr, vaddr))
=20
-static inline void __constant_change_bit(int nr, unsigned long *vaddr)
+static inline void __constant_change_bit(int nr, volatile unsigned lon=
g *vaddr)
{
char *p =3D (char *)vaddr + (nr ^ 31) / 8;
__asm__ __volatile__ ("bchg %1,%0"
: "+m" (*p) : "di" (nr & 7));
}
=20
-static inline void __generic_change_bit(int nr, unsigned long *vaddr)
+static inline void __generic_change_bit(int nr, volatile unsigned long=
*vaddr)
{
__asm__ __volatile__ ("bfchg %1{%0:#1}"
: : "d" (nr^31), "o" (*vaddr) : "memory");
--=20
1.7.0.4

[Arnd Bergmann]

Post by Geert Uytterhoeven
=20
This fixes a.o.
=20
drivers/ide/ide-io.c:415: warning: passing argument 2 of =E2=80=98__c=

onstant_test_and_set_bit=E2=80=99 discards qualifiers from pointer targ=
et type

Post by Geert Uytterhoeven
drivers/ide/ide-io.c:415: warning: passing argument 2 of =E2=80=98__g=

eneric_test_and_set_bit=E2=80=99 discards qualifiers from pointer targe=
t type

Post by Geert Uytterhoeven
=20

I think the correct fix would be to mark the variable not volatile, as =
it
clearly has no business be marked as such. That doesn't mean your patch
is wrong, though. It probably doesn't hurt to do both.

Arnd

[Geert Uytterhoeven]

Post by Geert Uytterhoeven
This fixes a.o.
drivers/ide/ide-io.c:415: warning: passing argument 2 of =E2=80=98__=

constant_test_and_set_bit=E2=80=99 discards qualifiers from pointer tar=
get type

Post by Geert Uytterhoeven
drivers/ide/ide-io.c:415: warning: passing argument 2 of =E2=80=98__=

generic_test_and_set_bit=E2=80=99 discards qualifiers from pointer targ=
et type

I think the correct fix would be to mark the variable not volatile, a=

s it

clearly has no business be marked as such. That doesn't mean your pat=

ch

You mean the host_busy variable in the IDE code?
That would also apply to context_flag in the DRM code:

drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
=E2=80=98__constant_test_and_set_bit=E2=80=99 discards qualifiers from =
pointer target
type
drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
=E2=80=98__generic_test_and_set_bit=E2=80=99 discards qualifiers from p=
ointer target
type

is wrong, though. It probably doesn't hurt to do both.

asm-generic/bitops/atomic.h has the volatiles everywhere. That's why
I'm wondering.

Gr{oetje,eeting}s,

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-=
m68k.org

In personal conversations with technical people, I call myself a hacker=
=2E But
when I'm talking to journalists I just say "programmer" or something li=
ke that.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Arnd Bergmann]

Post by Geert Uytterhoeven
You mean the host_busy variable in the IDE code?
=20
drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
=E2=80=98__constant_test_and_set_bit=E2=80=99 discards qualifiers fro=

m pointer target

Post by Geert Uytterhoeven
type
drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
=E2=80=98__generic_test_and_set_bit=E2=80=99 discards qualifiers from=

pointer target

Post by Geert Uytterhoeven
type

Yes, that fits the same category.

Post by Geert Uytterhoeven

Post by Arnd Bergmann
is wrong, though. It probably doesn't hurt to do both.

=20
asm-generic/bitops/atomic.h has the volatiles everywhere. That's why
I'm wondering.

I guess what happened is that some variables are traditionally marked
as volatile although they shouldn't be, and most architectures have
adapted their bitops to make the warnings go away. If you see more
warnings of that kind, it's probably fine to just do the same on m68k.
The volatile modifier doesn't really hurt in this case.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Ben Hutchings]

Post by Arnd Bergmann

Post by Geert Uytterhoeven
You mean the host_busy variable in the IDE code?
=20
drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
=E2=80=98__constant_test_and_set_bit=E2=80=99 discards qualifiers f=

rom pointer target

Post by Arnd Bergmann

Post by Geert Uytterhoeven
type
drivers/gpu/drm/drm_context.c:233: warning: passing argument 2 of
=E2=80=98__generic_test_and_set_bit=E2=80=99 discards qualifiers fr=

om pointer target

Post by Arnd Bergmann

Post by Geert Uytterhoeven
type

=20
Yes, that fits the same category.
=20

Post by Geert Uytterhoeven

Post by Arnd Bergmann
is wrong, though. It probably doesn't hurt to do both.

=20
asm-generic/bitops/atomic.h has the volatiles everywhere. That's wh=

y

Post by Arnd Bergmann

Post by Geert Uytterhoeven
I'm wondering.

=20
I guess what happened is that some variables are traditionally marked
as volatile although they shouldn't be, and most architectures have
adapted their bitops to make the warnings go away. If you see more
warnings of that kind, it's probably fine to just do the same on m68k=

=2E

Post by Arnd Bergmann
The volatile modifier doesn't really hurt in this case.

=20
These operations are required to be atomic and therefore they
must be suitable for use with volatile-qualified variables.

Ben.

--=20
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert =
Camus

[Arnd Bergmann]

Post by Ben Hutchings

Post by Arnd Bergmann
I guess what happened is that some variables are traditionally marked
as volatile although they shouldn't be, and most architectures have
adapted their bitops to make the warnings go away. If you see more
warnings of that kind, it's probably fine to just do the same on m68k.
The volatile modifier doesn't really hurt in this case.

These operations are required to be atomic and therefore they
must be suitable for use with volatile-qualified variables.

As I said, it's not wrong for them to have a volatile qualifier in the
argument list. However, there should also not be the need for the
qualifier in any of the callers, because the bitops only work if
all accesses to the data are done through bitops functions, and that
means that the qualifier on the variable is completely meaningless.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html