linux-kernel - Re: [PATCH] regmap: add iopoll-like polling macro

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1467995182.2365.45.camel@pengutronix.de>
Date:	Fri, 08 Jul 2016 18:26:22 +0200
From:	Philipp Zabel <p.zabel@...gutronix.de>
To:	Mark Brown <broonie@...nel.org>
Cc:	linux-kernel@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH] regmap: add iopoll-like polling macro

Am Freitag, den 08.07.2016, 16:39 +0200 schrieb Mark Brown:
> On Thu, Jul 07, 2016 at 12:01:43PM +0200, Philipp Zabel wrote:
> > Am Donnerstag, den 07.07.2016, 11:42 +0200 schrieb Mark Brown:
> > > On Wed, Jul 06, 2016 at 04:19:41PM +0200, Philipp Zabel wrote:
> 
> > > > This patch adds a macro regmap_read_poll_timeout that works similar
> > > > to the readx_poll_timeout defined in linux/iopoll.h, except that this
> > > > can also return the error value returned by a failed regmap_read.
> 
> > > Please make this a proper function.
> 
> > I can't, the condition has to be evaluated inside the loop. This is
> > basically a poor man's function template.
> 
> Given that the condition is always going to be essentially the same one
> checking that (read & mask) == value we could just parameterize it
> couldn't we?

The iopoll macros also allow comparisons like < > !=, more complicated
expressions, or even function calls.

Granted, the only existing example I am aware of is
drivers/dma/qcom/hidma_ll.c, which calls a static function in
readl_poll_wait_timeout, that evaluates to ((read == 1) || (read == 2)).

There's a similar condition in rt5659_headset_detect, although that may
be dismissed as it's using snd_soc_read and variable delays:
        while (i < 5) {
                msleep(sleep_time[i]);
                val = snd_soc_read(codec, RT5659_EJD_CTRL_2) & 0x0003;
                i++;
                if (val == 0x1 || val == 0x2 || val == 0x3)
                        break;
        }

A potential user for an inequality condition is
drivers/media/tuners/it913x.c, which has a loop where the condition is
(read != 0):
                #define TIMEOUT 50
                timeout = jiffies + msecs_to_jiffies(TIMEOUT);
                while (!time_after(jiffies, timeout)) {
                        ret = regmap_read(dev->regmap, 0x80ec82, &utmp);
                        if (ret)
                                goto err;

                        if (utmp)
                                break;
                }

Even if by far the most common cases can be covered as you suggest,
I would find it useful to keep this the same as the other
readx_poll_wait_timeout variants.

regards
Philipp