From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Yikai Tsai <yikai.tsai.wiwynn@gmail.com>,
Guenter Roeck <linux@roeck-us.net>,
Sasha Levin <sashal@kernel.org>,
mail@carsten-spiess.de, jdelvare@suse.com,
linux-hwmon@vger.kernel.org
Subject: [PATCH AUTOSEL 6.15 14/21] hwmon: (isl28022) Fix current reading calculation
Date: Fri, 6 Jun 2025 11:41:39 -0400 [thread overview]
Message-ID: <20250606154147.546388-14-sashal@kernel.org> (raw)
In-Reply-To: <20250606154147.546388-1-sashal@kernel.org>
From: Yikai Tsai <yikai.tsai.wiwynn@gmail.com>
[ Upstream commit b2446a16dbf2347a07af0cf994ca36576d94df77 ]
According to the ISL28022 datasheet, bit15 of the current register is
representing -32768. Fix the calculation to properly handle this bit,
ensuring correct measurements for negative values.
Signed-off-by: Yikai Tsai <yikai.tsai.wiwynn@gmail.com>
Link: https://lore.kernel.org/r/20250519084055.3787-2-yikai.tsai.wiwynn@gmail.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Now let me check for similar current register handling issues in other
hwmon drivers. Let me look at the commit to understand the exact issue:
Looking at the commit changes, I can see that:
**Before (broken):**
```c
*val = ((long)regval * 1250L * (long)data->gain) / (long)data->shunt;
```
**After (fixed):**
```c
u16 sign_bit;
sign_bit = (regval >> 15) & 0x01;
*val = (((long)(((u16)regval) & 0x7FFF) - (sign_bit * 32768)) * 1250L *
(long)data->gain) / (long)data->shunt;
```
The issue is that the ISL28022 current register is a 16-bit two's
complement signed integer where bit 15 represents the sign bit
(equivalent to -32768), but the original code was treating it as an
unsigned value.
Let me analyze based on the established criteria:
**Analysis:**
**YES - This commit should be backported to stable kernel trees.**
**Detailed Analysis:**
1. **Bug Fix Nature**: This is a clear bug fix that addresses incorrect
current readings for negative values. According to the ISL28022
datasheet mentioned in the commit, the current register (04h) is a
16-bit two's complement value where bit 15 represents -32768, but the
driver was treating it as unsigned.
2. **User Impact**: This bug significantly affects users who rely on
accurate current measurements, especially when current flows in the
negative direction. The bug would cause:
- Incorrect reporting of negative currents as large positive values
- Complete failure to detect reverse current flow
- Invalid power calculations that depend on current readings
3. **Code Analysis**:
- **Problem**: Lines 164-165 in
`/home/sasha/linux/drivers/hwmon/isl28022.c:164-165` treat `regval`
as unsigned, causing negative currents to be interpreted as large
positive values
- **Solution**: The fix properly implements sign extension by
extracting bit 15 as the sign bit and manually computing the two's
complement value: `(value & 0x7FFF) - (sign_bit * 32768)`
- **Consistency**: The fix follows the exact same pattern already
used in the shunt voltage reading function (`isl28022_read_in`) at
lines 120-122, showing this is the established correct approach in
this driver
4. **Risk Assessment**:
- **Minimal Risk**: The change is very small and localized to the
current reading calculation
- **Well-Tested Pattern**: Uses the identical sign handling logic
already proven correct in the shunt voltage function
- **No Side Effects**: Only affects current register interpretation,
doesn't change any other functionality
5. **Driver Context**:
- The ISL28022 driver was introduced in October 2024 (commit
39671a14df4f), making it relatively new
- The driver is hardware-specific and contained within the hwmon
subsystem
- This follows the same pattern as other successful backports shown
in the examples
6. **Comparison to Similar Commits**: This commit is nearly identical to
the reference examples that were marked for backporting:
- **Similar Commit #1** (ina2xx): Fixed sign extension for current
register by adding `(s16)` cast
- **Similar Commit #3** (tmp513): Fixed sign extension with
`sign_extend32(regval, 15)` and type casting
- **Similar Commit #4** (nct7904): Fixed sign extension by converting
to `s8`
- **Similar Commit #5** (ina3221): Fixed shunt sum register bit
shifting
7. **Critical Subsystem**: Hardware monitoring is critical for system
health monitoring, thermal management, and power management.
Incorrect current readings could lead to improper system behavior or
failure to detect dangerous conditions.
**Conclusion**: This is a textbook stable tree candidate - it fixes a
clear functional bug with minimal risk, follows established patterns,
and addresses a real user-impacting issue in hardware monitoring
functionality.
drivers/hwmon/isl28022.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/isl28022.c b/drivers/hwmon/isl28022.c
index 1fb9864635db9..1b4fb0824d6c0 100644
--- a/drivers/hwmon/isl28022.c
+++ b/drivers/hwmon/isl28022.c
@@ -154,6 +154,7 @@ static int isl28022_read_current(struct device *dev, u32 attr, long *val)
struct isl28022_data *data = dev_get_drvdata(dev);
unsigned int regval;
int err;
+ u16 sign_bit;
switch (attr) {
case hwmon_curr_input:
@@ -161,8 +162,9 @@ static int isl28022_read_current(struct device *dev, u32 attr, long *val)
ISL28022_REG_CURRENT, ®val);
if (err < 0)
return err;
- *val = ((long)regval * 1250L * (long)data->gain) /
- (long)data->shunt;
+ sign_bit = (regval >> 15) & 0x01;
+ *val = (((long)(((u16)regval) & 0x7FFF) - (sign_bit * 32768)) *
+ 1250L * (long)data->gain) / (long)data->shunt;
break;
default:
return -EOPNOTSUPP;
--
2.39.5
next prev parent reply other threads:[~2025-06-06 15:42 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-06 15:41 [PATCH AUTOSEL 6.15 01/21] cifs: Correctly set SMB1 SessionKey field in Session Setup Request Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 02/21] cifs: Fix cifs_query_path_info() for Windows NT servers Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 03/21] cifs: Fix encoding of SMB1 Session Setup NTLMSSP Request in non-UNICODE mode Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 04/21] NFSv4: Always set NLINK even if the server doesn't support it Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 05/21] NFSv4.2: fix listxattr to return selinux security label Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 06/21] NFSv4.2: fix setattr caching of TIME_[MODIFY|ACCESS]_SET when timestamps are delegated Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 07/21] mailbox: Not protect module_put with spin_lock_irqsave Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 08/21] mfd: max77541: Fix wakeup source leaks on device unbind Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 09/21] mfd: max14577: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 10/21] mfd: max77705: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 11/21] mfd: 88pm886: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 12/21] mfd: sprd-sc27xx: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 13/21] sunrpc: don't immediately retransmit on seqno miss Sasha Levin
2025-06-06 15:41 ` Sasha Levin [this message]
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 15/21] dm vdo indexer: don't read request structure after enqueuing Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 16/21] leds: multicolor: Fix intensity setting while SW blinking Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 17/21] fuse: fix race between concurrent setattrs from multiple nodes Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 18/21] cxl/region: Add a dev_err() on missing target list entries Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 19/21] cxl: core/region - ignore interleave granularity when ways=1 Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 20/21] NFSv4: xattr handlers should check for absent nfs filehandles Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 21/21] hwmon: (pmbus/max34440) Fix support for max34451 Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250606154147.546388-14-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mail@carsten-spiess.de \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
--cc=yikai.tsai.wiwynn@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.