From nobody Sat Jul 25 18:53:57 2026 Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5B1C73A453A for ; Tue, 14 Jul 2026 15:36:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784043419; cv=none; b=pPTXB5GgBJu5hvdOvJyErGE00JpXnD98b4nuWXbW7JFTNez+ziklsMf5DPHzmHFpXWCbbSq5SjsSvR4HvJen8OWWxcT7yp1vi8oJLYouFCnIIbZUloLWt/XogWlO5zXFmRi1BtEo/FM56wZlzMPOHR8BZqxkOG/Ha7r5zlGMrKg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784043419; c=relaxed/simple; bh=FUKRz5QZbEpD2ppSUUSJKph2gxfJRcgreInDDRklvQw=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:To:Cc; b=auP9VGOSXXHI/U6OFv/96iYC4nDw6d6wC3k0vF1PMNYbhpadu6abZdaVXdscsijmRsLIhHKEuwO68G2/IXuCq49evCVKGqfjM20CjkS5NykqcccV5geoJmicBGsVlAGMW34X0ggB1MIalPwVRkFNG3ek6Xv68GZRQ2OPca9IS8M= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=F93kF1UL; arc=none smtp.client-ip=209.85.128.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="F93kF1UL" Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-493c19bad03so40551485e9.2 for ; Tue, 14 Jul 2026 08:36:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1784043415; x=1784648215; darn=vger.kernel.org; h=cc:to:message-id:content-transfer-encoding:content-type :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to:content-type; bh=yFqzpW0R11/PPWKTn87rkaqLDktCu2fOc1hLjHnrcEc=; b=F93kF1ULnWBzz6LEBp3Y6NJ4vRPY+ApI5P9zC6IUlAKW7h1VYiO/UJ68cWqp3QBH8c 45R7hC7UyZtD1x3Rf8AX224zNCn/So1iiLi32M0BuIpXbSTE8UjhRB77Xy2dj18XiSdk CUUPSkuOvjwRd1loCSi4hGRMB0Qbpa6K2peW2reFsgAsy9CC6V4OfWYtXmQuRcyDeKgY yRqOjcKKgzNVAg4eudXsRGfWhuVsmjzsBIIpOYf9SvUfHlB6oEJBLjGPVb1Nz1hL7wKT 9GmU7sKz/DxGinWzB0WSBHvz3mvcorHx3G+NRGYyB6IBzvcG+BwvIsd7/mBTzkylONni fPjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784043415; x=1784648215; h=cc:to:message-id:content-transfer-encoding:content-type :mime-version:subject:date:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to:content-type; bh=yFqzpW0R11/PPWKTn87rkaqLDktCu2fOc1hLjHnrcEc=; b=ZBO2C3I8dNMUj8+A6GC8KVLHzqMNPNa33oRC3Yqaz5bfu1C4nGvyBN8PaK/g5EBCCb cm4zjmEhz47WJnYgHKtjpXT8RmOzmy/RntKP8bbRF3xQmAxXSxsYhpzCqDPInQNvhOQ/ iLQTXcLfDwSvemiGBlpBFPkFR0ClhBJpFLy7WcN121dt8S3ZoY175I1vlwx4634fBNDE 2cJKZ+ETIwK6Dau4zrNa2hcNaBos53+m07SLgbllDSSikGwTm7mya4CvYWJ8/s3uAgXX ue2luaADXUnJVpRgY3BRc8fl2ngDnxYwsJAA0wdLPpVBishW5gMT9iWOb63TLIrcR6T8 Wz2A== X-Forwarded-Encrypted: i=1; AHgh+RoAgtygGIS7TnUb3E0RZ2CSww1MFpIQl8HsZ79zwa6KfIz3zRd1PXsaIvAt80sCV3R2j5YVnLOKWnTPpvs=@vger.kernel.org X-Gm-Message-State: AOJu0Yzo8M75SWBkbcNWvXPY+52DYJm2csBx5H4h+DGOqGBWdojtMoaa yeb50G6uZLgd7Clvqq9uHzU8FCFJFwwxOS5lCTQmgKu7h9TqClv8LJDk X-Gm-Gg: AfdE7cng6D8pj5jGmhfMhXS/thRWEK2aSunvtHAKPG950w6fBe0qqQpQlVOJ2RIfl5X zP7Nm2aIHxY0zeTPBQAN3r4HM26jLe5ou5nkSYhjhgoO++6RvVdK3EPd+FeIJoa2l6SMbb4EhNi diEYRH9J5Y60Mb/L7A4ZPVKg1ivKyJDVT3PbphN3zNXDJyo1ZzbB9Dpiox0Uz58HsCFCZjeq1+V 4hzZKdQieti/KqCnmnpZWPAAzfo4pE2jauLE3wT2q8eQGdOD94LZ4rurlAIXgX3oYjywnogErwy Ht1z+5RvwWfvnuD8qyMuwqDw+60UR8LVmMnIb4+WXS75S6GRAwdApaFBJKjR+Qv/k4ZOdk7YQES Q+ojm6cFnjCFqpQLNgd2mqVFz9OLva7x1NEuek6U5HMFtyjmTAzogJPyCAWWjE7fP+5pwzFk1xB Njhnlvzye6iCxcvwO14KMNaGUbxt3Y3Ry7zfaPEJmL6VgJP+fmmMHBF1F3796S9ik7DhdL/JEem 1bPHoEvT0Wp47gymG7uZVr1fX4DzlGyeEoioKgLfZR53BmmczdQhoEeIHD5mJVhhV4OlM++n/BY Ghi0XkjsWCjtm1i5QhUJhXCIuClSMnAmQ6/1fo/2TDLLvDtscOtpgw== X-Received: by 2002:a05:600c:3143:b0:493:b87c:c87d with SMTP id 5b1f17b1804b1-493f87e9f43mr142113605e9.11.1784043415131; Tue, 14 Jul 2026 08:36:55 -0700 (PDT) Received: from [192.168.1.187] ([2a02:8308:4092:11f0::f9f]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f464b7e22sm8964103f8f.25.2026.07.14.08.36.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jul 2026 08:36:54 -0700 (PDT) From: Joshua Crofts Date: Tue, 14 Jul 2026 17:36:48 +0200 Subject: [PATCH v4] iio: light: opt3001: split opt3001_get_processed() logic Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20260714-opt3001-unwind-cleanup-v4-1-7ebea805bfac@gmail.com> X-B4-Tracking: v=1; b=H4sIAAAAAAAC/4XNTQqDMBCG4atI1k2ZJCZqV71H6SImowY0in9tE e/e6EoK0uX7wTyzkAF7hwO5RQvpcXaDa32I+BIRU2lfInU2NOHAFSSQ0rYbBQCjk385b6mpUfu po5lGtCiUVFKScNz1WLj3Dj+eoSs3jG3/2f/MbFv/kjOjjGYAOTeFtbnS97LRrr6atiEbOfMDw 9gpwwMTJ4ViCQIYJX8ZcWT4KSMCIwsthEoTlUN6ZNZ1/QLeyVTNSgEAAA== X-Change-ID: 20260708-opt3001-unwind-cleanup-9aeede365655 To: Jonathan Cameron , David Lechner , =?utf-8?q?Nuno_S=C3=A1?= , Andy Shevchenko Cc: linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, Joshua Crofts X-Mailer: b4 0.14.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1784043414; l=8125; i=joshua.crofts1@gmail.com; s=20260530; h=from:subject:message-id; bh=FUKRz5QZbEpD2ppSUUSJKph2gxfJRcgreInDDRklvQw=; b=6OEgFUke56iApxZ/GuMw0cqpKQm7j56FLGSu04VgsrpBCnV31v3TbAoFDOqztDkfL7c8biahf wU5Y76GFsIKBG9pzIoyCVJnRpeRBNohzxZM09jLJDVTUNlsKVc4oAY/ X-Developer-Key: i=joshua.crofts1@gmail.com; a=ed25519; pk=RTDOoVwgeL4oFdASj9U+cxJuIjXuXk73zkjnGOJKbEo= Split the logic inside the opt3001_get_processed() function, as the current flow is hard to read, mixing IRQ and non-IRQ code blocks. Separate the IRQ code path into its own function, same for the non-IRQ path. Suggested-by: Jonathan Cameron Signed-off-by: Joshua Crofts Reviewed-by: Andy Shevchenko --- This patch fixes the absolutely horrible code flow in the opt3001_get_processed() function, where the original implementation used checks whether IRQ mode is enabled to execute blocks of code, making the function hard to read. Ideas on how to improve the functions are more than welcome. Originally suggested by Jonathan Cameron. --- Changes in v4: - Simplify error checks and end of functions (Andy) - Refactor timeout check (Andy) - Link to v3: https://lore.kernel.org/r/20260712-opt3001-unwind-cleanup-v3-= 1-5fa336876b08@gmail.com Changes in v3: - Move duplicated code blocks to common functions (David) - Change wrapping of comments and function call (Jonathan) - Link to v2: https://lore.kernel.org/r/20260711-opt3001-unwind-cleanup-v2-= 1-47f617e00c65@gmail.com Changes in v2: - Remove ternary operator and add if/else (Andy) - Remove timeout variable (Andy) - Link to v1: https://lore.kernel.org/r/20260708-opt3001-unwind-cleanup-v1-= 1-900b2cfddb6a@gmail.com --- drivers/iio/light/opt3001.c | 192 ++++++++++++++++++++++++----------------= ---- 1 file changed, 106 insertions(+), 86 deletions(-) diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c index 434ef7e034fe..12cfb0e58870 100644 --- a/drivers/iio/light/opt3001.c +++ b/drivers/iio/light/opt3001.c @@ -315,35 +315,12 @@ static const struct iio_chan_spec opt3002_channels[] = =3D { IIO_CHAN_SOFT_TIMESTAMP(1), }; =20 -static int opt3001_get_processed(struct opt3001 *opt, int *val, int *val2) +static int opt3001_start_conversion(struct opt3001 *opt) { struct i2c_client *client =3D opt->client; struct device *dev =3D &client->dev; - int ret; - u16 mantissa; u16 reg; - u8 exponent; - u16 value; - long timeout; - - if (opt->use_irq) { - /* - * Enable the end-of-conversion interrupt mechanism. Note that - * doing so will overwrite the low-level limit value however we - * will restore this value later on. - */ - ret =3D i2c_smbus_write_word_swapped(client, - OPT3001_LOW_LIMIT, - OPT3001_LOW_LIMIT_EOC_ENABLE); - if (ret < 0) { - dev_err(dev, "failed to write register %02x\n", - OPT3001_LOW_LIMIT); - return ret; - } - - /* Allow IRQ to access the device despite lock being set */ - opt->ok_to_ignore_lock =3D true; - } + int ret; =20 /* Reset data-ready indicator flag */ opt->result_ready =3D false; @@ -353,85 +330,128 @@ static int opt3001_get_processed(struct opt3001 *opt= , int *val, int *val2) if (ret < 0) { dev_err(dev, "failed to read register %02x\n", OPT3001_CONFIGURATION); - goto err; + return ret; } =20 reg =3D ret; opt3001_set_mode(opt, ®, OPT3001_CONFIGURATION_M_SINGLE); =20 ret =3D i2c_smbus_write_word_swapped(client, OPT3001_CONFIGURATION, reg); - if (ret < 0) { + if (ret < 0) dev_err(dev, "failed to write register %02x\n", OPT3001_CONFIGURATION); + + return ret; +} + +static int opt3001_get_processed_irq(struct opt3001 *opt) +{ + struct i2c_client *client =3D opt->client; + struct device *dev =3D &client->dev; + u16 value; + int ret; + + /* + * Enable the end-of-conversion interrupt mechanism. Note that doing so + * will overwrite the low-level limit value however we will restore this + * value later on. + */ + ret =3D i2c_smbus_write_word_swapped(client, + OPT3001_LOW_LIMIT, + OPT3001_LOW_LIMIT_EOC_ENABLE); + if (ret < 0) { + dev_err(dev, "failed to write register %02x\n", + OPT3001_LOW_LIMIT); + return ret; + } + + /* Allow IRQ to access the device despite lock being set */ + opt->ok_to_ignore_lock =3D true; + + ret =3D opt3001_start_conversion(opt); + if (ret) goto err; - } =20 - if (opt->use_irq) { - /* Wait for the IRQ to indicate the conversion is complete */ - ret =3D wait_event_timeout(opt->result_ready_queue, - opt->result_ready, - msecs_to_jiffies(OPT3001_RESULT_READY_LONG)); - if (ret =3D=3D 0) { - ret =3D -ETIMEDOUT; - goto err; - } - } else { - /* Sleep for result ready time */ - timeout =3D (opt->int_time =3D=3D OPT3001_INT_TIME_SHORT) ? - OPT3001_RESULT_READY_SHORT : OPT3001_RESULT_READY_LONG; - msleep(timeout); - - /* Check result ready flag */ - ret =3D i2c_smbus_read_word_swapped(client, OPT3001_CONFIGURATION); - if (ret < 0) { - dev_err(dev, "failed to read register %02x\n", - OPT3001_CONFIGURATION); - goto err; - } - - if (!(ret & OPT3001_CONFIGURATION_CRF)) { - ret =3D -ETIMEDOUT; - goto err; - } - - /* Obtain value */ - ret =3D i2c_smbus_read_word_swapped(client, OPT3001_RESULT); - if (ret < 0) { - dev_err(dev, "failed to read register %02x\n", - OPT3001_RESULT); - goto err; - } - opt->result =3D ret; - opt->result_ready =3D true; - } + if (wait_event_timeout(opt->result_ready_queue, opt->result_ready, + msecs_to_jiffies(OPT3001_RESULT_READY_LONG))) + ret =3D 0; + else + ret =3D -ETIMEDOUT; =20 err: - if (opt->use_irq) - /* Disallow IRQ to access the device while lock is active */ - opt->ok_to_ignore_lock =3D false; + opt->ok_to_ignore_lock =3D false; =20 if (ret < 0) return ret; =20 - if (opt->use_irq) { - /* - * Disable the end-of-conversion interrupt mechanism by - * restoring the low-level limit value (clearing - * OPT3001_LOW_LIMIT_EOC_ENABLE). Note that selectively clearing - * those enable bits would affect the actual limit value due to - * bit-overlap and therefore can't be done. - */ - value =3D (opt->low_thresh_exp << 12) | opt->low_thresh_mantissa; - ret =3D i2c_smbus_write_word_swapped(client, - OPT3001_LOW_LIMIT, - value); - if (ret < 0) { - dev_err(dev, "failed to write register %02x\n", - OPT3001_LOW_LIMIT); - return ret; - } + /* + * Disable the end-of-conversion interrupt mechanism by restoring the + * low-level limit value (clearing OPT3001_LOW_LIMIT_EOC_ENABLE). Note + * that selectively clearing those enable bits would affect the actual + * limit value due to bit-overlap and therefore can't be done. + */ + value =3D (opt->low_thresh_exp << 12) | opt->low_thresh_mantissa; + ret =3D i2c_smbus_write_word_swapped(client, OPT3001_LOW_LIMIT, value); + if (ret < 0) + dev_err(dev, "failed to write register %02x\n", + OPT3001_LOW_LIMIT); + + return ret; +} + +static int opt3001_get_processed_noirq(struct opt3001 *opt) +{ + struct i2c_client *client =3D opt->client; + struct device *dev =3D &client->dev; + int ret; + + ret =3D opt3001_start_conversion(opt); + if (ret) + return ret; + + if (opt->int_time =3D=3D OPT3001_INT_TIME_SHORT) + msleep(OPT3001_RESULT_READY_SHORT); + else + msleep(OPT3001_RESULT_READY_LONG); + + /* Check result ready flag */ + ret =3D i2c_smbus_read_word_swapped(client, OPT3001_CONFIGURATION); + if (ret < 0) { + dev_err(dev, "failed to read register %02x\n", + OPT3001_CONFIGURATION); + return ret; } =20 + if (!(ret & OPT3001_CONFIGURATION_CRF)) + return -ETIMEDOUT; + + /* Obtain value */ + ret =3D i2c_smbus_read_word_swapped(client, OPT3001_RESULT); + if (ret < 0) { + dev_err(dev, "failed to read register %02x\n", + OPT3001_RESULT); + return ret; + } + + opt->result =3D ret; + opt->result_ready =3D true; + + return 0; +} + +static int opt3001_get_processed(struct opt3001 *opt, int *val, int *val2) +{ + u16 mantissa; + u8 exponent; + int ret; + + if (opt->use_irq) + ret =3D opt3001_get_processed_irq(opt); + else + ret =3D opt3001_get_processed_noirq(opt); + if (ret) + return ret; + exponent =3D OPT3001_REG_EXPONENT(opt->result); mantissa =3D OPT3001_REG_MANTISSA(opt->result); =20 --- base-commit: 2e2f2de7532cbbc2269de8be20ec709606c6e79b change-id: 20260708-opt3001-unwind-cleanup-9aeede365655 Best regards, --=20 Kind regards, Joshua Crofts