2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

[Martin Michlmayr]

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/

Re: 2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

[Patrick McHardy]

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

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!
>   

Does this patch fix it?



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 590 bytes --]

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,

Re: 2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

[Martin Michlmayr]

* Patrick McHardy <kaber@trash.net> [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/

[2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Adrian Bunk]

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 <bunk@kernel.org>

---

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;
 }
 

Re: [2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Martin Michlmayr]

* Adrian Bunk <bunk@kernel.org> [2008-02-26 17:34]:
> >   MODPOST 759 modules
> > ERROR: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!
> 
> Fix below.

Tested-by: Martin Michlmayr <tbm@cyrius.com>

-- 
Martin Michlmayr
http://www.cyrius.com/

Re: [2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Evgeniy Polyakov]

Hi Adrian.

On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk (bunk@kernel.org) 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 <bunk@kernel.org>

Yep, ndelay() uses division, thanks a lot Adrian for spotting this.

Herbert, please apply.

ACK.

-- 
	Evgeniy Polyakov

Re: [2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Andrew Morton]

On Tue, 26 Feb 2008 21:52:40 +0300 Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:

> Hi Adrian.
> 
> On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk (bunk@kernel.org) 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 <bunk@kernel.org>
> 
> 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)

Re: 2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

[David Rientjes]

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 <kaber@trash.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 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,

Re: [2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Adrian Bunk]

On Tue, Feb 26, 2008 at 04:04:39PM -0800, Andrew Morton wrote:
> On Tue, 26 Feb 2008 21:52:40 +0300 Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
> 
> > Hi Adrian.
> > 
> > On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk (bunk@kernel.org) 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 <bunk@kernel.org>
> > 
> > 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

Re: [2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Andrew Morton]

On Wed, 27 Feb 2008 08:22:07 +0200 Adrian Bunk <bunk@kernel.org> 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 <johnpol@2ka.mipt.ru> wrote:
> > 
> > > Hi Adrian.
> > > 
> > > On Tue, Feb 26, 2008 at 05:34:21PM +0200, Adrian Bunk (bunk@kernel.org) 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 <bunk@kernel.org>
> > > 
> > > 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);
_

Re: 2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

[Patrick McHardy]

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.

Re: 2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

[Ralf Baechle]

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

Re: [2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Evgeniy Polyakov]

On Tue, Feb 26, 2008 at 04:04:39PM -0800, Andrew Morton (akpm@linux-foundation.org) 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

Re: [2.6.25 patch] drivers/crypto/hifn_795x.c: fix 64bit division

[Adrian Bunk]

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

Re: 2.6.25-rc3: "__divdi3" [drivers/crypto/hifn_795x.ko] undefined!

[Andrew Morton]

On Tue, 26 Feb 2008 19:46:35 -0800 (PST)
David Rientjes <rientjes@google.com> 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 <kaber@trash.net>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  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 "/".