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
| ||
|
Message-ID: <20221009173606.5b8e5661@jic23-huawei> Date: Sun, 9 Oct 2022 17:36:06 +0100 From: Jonathan Cameron <jic23@...nel.org> To: Crt Mori <cmo@...exis.com> Cc: Christophe JAILLET <christophe.jaillet@...adoo.fr>, andy.shevchenko@...il.com, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v6 1/3] iio: temperature: mlx90632 Add runtime powermanagement modes On Mon, 3 Oct 2022 10:20:18 +0200 Crt Mori <cmo@...exis.com> wrote: > On Sun, 2 Oct 2022 at 18:09, Christophe JAILLET > <christophe.jaillet@...adoo.fr> wrote: > > > > Le 22/09/2022 à 10:13, cmo-fc6wVz46lShBDgjK7y7TUQ@...lic.gmane.org a écrit : > > > From: Crt Mori <cmo-fc6wVz46lShBDgjK7y7TUQ@...lic.gmane.org> > > > measurements in lower power mode (SLEEP_STEP), with the lowest refresh > > > rate (2 seconds). > > > > Hi, > > > > should there be a v7, a few nitpick below. > > > It was already applied, but I can spin a new patch for the suggested > changes (the s32 is mostly there because before this patch it was > returning value for further bit manipulation). Follow on patch welcome! Thanks, Jonathan > > > > > > > + ret = regmap_read_poll_timeout(data->regmap, MLX90632_REG_STATUS, > > > + reg_status, > > > + (reg_status & MLX90632_STAT_BUSY) == 0, > > > + 10000, 100 * 10000); > > > + if (ret < 0) { > > > + dev_err(&data->client->dev, "data not ready"); > > > + return -ETIMEDOUT; > > > > Why not "return ret;"? > > > If you came to this point there were already several i2c reads, so I > think it is more important to convert those to timeout. > > > > mutex_lock(&data->lock); > > > - measurement = mlx90632_perform_measurement(data); > > > - if (measurement < 0) { > > > - ret = measurement; > > > + ret = mlx90632_set_meas_type(data, MLX90632_MTYP_MEDICAL); > > > + if (ret < 0) > > > + goto read_unlock; > > > + > > > + switch (data->powerstatus) { > > > + case MLX90632_PWR_STATUS_CONTINUOUS: > > > + measurement = mlx90632_perform_measurement(data); > > > > ret = mlx90632_perform_measurement(data); > > and > > measurement = ret; > > on success would be less verbose (no need for {}, and save 1 LoC) and > > more in line with mlx90632_calculate_dataset_ready_time() above. > > > I wanted to change as few lines as possible to avoid clogging the > patch with unrelated changes. Also in most cases, we will be > in-success here, so limiting the number of variable copies in the > success path should be the priority.
Powered by blists - more mailing lists