From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932082AbcGMHh4 (ORCPT ); Wed, 13 Jul 2016 03:37:56 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:38955 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752492AbcGMHhf convert rfc822-to-8bit (ORCPT ); Wed, 13 Jul 2016 03:37:35 -0400 Message-ID: <1468395429.3638.21.camel@pengutronix.de> Subject: Re: [PATCH] regmap: add iopoll-like polling macro From: Philipp Zabel To: Mark Brown Cc: linux-kernel@vger.kernel.org, kernel@pengutronix.de Date: Wed, 13 Jul 2016 09:37:09 +0200 In-Reply-To: <1467995182.2365.45.camel@pengutronix.de> References: <1467814781-15007-1-git-send-email-p.zabel@pengutronix.de> <20160707094254.GK6247@sirena.org.uk> <1467885703.4236.40.camel@pengutronix.de> <20160708143925.GY6247@sirena.org.uk> <1467995182.2365.45.camel@pengutronix.de> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 2001:67c:670:100:96de:80ff:fec2:9969 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mark, Am Freitag, den 08.07.2016, 18:26 +0200 schrieb Philipp Zabel: > 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. Any comments on this? If I can't convince you to keep the macro, I could change it into a function: int regmap_poll_bits_wait_timeout(struct regmap *map, unsigned int reg, unsigned int mask, unsigned int val, unsigned long sleep, u64 timeout); With the changed name there would be no expectation for it to work the same as readx_poll_wait_timeout, and the mask and val condition is in the same place as mask and val for regmap_update_bits. regards Philipp