Re: [PATCH] hwmon: lm80: fix a missing check of return value
Hi Kangjie,
Thank you for the patch! Perhaps something to improve:
url:
https://github.com/0day-ci/linux/commits/Kangjie-Lu/hwmon-lm80-fix-a-missing-check-of-return-value/20181222-023000
base:
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
hwmon-next
smatch warnings:
drivers/hwmon/lm80.c:408 set_fan_div() warn: inconsistent returns
'mutex:>update_lock'.
Locked on: line 397
Unlocked on: line 367
#
https://github.com/0day-ci/linux/commit/7612ba0bef1defb6de3290accca07633ccfd3365
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 7612ba0bef1defb6de3290accca07633ccfd3365
vim +408 drivers/hwmon/lm80.c
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 348
1160631b1 drivers/hwmon/lm80.c Guenter Roeck 2012-01-19 349 /*
1160631b1 drivers/hwmon/lm80.c Guenter Roeck 2012-01-19 350 * Note:
we save and restore the fan minimum here, because its value is
1160631b1 drivers/hwmon/lm80.c Guenter Roeck 2012-01-19 351 *
determined in part by the fan divisor. This follows the principle of
1160631b1 drivers/hwmon/lm80.c Guenter Roeck 2012-01-19 352 * least
surprise; the user doesn't expect the fan minimum to change just
1160631b1 drivers/hwmon/lm80.c Guenter Roeck 2012-01-19 353 *
because the divisor changed.
1160631b1 drivers/hwmon/lm80.c Guenter Roeck 2012-01-19 354 */
f8181762a drivers/hwmon/lm80.c Jean Delvare 2008-01-05 355 static
ssize_t set_fan_div(struct device *dev, struct device_attribute *attr,
f8181762a drivers/hwmon/lm80.c Jean Delvare 2008-01-05 356 const
char *buf, size_t count)
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 357 {
f8181762a drivers/hwmon/lm80.c Jean Delvare 2008-01-05 358 int nr
= to_sensor_dev_attr(attr)->index;
118c9a61f drivers/hwmon/lm80.c Guenter Roeck 2014-04-04 359 struct
lm80_data *data = dev_get_drvdata(dev);
118c9a61f drivers/hwmon/lm80.c Guenter Roeck 2014-04-04 360 struct
i2c_client *client = data->client;
6a9e7c4c0 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-10 361
unsigned long min, val;
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 362 u8 reg;
7612ba0be drivers/hwmon/lm80.c Kangjie Lu 2018-12-21 363 int ret;
7612ba0be drivers/hwmon/lm80.c Kangjie Lu 2018-12-21 364
7612ba0be drivers/hwmon/lm80.c Kangjie Lu 2018-12-21 365 ret =
kstrtoul(buf, 10, );
7612ba0be drivers/hwmon/lm80.c Kangjie Lu 2018-12-21 366 if (ret
< 0)
7612ba0be drivers/hwmon/lm80.c Kangjie Lu 2018-12-21 367
return ret;
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 368
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 369 /* Save
fan_min */
9a61bf630 drivers/hwmon/lm80.c Ingo Molnar2006-01-18 370
mutex_lock(>update_lock);
85b3ee07b drivers/hwmon/lm80.c Guenter Roeck 2014-04-13 371 min =
FAN_FROM_REG(data->fan[f_min][nr],
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 372
DIV_FROM_REG(data->fan_div[nr]));
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 373
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 374 switch
(val) {
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 375 case 1:
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 376
data->fan_div[nr] = 0;
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 377
break;
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 378 case 2:
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 379
data->fan_div[nr] = 1;
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 380
break;
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 381 case 4:
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 382
data->fan_div[nr] = 2;
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 383
break;
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 384 case 8:
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 385
data->fan_div[nr] = 3;
66b3b1f75 drivers/hwmon/lm80.c Frans Meulenbroeks 2012-01-04 386
break;
^1da177e4 drivers/i2c/chips/lm80.c Linus Torvalds 2005-04-16 387 default:
118c9a61f drivers/hwmon/lm80.c Guenter Roeck 2014-04-04 388
dev_err(dev,
b55f37572 drivers/hwmon/lm80.c Guenter Roeck 2013-01-10 389
"fan_div value %ld not supported. Choose one of 1, 2, 4 or 8!\n",
b55f37572 drivers/hwmon/lm80.c Guenter Roeck 2013-01-10 390
val);
9a61bf630 drivers/hwmon/lm80.c Ingo Molnar2006-01-18 391
Re: [PATCH 1/5] hwmon: (core) Inherit power properties to hdev
Hi Nicolin,
I love your patch! Perhaps something to improve:
url:
https://github.com/0day-ci/linux/commits/Nicolin-Chen/hwmon-ina3221-Implement-PM-runtime-to-save-power/20181018-010754
base:
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
hwmon-next
smatch warnings:
drivers/hwmon/hwmon.c:631 __hwmon_device_register() warn: variable dereferenced
before check 'dev' (see line 628)
#
https://github.com/0day-ci/linux/commit/aa912316f2d30d4e699ac2f11b6197a4da011274
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout aa912316f2d30d4e699ac2f11b6197a4da011274
vim +/dev +631 drivers/hwmon/hwmon.c
d560168b Guenter Roeck2015-08-26 562
d560168b Guenter Roeck2015-08-26 563 static struct device *
d560168b Guenter Roeck2015-08-26 564 __hwmon_device_register(struct
device *dev, const char *name, void *drvdata,
d560168b Guenter Roeck2015-08-26 565 const struct
hwmon_chip_info *chip,
bab2243c Guenter Roeck2013-07-06 566 const struct
attribute_group **groups)
1236441f Mark M. Hoffman 2005-07-15 567 {
bab2243c Guenter Roeck2013-07-06 568 struct hwmon_device *hwdev;
d560168b Guenter Roeck2015-08-26 569 struct device *hdev;
d560168b Guenter Roeck2015-08-26 570 int i, j, err, id;
ded2b666 Mark M. Hoffman 2006-03-05 571
74d3b641 Guenter Roeck2017-01-27 572 /* Complain about invalid
characters in hwmon name attribute */
648cd48c Guenter Roeck2014-02-28 573 if (name && (!strlen(name) ||
strpbrk(name, "-* \t\n")))
74d3b641 Guenter Roeck2017-01-27 574 dev_warn(dev,
74d3b641 Guenter Roeck2017-01-27 575"hwmon: '%s'
is not a valid name attribute, please fix\n",
74d3b641 Guenter Roeck2017-01-27 576name);
648cd48c Guenter Roeck2014-02-28 577
4ca5f468 Jonathan Cameron 2011-10-31 578 id = ida_simple_get(_ida,
0, 0, GFP_KERNEL);
4ca5f468 Jonathan Cameron 2011-10-31 579 if (id < 0)
4ca5f468 Jonathan Cameron 2011-10-31 580 return ERR_PTR(id);
1236441f Mark M. Hoffman 2005-07-15 581
bab2243c Guenter Roeck2013-07-06 582 hwdev = kzalloc(sizeof(*hwdev),
GFP_KERNEL);
bab2243c Guenter Roeck2013-07-06 583 if (hwdev == NULL) {
bab2243c Guenter Roeck2013-07-06 584 err = -ENOMEM;
bab2243c Guenter Roeck2013-07-06 585 goto ida_remove;
bab2243c Guenter Roeck2013-07-06 586 }
1236441f Mark M. Hoffman 2005-07-15 587
d560168b Guenter Roeck2015-08-26 588 hdev = >dev;
d560168b Guenter Roeck2015-08-26 589
239552f4 Guenter Roeck2016-10-16 590 if (chip) {
d560168b Guenter Roeck2015-08-26 591 struct attribute
**attrs;
b2a4cc3a Guenter Roeck2016-10-16 592 int ngroups = 2; /*
terminating NULL plus >groups */
d560168b Guenter Roeck2015-08-26 593
d560168b Guenter Roeck2015-08-26 594 if (groups)
d560168b Guenter Roeck2015-08-26 595 for (i = 0;
groups[i]; i++)
d560168b Guenter Roeck2015-08-26 596
ngroups++;
d560168b Guenter Roeck2015-08-26 597
d560168b Guenter Roeck2015-08-26 598 hwdev->groups =
devm_kcalloc(dev, ngroups, sizeof(*groups),
d560168b Guenter Roeck2015-08-26 599
GFP_KERNEL);
38d8ed65 Colin Ian King 2016-10-23 600 if (!hwdev->groups) {
38d8ed65 Colin Ian King 2016-10-23 601 err = -ENOMEM;
38d8ed65 Colin Ian King 2016-10-23 602 goto free_hwmon;
38d8ed65 Colin Ian King 2016-10-23 603 }
d560168b Guenter Roeck2015-08-26 604
d560168b Guenter Roeck2015-08-26 605 attrs =
__hwmon_create_attrs(dev, drvdata, chip);
d560168b Guenter Roeck2015-08-26 606 if (IS_ERR(attrs)) {
d560168b Guenter Roeck2015-08-26 607 err =
PTR_ERR(attrs);
d560168b Guenter Roeck2015-08-26 608 goto free_hwmon;
d560168b Guenter Roeck2015-08-26 609 }
d560168b Guenter Roeck2015-08-26 610
d560168b Guenter Roeck2015-08-26 611 hwdev->group.attrs =
attrs;
d560168b Guenter Roeck2015-08-26 612 ngroups = 0;
d560168b Guenter Roeck2015-08-26 613
hwdev->groups[ngroups++] = >group;
d560168b Guenter Roeck2015-08-26 614
d560168b Guenter Roeck2015-08-26 615 if (groups) {
d560168b Guenter Roeck2015-08-26 616 for (i = 0;
groups[i]; i++)
d560168b Guenter Roeck2015-08-26 617
hwdev->groups[ngroups++] = groups[i];
d560168b Guenter Roeck2015-08-26 618 }
d560168b Guenter Roeck2015-08-26 619
Re: [bug report] hwmon: (nct6775) Add support for weighted fan control
On Wed, Sep 05, 2018 at 03:05:00PM -0700, Guenter Roeck wrote:
> Hi Dan,
>
> On 09/05/2018 12:57 AM, Dan Carpenter wrote:
> > Done. Btw, I saw another that looks legit.
> >
> Thanks ...
>
> > drivers/hwmon/nct6775.c:1687 nct6775_update_device()
> > error: buffer overflow 'data->FAN_PULSE_SHIFT' 6 <= 6
> >
> > drivers/hwmon/nct6775.c
> >1667 data->in[i][2] = nct6775_read_value(data,
> >1668
> > data->REG_IN_MINMAX[1][i]);
> >1669 }
> >1670
> >1671 /* Measured fan speeds and limits */
> >1672 for (i = 0; i < ARRAY_SIZE(data->rpm); i++) {
> >
> > This is a 7 element array.
> >
> >1673 u16 reg;
> >1674
> >1675 if (!(data->has_fan & BIT(i)))
> >^^
> >
> > I probably wouldn't have reported this originally because Smatch is bad
> > at these checks. (I just haven't gotten around to it yet). But we do
> > have fan7pin so it looks like BIT(6) can be set.
> >
> >1676 continue;
> >1677
> >1678 reg = nct6775_read_value(data,
> > data->REG_FAN[i]);
> >1679 data->rpm[i] = data->fan_from_reg(reg,
> >1680
> > data->fan_div[i]);
> >1681
> >1682 if (data->has_fan_min & BIT(i))
> >1683 data->fan_min[i] =
> > nct6775_read_value(data,
> >1684 data->REG_FAN_MIN[i]);
> >1685 data->fan_pulses[i] =
> >1686(nct6775_read_value(data,
> > data->REG_FAN_PULSES[i])
> >1687 >> data->FAN_PULSE_SHIFT[i]) &
> > 0x03;
> >
> > FAN_PULSE_SHIFT is either 5 or 6 elements.
> >
>
> Array size of xxx_REG_FAN_PULSES[] and xxx_FAN_PULSE_SHIFT[] should probably
> be NUM_FAN to be on the safe side. There is a slight secondary problem,
> though:
> data->REG_FAN_PULSES[i] should not be read if it is 0. It doesn't matter much
> -
> it results in an unnecessary read from register 0 - but it is still
> undesirable.
>
> Care to send another patch ?
Can you do that one and give me a Reported-by tag? It seems slightly
more involved and I am dumb.
regards,
dan carpenter
Re: [bug report] hwmon: (nct6775) Add support for weighted fan control
Done. Btw, I saw another that looks legit.
drivers/hwmon/nct6775.c:1687 nct6775_update_device()
error: buffer overflow 'data->FAN_PULSE_SHIFT' 6 <= 6
drivers/hwmon/nct6775.c
1667 data->in[i][2] = nct6775_read_value(data,
1668data->REG_IN_MINMAX[1][i]);
1669 }
1670
1671 /* Measured fan speeds and limits */
1672 for (i = 0; i < ARRAY_SIZE(data->rpm); i++) {
This is a 7 element array.
1673 u16 reg;
1674
1675 if (!(data->has_fan & BIT(i)))
^^
I probably wouldn't have reported this originally because Smatch is bad
at these checks. (I just haven't gotten around to it yet). But we do
have fan7pin so it looks like BIT(6) can be set.
1676 continue;
1677
1678 reg = nct6775_read_value(data,
data->REG_FAN[i]);
1679 data->rpm[i] = data->fan_from_reg(reg,
1680
data->fan_div[i]);
1681
1682 if (data->has_fan_min & BIT(i))
1683 data->fan_min[i] =
nct6775_read_value(data,
1684 data->REG_FAN_MIN[i]);
1685 data->fan_pulses[i] =
1686(nct6775_read_value(data,
data->REG_FAN_PULSES[i])
1687 >> data->FAN_PULSE_SHIFT[i]) & 0x03;
FAN_PULSE_SHIFT is either 5 or 6 elements.
1688
1689 nct6775_select_fan_div(dev, data, i, reg);
1690 }
1691
1692 nct6775_update_pwm(dev);
1693 nct6775_update_pwm_limits(dev);
1694
regards,
dan carpenter
[PATCH 1/2] hwmon: (nct6775) Set weight source to zero correctly
This is dead code because j can never be 1 at this point. We had
intended to just test if the bit was clear.
Fixes: bbd8decd4123 ("hwmon: (nct6775) Add support for weighted fan control")
Signed-off-by: Dan Carpenter
diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c
index 944f5b63aecd..139781ae830b 100644
--- a/drivers/hwmon/nct6775.c
+++ b/drivers/hwmon/nct6775.c
@@ -1558,7 +1558,7 @@ static void nct6775_update_pwm(struct device *dev)
reg = nct6775_read_value(data, data->REG_WEIGHT_TEMP_SEL[i]);
data->pwm_weight_temp_sel[i] = reg & 0x1f;
/* If weight is disabled, report weight source as 0 */
- if (j == 1 && !(reg & 0x80))
+ if (!(reg & 0x80))
data->pwm_weight_temp_sel[i] = 0;
/* Weight temp data */
[PATCH 2/2] hwmon: (nct6775) Clean up a condition
I removed the "dsw_en &&" chunk of the condition because we know that "dsw_en" is set. Signed-off-by: Dan Carpenter diff --git a/drivers/hwmon/nct6775.c b/drivers/hwmon/nct6775.c index 139781ae830b..719effe53ceb 100644 --- a/drivers/hwmon/nct6775.c +++ b/drivers/hwmon/nct6775.c @@ -3533,8 +3533,7 @@ nct6775_check_fan_inputs(struct nct6775_data *data) if (!fan6pin) fan6pin = (regval_2a & BIT(4)) && - (!dsw_en || - (dsw_en && (regval_ed & BIT(4; + (!dsw_en || (regval_ed & BIT(4))); if (!pwm6pin) pwm6pin = (regval_2a & BIT(3)) && (regval_ed & BIT(2));
[bug report] hwmon: (nct6775) Add support for weighted fan control
Hello Guenter Roeck,
The patch bbd8decd4123: "hwmon: (nct6775) Add support for weighted
fan control" from Dec 4, 2012, leads to the following static checker
warning:
drivers/hwmon/nct6775.c:1562 nct6775_update_pwm()
warn: dead code because of 'j == 1' and 'j < (56 / 8 + 0)'
drivers/hwmon/nct6775.c
1503 static void nct6775_update_pwm(struct device *dev)
1504 {
1505 struct nct6775_data *data = dev_get_drvdata(dev);
1506 int i, j;
1507 int fanmodecfg, reg;
1508 bool duty_is_dc;
1509
1510 for (i = 0; i < data->pwm_num; i++) {
1511 if (!(data->has_pwm & BIT(i)))
1512 continue;
1513
1514 duty_is_dc = data->REG_PWM_MODE[i] &&
1515(nct6775_read_value(data, data->REG_PWM_MODE[i])
1516 & data->PWM_MODE_MASK[i]);
1517 data->pwm_mode[i] = !duty_is_dc;
1518
1519 fanmodecfg = nct6775_read_value(data,
data->REG_FAN_MODE[i]);
1520 for (j = 0; j < ARRAY_SIZE(data->REG_PWM); j++) {
^
j is set to ARRAY_SIZE(data->REG_PWM) which is 7 at the end of this
loop.
1521 if (data->REG_PWM[j] && data->REG_PWM[j][i]) {
1522 data->pwm[j][i]
1523= nct6775_read_value(data,
1524
data->REG_PWM[j][i]);
1525 }
1526 }
1527
1528 data->pwm_enable[i] = reg_to_pwm_enable(data->pwm[0][i],
1529 (fanmodecfg >>
4) & 7);
1530
1531 if (!data->temp_tolerance[0][i] ||
1532 data->pwm_enable[i] != speed_cruise)
1533 data->temp_tolerance[0][i] = fanmodecfg & 0x0f;
1534 if (!data->target_speed_tolerance[i] ||
1535 data->pwm_enable[i] == speed_cruise) {
1536 u8 t = fanmodecfg & 0x0f;
1537
1538 if (data->REG_TOLERANCE_H) {
1539 t |= (nct6775_read_value(data,
1540data->REG_TOLERANCE_H[i]) & 0x70)
>> 1;
1541 }
1542 data->target_speed_tolerance[i] = t;
1543 }
1544
1545 data->temp_tolerance[1][i] =
1546 nct6775_read_value(data,
1547
data->REG_CRITICAL_TEMP_TOLERANCE[i]);
1548
1549 reg = nct6775_read_value(data, data->REG_TEMP_SEL[i]);
1550 data->pwm_temp_sel[i] = reg & 0x1f;
1551 /* If fan can stop, report floor as 0 */
1552 if (reg & 0x80)
1553 data->pwm[2][i] = 0;
1554
1555 if (!data->REG_WEIGHT_TEMP_SEL[i])
1556 continue;
1557
1558 reg = nct6775_read_value(data,
data->REG_WEIGHT_TEMP_SEL[i]);
1559 data->pwm_weight_temp_sel[i] = reg & 0x1f;
1560 /* If weight is disabled, report weight source as 0 */
1561 if (j == 1 && !(reg & 0x80))
^^
So it can't be 1 here. Did you intend to say "i == 1"?
1562 data->pwm_weight_temp_sel[i] = 0;
1563
1564 /* Weight temp data */
1565 for (j = 0; j < ARRAY_SIZE(data->weight_temp); j++) {
1566 data->weight_temp[j][i]
1567= nct6775_read_value(data,
1568
data->REG_WEIGHT_TEMP[j][i]);
1569 }
1570 }
1571 }
regards,
dan carpenter
[PATCH 2/2 v2] hwmon: (adt7475) Make adt7475_read_word() return errors
The adt7475_read_word() function was meant to return negative error
codes on failure.
Signed-off-by: Dan Carpenter
---
v2: In my first patch I just removed the error handling in the callers
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 90837f7c7d0f..ec03359536aa 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -302,14 +302,18 @@ static inline u16 volt2reg(int channel, long volt, u8
bypass_attn)
return clamp_val(reg, 0, 1023) & (0xff << 2);
}
-static u16 adt7475_read_word(struct i2c_client *client, int reg)
+static int adt7475_read_word(struct i2c_client *client, int reg)
{
- u16 val;
+ int val1, val2;
- val = i2c_smbus_read_byte_data(client, reg);
- val |= (i2c_smbus_read_byte_data(client, reg + 1) << 8);
+ val1 = i2c_smbus_read_byte_data(client, reg);
+ if (val1 < 0)
+ return val1;
+ val2 = i2c_smbus_read_byte_data(client, reg + 1);
+ if (val2 < 0)
+ return val2;
- return val;
+ return val1 | (val2 << 8);
}
static void adt7475_write_word(struct i2c_client *client, int reg, u16 val)
Re: [PATCH 2/2] hwmon: (adt7475) Remove some unnecessary checks
On Tue, Aug 14, 2018 at 09:34:30AM +, IKEGAMI Tokunori wrote: > Hi Dan-san, > > Thank you so much for this fix also. > > Reviewed-by: Tokunori Ikegami > > By the way I will do try to change the adt7475_read_word() to return the > error correctly in near future. > I can do that. Drop this patch. I will resend. regards, dan carpenter
[PATCH 2/2] hwmon: (adt7475) Remove some unnecessary checks
The adt7475_read_word() returns u16 values, so it's impossible for
"ret" to be negative. The check is harmless, but static checkers
complain about it.
Signed-off-by: Dan Carpenter
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 16045149f3db..9d3da8ea38ba 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -1492,10 +1492,7 @@ static int adt7475_update_limits(struct i2c_client
*client)
for (i = 0; i < ADT7475_TACH_COUNT; i++) {
if (i == 3 && !data->has_fan4)
continue;
- ret = adt7475_read_word(client, TACH_MIN_REG(i));
- if (ret < 0)
- return ret;
- data->tach[MIN][i] = ret;
+ data->tach[MIN][i] = adt7475_read_word(client, TACH_MIN_REG(i));
}
for (i = 0; i < ADT7475_PWM_COUNT; i++) {
@@ -1881,10 +1878,7 @@ static int adt7475_update_measure(struct device *dev)
for (i = 0; i < ADT7475_TACH_COUNT; i++) {
if (i == 3 && !data->has_fan4)
continue;
- ret = adt7475_read_word(client, TACH_REG(i));
- if (ret < 0)
- return ret;
- data->tach[INPUT][i] = ret;
+ data->tach[INPUT][i] = adt7475_read_word(client, TACH_REG(i));
}
/* Updated by hw when in auto mode */
[PATCH 1/2] hwmon: (adt7475) Potential error pointer dereferences
The adt7475_update_device() function returns error pointers. The
problem is that in show_pwmfreq() we dereference it before the check.
And then in pwm_use_point2_pwm_at_crit_show() there isn't a check at
all. I don't know if it's required, but it silences a static checker
warning and it's doesn't hurt anything to check.
Signed-off-by: Dan Carpenter
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 90837f7c7d0f..16045149f3db 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -962,13 +962,14 @@ static ssize_t show_pwmfreq(struct device *dev, struct
device_attribute *attr,
{
struct adt7475_data *data = adt7475_update_device(dev);
struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
- int i = clamp_val(data->range[sattr->index] & 0xf, 0,
- ARRAY_SIZE(pwmfreq_table) - 1);
+ int idx;
if (IS_ERR(data))
return PTR_ERR(data);
+ idx = clamp_val(data->range[sattr->index] & 0xf, 0,
+ ARRAY_SIZE(pwmfreq_table) - 1);
- return sprintf(buf, "%d\n", pwmfreq_table[i]);
+ return sprintf(buf, "%d\n", pwmfreq_table[idx]);
}
static ssize_t set_pwmfreq(struct device *dev, struct device_attribute *attr,
@@ -1004,6 +1005,10 @@ static ssize_t pwm_use_point2_pwm_at_crit_show(struct
device *dev,
char *buf)
{
struct adt7475_data *data = adt7475_update_device(dev);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
return sprintf(buf, "%d\n", !!(data->config4 & CONFIG4_MAXDUTY));
}
Re: [PATCH v4 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
Hi Tomer,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.18-rc2 next-20180622]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Tomer-Maimon/dt-binding-hwmon-Add-NPCM7xx-PWM-and-Fan-controller-documentation/20180624-205017
base:
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
hwmon-next
New smatch warnings:
drivers/hwmon/npcm750-pwm-fan.c:261 npcm7xx_pwm_config_set() warn: inconsistent
returns 'mutex:>pwm_lock[module]'.
Locked on: line 245
Unlocked on: line 261
Old smatch warnings:
drivers/hwmon/npcm750-pwm-fan.c:836 npcm7xx_pwm_cz_set_cur_state() warn:
potential spectre issue 'cdev->cooling_levels'
#
https://github.com/0day-ci/linux/commit/5ef6a0a11de5f3f0711993a20b13820cc0884c7e
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 5ef6a0a11de5f3f0711993a20b13820cc0884c7e
vim +261 drivers/hwmon/npcm750-pwm-fan.c
5ef6a0a1 Tomer Maimon 2018-06-24 210
5ef6a0a1 Tomer Maimon 2018-06-24 211 static int npcm7xx_pwm_config_set(struct
npcm7xx_pwm_fan_data *data,
5ef6a0a1 Tomer Maimon 2018-06-24 212 int channel,
u16 val)
5ef6a0a1 Tomer Maimon 2018-06-24 213 {
5ef6a0a1 Tomer Maimon 2018-06-24 214 u32 pwm_ch = (channel %
NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
5ef6a0a1 Tomer Maimon 2018-06-24 215 u32 module = (channel /
NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
5ef6a0a1 Tomer Maimon 2018-06-24 216 u32 tmp_buf, ctrl_en_bit, env_bit;
5ef6a0a1 Tomer Maimon 2018-06-24 217
5ef6a0a1 Tomer Maimon 2018-06-24 218 /*
5ef6a0a1 Tomer Maimon 2018-06-24 219* Config PWM Comparator register for
setting duty cycle
5ef6a0a1 Tomer Maimon 2018-06-24 220*/
5ef6a0a1 Tomer Maimon 2018-06-24 221 mutex_lock(>pwm_lock[module]);
5ef6a0a1 Tomer Maimon 2018-06-24 222
5ef6a0a1 Tomer Maimon 2018-06-24 223 /* write new CMR value */
5ef6a0a1 Tomer Maimon 2018-06-24 224 iowrite32(val,
NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, pwm_ch));
5ef6a0a1 Tomer Maimon 2018-06-24 225 tmp_buf =
ioread32(NPCM7XX_PWM_REG_CR(data->pwm_base, module));
5ef6a0a1 Tomer Maimon 2018-06-24 226
5ef6a0a1 Tomer Maimon 2018-06-24 227 switch (pwm_ch) {
5ef6a0a1 Tomer Maimon 2018-06-24 228 case 0:
5ef6a0a1 Tomer Maimon 2018-06-24 229 ctrl_en_bit =
NPCM7XX_PWM_CTRL_CH0_EN_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 230 env_bit =
NPCM7XX_PWM_CTRL_CH0_INV_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 231 break;
5ef6a0a1 Tomer Maimon 2018-06-24 232 case 1:
5ef6a0a1 Tomer Maimon 2018-06-24 233 ctrl_en_bit =
NPCM7XX_PWM_CTRL_CH1_EN_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 234 env_bit =
NPCM7XX_PWM_CTRL_CH1_INV_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 235 break;
5ef6a0a1 Tomer Maimon 2018-06-24 236 case 2:
5ef6a0a1 Tomer Maimon 2018-06-24 237 ctrl_en_bit =
NPCM7XX_PWM_CTRL_CH2_EN_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 238 env_bit =
NPCM7XX_PWM_CTRL_CH2_INV_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 239 break;
5ef6a0a1 Tomer Maimon 2018-06-24 240 case 3:
5ef6a0a1 Tomer Maimon 2018-06-24 241 ctrl_en_bit =
NPCM7XX_PWM_CTRL_CH3_EN_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 242 env_bit =
NPCM7XX_PWM_CTRL_CH3_INV_BIT;
5ef6a0a1 Tomer Maimon 2018-06-24 243 break;
5ef6a0a1 Tomer Maimon 2018-06-24 244 default:
5ef6a0a1 Tomer Maimon 2018-06-24 245 return -ENODEV;
^^
5ef6a0a1 Tomer Maimon 2018-06-24 246 }
5ef6a0a1 Tomer Maimon 2018-06-24 247
5ef6a0a1 Tomer Maimon 2018-06-24 248 if (val == 0) {
5ef6a0a1 Tomer Maimon 2018-06-24 249 /* Disable PWM */
5ef6a0a1 Tomer Maimon 2018-06-24 250 tmp_buf &= ~ctrl_en_bit;
5ef6a0a1 Tomer Maimon 2018-06-24 251 tmp_buf |= env_bit;
5ef6a0a1 Tomer Maimon 2018-06-24 252 } else {
5ef6a0a1 Tomer Maimon 2018-06-24 253 /* Enable PWM */
5ef6a0a1 Tomer Maimon 2018-06-24 254 tmp_buf |= ctrl_en_bit;
5ef6a0a1 Tomer Maimon 2018-06-24 255 tmp_buf &= ~env_bit;
5ef6a0a1 Tomer Maimon 2018-06-24 256 }
5ef6a0a1 Tomer Maimon 2018-06-24 257
5ef6a0a1 Tomer Maimon 2018-06-24 258 iowrite32(tmp_buf,
NPCM7XX_PWM_REG_CR(data->pwm_base, module));
5ef6a0a1 Tomer Maimon 2018-06-24 259 mutex_unlock(>pwm_lock[module]);
5ef6a0a1 Tomer Maimon 2018-06-24 260
5ef6a0a1 Tomer Maimon 2018-06-24 @261 return 0;
5ef6a0a1 Tomer Maimon 2018-06-24 262 }
5ef6a0a1 Tomer Maimon 2018-06-24 263
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to
[PATCH] hwmon: (w83773g) Fix a precedence bug in get_fault()
Shift has higher precedence than mask but presumably we wanted to set
*val to 1 if BIT(2) was set or 0 if the bit was clear.
Fixes: de0a524e3e09 ("hwmon: Add W83773G driver")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/hwmon/w83773g.c b/drivers/hwmon/w83773g.c
index 0b97c285b049..e858093ac806 100644
--- a/drivers/hwmon/w83773g.c
+++ b/drivers/hwmon/w83773g.c
@@ -102,7 +102,7 @@ static int get_fault(struct regmap *regmap, int index, long
*val)
if (ret < 0)
return ret;
- *val = (u8)regval & 0x04 >> 2;
+ *val = (regval & 0x04) >> 2;
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] hwmon: (max6621) Inverted if condition in max6621_read()
We intended to test for failure here but accidentally tested for
success. It means that we don't set "*val" to true and it means that
if i2c_smbus_write_byte() does fail then we return success.
Fixes: e7895864b0d7 ("hwmon: (max6621) Add support for Maxim MAX6621
temperature sensor")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/hwmon/max6621.c b/drivers/hwmon/max6621.c
index 22079ec29660..3f0eefb9 100644
--- a/drivers/hwmon/max6621.c
+++ b/drivers/hwmon/max6621.c
@@ -296,7 +296,7 @@ max6621_read(struct device *dev, enum hwmon_sensor_types
type, u32 attr,
if (regval) {
ret = i2c_smbus_write_byte(data->client,
MAX6621_CLEAR_ALERT_REG);
- if (!ret)
+ if (ret)
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (pmbus) Add missing break statements
On Thu, May 04, 2017 at 09:28:19AM +0200, walter harms wrote:
>
>
> Am 03.05.2017 21:31, schrieb Dan Carpenter:
> > Static checkers complain about these missing break statements.
> >
> > Fixes: 6eaaea144dc5 ("hwmon: (pmbus) Add client driver for IR35221")
> > Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> >
> > diff --git a/drivers/hwmon/pmbus/ir35221.c b/drivers/hwmon/pmbus/ir35221.c
> > index cc7b3b542531..00e4a1e264e2 100644
> > --- a/drivers/hwmon/pmbus/ir35221.c
> > +++ b/drivers/hwmon/pmbus/ir35221.c
> > @@ -129,6 +129,7 @@ static int ir35221_read_word_data(struct i2c_client
> > *client, int page, int reg)
> > case PMBUS_IIN_OC_WARN_LIMIT:
> > ret = pmbus_read_word_data(client, page, reg);
> > ret = ir35221_scale_result(ret, -1, PSC_CURRENT_IN);
> > + break;
> > case PMBUS_READ_VIN:
> > ret = pmbus_read_word_data(client, page, PMBUS_READ_VIN);
> > ret = ir35221_scale_result(ret, -5, PSC_VOLTAGE_IN);
>
> Just a remark:
> the naming of the variable for pmbus_read_word_data() is unfortunate.
> It would be nice to have it like below: val
>
Yeah. I thought so too.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] hwmon: (pmbus) Add missing break statements
Static checkers complain about these missing break statements.
Fixes: 6eaaea144dc5 ("hwmon: (pmbus) Add client driver for IR35221")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/hwmon/pmbus/ir35221.c b/drivers/hwmon/pmbus/ir35221.c
index cc7b3b542531..00e4a1e264e2 100644
--- a/drivers/hwmon/pmbus/ir35221.c
+++ b/drivers/hwmon/pmbus/ir35221.c
@@ -129,6 +129,7 @@ static int ir35221_read_word_data(struct i2c_client
*client, int page, int reg)
case PMBUS_IIN_OC_WARN_LIMIT:
ret = pmbus_read_word_data(client, page, reg);
ret = ir35221_scale_result(ret, -1, PSC_CURRENT_IN);
+ break;
case PMBUS_READ_VIN:
ret = pmbus_read_word_data(client, page, PMBUS_READ_VIN);
ret = ir35221_scale_result(ret, -5, PSC_VOLTAGE_IN);
@@ -222,6 +223,7 @@ static int ir35221_write_word_data(struct i2c_client
*client, int page, int reg,
case PMBUS_IIN_OC_WARN_LIMIT:
val = ir35221_scale_result(word, 1, PSC_CURRENT_IN);
ret = pmbus_write_word_data(client, page, reg, val);
+ break;
default:
ret = -ENODATA;
break;
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] hwmon: (tacho) Remove an unused variable
We never use this global variable.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
index 2f22add5c73b..b96fff50a3ee 100644
--- a/drivers/hwmon/aspeed-pwm-tacho.c
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -153,8 +153,6 @@
#define M_TACH_UNIT 0x1000
#define INIT_FAN_CTRL 0xFF
-void __iomem *regs;
-
struct aspeed_pwm_tacho_data {
struct regmap *regmap;
unsigned long clk_freq;
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] hwmon: (tacho) Fix a condition in aspeed_get_fan_tach_ch_rpm()
What we want is for regmap_read() to return zero meaning success and
for RESULT_STATUS_MASK which is BIT(31) to be set set.
The current code is buggy in two ways. It requires both failure
conditions should be met before it fails. And if it times out then it
returns zero instead of -EIO.
Fixes: cc96f6f6d766 ("hwmon: Support for ASPEED PWM/Fan tach")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
Static analysis. Not tested.
diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
index 29010ad94208..2f22add5c73b 100644
--- a/drivers/hwmon/aspeed-pwm-tacho.c
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -512,11 +512,11 @@ static u32 aspeed_get_fan_tach_ch_rpm(struct
aspeed_pwm_tacho_data *priv,
msleep(sec);
- while (!(regmap_read(priv->regmap, ASPEED_PTCR_RESULT, ))
- & !(val & RESULT_STATUS_MASK)) {
+ while (regmap_read(priv->regmap, ASPEED_PTCR_RESULT, ) ||
+ !(val & RESULT_STATUS_MASK)) {
timeout++;
if (timeout > 1)
- return 0;
+ return -EIO;
msleep(sec);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] hwmon: (max31790) potential ERR_PTR dereference
We should only dereference "data" after we check if it is an error
pointer.
Fixes: 54187ff9d766 ('hwmon: (max31790) Convert to use new hwmon registration
API')
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/hwmon/max31790.c b/drivers/hwmon/max31790.c
index bef84e0..c1b9275 100644
--- a/drivers/hwmon/max31790.c
+++ b/drivers/hwmon/max31790.c
@@ -268,11 +268,13 @@ static int max31790_read_pwm(struct device *dev, u32
attr, int channel,
long *val)
{
struct max31790_data *data = max31790_update_device(dev);
- u8 fan_config = data->fan_config[channel];
+ u8 fan_config;
if (IS_ERR(data))
return PTR_ERR(data);
+ fan_config = data->fan_config[channel];
+
switch (attr) {
case hwmon_pwm_input:
*val = data->pwm[channel] >> 8;
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
