Skip to contentSkip to conversation

Tiny delays in drivers/mtd/nand/nand_base.c

DISCUSSION Mason 2 participants 2 messages Apr 1, 2015 Memory technology devices (MTD)

lore
Share & tools
lore

Mason

author
#0 lore raw

Hello everyone,

In drivers/mtd/nand/nand_base.c there are several instances of the following code:

	/*
	 * Apply this short delay always to ensure that we do wait tWB in
	 * any case on any machine.
	 */
	ndelay(100);

Is the intent to spin for 100 nanoseconds?

It seems that, for most platforms, ndelay is defined as:
udelay(DIV_ROUND_UP(x, 1000))

So it will resolve to udelay(1) if I understand correctly? (I suppose sleeping longer is not a problem.)

However the comment implies that 100 ns are sufficient, right?

So if I override ndelay with a function that sleeps *exactly* the amount requested, everything should keep working?

The reason I ask is because someone added this comment in my source tree:

#ifdef CONFIG_TANGOX
udelay(1); /* needs to make it much longer than tWB */

#else
	ndelay(100);

#endif

Also I have to figure out why the build is not picking up this definition for ndelay (from include/asm-generic/delay.h)

/* 0x5 is 2**32 / 1000000000 (rounded up) */

#define ndelay(n)							\
	({								\
		if (__builtin_constant_p(n)) {				\
			if ((n) / 20000 >= 1)				\
				__bad_ndelay();				\
			else						\
				__const_udelay((n) * 5ul);		\
		} else {						\
			__ndelay(n);					\
		}							\
	})

Although it seems it should take HZ into account, as the argument is then multiplied by ticks_per_jiffy (which is FREQ/HZ).

Regards.

#1 lore raw
Quoted 5 lines · Mason

Hello everyone,

In drivers/mtd/nand/nand_base.c there are several instances of the following code:

Quoted 8 lines
        /*
         * Apply this short delay always to ensure that we do wait tWB in
         * any case on any machine.
         */
        ndelay(100);

Is the intent to spin for 100 nanoseconds?

Yes. tWB is 100ns.

Quoted 4 lines

It seems that, for most platforms, ndelay is defined as:
udelay(DIV_ROUND_UP(x, 1000))

So it will resolve to udelay(1) if I understand correctly?

If your kernel has the above definition, yes.

Quoted 1 line

(I suppose sleeping longer is not a problem.)

That covers my knowledge.

Quoted 1 line

However the comment implies that 100 ns are sufficient, right?

Spec wants tWB to be at least 100ns.

Quoted 2 lines

So if I override ndelay with a function that sleeps *exactly* the amount requested, everything should keep working?

Yes.

Quoted 8 lines

The reason I ask is because someone added this comment in my source tree:

#ifdef CONFIG_TANGOX
udelay(1); /* needs to make it much longer than tWB */

#else
        ndelay(100);

#endif

Looks fishy. Maybe this papers over a BSP bug.

Quoted 2 lines

Also I have to figure out why the build is not picking up this definition for ndelay (from include/asm-generic/delay.h)

Not all archs use generic delay.h

HTH,
//richard