Re: [PATCH] hwmon (ina2xx) Fix NULL id pointer in probe()
On Fri, Nov 09, 2018 at 04:42:14PM -0800, Nicolin Chen wrote:
> When using DT configurations, the id pointer might turn out to
> be NULL. Then the driver encounters NULL pointer access:
>
> Unable to handle kernel read from unreadable memory at vaddr 0018
> [...]
> PC is at ina2xx_probe+0x114/0x200
> LR is at ina2xx_probe+0x10c/0x200
> [...]
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
>
> The reason is that i2c core returns the id pointer by matching
> id_table with client->name, while the client->name is actually
> using the name from the first string in the DT compatible list,
> not the best one. So i2c core would fail to match the id_table
> if the best matched compatible string isn't the first one, and
> then would return a NULL id pointer.
>
> This probably should be fixed in i2c core. But it doesn't hurt
> to make the driver robust. So this patch fixes it by using the
> "chip" that's added to unify both DT and non-DT configurations.
>
> Additionally, since id pointer could be null, so as id->name:
> ina2xx 10-0047: power monitor (null) (Rshunt = 1000 uOhm)
> ina2xx 10-0048: power monitor (null) (Rshunt = 1 uOhm)
>
> So this patch also fixes NULL name pointer, using client->name
> to play safe and to align with hwmon->name.
>
> Fixes: bd0ddd4d0883 ("hwmon: (ina2xx) Add OF device ID table")
> Signed-off-by: Nicolin Chen
Applied.
Thanks,
Guenter
> ---
> drivers/hwmon/ina2xx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index 71d3445ba869..c2252cf452f5 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -491,7 +491,7 @@ static int ina2xx_probe(struct i2c_client *client,
> }
>
> data->groups[group++] = _group;
> - if (id->driver_data == ina226)
> + if (chip == ina226)
> data->groups[group++] = _group;
>
> hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
> @@ -500,7 +500,7 @@ static int ina2xx_probe(struct i2c_client *client,
> return PTR_ERR(hwmon_dev);
>
> dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
> - id->name, data->rshunt);
> + client->name, data->rshunt);
>
> return 0;
> }
> --
> 2.17.1
>
[PATCH] hwmon (ina2xx) Fix NULL id pointer in probe()
When using DT configurations, the id pointer might turn out to
be NULL. Then the driver encounters NULL pointer access:
Unable to handle kernel read from unreadable memory at vaddr 0018
[...]
PC is at ina2xx_probe+0x114/0x200
LR is at ina2xx_probe+0x10c/0x200
[...]
Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
The reason is that i2c core returns the id pointer by matching
id_table with client->name, while the client->name is actually
using the name from the first string in the DT compatible list,
not the best one. So i2c core would fail to match the id_table
if the best matched compatible string isn't the first one, and
then would return a NULL id pointer.
This probably should be fixed in i2c core. But it doesn't hurt
to make the driver robust. So this patch fixes it by using the
"chip" that's added to unify both DT and non-DT configurations.
Additionally, since id pointer could be null, so as id->name:
ina2xx 10-0047: power monitor (null) (Rshunt = 1000 uOhm)
ina2xx 10-0048: power monitor (null) (Rshunt = 1 uOhm)
So this patch also fixes NULL name pointer, using client->name
to play safe and to align with hwmon->name.
Fixes: bd0ddd4d0883 ("hwmon: (ina2xx) Add OF device ID table")
Signed-off-by: Nicolin Chen
---
drivers/hwmon/ina2xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 71d3445ba869..c2252cf452f5 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -491,7 +491,7 @@ static int ina2xx_probe(struct i2c_client *client,
}
data->groups[group++] = _group;
- if (id->driver_data == ina226)
+ if (chip == ina226)
data->groups[group++] = _group;
hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
@@ -500,7 +500,7 @@ static int ina2xx_probe(struct i2c_client *client,
return PTR_ERR(hwmon_dev);
dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
-id->name, data->rshunt);
+client->name, data->rshunt);
return 0;
}
--
2.17.1
Re: [PATCH v6 00/10] hwmon and fsi: Add On-Chip Controller Driver
On Thu, Nov 08, 2018 at 03:05:19PM -0600, Eddie James wrote: > From: Eddie James > > This series adds a hwmon driver to support the OCC on POWER8 and POWER9 > processors. The OCC is an embedded processor that provides realtime power and > thermal monitoring and management. > > The series also adds a "bus" driver to handle atomic communication between the > service processor and the OCC on a POWER9 chip. This communication takes place > over FSI bus to the SBE (Self-Boot engine) FIFO, which in turn communicates > with the OCC. The driver for the SBEFIFO is already available as an FSI client > driver. > > For POWER8 OCCs, communication between the service processor and the OCC is > achieved over I2C bus. > I am not entirely happy with the series - there are still lots of proprietary attributes, and I would have preferred the use of the _info API at this point - but this has taken long enough. Series applied to hwmon-next. Please send any fixes as follow-up patches. Thanks, Guenter > Changes since v5: > * Makefile fix when compiling both P8 and P9 versions > * Spelling fix in hwmon doc > * Added an additional sentence for P9 binding doc to explain that OCC isn't >an FSI slave device. > > Changes since v4: > * Make the hwmon attributes conform almost completely to standard names and >values. The only exception is powerX_cap_user and powerX_cap_user_source. > * Improve hwmon documentation. > * Add ibm,p9-occ dt documentation. > > Changes since v3: > * Add the FSI OCC driver. > * Pull the sysfs attribute code into it's own file for cleanliness. > * Various fixes for attribute creation and integer overflow. > > Changes since v2: > * Add sysfs_notify for the error and throttling attributes when change is >detected. > * Removed occs_present counting of devices bound. > * Improved remove() of P9 driver to avoid bad behavior with relation to OCC >driver when unbound. > * Added default cases (return EINVAL) for all sensor show functions. > * Added temperature fault sensor. > * Added back dt binding documentation for P9 to address checkpatch warning. > * Added occs_present attribute from the poll response. > > Changes since v1: > * Remove wait loop in P9 code, as that is now handled by FSI OCC driver. > * Removed dt binding documentation for P9, FSI OCC driver will probe OCC > hwmon >driver automatically. > * Moved OCC response code definitions to the OCC include file. > * Fixed includes. > * Changed some structure fields to __beXX as that is what they are. > * Changed some errnos. > * Removed some dev_err(). > * Refactored P8 code a bit to use #defined addresses and magic values, and >changed "goto retry" to a loop. > * Refactored error handling a bit. > > Eddie James (10): > dt-bindings: fsi: Add P9 OCC device documentation > fsi: Add On-Chip Controller (OCC) driver > Documentation: hwmon: Add OCC documentation > dt-bindings: i2c: Add P8 OCC hwmon device documentation > hwmon: Add On-Chip Controller (OCC) hwmon driver > hwmon (occ): Add command transport method for P8 and P9 > hwmon (occ): Parse OCC poll response > hwmon (occ): Add sensor types and versions > hwmon (occ): Add sensor attributes and register hwmon device > hwmon (occ): Add sysfs attributes for additional OCC data > > .../devicetree/bindings/fsi/ibm,p9-occ.txt | 16 + > .../devicetree/bindings/i2c/ibm,p8-occ-hwmon.txt | 25 + > Documentation/hwmon/occ| 112 ++ > drivers/fsi/Kconfig| 10 + > drivers/fsi/Makefile |1 + > drivers/fsi/fsi-occ.c | 599 +++ > drivers/hwmon/Kconfig |2 + > drivers/hwmon/Makefile |1 + > drivers/hwmon/occ/Kconfig | 31 + > drivers/hwmon/occ/Makefile |5 + > drivers/hwmon/occ/common.c | 1098 > > drivers/hwmon/occ/common.h | 128 +++ > drivers/hwmon/occ/p8_i2c.c | 255 + > drivers/hwmon/occ/p9_sbe.c | 106 ++ > drivers/hwmon/occ/sysfs.c | 188 > include/linux/fsi-occ.h| 25 + > 16 files changed, 2602 insertions(+) > create mode 100644 Documentation/devicetree/bindings/fsi/ibm,p9-occ.txt > create mode 100644 Documentation/devicetree/bindings/i2c/ibm,p8-occ-hwmon.txt > create mode 100644 Documentation/hwmon/occ > create mode 100644 drivers/fsi/fsi-occ.c > create mode 100644 drivers/hwmon/occ/Kconfig > create mode 100644 drivers/hwmon/occ/Makefile > create mode 100644 drivers/hwmon/occ/common.c > create mode 100644 drivers/hwmon/occ/common.h > create mode 100644 drivers/hwmon/occ/p8_i2c.c > create mode 100644 drivers/hwmon/occ/p9_sbe.c > create mode 100644 drivers/hwmon/occ/sysfs.c > create
Re: [PATCH v5 0/4] hwmon: (ina3221) Implement PM runtime to save power
On Mon, Nov 05, 2018 at 12:48:39PM -0800, Nicolin Chen wrote: > This series patches implement PM runtime feature in the ina3221 hwmon > driver (PATCH-4). However, PATCH-[1:3] are required to make sure that > the PM runtime feature would be functional and safe. > Series applied to hwmon-next. Thanks, Guenter > Changelog > v4->v5: > * Dropped the change for hwmon core; Shifted all the patch indexes > * Used i2c_client dev pointer for pm runtime callbacks (PATCH-4) > v3->v4: > * Added generic pm runtime functions to hwmon class (PATCH-1) > * Updated to pass pm pointer via _with_info API (PATCH-5) > v2->v3: > * Dropped timeout dev_err messages as it's indicated in errno (PATCH-4) > * Improved a dev_err message and comments (PATCH-5) > v1->v2: > * Added device pointer check (PATCH-1) > * Returned 0 for alert flags (PATCH-2) > * Moved CVRF polling to data read routine (PATCH-4) > * Bypassed i2c_client->dev in suspend/resume() (PATCH-5) > > Nicolin Chen (4): > hwmon: (ina3221) Check channel status for alarms attribute read > hwmon: (ina3221) Serialize sysfs ABI accesses > hwmon: (ina3221) Make sure data is ready before reading > hwmon: (ina3221) Add PM runtime support > > drivers/hwmon/ina3221.c | 190 +++- > 1 file changed, 166 insertions(+), 24 deletions(-) > > -- > 2.17.1 >
