Hi Boris,
On Mon, May 14, 2018 at 2:13 PM, Boris Brezillon
<boris.brezillon@xxxxxxxxxxx> wrote:
On Mon, 14 May 2018 14:00:19 +0200
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
On Mon, May 14, 2018 at 1:46 PM, Boris Brezillon
<boris.brezillon@xxxxxxxxxxx> wrote:
On Mon, 14 May 2018 13:32:30 +0200
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
On Mon, May 14, 2018 at 1:23 PM, Boris Brezillon
<boris.brezillon@xxxxxxxxxxx> wrote:
On Mon, 14 May 2018 12:49:37 +0200
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
The __DIVIDE() macro checks whether it is called with a 32-bit or 64-bit
dividend, to select the appropriate divide-and-round-up routine.
As the check uses the ternary operator, the result will always be
promoted to a type that can hold both results, i.e. unsigned long long.
When using this result in a division on a 32-bit system, this may lead
to link errors like:
ERROR: "__udivdi3" [drivers/mtd/nand/raw/nand.ko] undefined!
Fix this by casting the result of the 64-bit division to the type of the
dividend.
Fixes: 8878b126df769831 ("mtd: nand: add ->exec_op() implementation")
Signed-off-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
---
This fixes the root cause of the link failure seen with
m68k/allmodconfig since commit 3057fcef385348fe ("mtd: rawnand: Make
sure we wait tWB before polling the STATUS reg").
An alternative mitigation was posted as "[PATCH] m68k: Implement
ndelay() as an inline function to force type checking/casting"
(https://lkml.org/lkml/2018/5/13/102).
---
include/linux/mtd/rawnand.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 5dad59b312440a9c..d06dc428ea0102ae 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -871,7 +871,7 @@ struct nand_op_instr {
#define __DIVIDE(dividend, divisor) ({ \
sizeof(dividend) == sizeof(u32) ? \
DIV_ROUND_UP(dividend, divisor) : \
- DIV_ROUND_UP_ULL(dividend, divisor); \
+ (__typeof__(dividend))DIV_ROUND_UP_ULL(dividend, divisor); \
Hm, it's a bit hard to follow when you place the cast here. One could
wonder why a cast to (__typeof__(dividend)) is needed since
DIV_ROUND_UP_ULL() already returns a (__typeof__(dividend)) type.
DIV_ROUND_UP_ULL() does not return __typeof__(dividend), but
unsigned long long.
Except if you entered this branch, that means you passed an unsigned
long long dividend (AKA u64), otherwise you would go in DIV_ROUND_UP().
Am I missing something?
Sure, inside that branch, it does.
But the compiler considers the whole ternary operator construction, i.e.
both branches.
Yes, and that's my point. The (__typeof__(dividend)) when placed like
you did is ambiguous. It looks like you're doing a useless cast, while
what you're actually fixing is the case where dividend is an u32 (AKA
unsigned long), and from the reader PoV, the code you're fixing
shouldn't even be reached. Hence my suggestion to move the case one
level up and add a comment ;-).
OK.
How about:
/*
* Cast to type of dividend is needed here to guarantee that the
* result won't be an unsigned long long when the dividend is an
* unsigned long, which is what the compiler does when it sees a
s/an unsigned long/32-bit/
* ternary operator with 2 different return types.
*/
(__typeof__(dividend))(sizeof(dividend) == sizeof(u32) ? \
To be completely safe and handle cases where dividend is an unsigned
short or an unsigned, we should probably have:
(__typeof__(dividend))(sizeof(dividend) == sizeof(unsigned long long) ? \
"> sizeof(u32)"?
/me starts to think about uint128_t...
sizeof(dividend) <= sizeof(unsigned long) ?
DIV_ROUND_UP(dividend, divisor) :
DIV_ROUND_UP_ULL(dividend, divisor);
Problem solved :-)
BTW, this will still fail (silently) with uint128_t. But we don't care.
And it will do the right thing when passed an unsigned long on 64-bit
systems.
Is the following version okay?
I think it is.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
