With 2.6.25-rc3 and a config file with
CONFIG_CRYPTO_DEV_HIFN_795X=m
CONFIG_CRYPTO_DEV_HIFN_795X_RNG=y
I get the following build error on at least ARM and MIPS:
Building modules, stage 2.
MODPOST 759 modules
ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
--
Martin Michlmayr
http://www.cyrius.com/
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 3110bf7..92c53ce 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -825,8 +825,8 @@ static int hifn_register_rng(struct hifn_device *dev)
/*
* We must wait at least 256 Pk_clk cycles between two reads of the rng.
*/
- dev->rng_wait_time = DIV_ROUND_UP(NSEC_PER_SEC, dev->pk_clk_freq) *
- 256;
+ dev->rng_wait_time = DIV_ROUND_UP((unsigned int)NSEC_PER_SEC,
+ dev->pk_clk_freq) * 256;
dev->rng.name = dev->name;
dev->rng.data_present = hifn_rng_data_present,
* Patrick McHardy <[email protected]> [2008-02-26 13:28]:
>> I get the following build error on at least ARM and MIPS:
>> Building modules, stage 2.
>> MODPOST 759 modules
>> ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
>
> Does this patch fix it?
Nope.
--
Martin Michlmayr
http://www.cyrius.com/
On Tue, Feb 26, 2008 at 01:21:00PM +0100, Martin Michlmayr wrote:
> With 2.6.25-rc3 and a config file with
>
> CONFIG_CRYPTO_DEV_HIFN_795X=m
> CONFIG_CRYPTO_DEV_HIFN_795X_RNG=y
>
> I get the following build error on at least ARM and MIPS:
>
> Building modules, stage 2.
> MODPOST 759 modules
> ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
Fix below.
> Martin Michlmayr
cu
Adrian
<-- snip -->
Using ndelay() with a 64bit variable as parameter can result in build
errors like the following on some 32bit systems when it results in a
64bit division:
<-- snip -->
...
MODPOST 759 modules
ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
<-- snip -->
Reported by Martin Michlmayr.
Signed-off-by: Adrian Bunk <[email protected]>
---
40b45041ddc587c20b872a86a6a36952c28b02c7 diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 3110bf7..b1541c6 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -807,7 +807,7 @@ static int hifn_rng_data_present(struct hwrng *rng, int wait)
return 1;
if (!wait)
return 0;
- ndelay(nsec);
+ ndelay((u32)nsec);
return 1;
}
* Adrian Bunk <[email protected]> [2008-02-26 17:34]:
> > MODPOST 759 modules
> > ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
>
> Fix below.
Tested-by: Martin Michlmayr <[email protected]>
--
Martin Michlmayr
http://www.cyrius.com/
Hi Adrian.
On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk ([email protected]) wrote:
> Using ndelay() with a 64bit variable as parameter can result in build
> errors like the following on some 32bit systems when it results in a
> 64bit division:
>
> <-- snip -->
>
> ...
> MODPOST 759 modules
> ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
>
> <-- snip -->
>
> Reported by Martin Michlmayr.
>
> Signed-off-by: Adrian Bunk <[email protected]>
Yep, ndelay() uses division, thanks a lot Adrian for spotting this.
Herbert, please apply.
ACK.
--
Evgeniy Polyakov
On Tue, 26 Feb 2008 21:52:40 +0300 Evgeniy Polyakov <[email protected]> wrote:
> Hi Adrian.
>
> On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk ([email protected]) wrote:
> > Using ndelay() with a 64bit variable as parameter can result in build
> > errors like the following on some 32bit systems when it results in a
> > 64bit division:
> >
> > <-- snip -->
> >
> > ...
> > MODPOST 759 modules
> > ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
> >
> > <-- snip -->
> >
> > Reported by Martin Michlmayr.
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
>
> Yep, ndelay() uses division, thanks a lot Adrian for spotting this.
hm. Where?
> Herbert, please apply.
>
> ACK.
udelay() might be exposed to the same problem. It would be better to fix
ndelay() and udelay() rather than callers. It is reasonable to pass a u64
into ndelay() and to expect the build to not explode.
(Geeze macros suck)
On Tue, 26 Feb 2008, Martin Michlmayr wrote:
> With 2.6.25-rc3 and a config file with
>
> CONFIG_CRYPTO_DEV_HIFN_795X=m
> CONFIG_CRYPTO_DEV_HIFN_795X_RNG=y
>
> I get the following build error on at least ARM and MIPS:
>
> Building modules, stage 2.
> MODPOST 759 modules
> ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
>
crypto: use do_div() when registering the rng
Use do_div() instead of the divide operator.
Cc: Patrick McHardy <[email protected]>
Cc: Herbert Xu <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
Looks like this was triggered by f881d829, which causes this code to be
compiled for CONFIG_CRYPTO_DEV_HIFN_795X_RNG.
drivers/crypto/hifn_795x.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -822,12 +822,14 @@ static int hifn_rng_data_read(struct hwrng *rng, u32 *data)
static int hifn_register_rng(struct hifn_device *dev)
{
+ unsigned int tmp;
+
/*
* We must wait at least 256 Pk_clk cycles between two reads of the rng.
*/
- dev->rng_wait_time = DIV_ROUND_UP(NSEC_PER_SEC, dev->pk_clk_freq) *
- 256;
-
+ tmp = NSEC_PER_SEC + dev->pk_clk_freq - 1;
+ do_div(tmp, dev->pk_clk_freq);
+ dev->rng_wait_time = tmp * 256;
dev->rng.name = dev->name;
dev->rng.data_present = hifn_rng_data_present,
dev->rng.data_read = hifn_rng_data_read,
On Tue, Feb 26, 2008 at 04:04:39PM -0800, Andrew Morton wrote:
> On Tue, 26 Feb 2008 21:52:40 +0300 Evgeniy Polyakov <[email protected]> wrote:
>
> > Hi Adrian.
> >
> > On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk ([email protected]) wrote:
> > > Using ndelay() with a 64bit variable as parameter can result in build
> > > errors like the following on some 32bit systems when it results in a
> > > 64bit division:
> > >
> > > <-- snip -->
> > >
> > > ...
> > > MODPOST 759 modules
> > > ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
> > >
> > > <-- snip -->
> > >
> > > Reported by Martin Michlmayr.
> > >
> > > Signed-off-by: Adrian Bunk <[email protected]>
> >
> > Yep, ndelay() uses division, thanks a lot Adrian for spotting this.
>
> hm. Where?
>...
include/linux/delay.h:35
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Wed, 27 Feb 2008 08:22:07 +0200 Adrian Bunk <[email protected]> wrote:
> On Tue, Feb 26, 2008 at 04:04:39PM -0800, Andrew Morton wrote:
> > On Tue, 26 Feb 2008 21:52:40 +0300 Evgeniy Polyakov <[email protected]> wrote:
> >
> > > Hi Adrian.
> > >
> > > On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk ([email protected]) wrote:
> > > > Using ndelay() with a 64bit variable as parameter can result in build
> > > > errors like the following on some 32bit systems when it results in a
> > > > 64bit division:
> > > >
> > > > <-- snip -->
> > > >
> > > > ...
> > > > MODPOST 759 modules
> > > > ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
> > > >
> > > > <-- snip -->
> > > >
> > > > Reported by Martin Michlmayr.
> > > >
> > > > Signed-off-by: Adrian Bunk <[email protected]>
> > >
> > > Yep, ndelay() uses division, thanks a lot Adrian for spotting this.
> >
> > hm. Where?
> >...
>
> include/linux/delay.h:35
>
well found.
Something like this:
--- a/include/linux/delay.h~a
+++ a/include/linux/delay.h
@@ -7,10 +7,12 @@
* Delay routines, using a pre-computed "loops_per_jiffy" value.
*/
-extern unsigned long loops_per_jiffy;
+#include <linux/kernel.h>
#include <asm/delay.h>
+extern unsigned long loops_per_jiffy;
+
/*
* Using udelay() for intervals greater than a few milliseconds can
* risk overflow for high loops_per_jiffy (high bogomips) machines. The
@@ -32,7 +34,11 @@ extern unsigned long loops_per_jiffy;
#endif
#ifndef ndelay
-#define ndelay(x) udelay(((x)+999)/1000)
+static inline void ndelay(unsigned long x)
+{
+ udelay(DIV_ROUND_UP(x, 1000));
+}
+#define ndelay(x) ndelay(x)
#endif
void calibrate_delay(void);
_
David Rientjes wrote:
> On Tue, 26 Feb 2008, Martin Michlmayr wrote:
>
>
>> ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
>>
>
> crypto: use do_div() when registering the rng
>
> Use do_div() instead of the divide operator.
>
>
This is similar to my patch, which didn't fix the problem. Adrian
already fixed it.
On Tue, Feb 26, 2008 at 01:21:00PM +0100, Martin Michlmayr wrote:
References to __divdi3 / __udivdi3 are becoming a somewhat regular bug.
I've created a patch to add these to the kernel but I'd rather not push
it unless I have to. 64-bit operations but especially divisions are slow
on 32-bit hardware so undefined references serve as an important detector.
Ralf
On Tue, Feb 26, 2008 at 04:04:39PM -0800, Andrew Morton ([email protected]) wrote:
> udelay() might be exposed to the same problem. It would be better to fix
> ndelay() and udelay() rather than callers. It is reasonable to pass a u64
> into ndelay() and to expect the build to not explode.
Well, if you think it is resonable to pass u64 into function, which is
supposed to sleep no more than several cpu cycles. I do not want to
start any kind of flame about it, but this looks like an overkill.
> (Geeze macros suck)
Absolutely.
--
Evgeniy Polyakov
On Tue, Feb 26, 2008 at 10:47:19PM -0800, Andrew Morton wrote:
>...
> Something like this:
Generally agreed, but
> --- a/include/linux/delay.h~a
> +++ a/include/linux/delay.h
> @@ -7,10 +7,12 @@
> * Delay routines, using a pre-computed "loops_per_jiffy" value.
> */
>
> -extern unsigned long loops_per_jiffy;
> +#include <linux/kernel.h>
>
> #include <asm/delay.h>
>
> +extern unsigned long loops_per_jiffy;
> +
>...
this part of your patch breaks m68k:
<-- snip -->
...
CC init/main.o
In file included from
/home/bunk/linux/kernel-2.6/git/linux-2.6/include/linux/delay.h:12,
from
/home/bunk/linux/kernel-2.6/git/linux-2.6/init/main.c:19:
include2/asm/delay.h: In function '__const_udelay':
include2/asm/delay.h:33: error: 'loops_per_jiffy' undeclared (first use in this function)
include2/asm/delay.h:33: error: (Each undeclared identifier is reported only once
include2/asm/delay.h:33: error: for each function it appears in.)
make[2]: *** [init/main.o] Error 1
<-- snip -->
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Tue, 26 Feb 2008 19:46:35 -0800 (PST)
David Rientjes <[email protected]> wrote:
> On Tue, 26 Feb 2008, Martin Michlmayr wrote:
>
> > With 2.6.25-rc3 and a config file with
> >
> > CONFIG_CRYPTO_DEV_HIFN_795X=m
> > CONFIG_CRYPTO_DEV_HIFN_795X_RNG=y
> >
> > I get the following build error on at least ARM and MIPS:
> >
> > Building modules, stage 2.
> > MODPOST 759 modules
> > ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
> >
>
> crypto: use do_div() when registering the rng
>
> Use do_div() instead of the divide operator.
>
> Cc: Patrick McHardy <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Signed-off-by: David Rientjes <[email protected]>
> ---
> Looks like this was triggered by f881d829, which causes this code to be
> compiled for CONFIG_CRYPTO_DEV_HIFN_795X_RNG.
>
> drivers/crypto/hifn_795x.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
> --- a/drivers/crypto/hifn_795x.c
> +++ b/drivers/crypto/hifn_795x.c
> @@ -822,12 +822,14 @@ static int hifn_rng_data_read(struct hwrng *rng, u32 *data)
>
> static int hifn_register_rng(struct hifn_device *dev)
> {
> + unsigned int tmp;
> +
> /*
> * We must wait at least 256 Pk_clk cycles between two reads of the rng.
> */
> - dev->rng_wait_time = DIV_ROUND_UP(NSEC_PER_SEC, dev->pk_clk_freq) *
> - 256;
> -
> + tmp = NSEC_PER_SEC + dev->pk_clk_freq - 1;
> + do_div(tmp, dev->pk_clk_freq);
> + dev->rng_wait_time = tmp * 256;
> dev->rng.name = dev->name;
> dev->rng.data_present = hifn_rng_data_present,
> dev->rng.data_read = hifn_rng_data_read,
do_div takes a u64 as its first arg. If we're going to make the above
change then we may as well use plain old "/".