* Re: [PATCH RFC] hwmon: Add Support for PMIC5000 Power Management ICs
2026-08-19 13:37 [PATCH RFC] hwmon: Add Support for PMIC5000 Power Management ICs Stephen Horvath
@ 2026-08-19 13:51 ` sashiko-bot
2026-08-20 15:34 ` Guenter Roeck
1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-19 13:51 UTC (permalink / raw)
To: Stephen Horvath; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] Race condition: sysfs registration before hardware initialization
- [Medium] Incorrect error code for out-of-range values
- [Medium] hwmon update_interval attribute violates ABI units (returns raw values instead of milliseconds)
- [Medium] Architectural layering violation: PMIC directly registered as hwmon I2C driver
- [Low] C11 syntax error due to declaration directly after a label
--
--- Patch [1]: [PATCH RFC] hwmon: Add Support for PMIC5000 Power Management ICs ---
commit 4dd6cacf5a9ff70a4083c91afc8539b2aae42832
Author: Stephen Horvath <linux@stevetech.au>
hwmon: Add Support for PMIC5000 Power Management ICs
Add support for sensors present on PMIC5000 (JEDEC JESD301) compliant
power management ICs. These chips are commonly found on DDR5 memory
modules.
> diff --git a/drivers/hwmon/pmic5000.c b/drivers/hwmon/pmic5000.c
> index 0000000000000..d3f3126dd9c80
> --- /dev/null
> +++ b/drivers/hwmon/pmic5000.c
[ ... ]
> +static int pmic5000_read_temp(struct regmap *regmap, u32 attr, int channel,
> + long *val)
> +{
> + int err;
> + u32 regval;
> +
> + if (channel != 0)
> + return -EOPNOTSUPP;
> +
> + switch (attr) {
> + case hwmon_temp_input: {
> + err = regmap_read(regmap, PMIC5000_REG_TEMPERATURE, ®val);
> + if (err)
> + return err;
> + regval >>= 5;
> + /* Below 85°C */
> + if (regval == 0)
> + return -EOPNOTSUPP;
[Severity: Medium]
Since returning -EOPNOTSUPP causes userspace tools to treat the attribute
as permanently unsupported by the chip, should this return -ENODATA instead
when the temperature is temporarily out of range?
> + /* 0b001 = 85°C, 0b010 = 95°C, etc. */
> + *val = (75 + regval * 10) * MILLIDEGREE_PER_DEGREE;
> + return 0;
> + }
[ ... ]
> +static int pmic5000_read_volt_thresholds(struct regmap *regmap, u32 attr,
> + int channel, long *val)
> +{
[ ... ]
> + volt_set = range_regval & range_bit ? base_volts[1] : base_volts[0];
> + volt_set += (set_regval >> 1) * 5;
> +
> + switch (attr) {
> + case hwmon_in_min:
> + /* 10%, 12.5%, Reserved, Reserved */
> + const int min_permilles[4] = { 100, 125, PERMILLE, PERMILLE };
[Severity: Low]
Can declaring a variable immediately following a case label cause a build
failure? The C11 standard expects a statement after a label, rather than
a declaration. Would wrapping the contents of this case block in braces
resolve this syntax error?
> + *val = volt_set - (min_permilles[(thresh_regval >> 2) & 0x03] *
> + volt_set / PERMILLE);
> + return 0;
> + case hwmon_in_max:
> + /* 7.5%, 10%, 12.5%, Reserved */
> + const int max_permilles[4] = { 75, 100, 125, PERMILLE };
> + *val = volt_set + (max_permilles[(thresh_regval >> 4) & 0x03] *
> + volt_set / PERMILLE);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
[ ... ]
> +static int pmic5000_read_interval(struct regmap *regmap, u32 attr, long *val)
> +{
> + unsigned int regval;
> + int err;
> +
> + if (attr != hwmon_chip_update_interval)
> + return -EOPNOTSUPP;
> +
> + err = regmap_read(regmap, PMIC5000_REG_ADC_CONFIG, ®val);
> + if (err < 0)
> + return err;
> + *val = 1 << (regval & 0x03);
> + return 0;
> +}
[Severity: Medium]
Is this exposing raw values like 1, 2, 4, or 8 instead of milliseconds?
The hwmon ABI specifies that update_interval should be reported and set
in milliseconds. Do these values need to be translated to match the ABI
requirements?
[ ... ]
> +static int pmic5000_common_probe(struct device *dev, struct regmap *regmap)
> +{
> + unsigned int revision, vendor, bank;
> + struct pmic5000_data *data;
> + struct device *hwmon_dev;
> + int err;
[ ... ]
> + hwmon_dev = devm_hwmon_device_register_with_info(
> + dev, "pmic5000", data, &pmic5000_chip_info, NULL);
> + if (IS_ERR(hwmon_dev))
> + return PTR_ERR(hwmon_dev);
> +
> + dev_info(dev, "DDR5 PMIC sensor: vendor 0x%02x:0x%02x revision %d.%d\n",
> + bank & 0x7f, vendor, ((revision >> 4) & 0x03) + 1,
> + ((revision >> 1) & 0x07) + 1);
> +
> + /* Enable individual measurements and enable ADC */
> + err = regmap_update_bits(regmap, PMIC5000_REG_OUTPUT_SELECT,
> + PMIC5000_OUTPUT_SELECT,
> + PMIC5000_OUTPUT_SELECT);
> + if (err)
> + return err;
> + err = regmap_update_bits(regmap, PMIC5000_REG_ADC_CONFIG,
> + PMIC5000_ADC_ENABLE, PMIC5000_ADC_ENABLE);
[Severity: Medium]
Does this expose the sysfs files before the ADC is actually enabled? If
userspace polling tools access the sysfs attributes right after
devm_hwmon_device_register_with_info() completes, could it trigger reads on
uninitialized hardware? Should the hardware initialization happen before
registration?
[ ... ]
> +static struct i2c_driver pmic5000_i2c_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "pmic5000",
> + .of_match_table = pmic5000_of_ids,
> + .pm = pm_sleep_ptr(&pmic5000_pm_ops),
> + },
> + .probe = pmic5000_i2c_probe,
> + .id_table = pmic5000_i2c_id,
> +};
> +
> +module_i2c_driver(pmic5000_i2c_driver);
[Severity: Medium]
Is this driver placed in the correct subsystem? For PMIC chips where hardware
monitoring isn't the primary function, the hwmon subsystem guidelines state
the core driver should reside in drivers/mfd or drivers/regulator, and
instantiate the hwmon functionality as an auxiliary device. Does binding
directly to the I2C device here prevent regulator drivers from attaching to
this PMIC later?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-pmic5000-v1-1-e5e0ddcf0863@stevetech.au?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFC] hwmon: Add Support for PMIC5000 Power Management ICs
2026-08-19 13:37 [PATCH RFC] hwmon: Add Support for PMIC5000 Power Management ICs Stephen Horvath
2026-08-19 13:51 ` sashiko-bot
@ 2026-08-20 15:34 ` Guenter Roeck
2026-08-21 3:27 ` Stephen Horvath
1 sibling, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2026-08-20 15:34 UTC (permalink / raw)
To: Stephen Horvath
Cc: Andi Shyti, linux-hwmon, linux-i2c, linux-kernel, Stephen Horvath
On Wed, Aug 19, 2026 at 11:37:45PM +1000, Stephen Horvath wrote:
> Add support for sensors present on PMIC5000 (JEDEC JESD301) compliant
> power management ICs. These chips are commonly found on DDR5 memory
> modules.
>
> Tested against PMIC5100 chips, but only features present in the
> PMIC5000 (JESD301-1) specification have been implemented.
>
> Signed-off-by: Stephen Horvath <linux@stevetech.au>
> ---
> Hi maintainers, I wrote this over the past couple days and I'm looking
> for feedback.
>
> Here's a few concerns I have:
> - I'm not too experienced with kernel development.
> - I don't think there's a good way to auto-detect PMIC5000 chips
> standalone, would it be reasonable to instantiate it from
> spd5118?
> - I'm unsure what to do with temperatures below 85°C, I'd rather not
> return 85 as that would make it seem hotter than it is, but
> returning -EOPNOTSUPP seems wrong.
> - Toggling between current and power seems like it's a bit of a hack,
> is that okay, or should I just pick one and drop the other?
> - Does this belong in HWMON, or should it be part of the regulator
> subsystem?
It depends on its functionality. Is this just for monitoring, or are the voltages
controllable ?
> ---
> .../devicetree/bindings/trivial-devices.yaml | 2 +
> Documentation/hwmon/index.rst | 1 +
> Documentation/hwmon/pmic5000.rst | 103 +++
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/pmic5000.c | 896 +++++++++++++++++++++
> 6 files changed, 1015 insertions(+)
>
>
> ---
> base-commit: 75f2c0b3690702c90863c2e138cb5520670845ea
> change-id: 20260819-pmic5000-3840f8a9ade1
>
> Best regards,
> --
> Stephen Horvath <linux@stevetech.au>
>
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 2de8eb09cb7d..86e76be7d41b 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -194,6 +194,8 @@ properties:
> - isil,isl29030
> # Intersil ISL76682 Ambient Light Sensor
> - isil,isl76682
> + # JEDEC JESD301 (PMIC5000) Power Management IC
> + - jedec,pmic5000
> # JEDEC JESD300 (SPD5118) Hub and Serial Presence Detect
> - jedec,spd5118
> # Kandou KB9002 PCIe 5.0 retimer
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index d979e6d6e9f2..4bb518d1927c 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -226,6 +226,7 @@ Hardware Monitoring Kernel Drivers
> peci-cputemp
> peci-dimmtemp
> pmbus
> + pmic5000
> powerz
> powr1220
> prom21-xhci
> diff --git a/Documentation/hwmon/pmic5000.rst b/Documentation/hwmon/pmic5000.rst
> new file mode 100644
> index 000000000000..e7d864088ba5
> --- /dev/null
> +++ b/Documentation/hwmon/pmic5000.rst
> @@ -0,0 +1,103 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Kernel driver pmic5000
> +=====================
> +
> +Supported chips:
> +
> + * PMIC5000 (JEDEC JESD301) compliant power management chips
> +
> + JEDEC standard download:
> + https://www.jedec.org/standards-documents/docs/jesd301-1a03
> + (account required)
> +
> +
> + Prefix: 'pmic5000'
> +
> +Author:
> + Stephen Horvath <linux@stevetech.au>
> +
> +
> +Description
> +-----------
> +
> +This driver implements support for PMIC5000 (JEDEC JESD301) compliant power
> +management chips, which are used on many DDR5 memory modules.
> +
> +The driver does not auto-detect PMIC5000 compliant chips, and must be
> +instantiated manually or via device tree. The address of these chips are
> +usually between (inclusive) 0x48 and 0x4F.
> +
> +
> +Hardware monitoring sysfs entries
> +---------------------------------
> +
> +======================= ==================================
> +in0_enable Whether SWA is enabled (RO)
Does that enable the voltage or its monitoriong ? The enable attribute is
only to enable monitoring, not to enable the voltage. If this is a
controllable voltage, the driver should be a regulator driver, as Sashiko
suggested.
> +in0_input SWA Voltage (RO)
> +in0_min Minimum SWA Voltage (RO)
> +in0_max Maximum SWA Voltage (RO)
> +in0_min_alarm SWA Voltage low alarm
> +in0_max_alarm SWA Voltage high alarm
> +
> +in1_enable Whether SWB is enabled (RO)
> +in1_input SWB Voltage (RO)
> +in1_min Minimum SWB Voltage (RO)
> +in1_max Maximum SWB Voltage (RO)
> +in1_min_alarm SWB Voltage low alarm
> +in1_max_alarm SWB Voltage high alarm
> +
> +in2_enable Whether SWC is enabled (RO)
> +in2_input SWC Voltage (RO)
> +in2_min Minimum SWC Voltage (RO)
> +in2_max Maximum SWC Voltage (RO)
> +in2_min_alarm SWC Voltage low alarm
> +in2_max_alarm SWC Voltage high alarm
> +
> +in3_enable Whether SWD is enabled (RO)
> +in3_input SWD Voltage (RO)
> +in3_min Minimum SWD Voltage (RO)
> +in3_max Maximum SWD Voltage (RO)
> +in3_min_alarm SWD Voltage low alarm
> +in3_max_alarm SWD Voltage high alarm
> +
> +in5_input VIN_Bulk Voltage (RO)
> +in5_max Maximum VIN_Bulk Voltage (RO)
> +in5_max_alarm VIN_Bulk Voltage high alarm
> +
> +in6_input VIN_Mgmt Voltage (RO)
> +in6_max Maximum VIN_Mgmt Voltage (RO)
> +in6_max_alarm VIN_Mgmt Voltage high alarm
> +
> +in7_input VBias Voltage (RO)
> +in8_input VOUT_1.8V Voltage (RO)
> +in9_input VOUT_1.0V Voltage (RO)
> +
> +temp1_input Temperature (RO)
> +temp1_max Maximum temperature (RO)
> +temp1_max_alarm Temperature high alarm
> +
> +power1_input SWA Wattage (RO)
> +power2_input SWB Wattage (RO)
> +power3_input SWC Wattage (RO)
> +power4_input SWD Wattage (RO)
> +
> +curr1_input SWA Current (RO)
> +curr1_max Maximum SWA Current (RO)
> +curr1_max_alarm SWA Current high alarm
> +
> +curr2_input SWB Current (RO)
> +curr2_max Maximum SWB Current (RO)
> +curr2_max_alarm SWB Current high alarm
> +
> +curr3_input SWC Current (RO)
> +curr3_max Maximum SWC Current (RO)
> +curr3_max_alarm SWC Current high alarm
> +
> +curr4_input SWD Current (RO)
> +curr4_max Maximum SWD Current (RO)
> +curr4_max_alarm SWD Current high alarm
> +
> +update_interval Update interval in milliseconds (RW), Allowed values: 1, 2, 4, 8
> +
> +======================= ==================================
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 81a9a1d40eec..5ed5f9499c43 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -950,6 +950,18 @@ config SENSORS_JC42
> This driver can also be built as a module. If so, the module
> will be called jc42.
>
> +config SENSORS_PMIC5000
> + tristate "PMIC5000 Compliant Sensors"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + If you say yes here you get support for PMIC5000 (JEDEC JESD301)
> + compliant sensors. Such sensors are found on DDR5 memory
> + modules.
> +
> + This driver can also be built as a module. If so, the module
> + will be called pmic5000.
> +
> config SENSORS_POLARFIRE_SOC_TVS
> tristate "PolarFire SoC (MPFS) temperature and voltage sensor"
> depends on POLARFIRE_SOC_SYSCONS || COMPILE_TEST
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 0cad7e21634c..ac519123796f 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -199,6 +199,7 @@ obj-$(CONFIG_SENSORS_NZXT_SMART2) += nzxt-smart2.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> +obj-$(CONFIG_SENSORS_PMIC5000) += pmic5000.o
> obj-$(CONFIG_SENSORS_POLARFIRE_SOC_TVS) += tvs-mpfs.o
> obj-$(CONFIG_SENSORS_POWERZ) += powerz.o
> obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
> diff --git a/drivers/hwmon/pmic5000.c b/drivers/hwmon/pmic5000.c
> new file mode 100644
> index 000000000000..d3f3126dd9c8
> --- /dev/null
> +++ b/drivers/hwmon/pmic5000.c
> @@ -0,0 +1,896 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Driver for Jedec PMIC5000 compliant sensors
> + *
> + * Copyright (c) 2026 Stephen Horvath
> + *
> + * Inspired by spd5118.c.
> + *
> + * PMIC5000 compliant sensors are typically used on DDR5 memory modules.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/bits.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/regmap.h>
> +#include <linux/units.h>
> +
> +/* PMIC5000 registers. */
> +// clang-format off
No way. Are you serious ?
> +#define PMIC5000_REG_OVER_VOLT_IN 0x08
> +#define PMIC5000_REG_OVER_CURRENT 0x09
> + #define PMIC5000_HIGH_TEMP BIT(7)
> +#define PMIC5000_REG_OVER_VOLTAGE 0x0A
> +#define PMIC5000_REG_UNDER_VOLTAGE 0x0B
> +#define PMIC5000_REG_SWA_POWER 0x0C
> +#define PMIC5000_REG_SWB_POWER 0x0D
> +#define PMIC5000_REG_SWC_POWER 0x0E
> +#define PMIC5000_REG_SWD_POWER 0x0F
> +#define PMIC5000_REG_OUTPUT_SELECT 0x1A
> + #define PMIC5000_OUTPUT_SELECT BIT(1)
> +#define PMIC5000_REG_THRES_AND_SEL 0x1B
> + #define PMIC5000_VIN_MGMT_THRESH BIT(5)
> + #define PMIC5000_CURR_OR_PWR BIT(6)
> + #define PMIC5000_VIN_BULK_THRESH BIT(7)
> +#define PMIC5000_REG_SWA_CURR_WARN 0x1C
> +#define PMIC5000_REG_SWB_CURR_WARN 0x1D
> +#define PMIC5000_REG_SWC_CURR_WARN 0x1E
> +#define PMIC5000_REG_SWD_CURR_WARN 0x1F
> +#define PMIC5000_REG_SWA_VOLT_SET 0x21
> +#define PMIC5000_REG_SWA_THRESH 0x22
> +#define PMIC5000_REG_SWB_VOLT_SET 0x23
> +#define PMIC5000_REG_SWB_THRESH 0x24
> +#define PMIC5000_REG_SWC_VOLT_SET 0x25
> +#define PMIC5000_REG_SWC_THRESH 0x26
> +#define PMIC5000_REG_SWD_VOLT_SET 0x27
> +#define PMIC5000_REG_SWD_THRESH 0x28
> +#define PMIC5000_REG_SW_VOLT_RANGE 0x2B
> + #define PMIC5000_SWD_RANGE BIT(0)
> + #define PMIC5000_SWC_RANGE BIT(3)
> + #define PMIC5000_SWB_RANGE BIT(4)
> + #define PMIC5000_SWA_RANGE BIT(5)
> +#define PMIC5000_REG_REGULATOR_CONTROL 0x2F
> + #define PMIC5000_SWD_CONTROL BIT(3)
> + #define PMIC5000_SWC_CONTROL BIT(4)
> + #define PMIC5000_SWB_CONTROL BIT(5)
> + #define PMIC5000_SWA_CONTROL BIT(6)
> +#define PMIC5000_REG_ADC_CONFIG 0x30
> + #define PMIC5000_ADC_SELECT_MASK GENMASK(6, 3)
> + #define PMIC5000_ADC_ENABLE BIT(7)
> +#define PMIC5000_REG_ADC_VOLTAGE 0x31
> +#define PMIC5000_REG_TEMPERATURE 0x33
> +#define PMIC5000_REG_REVISION 0x3B
> +#define PMIC5000_REG_VENDOR 0x3C
> +
> +
Please run checkpatch --strict on your patches and fix what it reports.
> +/* 125 mA multiplier */
> +#define PMIC5000_CURR_UNIT 125
> +/* 125 mW multiplier */
> +#define PMIC5000_POWER_UNIT (125 * MILLIWATT_PER_WATT)
> +/* mV multipliers */
> +#define PMIC5000_VOLT_UNIT 15
> +#define PMIC5000_VINBULK_UNIT 70
> +#define PMIC5000_VBIAS_UNIT 25
> +// clang-format on
> +
> +struct pmic5000_data {
> + struct regmap *regmap;
> + struct mutex mode_lock;
> + struct mutex adc_lock;
Explain why those are needed on top of the hwmon subsystem lock.
> +};
> +
> +static const char *const pmic5000_power_labels[] = { "SWA", "SWB", "SWC",
> + "SWD" };
> +
> +static const char *const pmic5000_voltage_labels[] = {
> + "SWA", "SWB", "SWC", "SWD", NULL,
> + "VIN_Bulk", "VIN_Mgmt", "VBias", "VOUT_1.8V", "VOUT_1.0V"
> +};
> +
> +/* hwmon */
> +
> +static int pmic5000_check_regulator_enabled(struct regmap *regmap, int channel)
Pretty clear indication that this should be a regulator driver.
> +{
> + u32 regval;
> + int err;
> +
> + err = regmap_read(regmap, PMIC5000_REG_REGULATOR_CONTROL, ®val);
> + if (err)
> + return err;
> + switch (channel) {
> + case 0:
> + return !!(regval & PMIC5000_SWA_CONTROL);
> + case 1:
> + return !!(regval & PMIC5000_SWB_CONTROL);
> + case 2:
> + return !!(regval & PMIC5000_SWC_CONTROL);
> + case 3:
> + return !!(regval & PMIC5000_SWD_CONTROL);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int pmic5000_read_temp(struct regmap *regmap, u32 attr, int channel,
> + long *val)
> +{
> + int err;
> + u32 regval;
> +
> + if (channel != 0)
> + return -EOPNOTSUPP;
Unnecessary check.
> +
> + switch (attr) {
> + case hwmon_temp_input: {
> + err = regmap_read(regmap, PMIC5000_REG_TEMPERATURE, ®val);
> + if (err)
> + return err;
> + regval >>= 5;
> + /* Below 85°C */
> + if (regval == 0)
> + return -EOPNOTSUPP;
> + /* 0b001 = 85°C, 0b010 = 95°C, etc. */
> + *val = (75 + regval * 10) * MILLIDEGREE_PER_DEGREE;
> + return 0;
> + }
> + case hwmon_temp_max: {
> + err = regmap_read(regmap, PMIC5000_REG_THRES_AND_SEL, ®val);
> + if (err)
> + return err;
> + regval &= 0x07;
> + /* Reserved */
> + if (regval == 0 || regval == 0x7)
> + return -EOPNOTSUPP;
> + *val = (75 + regval * 10) * MILLIDEGREE_PER_DEGREE;
> + return 0;
> + }
> + case hwmon_temp_max_alarm: {
> + err = regmap_read(regmap, PMIC5000_REG_OVER_CURRENT, ®val);
> + if (err)
> + return err;
> + *val = !!(regval & PMIC5000_HIGH_TEMP);
> + return 0;
> + }
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int pmic5000_read_curr(struct pmic5000_data *data, u32 attr, int channel,
> + long *val)
> +{
> + struct regmap *regmap = data->regmap;
> + int reg, err;
> + int shift = 0;
> + u32 regval;
> +
> + if (attr == hwmon_curr_input) {
> + mutex_lock(&data->mode_lock);
I don't see why this is needed on top of the hwmon subsystem lock.
Same for other usage of this lock.
> + /* Select power measurements */
> + err = regmap_update_bits(regmap, PMIC5000_REG_THRES_AND_SEL,
> + PMIC5000_CURR_OR_PWR, 0);
> + if (err)
> + goto error;
> +
> + switch (channel) {
> + case 0:
> + reg = PMIC5000_REG_SWA_POWER;
> + break;
> + case 1:
> + reg = PMIC5000_REG_SWB_POWER;
> + break;
> + case 2:
> + reg = PMIC5000_REG_SWC_POWER;
> + break;
> + case 3:
> + reg = PMIC5000_REG_SWD_POWER;
> + break;
> + default:
> + err = -EOPNOTSUPP;
> + goto error;
> + }
> + } else if (attr == hwmon_curr_max) {
> + shift = 2;
> + switch (channel) {
> + case 0:
> + reg = PMIC5000_REG_SWA_CURR_WARN;
> + break;
> + case 1:
> + reg = PMIC5000_REG_SWB_CURR_WARN;
> + break;
> + case 2:
> + reg = PMIC5000_REG_SWC_CURR_WARN;
> + break;
> + case 3:
> + reg = PMIC5000_REG_SWD_CURR_WARN;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> + } else if (attr == hwmon_curr_max_alarm && channel >= 0 &&
> + channel <= 3) {
Unnecessary channel checks (at least if the is_visible function does its job
and the info data is correct).
> + err = regmap_read(regmap, PMIC5000_REG_OVER_CURRENT, ®val);
> + if (err)
> + return err;
> + *val = regval >> (3 - channel) & 0x01;
> + return 0;
> + } else {
> + return -EOPNOTSUPP;
> + }
> +
> + err = regmap_read(regmap, reg, ®val);
> + if (err)
> + goto error;
> +
> + if (attr == hwmon_curr_input)
> + mutex_unlock(&data->mode_lock);
> +
> + *val = regval * PMIC5000_CURR_UNIT >> shift;
> + return 0;
> +
> +error:
> + if (attr == hwmon_curr_input)
> + mutex_unlock(&data->mode_lock);
> + return err;
> +}
> +
> +static int pmic5000_read_power(struct pmic5000_data *data, u32 attr,
> + int channel, long *val)
> +{
> + struct regmap *regmap = data->regmap;
> + int reg, err;
> + u32 regval;
> +
> + if (attr != hwmon_power_input)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&data->mode_lock);
> + /* Select power measurements */
> + err = regmap_update_bits(regmap, PMIC5000_REG_THRES_AND_SEL,
> + PMIC5000_CURR_OR_PWR, PMIC5000_CURR_OR_PWR);
> + if (err)
> + goto error;
> +
> + switch (channel) {
> + case 0:
> + reg = PMIC5000_REG_SWA_POWER;
> + break;
> + case 1:
> + reg = PMIC5000_REG_SWB_POWER;
> + break;
> + case 2:
> + reg = PMIC5000_REG_SWC_POWER;
> + break;
> + case 3:
> + reg = PMIC5000_REG_SWD_POWER;
> + break;
> + default:
> + err = -EOPNOTSUPP;
> + goto error;
> + }
> +
> + err = regmap_read(regmap, reg, ®val);
> + if (err)
> + goto error;
> +
> + mutex_unlock(&data->mode_lock);
> +
> + *val = regval * PMIC5000_POWER_UNIT;
> + return 0;
> +
> +error:
> + mutex_unlock(&data->mode_lock);
> + return err;
> +}
> +
> +static int pmic5000_read_volt_thresholds(struct regmap *regmap, u32 attr,
> + int channel, long *val)
> +{
> + int err;
> + u32 set_reg, thresh_reg, range_bit;
> + u32 set_regval, thresh_regval, range_regval;
> + u32 volt_set;
> + int base_volts[2];
> +
> + switch (channel) {
> + case 0:
> + set_reg = PMIC5000_REG_SWA_VOLT_SET;
> + thresh_reg = PMIC5000_REG_SWA_THRESH;
> + range_bit = PMIC5000_SWA_RANGE;
> + base_volts[0] = 800;
> + base_volts[1] = 600;
> + break;
> + case 1:
> + set_reg = PMIC5000_REG_SWB_VOLT_SET;
> + thresh_reg = PMIC5000_REG_SWB_THRESH;
> + range_bit = PMIC5000_SWB_RANGE;
> + base_volts[0] = 800;
> + base_volts[1] = 600;
> + break;
> + case 2:
> + set_reg = PMIC5000_REG_SWC_VOLT_SET;
> + thresh_reg = PMIC5000_REG_SWC_THRESH;
> + range_bit = PMIC5000_SWC_RANGE;
> + base_volts[0] = 800;
> + base_volts[1] = 600;
> + break;
> + case 3:
> + set_reg = PMIC5000_REG_SWD_VOLT_SET;
> + thresh_reg = PMIC5000_REG_SWD_THRESH;
> + range_bit = PMIC5000_SWD_RANGE;
> + base_volts[0] = 1500;
> + base_volts[1] = 2200;
> + break;
> + case 5:
> + case 6: {
Why "{" ?
> + err = regmap_read(regmap, PMIC5000_REG_THRES_AND_SEL,
> + &thresh_regval);
> + if (err)
> + return err;
> + if (channel == 5) {
> + if (thresh_regval & PMIC5000_VIN_BULK_THRESH)
> + *val = 14500;
> + else
> + *val = 16000;
> + return 0;
> + } else {
> + if (thresh_regval & PMIC5000_VIN_MGMT_THRESH)
> + *val = 3800;
> + else
> + *val = 3700;
> + return 0;
> + }
> + }
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + err = regmap_read(regmap, set_reg, &set_regval);
> + if (err)
> + return err;
> + err = regmap_read(regmap, thresh_reg, &thresh_regval);
> + if (err)
> + return err;
> + err = regmap_read(regmap, PMIC5000_REG_SW_VOLT_RANGE, &range_regval);
> + if (err)
> + return err;
> +
> + volt_set = range_regval & range_bit ? base_volts[1] : base_volts[0];
> + volt_set += (set_regval >> 1) * 5;
> +
> + switch (attr) {
> + case hwmon_in_min:
> + /* 10%, 12.5%, Reserved, Reserved */
> + const int min_permilles[4] = { 100, 125, PERMILLE, PERMILLE };
> + *val = volt_set - (min_permilles[(thresh_regval >> 2) & 0x03] *
> + volt_set / PERMILLE);
> + return 0;
> + case hwmon_in_max:
> + /* 7.5%, 10%, 12.5%, Reserved */
> + const int max_permilles[4] = { 75, 100, 125, PERMILLE };
> + *val = volt_set + (max_permilles[(thresh_regval >> 4) & 0x03] *
> + volt_set / PERMILLE);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int pmic5000_read_adc_alarms(struct regmap *regmap, u32 attr,
> + int channel, long *val)
> +{
> + int err;
> + u32 regval;
> +
> + if (channel >= 0 && channel <= 3) {
channel is always >= 0.
> + switch (attr) {
> + case hwmon_in_min_alarm: {
> + err = regmap_read(regmap, PMIC5000_REG_UNDER_VOLTAGE,
> + ®val);
> + if (err)
> + return err;
> + *val = regval >> (3 - channel) & 0x01;
> + return 0;
> + }
> + case hwmon_in_max_alarm: {
> + err = regmap_read(regmap, PMIC5000_REG_OVER_VOLTAGE,
> + ®val);
> + if (err)
> + return err;
> + *val = regval >> (7 - channel) & 0x01;
> + return 0;
> + }
> + }
Alignment.
> + } else if (channel == 5 || channel == 6) {
> + switch (attr) {
> + case hwmon_in_max_alarm: {
> + err = regmap_read(regmap, PMIC5000_REG_OVER_VOLT_IN,
> + ®val);
> + if (err)
> + return err;
> + *val = regval >> (channel - 4) & 0x01;
> + return 0;
> + }
> + }
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int pmic5000_read_adc(struct pmic5000_data *data, u32 attr, int channel,
> + long *val)
> +{
> + struct regmap *regmap = data->regmap;
> + int err, mult;
> + u32 regval;
> +
> + switch (attr) {
> + case hwmon_in_enable:
> + err = pmic5000_check_regulator_enabled(regmap, channel);
> + if (err < 0)
> + return err;
> + *val = err;
> + return 0;
The enable attribute is supposed to enable or disable monitoring.
It is not supposed to report regulator status.
> + case hwmon_in_input:
> + break;
> + case hwmon_in_min:
> + case hwmon_in_max:
> + return pmic5000_read_volt_thresholds(regmap, attr, channel,
> + val);
> + case hwmon_in_min_alarm:
> + case hwmon_in_max_alarm:
> + return pmic5000_read_adc_alarms(regmap, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + /* Channel 4 is reserved */
> + if (channel < 0 || channel > 9 || channel == 4)
> + return -EOPNOTSUPP;
Then it should not be enabled by the is_visible function.
Also, channel is never < 0 or > 9.
> +
> + switch (channel) {
+ case 5:
> + mult = PMIC5000_VINBULK_UNIT;
> + break;
> + case 7:
> + mult = PMIC5000_VBIAS_UNIT;
> + break;
> + default:
> + mult = PMIC5000_VOLT_UNIT;
> + break;
> + }
> +
> + mutex_lock(&data->adc_lock);
> +
> + err = regmap_update_bits(regmap, PMIC5000_REG_ADC_CONFIG,
> + PMIC5000_ADC_SELECT_MASK, channel << 3);
> + if (err)
> + goto error;
> +
> + /*
> + * The host shall wait minimum of 9 ms delay after the input selection
> + * for ADC readout and the actual readout
> + *
> + * msleep may sleep for up to 20ms, which is fine.
No, it isn't fine.
Also, why wait if the channel was not changed ?
> + */
> + msleep(9);
> +
> + err = regmap_read(regmap, PMIC5000_REG_ADC_VOLTAGE, ®val);
> + if (err)
> + goto error;
> +
> + mutex_unlock(&data->adc_lock);
> +
> + *val = regval * mult;
> + return 0;
> +
> +error:
> + mutex_unlock(&data->adc_lock);
> + return err;
> +}
> +
> +static int pmic5000_read_interval(struct regmap *regmap, u32 attr, long *val)
> +{
> + unsigned int regval;
> + int err;
> +
> + if (attr != hwmon_chip_update_interval)
> + return -EOPNOTSUPP;
> +
> + err = regmap_read(regmap, PMIC5000_REG_ADC_CONFIG, ®val);
> + if (err < 0)
> + return err;
> + *val = 1 << (regval & 0x03);
BIT()
> + return 0;
> +}
> +
> +static int pmic5000_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct pmic5000_data *data = dev_get_drvdata(dev);
> + struct regmap *regmap = data->regmap;
> +
> + switch (type) {
> + case hwmon_chip:
> + return pmic5000_read_interval(regmap, attr, val);
> + case hwmon_temp:
> + return pmic5000_read_temp(regmap, attr, channel, val);
> + case hwmon_in:
> + return pmic5000_read_adc(data, attr, channel, val);
> + case hwmon_curr:
> + return pmic5000_read_curr(data, attr, channel, val);
> + case hwmon_power:
> + return pmic5000_read_power(data, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int pmic5000_read_string(struct device *dev,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + if (type == hwmon_curr && attr == hwmon_curr_label) {
> + if (channel < 0 || channel > 3)
> + return -EOPNOTSUPP;
Again, channel is never < 0, and unless is_visible and/or the configuration data is wrong
it should never be > 3.
> + *str = pmic5000_power_labels[channel];
> + } else if (type == hwmon_power && attr == hwmon_power_label) {
> + if (channel < 0 || channel > 3)
> + return -EOPNOTSUPP;
> + *str = pmic5000_power_labels[channel];
> + } else if (type == hwmon_in && attr == hwmon_in_label) {
> + if (channel < 0 || channel > 9 || channel == 4)
Many more unencessary channel checks.
> + return -EOPNOTSUPP;
> + *str = pmic5000_voltage_labels[channel];
> + } else {
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int pmic5000_write_interval(struct pmic5000_data *data, long val)
> +{
> + struct regmap *regmap = data->regmap;
> + u32 regval;
> + int err;
> +
> + switch (val) {
> + case 1:
> + regval = 0;
> + break;
> + case 2:
> + regval = 1;
> + break;
> + case 4:
> + regval = 2;
> + break;
> + case 8:
> + regval = 3;
> + break;
> + default:
> + return -EINVAL;
> + }
find_closest() would be more appropriate here. We don't usually expect users to know valid
update intervals.
> +
> + mutex_lock(&data->adc_lock);
> + err = regmap_update_bits(regmap, PMIC5000_REG_ADC_CONFIG, 0x03, regval);
> + mutex_unlock(&data->adc_lock);
Another unnecessary lock.
> + return err;
> +}
> +
> +static int pmic5000_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + struct pmic5000_data *data = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_chip:
> + return pmic5000_write_interval(data, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t pmic5000_is_visible(const void *data,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + int ret;
> + struct pmic5000_data *pmic_data = (struct pmic5000_data *)data;
> + struct regmap *regmap = pmic_data->regmap;
> +
> + switch (type) {
> + case hwmon_chip:
> + if (attr == hwmon_chip_update_interval)
> + return 0644;
> + break;
> + case hwmon_temp:
> + return 0444;
> + case hwmon_in:
> + if (channel == 4)
> + return 0;
> + if (channel >= 0 && channel <= 3 && (attr != hwmon_in_enable)) {
> + ret = pmic5000_check_regulator_enabled(regmap, channel);
> + if (!ret || ret < 0)
> + return 0;
> + }
> + return 0444;
> + case hwmon_power:
> + if (channel >= 0 && channel <= 3) {
> + ret = pmic5000_check_regulator_enabled(regmap, channel);
> + if (!ret || ret < 0)
> + return 0;
> + }
> + return 0444;
> + case hwmon_curr:
> + if (channel >= 0 && channel <= 3) {
> + ret = pmic5000_check_regulator_enabled(regmap, channel);
> + if (!ret || ret < 0)
> + return 0;
> + }
> + return 0444;
> + default:
> + break;
> + }
> + return 0444;
> +}
> +
> +/*
> + * Bank and vendor id are 8-bit fields with seven data bits and odd parity.
> + * Vendor IDs 0 and 0x7f are invalid.
> + * See Jedec standard JEP106BJ for details and a list of assigned vendor IDs.
> + */
> +static bool pmic5000_vendor_valid(u8 bank, u8 id)
> +{
> + if (parity8(bank) == 0 || parity8(id) == 0)
> + return false;
> +
> + id &= 0x7f;
> + return id && id != 0x7f;
> +}
> +
> +static const struct hwmon_channel_info *pmic5000_info[] = {
> + HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_ALARM),
> + HWMON_CHANNEL_INFO(
> + in,
> + HWMON_I_ENABLE | HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX |
> + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | HWMON_I_LABEL,
> + HWMON_I_ENABLE | HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX |
> + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | HWMON_I_LABEL,
> + HWMON_I_ENABLE | HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX |
> + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | HWMON_I_LABEL,
> + HWMON_I_ENABLE | HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX |
> + HWMON_I_MIN_ALARM | HWMON_I_MAX_ALARM | HWMON_I_LABEL,
> + HWMON_I_INPUT,
> + HWMON_I_INPUT | HWMON_I_MAX | HWMON_I_MAX_ALARM | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_MAX | HWMON_I_MAX_ALARM | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL, HWMON_I_INPUT | HWMON_I_LABEL,
> + HWMON_I_INPUT | HWMON_I_LABEL),
> + HWMON_CHANNEL_INFO(
> + curr,
> + HWMON_C_INPUT | HWMON_C_MAX | HWMON_C_MAX_ALARM | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_MAX | HWMON_C_MAX_ALARM | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_MAX | HWMON_C_MAX_ALARM | HWMON_C_LABEL,
> + HWMON_C_INPUT | HWMON_C_MAX | HWMON_C_MAX_ALARM |
> + HWMON_C_LABEL),
> + HWMON_CHANNEL_INFO(power, HWMON_P_INPUT | HWMON_P_LABEL,
> + HWMON_P_INPUT | HWMON_P_LABEL,
> + HWMON_P_INPUT | HWMON_P_LABEL,
> + HWMON_P_INPUT | HWMON_P_LABEL),
> + NULL
> +};
> +
> +static const struct hwmon_ops pmic5000_hwmon_ops = {
> + .is_visible = pmic5000_is_visible,
> + .read = pmic5000_read,
> + .read_string = pmic5000_read_string,
> + .write = pmic5000_write,
> +};
> +
> +static const struct hwmon_chip_info pmic5000_chip_info = {
> + .ops = &pmic5000_hwmon_ops,
> + .info = pmic5000_info,
> +};
> +
> +/* regmap */
> +
> +static bool pmic5000_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case PMIC5000_REG_OUTPUT_SELECT:
> + case PMIC5000_REG_THRES_AND_SEL:
> + case PMIC5000_REG_ADC_CONFIG:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool pmic5000_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case PMIC5000_REG_OVER_VOLT_IN:
> + case PMIC5000_REG_OVER_CURRENT:
> + case PMIC5000_REG_OVER_VOLTAGE:
> + case PMIC5000_REG_UNDER_VOLTAGE:
> + case PMIC5000_REG_SWA_POWER:
> + case PMIC5000_REG_SWB_POWER:
> + case PMIC5000_REG_SWC_POWER:
> + case PMIC5000_REG_SWD_POWER:
> + case PMIC5000_REG_ADC_VOLTAGE:
> + case PMIC5000_REG_TEMPERATURE:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config pmic5000_regmap8_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x3f,
> + .writeable_reg = pmic5000_writeable_reg,
> + .volatile_reg = pmic5000_volatile_reg,
> + .cache_type = REGCACHE_MAPLE,
> +};
> +
> +static int pmic5000_suspend(struct device *dev)
> +{
> + struct pmic5000_data *data = dev_get_drvdata(dev);
> + struct regmap *regmap = data->regmap;
> + u32 regval;
> + int err;
> +
> + /*
> + * Make sure the configuration register in the regmap cache is current
> + * before bypassing it.
> + */
> + err = regmap_read(regmap, PMIC5000_REG_ADC_CONFIG, ®val);
> + if (err < 0)
> + return err;
> +
> + regcache_cache_bypass(regmap, true);
> + regmap_update_bits(regmap, PMIC5000_REG_ADC_CONFIG, PMIC5000_ADC_ENABLE,
> + 0);
> + regcache_cache_bypass(regmap, false);
> +
> + regcache_cache_only(regmap, true);
> + regcache_mark_dirty(regmap);
> +
> + return 0;
> +}
> +
> +static int pmic5000_resume(struct device *dev)
> +{
> + struct pmic5000_data *data = dev_get_drvdata(dev);
> + struct regmap *regmap = data->regmap;
> +
> + regcache_cache_only(regmap, false);
> + return regcache_sync(regmap);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(pmic5000_pm_ops, pmic5000_suspend,
> + pmic5000_resume);
> +
> +static int pmic5000_common_probe(struct device *dev, struct regmap *regmap)
Why pmic5000_common_probe() ? There is only one caller.
> +{
> + unsigned int revision, vendor, bank;
> + struct pmic5000_data *data;
> + struct device *hwmon_dev;
> + int err;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + err = regmap_read(regmap, PMIC5000_REG_REVISION, &revision);
> + if (err)
> + return err;
> +
> + err = regmap_read(regmap, PMIC5000_REG_VENDOR, &bank);
> + if (err)
> + return err;
> + err = regmap_read(regmap, PMIC5000_REG_VENDOR + 1, &vendor);
> + if (err)
> + return err;
> + if (!pmic5000_vendor_valid(bank, vendor))
> + return -ENODEV;
> +
> + data->regmap = regmap;
> + mutex_init(&data->mode_lock);
> + mutex_init(&data->adc_lock);
> + dev_set_drvdata(dev, data);
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(
> + dev, "pmic5000", data, &pmic5000_chip_info, NULL);
> + if (IS_ERR(hwmon_dev))
> + return PTR_ERR(hwmon_dev);
> +
> + dev_info(dev, "DDR5 PMIC sensor: vendor 0x%02x:0x%02x revision %d.%d\n",
> + bank & 0x7f, vendor, ((revision >> 4) & 0x03) + 1,
> + ((revision >> 1) & 0x07) + 1);
> +
> + /* Enable individual measurements and enable ADC */
> + err = regmap_update_bits(regmap, PMIC5000_REG_OUTPUT_SELECT,
> + PMIC5000_OUTPUT_SELECT,
> + PMIC5000_OUTPUT_SELECT);
> + if (err)
> + return err;
> + err = regmap_update_bits(regmap, PMIC5000_REG_ADC_CONFIG,
> + PMIC5000_ADC_ENABLE, PMIC5000_ADC_ENABLE);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +/* I2C */
> +
> +static int pmic5000_i2c_init(struct i2c_client *client)
> +{
> + struct i2c_adapter *adapter = client->adapter;
> +
> + /*
> + * Register accesses are 8-bit, so require byte-data transactions only.
> + * Requiring WORD_DATA here rejects otherwise valid adapters.
> + */
> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static int pmic5000_i2c_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct regmap *regmap;
> + int err;
> +
> + err = pmic5000_i2c_init(client);
> + if (err)
> + return dev_err_probe(dev, err, "I2C capability check failed\n");
> +
> + regmap = devm_regmap_init_i2c(client, &pmic5000_regmap8_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "regmap init failed\n");
> +
> + return pmic5000_common_probe(dev, regmap);
> +}
> +
> +static const struct i2c_device_id pmic5000_i2c_id[] = { { .name = "pmic5000" },
> + {} };
> +MODULE_DEVICE_TABLE(i2c, pmic5000_i2c_id);
> +
> +static const struct of_device_id pmic5000_of_ids[] = {
> + {
> + .compatible = "jedec,pmic5000",
jedec,pmic5000 is not documented.
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, pmic5000_of_ids);
> +
> +static struct i2c_driver pmic5000_i2c_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "pmic5000",
> + .of_match_table = pmic5000_of_ids,
> + .pm = pm_sleep_ptr(&pmic5000_pm_ops),
> + },
> + .probe = pmic5000_i2c_probe,
> + .id_table = pmic5000_i2c_id,
> +};
> +
> +module_i2c_driver(pmic5000_i2c_driver);
> +
> +MODULE_AUTHOR("Stephen Horvath <linux@stevetech.au>");
> +MODULE_DESCRIPTION("PMIC5000 driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 5+ messages in thread