linux-next: manual merge of the akpm tree with the openrisc tree

linux-next: manual merge of the akpm tree with the openrisc tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

Hi Andrew,

Today's linux-next merge of the scsi-post-merge tree got a conflict in
arch/x86/include/asm/delay.h between commit 10f53642f115 ("asm-generic:
move archictures to common delay.h") from the openrisc tree and commit
"With a non-constant 8-bit argument, a call to udelay() generates a
warning:" from the akpm tree.

I just dropped this patch from the akpm tree as the former commit
consolidated this code into asm-generic/delay.h (which doesn't solve the
problem).
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: manual merge of the akpm tree with the openrisc tree

[Jonas Bonn]

On Mon, 2011-07-18 at 18:56 +1000, Stephen Rothwell wrote:
> Hi Andrew,
> 
> Today's linux-next merge of the scsi-post-merge tree got a conflict in
> arch/x86/include/asm/delay.h between commit 10f53642f115 ("asm-generic:
> move archictures to common delay.h") from the openrisc tree and commit
> "With a non-constant 8-bit argument, a call to udelay() generates a
> warning:" from the akpm tree.
> 
> I just dropped this patch from the akpm tree as the former commit
> consolidated this code into asm-generic/delay.h (which doesn't solve the
> problem).

Applying the change from Andrew's patch to asm-generic/delay.h looks
like the sane thing to do.  I can do that in my tree and then Andrew can
drop the patch from his tree... does that sound reasonable?

Just so others don't have to go digging for it, the patch in question
is:

http://userweb.kernel.org/~akpm/mmotm/broken-out/arch-x86-include-asm-delayh-fix-udelay-and-ndelay-for-8-bit-args.patch

/Jonas

[PATCH 1/1] asm-generic: delay.h fix udelay and ndelay for 8 bit args

[Jonas Bonn]

From: Andrew Morton <akpm@linux-foundation.org>

With a non-constant 8-bit argument, a call to udelay() generates a warning:

drivers/gpu/drm/radeon/atom.c: In function 'atom_op_delay':
drivers/gpu/drm/radeon/atom.c:654: warning: comparison is always false due to limited range of data type

The code looks like it works OK with an 8-bit arg, and the calling code is
doing nothing wrong, so udelay() needs fixing.

Fixing it was rather tricky.  Simply typecasting `n' in the comparison with
20000 didn't change anything.  Hence the divide-by-20000 trick.

Using a do{}while loop didn't work because udelay() is used in ?: statements,
hence the ({...}) construct.

While I was there I replaced the brain-bending ?:?:?: mess with nice if/else
code.

Probably other architectures are generating the same warning and can use a
similar change.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <linux-arch@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jonas Bonn <jonas@southpole.se>
---

Here's a patch that should resolve the merge conflict.  This applies
Andrew's changes on top of the new asm-generic/delay.h instead of the x86
arch-specific one.  I've tested this for OpenRISC and the changed macros
don't cause any problems there.

Let me know if this is OK and I'll throw it into the OpenRISC tree together
with the other delay.h modifications.

Thanks,
Jonas

 include/asm-generic/delay.h |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/include/asm-generic/delay.h b/include/asm-generic/delay.h
index 6511b99..0f79054 100644
--- a/include/asm-generic/delay.h
+++ b/include/asm-generic/delay.h
@@ -10,14 +10,35 @@ extern void __ndelay(unsigned long nsecs);
 extern void __const_udelay(unsigned long xloops);
 extern void __delay(unsigned long loops);
 
+/*
+ * The weird n/20000 thing suppresses a "comparison is always false due to
+ * limited range of data type" warning with non-const 8-bit arguments.
+ */
+
 /* 0x10c7 is 2**32 / 1000000 (rounded up) */
-#define udelay(n) (__builtin_constant_p(n) ? \
-	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c7ul)) : \
-	__udelay(n))
+#define udelay(n)							\
+	({								\
+		if (__builtin_constant_p(n)) {				\
+			if ((n) / 20000 >= 1)				\
+				 __bad_udelay();			\
+			else						\
+				__const_udelay((n) * 0x10c7ul);		\
+		} else {						\
+			__udelay(n);					\
+		}							\
+	})
 
 /* 0x5 is 2**32 / 1000000000 (rounded up) */
-#define ndelay(n) (__builtin_constant_p(n) ? \
-	((n) > 20000 ? __bad_ndelay() : __const_udelay((n) * 5ul)) : \
-	__ndelay(n))
+#define ndelay(n)							\
+	({								\
+		if (__builtin_constant_p(n)) {				\
+			if ((n) / 20000 >= 1)				\
+				__bad_ndelay();				\
+			else						\
+				__const_udelay((n) * 5ul);		\
+		} else {						\
+			__ndelay(n);					\
+		}							\
+	})
 
 #endif /* __ASM_GENERIC_DELAY_H */
-- 
1.7.4.1

Re: [PATCH 1/1] asm-generic: delay.h fix udelay and ndelay for 8 bit args

[Jonas Bonn]

On Mon, 2011-07-18 at 16:05 +0200, Jonas Bonn wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
> 
> With a non-constant 8-bit argument, a call to udelay() generates a warning:
> 
<snip>

> ---
> 
> Here's a patch that should resolve the merge conflict.  This applies
> Andrew's changes on top of the new asm-generic/delay.h instead of the x86
> arch-specific one.  I've tested this for OpenRISC and the changed macros
> don't cause any problems there.
> 
> Let me know if this is OK and I'll throw it into the OpenRISC tree together
> with the other delay.h modifications.
> 

I haven't gotten any feedback on this... since the change looks
appropriate to merge with the other changes to asm-generic/delay.h, I'll
apply this patch there and carry the patch together with the other
asm-generic/delay.h changes in the 'openrisc' tree.  Andrew can drop the
patch from his series to avoid the merge conflict.

I hope that works for everybody...

Thanks,
Jonas

Re: [PATCH 1/1] asm-generic: delay.h fix udelay and ndelay for 8 bit args

[H. Peter Anvin]

On 07/22/2011 12:04 PM, Jonas Bonn wrote:
> 
> On Mon, 2011-07-18 at 16:05 +0200, Jonas Bonn wrote:
>> From: Andrew Morton <akpm@linux-foundation.org>
>>
>> With a non-constant 8-bit argument, a call to udelay() generates a warning:
>>
> <snip>
> 
>> ---
>>
>> Here's a patch that should resolve the merge conflict.  This applies
>> Andrew's changes on top of the new asm-generic/delay.h instead of the x86
>> arch-specific one.  I've tested this for OpenRISC and the changed macros
>> don't cause any problems there.
>>
>> Let me know if this is OK and I'll throw it into the OpenRISC tree together
>> with the other delay.h modifications.
>>
> 
> I haven't gotten any feedback on this... since the change looks
> appropriate to merge with the other changes to asm-generic/delay.h, I'll
> apply this patch there and carry the patch together with the other
> asm-generic/delay.h changes in the 'openrisc' tree.  Andrew can drop the
> patch from his series to avoid the merge conflict.
> 
> I hope that works for everybody...
> 

You may want to Cc: Arnd on this, since he's the asm-generic maintainer.

Otherwise, since this is zero work on our part, it obviously works for me.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 1/1] asm-generic: delay.h fix udelay and ndelay for 8 bit args

[Jonas Bonn]

On Fri, 2011-07-22 at 12:11 -0700, H. Peter Anvin wrote:
> On 07/22/2011 12:04 PM, Jonas Bonn wrote:
> > 
> > On Mon, 2011-07-18 at 16:05 +0200, Jonas Bonn wrote:
> >> From: Andrew Morton <akpm@linux-foundation.org>
> >>
> >> With a non-constant 8-bit argument, a call to udelay() generates a warning:
> >>
> > <snip>
> > 
> >> ---
> >>
> >> Here's a patch that should resolve the merge conflict.  This applies
> >> Andrew's changes on top of the new asm-generic/delay.h instead of the x86
> >> arch-specific one.  I've tested this for OpenRISC and the changed macros
> >> don't cause any problems there.
> >>
> >> Let me know if this is OK and I'll throw it into the OpenRISC tree together
> >> with the other delay.h modifications.
> >>
> > 
> > I haven't gotten any feedback on this... since the change looks
> > appropriate to merge with the other changes to asm-generic/delay.h, I'll
> > apply this patch there and carry the patch together with the other
> > asm-generic/delay.h changes in the 'openrisc' tree.  Andrew can drop the
> > patch from his series to avoid the merge conflict.
> > 
> > I hope that works for everybody...
> > 
> 
> You may want to Cc: Arnd on this, since he's the asm-generic maintainer.
> 
> Otherwise, since this is zero work on our part, it obviously works for me.
> 

Good idea!

CC:ing Arnd, too...

Arnd has already reviewed the changes to delay.h.  This is only about
getting Andrew's fixes out of his tree where they cause a merge conflict
and into asm-generic/delay.h where they belong.

Thanks,
Jonas