On Tue, Jun 28, 2011 at 04:59:57PM -0700, Colin Cross wrote:

On Tue, Jun 28, 2011 at 4:46 PM, Russell King - ARM Linux
[off-list ref] wrote:

quoted

On Tue, Jun 28, 2011 at 04:37:08PM -0700, Colin Cross wrote:

quoted

I don't think it affects bogomips - loops_per_jiffy can still be
calibrated and updated by cpufreq, udelay just wont use them.

No, you can't avoid it. ?__delay(), udelay(), and the global
loops_per_jiffy are all linked together - for instance this is how
the spinlock code has a 1s timeout:

static void __spin_lock_debug(raw_spinlock_t *lock)
{
? ? ? ?u64 loops = loops_per_jiffy * HZ;
? ? ? ?int print_once = 1;

? ? ? ?for (;;) {
? ? ? ? ? ? ? ?for (i = 0; i < loops; i++) {
? ? ? ? ? ? ? ? ? ? ? ?if (arch_spin_trylock(&lock->raw_lock))
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?return;
? ? ? ? ? ? ? ? ? ? ? ?__delay(1);
? ? ? ? ? ? ? ?}

which goes wrong for all the same reasons you're pointing out about
udelay(). ?So just restricting your argument to udelay() is not
realistic.

True, there are a few other users of loops_per_jiffy and __delay, but
it looks like __spin_lock_debug is the only one worth worrying about,
and it's timing is not as critical as udelay - worst case here is that
the warning occurs after 250 ms instead of 1s.  Leaving
loops_per_jiffy and __delay intact, and fixing udelay, would still be
a net gain.

Other users of loops_per_jiffy are incorrect in any case:

static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,                                                   unsigned long bit)
{
        unsigned long i = 0;
        unsigned long limit = (loops_per_jiffy *
                                msecs_to_jiffies(MMC_TIMEOUT_MS));
...
        if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) {
                while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
                                        && (i++ < limit))
                        cpu_relax();
        }

Is rubbish.

static void omap_write_buf_pref(struct mtd_info *mtd,
                                        const u_char *buf, int len)
{
...
                /* wait for data to flushed-out before reset the prefetch */
                tim = 0;
                limit = (loops_per_jiffy *
                                        msecs_to_jiffies(OMAP_NAND_TIMEOUT_MS));                while (gpmc_read_status(GPMC_PREFETCH_COUNT) && (tim++ < limit))                        cpu_relax();

Another load of rubbish.

static int flush(struct pl022 *pl022)
{
        unsigned long limit = loops_per_jiffy << 1;

        dev_dbg(&pl022->adev->dev, "flush\n");
        do {
                while (readw(SSP_SR(pl022->virtbase)) & SSP_SR_MASK_RNE)
                        readw(SSP_DR(pl022->virtbase));
        } while ((readw(SSP_SR(pl022->virtbase)) & SSP_SR_MASK_BSY) && limit--);

Yet more...

static int flush(struct driver_data *drv_data)
{
        unsigned long limit = loops_per_jiffy << 1;

        void __iomem *reg = drv_data->ioaddr;

        do {
                while (read_SSSR(reg) & SSSR_RNE) {
                        read_SSDR(reg);
                }
        } while ((read_SSSR(reg) & SSSR_BSY) && --limit);

and...

sound/soc/samsung/i2s.c:
#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
        /* Be patient */
        val = msecs_to_loops(1) / 1000; /* 1 usec */
        while (--val)
                cpu_relax();

even worse.

#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
static int s3c2412_snd_lrsync(struct s3c_i2sv2_info *i2s)
{
        u32 iiscon;
        unsigned long loops = msecs_to_loops(5);

        while (--loops) {
                iiscon = readl(i2s->regs + S3C2412_IISCON);
                if (iiscon & S3C2412_IISCON_LRINDEX)
                        break;

                cpu_relax();
        }

It just goes on...

And strangely the major offender of this stuff are ARM drivers, not other
peoples stuff and not stuff in drivers/staging.

So I don't think its sane to ignore loops_per_jiffy and __delay, and just
concentrate on udelay().