diff mbox series

[linux,dev-5.8] hwmon: Ampere Computing ALTRA SMPMPRO sensor driver

Message ID 1601504817-16752-1-git-send-email-lancelot.kao@fii-usa.com
State Changes Requested, archived
Headers show
Series [linux,dev-5.8] hwmon: Ampere Computing ALTRA SMPMPRO sensor driver | expand

Commit Message

Lancelot Kao Sept. 30, 2020, 10:26 p.m. UTC
From: Lancelot Kao <lancelot.kao@fii-usa.com>

Add SMPMPro-hwmon driver to monitor Ampere CPU/Memory/VR via an
i2c interface of the CPU's smpmpro management device.

Signed-off-by: Xiaopeng XP Chen <xiao-peng.chen@fii-na.com>
Signed-off-by: Lancelot Kao <lancelot.kao@fii-usa.com>
---
 MAINTAINERS                   |   8 +
 drivers/hwmon/Kconfig         |  10 +
 drivers/hwmon/Makefile        |   1 +
 drivers/hwmon/smpmpro-hwmon.c | 903 ++++++++++++++++++++++++++++++++++
 4 files changed, 922 insertions(+)
 create mode 100644 drivers/hwmon/smpmpro-hwmon.c

Comments

Patrick Williams Oct. 1, 2020, 12:32 p.m. UTC | #1
On Wed, Sep 30, 2020 at 05:26:57PM -0500, Lancelot wrote:
> From: Lancelot Kao <lancelot.kao@fii-usa.com>
> 
> Add SMPMPro-hwmon driver to monitor Ampere CPU/Memory/VR via an
> i2c interface of the CPU's smpmpro management device.
> 
> Signed-off-by: Xiaopeng XP Chen <xiao-peng.chen@fii-na.com>
> Signed-off-by: Lancelot Kao <lancelot.kao@fii-usa.com>

Nice work at adding this driver.

It does look like you've missed CC'ing upstream though.  Was this
intentional?  (linux-hwmon@vger.kernel.org)

> +/* Capability Registers  */
> +#define TEMP_SENSOR_SUPPORT_REG	0x05
> +#define PWR_SENSOR_SUPPORT_REG	0x06
> +#define VOLT_SENSOR_SUPPORT_REG	0x07
> +#define OTHER_CAP_REG		    0x08
> +#define CORE_CLUSTER_CNT_REG	0x0B
> +#define SYS_CACHE_PCIE_CNT_REG	0x0C
> +#define SOCKET_INFO_REG	        0x0D

There seems to be some sporatic indentation throughout all the #defines
in this file, where it appears you attempted to align the values.  Make
sure you have tabs set to 8-step spacing for kernel code.

> +static void smpmpro_init_device(struct i2c_client *client,
> +				struct smpmpro_data *data)
> +{
> +	u16 ret;
> +
> +	ret = i2c_smbus_read_word_swapped(client, TEMP_SENSOR_SUPPORT_REG);
> +	if (ret < 0)
> +		return;
> +	data->temp_support_regs = ret;

i2c_smbus_read_word_swapped returns a s32 even though you're looking for
a u16.  By setting `ret` to u16 you've caused two problems:

    * You are immediately truncating -ERRNO values into a u16 so that
      you are unable to differentiate values like 0xFFFFFFFF as a
      register value and -1 as an errno.

    * The if condition here can never be true, so you're never catching
      error conditions.  (An u16 can never be negative, so ret < 0 can
      never be true.)

This issue occurs throughout the driver.

> +static int smpmpro_read_temp(struct device *dev, u32 attr, int channel,
> +			     long *val)
> +{
> +	struct smpmpro_data *data = dev_get_drvdata(dev);
> +	struct i2c_client *client = data->client;
> +	int ret;

You might want a sized int on this one?  Repeated in most other
functions.

> +static int smpmpro_read_power(struct device *dev, u32 attr, int channel,
> +			     long *val)
> +{
> +	struct smpmpro_data *data = dev_get_drvdata(dev);
> +	struct i2c_client *client = data->client;
> +	int ret, ret_mw;
> +	int ret2 = 0, ret2_mw = 0;

Any reason to not initialize ret/ret_mw?  By it being different from
ret2/ret2_mw it makes me question "is this ok?", which spends more time
in review.

> +static int smpmpro_i2c_probe(struct i2c_client *client,
> +			  const struct i2c_device_id *id)
...
> +	/* Initialize the Altra SMPMPro chip */
> +	smpmpro_init_device(client, data);

I didn't see anything in the smpmpro_init_device function, but is there
anything you can or should do to ensure this device really is an
SMPMPro rather than exclusively relying on the device tree compatible?

> +static struct i2c_driver smpmpro_driver = {
> +	.class		= I2C_CLASS_HWMON,
> +	.probe		= smpmpro_i2c_probe,
> +	.driver = {
> +		.name	= "smpmpro",
> +	},
> +	.id_table	= smpmpro_i2c_id,
> +};
> +
> +module_i2c_driver(smpmpro_driver);

Are you missing the .of_match_table inside .driver?  Is that necessary
or useful for your use?  I'm not sure if you can have device tree
entries that automatically instantiate the hwmon driver otherwise.
Joel Stanley Oct. 5, 2020, 11:05 p.m. UTC | #2
On Thu, 1 Oct 2020 at 12:32, Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Wed, Sep 30, 2020 at 05:26:57PM -0500, Lancelot wrote:
> > From: Lancelot Kao <lancelot.kao@fii-usa.com>
> >
> > Add SMPMPro-hwmon driver to monitor Ampere CPU/Memory/VR via an
> > i2c interface of the CPU's smpmpro management device.
> >
> > Signed-off-by: Xiaopeng XP Chen <xiao-peng.chen@fii-na.com>
> > Signed-off-by: Lancelot Kao <lancelot.kao@fii-usa.com>
>
> Nice work at adding this driver.
>
> It does look like you've missed CC'ing upstream though.  Was this
> intentional?  (linux-hwmon@vger.kernel.org)

As Patrick mentioned, let's review this on the upstream list.

Cheers,

Joel

>
> > +/* Capability Registers  */
> > +#define TEMP_SENSOR_SUPPORT_REG      0x05
> > +#define PWR_SENSOR_SUPPORT_REG       0x06
> > +#define VOLT_SENSOR_SUPPORT_REG      0x07
> > +#define OTHER_CAP_REG                    0x08
> > +#define CORE_CLUSTER_CNT_REG 0x0B
> > +#define SYS_CACHE_PCIE_CNT_REG       0x0C
> > +#define SOCKET_INFO_REG              0x0D
>
> There seems to be some sporatic indentation throughout all the #defines
> in this file, where it appears you attempted to align the values.  Make
> sure you have tabs set to 8-step spacing for kernel code.
>
> > +static void smpmpro_init_device(struct i2c_client *client,
> > +                             struct smpmpro_data *data)
> > +{
> > +     u16 ret;
> > +
> > +     ret = i2c_smbus_read_word_swapped(client, TEMP_SENSOR_SUPPORT_REG);
> > +     if (ret < 0)
> > +             return;
> > +     data->temp_support_regs = ret;
>
> i2c_smbus_read_word_swapped returns a s32 even though you're looking for
> a u16.  By setting `ret` to u16 you've caused two problems:
>
>     * You are immediately truncating -ERRNO values into a u16 so that
>       you are unable to differentiate values like 0xFFFFFFFF as a
>       register value and -1 as an errno.
>
>     * The if condition here can never be true, so you're never catching
>       error conditions.  (An u16 can never be negative, so ret < 0 can
>       never be true.)
>
> This issue occurs throughout the driver.
>
> > +static int smpmpro_read_temp(struct device *dev, u32 attr, int channel,
> > +                          long *val)
> > +{
> > +     struct smpmpro_data *data = dev_get_drvdata(dev);
> > +     struct i2c_client *client = data->client;
> > +     int ret;
>
> You might want a sized int on this one?  Repeated in most other
> functions.
>
> > +static int smpmpro_read_power(struct device *dev, u32 attr, int channel,
> > +                          long *val)
> > +{
> > +     struct smpmpro_data *data = dev_get_drvdata(dev);
> > +     struct i2c_client *client = data->client;
> > +     int ret, ret_mw;
> > +     int ret2 = 0, ret2_mw = 0;
>
> Any reason to not initialize ret/ret_mw?  By it being different from
> ret2/ret2_mw it makes me question "is this ok?", which spends more time
> in review.
>
> > +static int smpmpro_i2c_probe(struct i2c_client *client,
> > +                       const struct i2c_device_id *id)
> ...
> > +     /* Initialize the Altra SMPMPro chip */
> > +     smpmpro_init_device(client, data);
>
> I didn't see anything in the smpmpro_init_device function, but is there
> anything you can or should do to ensure this device really is an
> SMPMPro rather than exclusively relying on the device tree compatible?
>
> > +static struct i2c_driver smpmpro_driver = {
> > +     .class          = I2C_CLASS_HWMON,
> > +     .probe          = smpmpro_i2c_probe,
> > +     .driver = {
> > +             .name   = "smpmpro",
> > +     },
> > +     .id_table       = smpmpro_i2c_id,
> > +};
> > +
> > +module_i2c_driver(smpmpro_driver);
>
> Are you missing the .of_match_table inside .driver?  Is that necessary
> or useful for your use?  I'm not sure if you can have device tree
> entries that automatically instantiate the hwmon driver otherwise.
>
> --
> Patrick Williams
Guenter Roeck Oct. 6, 2020, 3:13 a.m. UTC | #3
On 10/5/20 4:05 PM, Joel Stanley wrote:
> On Thu, 1 Oct 2020 at 12:32, Patrick Williams <patrick@stwcx.xyz> wrote:
>>
>> On Wed, Sep 30, 2020 at 05:26:57PM -0500, Lancelot wrote:
>>> From: Lancelot Kao <lancelot.kao@fii-usa.com>
>>>
>>> Add SMPMPro-hwmon driver to monitor Ampere CPU/Memory/VR via an
>>> i2c interface of the CPU's smpmpro management device.
>>>
>>> Signed-off-by: Xiaopeng XP Chen <xiao-peng.chen@fii-na.com>
>>> Signed-off-by: Lancelot Kao <lancelot.kao@fii-usa.com>
>>
>> Nice work at adding this driver.
>>
>> It does look like you've missed CC'ing upstream though.  Was this
>> intentional?  (linux-hwmon@vger.kernel.org)
> 
> As Patrick mentioned, let's review this on the upstream list.
> 

I can not really comment, not having seen the entire patch.
However, looking it up on the OpenBMC patchwork, couple of
high level comments:

- Label attributes are handled by the hwmon core. Label attributes
  outside the core are unacceptable.
- There is no discussion about the non-standard attributes, nor a description
  of those, nor an explanation for why they are needed (as hwmon sysfs attributes)
  or what they report. This is unacceptable.
  Besides, many of those attributes - say, gpi22_input, which seems to report
  the content of GPI_WDT_STS_REG, suggesting association with a watchdog -
  seem inappropriate for a hwmon driver to start with. It seems like the
  hwmon driver us used as catch-all driver for this chip. Sorry,
  that is an absolute no-go.

If this patch is supposed to be submitted as upstream patch, I would suggest
to read and follow the guidance in Documentation/hwmon/submitting-patches.rst.

Guenter

> Cheers,
> 
> Joel
> 
>>
>>> +/* Capability Registers  */
>>> +#define TEMP_SENSOR_SUPPORT_REG      0x05
>>> +#define PWR_SENSOR_SUPPORT_REG       0x06
>>> +#define VOLT_SENSOR_SUPPORT_REG      0x07
>>> +#define OTHER_CAP_REG                    0x08
>>> +#define CORE_CLUSTER_CNT_REG 0x0B
>>> +#define SYS_CACHE_PCIE_CNT_REG       0x0C
>>> +#define SOCKET_INFO_REG              0x0D
>>
>> There seems to be some sporatic indentation throughout all the #defines
>> in this file, where it appears you attempted to align the values.  Make
>> sure you have tabs set to 8-step spacing for kernel code.
>>
>>> +static void smpmpro_init_device(struct i2c_client *client,
>>> +                             struct smpmpro_data *data)
>>> +{
>>> +     u16 ret;
>>> +
>>> +     ret = i2c_smbus_read_word_swapped(client, TEMP_SENSOR_SUPPORT_REG);
>>> +     if (ret < 0)
>>> +             return;
>>> +     data->temp_support_regs = ret;
>>
>> i2c_smbus_read_word_swapped returns a s32 even though you're looking for
>> a u16.  By setting `ret` to u16 you've caused two problems:
>>
>>     * You are immediately truncating -ERRNO values into a u16 so that
>>       you are unable to differentiate values like 0xFFFFFFFF as a
>>       register value and -1 as an errno.
>>
>>     * The if condition here can never be true, so you're never catching
>>       error conditions.  (An u16 can never be negative, so ret < 0 can
>>       never be true.)
>>
>> This issue occurs throughout the driver.
>>
>>> +static int smpmpro_read_temp(struct device *dev, u32 attr, int channel,
>>> +                          long *val)
>>> +{
>>> +     struct smpmpro_data *data = dev_get_drvdata(dev);
>>> +     struct i2c_client *client = data->client;
>>> +     int ret;
>>
>> You might want a sized int on this one?  Repeated in most other
>> functions.
>>
>>> +static int smpmpro_read_power(struct device *dev, u32 attr, int channel,
>>> +                          long *val)
>>> +{
>>> +     struct smpmpro_data *data = dev_get_drvdata(dev);
>>> +     struct i2c_client *client = data->client;
>>> +     int ret, ret_mw;
>>> +     int ret2 = 0, ret2_mw = 0;
>>
>> Any reason to not initialize ret/ret_mw?  By it being different from
>> ret2/ret2_mw it makes me question "is this ok?", which spends more time
>> in review.
>>
>>> +static int smpmpro_i2c_probe(struct i2c_client *client,
>>> +                       const struct i2c_device_id *id)
>> ...
>>> +     /* Initialize the Altra SMPMPro chip */
>>> +     smpmpro_init_device(client, data);
>>
>> I didn't see anything in the smpmpro_init_device function, but is there
>> anything you can or should do to ensure this device really is an
>> SMPMPro rather than exclusively relying on the device tree compatible?
>>
>>> +static struct i2c_driver smpmpro_driver = {
>>> +     .class          = I2C_CLASS_HWMON,
>>> +     .probe          = smpmpro_i2c_probe,
>>> +     .driver = {
>>> +             .name   = "smpmpro",
>>> +     },
>>> +     .id_table       = smpmpro_i2c_id,
>>> +};
>>> +
>>> +module_i2c_driver(smpmpro_driver);
>>
>> Are you missing the .of_match_table inside .driver?  Is that necessary
>> or useful for your use?  I'm not sure if you can have device tree
>> entries that automatically instantiate the hwmon driver otherwise.
>>
>> --
>> Patrick Williams
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 5fecb388d073..c169fd9a4d7c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15759,6 +15759,14 @@  S:	Maintained
 F:	Documentation/hwmon/smm665.rst
 F:	drivers/hwmon/smm665.c
 
+SMPMPRO HARDWARE MONITOR DRIVER
+M:	Lancelot Kao <lancelot.kao@fii-usa.com>
+M:	Xiaopeng XP Chen <xiao-peng.chen@fii-na.com>
+M:	Mohaimen Alsamarai <Mohaimen.Alsamarai@fii-na.com>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	drivers/hwmon/smpmpro-hwmon.c
+
 SMSC EMC2103 HARDWARE MONITOR DRIVER
 M:	Steve Glendinning <steve.glendinning@shawell.net>
 L:	linux-hwmon@vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9aa89d7d4193..50881ebcb022 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1777,6 +1777,16 @@  config SENSORS_INA3221
 	  This driver can also be built as a module. If so, the module
 	  will be called ina3221.
 
+config SENSORS_SMPMPRO
+	tristate "Ampere Computing Altra SMPMPRO sensor chip"
+	depends on I2C
+	help
+	  If you say yes here you get support for Ampere Computing Altra SMPMPRO
+	  sensor chip. This will enable monitoring of Ampere CPU Sensors; via hwmon.
+
+	  This driver can also be built as a module. If so, the module 
+	  will be called smpmpro-hwmon.
+
 config SENSORS_TC74
 	tristate "Microchip TC74"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index ae41ee71a71b..49a1a8e0c73f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -166,6 +166,7 @@  obj-$(CONFIG_SENSORS_SHT3x)	+= sht3x.o
 obj-$(CONFIG_SENSORS_SHTC1)	+= shtc1.o
 obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
 obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
+obj-$(CONFIG_SENSORS_SMPMPRO)	+= smpmpro-hwmon.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
 obj-$(CONFIG_SENSORS_SMSC47M1)	+= smsc47m1.o
 obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
diff --git a/drivers/hwmon/smpmpro-hwmon.c b/drivers/hwmon/smpmpro-hwmon.c
new file mode 100644
index 000000000000..37b48c42a372
--- /dev/null
+++ b/drivers/hwmon/smpmpro-hwmon.c
@@ -0,0 +1,903 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Ampere Computing SoC Hardware Monitoring Driver
+ *
+ * Copyright (c) 2019-2020, Ampere Computing LLC
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+/* Identification Registers */
+#define REG_SPEC_VERSION_REG	0x00
+#define SCP_VERSION_REG	        0x01
+#define MANUFACTURER_ID_REG	    0x02
+#define DEVICE_ID_REG	        0x03
+#define SCP_BUILD_ID_LO_REG	    0x09
+#define SCP_BUILD_ID_HI_REG	    0x0A
+
+/* Capability Registers  */
+#define TEMP_SENSOR_SUPPORT_REG	0x05
+#define PWR_SENSOR_SUPPORT_REG	0x06
+#define VOLT_SENSOR_SUPPORT_REG	0x07
+#define OTHER_CAP_REG		    0x08
+#define CORE_CLUSTER_CNT_REG	0x0B
+#define SYS_CACHE_PCIE_CNT_REG	0x0C
+#define SOCKET_INFO_REG	        0x0D
+
+/* Logical Power Sensor Registers */
+#define SOC_TEMP_REG		0x10
+#define SOC_VRD_TEMP_REG	0x11
+#define DIMM_VRD_TEMP_REG	0x12
+#define CORE_VRD_TEMP_REG	0x13
+#define CH0_DIMM_TEMP_REG	0x14
+#define CH1_DIMM_TEMP_REG	0x15
+#define CH2_DIMM_TEMP_REG	0x16
+#define CH3_DIMM_TEMP_REG	0x17
+#define CH4_DIMM_TEMP_REG	0x18
+#define CH5_DIMM_TEMP_REG	0x19
+#define CH6_DIMM_TEMP_REG	0x1A
+#define CH7_DIMM_TEMP_REG	0x1B
+#define CORE_VRD_PWR_REG		0x20
+#define SOC_VRD_PWR_REG		0x21
+#define DIMM_VRD1_PWR_REG	0x22
+#define DIMM_VRD2_PWR_REG	0x23
+#define CORE_VRD_PWR_MW_REG	0x26
+#define SOC_VRD_PWR_MW_REG	0x27
+#define DIMM_VRD1_PWR_MW_REG	0x28
+#define DIMM_VRD2_PWR_MW_REG	0x29
+#define MEM_HOT_THRESHOLD_REG	0x32
+#define SOC_VR_HOT_THRESHOLD_REG	0x33
+#define CORE_VRD_VOLT_REG	0x34
+#define SOC_VRD_VOLT_REG	0x35
+#define DIMM_VRD1_VOLT_REG	0x36
+#define DIMM_VRD2_VOLT_REG	0x37
+#define DIMM_CE_THRESHOLD_REG	0x38
+
+/* GPI Control set  Registers */
+#define GPI_CTRL0_REG		0x50
+#define GPI_CTRL1_REG		0x51
+#define GPI_CTRL2_REG		0x52
+#define GPI_CTRL3_REG		0x53
+#define GPI_CE_UE_MASK_REG		0x54
+
+/* GPI data set Registers */
+#define GPI_DATA_SET_REG	0x60
+#define GPI_DATA_SET0_REG	0x61
+#define GPI_DATA_SET1_REG	0x62
+#define GPI_DATA_SET2_REG	0x63
+#define GPI_DATA_SET3_REG	0x64
+
+/* GPI Status Registers */
+#define GPI_CLUSTER_ERR_SET0_REG	0x70
+#define GPI_CLUSTER_ERR_SEL_REG	 0x71
+#define GPI_MCU_ERR_REG		0x72
+#define GPI_PCIE_ERR_REG	0x73
+#define GPI_SYS_CACHE_ERR_SEL_REG	0x74
+#define GPI_SYS_CACHE_ERR_REG	0x75
+#define GPI_PCIE_ERR_SEL_REG	0x76
+#define GPI_VRD_FAULT_ERR_REG	0x78
+#define GPI_VRD_HOT_ERR_REG	0x79
+#define GPI_DIMM_HOT_ERR_REG	0x7A
+#define GPI_BOOT_ERR1_REG	0x7B
+#define GPI_BOOT_ERR2_REG	0x7C
+
+/* GPI RAS Error Registers */
+#define GPI_WDT_STS_REG		0x7D
+#define GPI_RAS_ERR_REG		0x7E
+
+/* Core and L2C Error Registers */
+#define GPI_CORE_CLUSTER_SEL_REG	0x80
+#define GPI_CORE_L1_ERR_REG	0x81
+#define GPI_CORE_L2_ERR_REG	0x82
+#define GPI_SYS_CACHE_INST_SEL_REG	0x86
+#define GPI_SYS_CACHE_ERR_REG	0x87
+
+/* Memory Error Registers */
+#define GPI_MCU_DIMM_SELECT_REG	0x90
+#define GPI_MCU_DIMM_ERR_REG	0x91
+
+/* RAS Error/Warning Registers */
+#define GPI_RAS_ERR_SMPRO_TYPE_REG	0xA0
+#define GPI_RAS_ERR_PMPRO_TYPE_REG	0xA1
+#define GPI_RAS_ERR_SMPRO_INFO_LO_REG	0xA2
+#define GPI_RAS_ERR_SMPRO_INFO_HI_REG	0xA3
+#define GPI_RAS_ERR_SMPRO_DATA_LO_REG	0xA4
+#define GPI_RAS_ERR_SMPRO_DATA_HI_REG	0xA5
+#define GPI_RAS_WARN_SMPRO_INFO_LO_REG	0xAA
+#define GPI_RAS_WARN_SMPRO_INFO_HI_REG	0xAB
+#define GPI_RAS_ERR_PMPRO_INFO_LO_REG	0xA6
+#define GPI_RAS_ERR_PMPRO_INFO_HI_REG	0xA7
+#define GPI_RAS_ERR_PMPRO_DATA_LO_REG	0xA8
+#define GPI_RAS_ERR_PMPRO_DATA_HI_REG	0xA9
+#define GPI_RAS_WARN_PMPRO_INFO_LO_REG	0xAC
+#define GPI_RAS_WARN_PMPRO_INFO_HI_REG	0xAD
+
+/* Boot Stage/Progress Registers */
+#define GPI_BOOT_STAGE_SELECT_REG	0xB0
+#define GPI_BOOT_STAGE_STATUS_LO_REG	0xB1
+#define GPI_BOOT_STAGE_CUR_STAGE_REG	0xB2
+
+/* PCIE Error Registers */
+#define GPI_PCIE_ERR_SEL_REG		0xC0
+#define GPI_PCIE_ERR_TYPE_REG		0xC1
+#define GPI_PCIE_ERR_DEVICE_REG	0xC2
+
+/* Other Error Registers */
+#define OTHER_CE_ERR_CNT_REG	0xD0
+#define OTHER_CE_ERR_LEN_REG	0xD1
+#define OTHER_CE_ERR_DATA_REG	0xD2
+#define OTHER_UE_ERR_CNT_REG	0xD0
+#define OTHER_UE_ERR_LEN_REG	0xD1
+#define OTHER_UE_ERR_DATA_REG	0xD2
+
+/* ACPI State Registers */
+#define ACPI_SYSTEM_STATE_REG		0xE0
+#define ACPI_CPPC_CLUSTER_SEL_REG	0xE3
+#define ACPI_CPPC_CLUSTER_DATA_REG		0xE4
+#define ACPI_POWER_LIMIT_REG		0xE5
+
+
+struct smpmpro_data {
+	struct i2c_client *client;
+
+	u16 temp_support_regs;
+	u16 pwr_support_regs;
+	u16 volt_support_regs;
+	u16 other_caps;
+	u16 core_cluster_cnt_reg;
+	u16 sys_cache_pcie_cnt_reg;
+	u16 socket_info_reg;
+};
+
+static const u8 temp_regs[] = {
+	SOC_TEMP_REG,
+	SOC_VRD_TEMP_REG,
+	DIMM_VRD_TEMP_REG,
+	CORE_VRD_TEMP_REG,
+	CH0_DIMM_TEMP_REG,
+	CH1_DIMM_TEMP_REG,
+	CH2_DIMM_TEMP_REG,
+	CH3_DIMM_TEMP_REG,
+	CH4_DIMM_TEMP_REG,
+	CH5_DIMM_TEMP_REG,
+	CH6_DIMM_TEMP_REG,
+	CH7_DIMM_TEMP_REG,
+	MEM_HOT_THRESHOLD_REG,
+	SOC_VR_HOT_THRESHOLD_REG
+};
+
+static const u8 volt_regs[] = {
+	CORE_VRD_VOLT_REG,
+	SOC_VRD_VOLT_REG,
+	DIMM_VRD1_VOLT_REG,
+	DIMM_VRD2_VOLT_REG
+};
+
+enum pwr_regs {
+	PMD_VRD_PWR,
+	SOC_VRD_PWR,
+	DIMM_VRD1_PWR,
+	DIMM_VRD2_PWR,
+	CPU_VRD_PWR,
+	DIMM_VRD_PWR,
+};
+
+static const char * const label[] = {
+	"SoC",
+	"SoC VRD",
+	"DIMM VRD",
+	"DIMM VRD1",
+	"DIMM VRD2",
+	"PMD VRD",
+	"CH0 DIMM",
+	"CH1 DIMM",
+	"CH2 DIMM",
+	"CH3 DIMM",
+	"CH4 DIMM",
+	"CH5 DIMM",
+	"CH6 DIMM",
+	"CH7 DIMM",
+	"MEM HOT",
+	"SoC VR HOT",
+	"CPU VRD",
+};
+
+static const char * const gpi_label[] = {
+	"GPI CTRL 0",
+	"GPI CTRL 1",
+	"GPI CTRL 2",
+	"GPI CTRL 3",
+	"GPI CE UE MASK",
+	"GPI DATA SET",
+	"GPI DATA SET 0",
+	"GPI DATA SET 1",
+	"GPI DATA SET 2",
+	"GPI DATA SET 3",
+	"CLUSTER ERROR SET 0",
+	"CLUSTER ERROR SEL",
+	"MCU ERROR",
+	"PCIE ERROR",
+	"SYS CACHE ERR SEL",
+	"SYS CACHE ERR",
+	"PCIE ERR SEL",
+	"VRD FAULT ERR",
+	"VRD HOT ERROR",
+	"DIMM HOT ERROR",
+	"BOOT 1 ERROR",
+	"BOOT 2 ERROR",
+	"WATCHDOG STATUS",
+	"RAS INTERNAL ERROR",
+};
+
+static void smpmpro_init_device(struct i2c_client *client,
+				struct smpmpro_data *data)
+{
+	u16 ret;
+
+	ret = i2c_smbus_read_word_swapped(client, TEMP_SENSOR_SUPPORT_REG);
+	if (ret < 0)
+		return;
+	data->temp_support_regs = ret;
+
+	ret = i2c_smbus_read_word_swapped(client, PWR_SENSOR_SUPPORT_REG);
+	if (ret < 0)
+		return;
+	data->pwr_support_regs = ret;
+
+	ret = i2c_smbus_read_word_swapped(client, VOLT_SENSOR_SUPPORT_REG);
+	if (ret < 0)
+		return;
+	data->volt_support_regs = ret;
+
+	ret = i2c_smbus_read_word_swapped(client, OTHER_CAP_REG);
+	if (ret < 0)
+		return;
+	data->other_caps = ret;
+
+	ret = i2c_smbus_read_word_swapped(client, CORE_CLUSTER_CNT_REG);
+	if (ret < 0)
+		return;
+	data->core_cluster_cnt_reg = ret;
+
+	ret = i2c_smbus_read_word_swapped(client, SYS_CACHE_PCIE_CNT_REG);
+	if (ret < 0)
+		return;
+	data->sys_cache_pcie_cnt_reg = ret;
+
+	ret = i2c_smbus_read_word_swapped(client, SOCKET_INFO_REG);
+	if (ret < 0)
+		return;
+	data->socket_info_reg = ret;
+}
+
+static int smpmpro_read_temp(struct device *dev, u32 attr, int channel,
+			     long *val)
+{
+	struct smpmpro_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int ret;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		ret = i2c_smbus_read_word_swapped(client, temp_regs[channel]);
+		if (ret < 0)
+			return ERR_PTR(ret);
+		*val = (ret & 0x1ff) * 1000;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static umode_t smpmpro_temp_is_visible(const void *_data, u32 attr, int channel)
+{
+	return 0444;
+}
+
+static int smpmpro_read_in(struct device *dev, u32 attr, int channel,
+			     long *val)
+{
+	struct smpmpro_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int ret;
+
+	switch (attr) {
+	case hwmon_in_input:
+		ret = i2c_smbus_read_word_swapped(client, volt_regs[channel]);
+		if (ret < 0)
+			return ERR_PTR(ret);
+		*val = ret & 0x7fff;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static umode_t smpmpro_in_is_visible(const void *_data, u32 attr, int channel)
+{
+	return 0444;
+}
+
+static int smpmpro_read_power(struct device *dev, u32 attr, int channel,
+			     long *val)
+{
+	struct smpmpro_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int ret, ret_mw;
+	int ret2 = 0, ret2_mw = 0;
+
+	switch (attr) {
+	case hwmon_power_input:
+		switch (channel) {
+		case PMD_VRD_PWR:
+			ret = i2c_smbus_read_word_swapped(client, CORE_VRD_PWR_REG);
+			ret_mw = i2c_smbus_read_word_swapped(client, CORE_VRD_PWR_MW_REG);
+			break;
+		case SOC_VRD_PWR:
+			ret = i2c_smbus_read_word_swapped(client, SOC_VRD_PWR_REG);
+			ret_mw = i2c_smbus_read_word_swapped(client, SOC_VRD_PWR_MW_REG);
+			break;
+		case DIMM_VRD1_PWR:
+			ret = i2c_smbus_read_word_swapped(client, DIMM_VRD1_PWR_REG);
+			ret_mw = i2c_smbus_read_word_swapped(client, DIMM_VRD1_PWR_MW_REG);
+			break;
+		case DIMM_VRD2_PWR:
+			ret = i2c_smbus_read_word_swapped(client, DIMM_VRD2_PWR_REG);
+			ret_mw = i2c_smbus_read_word_swapped(client, DIMM_VRD2_PWR_MW_REG);
+			break;
+		case CPU_VRD_PWR:
+			ret = i2c_smbus_read_word_swapped(client, CORE_VRD_PWR_REG);
+			ret_mw = i2c_smbus_read_word_swapped(client, CORE_VRD_PWR_MW_REG);
+			ret2 = i2c_smbus_read_word_swapped(client, SOC_VRD_PWR_REG);
+			ret2_mw = i2c_smbus_read_word_swapped(client, SOC_VRD_PWR_MW_REG);
+			break;
+		case DIMM_VRD_PWR:
+			ret = i2c_smbus_read_word_swapped(client, DIMM_VRD1_PWR_REG);
+			ret_mw = i2c_smbus_read_word_swapped(client, DIMM_VRD1_PWR_MW_REG);
+			ret2 = i2c_smbus_read_word_swapped(client, DIMM_VRD2_PWR_REG);
+			ret2_mw = i2c_smbus_read_word_swapped(client, DIMM_VRD2_PWR_MW_REG);
+			break;
+		default:
+			return -EOPNOTSUPP;
+		}
+		if (ret < 0 || ret_mw < 0 || ret2 < 0 || ret2_mw < 0)
+			return ERR_PTR(ret < 0 ? ret : ret_mw < 0 ? ret_mw
+					: ret2 < 0 ? ret2 : ret2_mw);
+		*val = (ret + ret2)*1000000 + (ret_mw + ret2_mw)*1000;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static umode_t smpmpro_power_is_visible(const void *_data, u32 attr, int channel)
+{
+	return 0444;
+}
+
+
+static int smpmpro_read_gpi(struct device *dev,
+				 struct device_attribute *da, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct smpmpro_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	int ret;
+
+	ret = i2c_smbus_read_word_swapped(client, attr->index);
+
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
+}
+
+static int smpmpro_read(struct device *dev, enum hwmon_sensor_types type,
+			 u32 attr, int channel, long *val)
+{
+	switch (type) {
+	case hwmon_temp:
+		return smpmpro_read_temp(dev, attr, channel, val);
+	case hwmon_in:
+		return smpmpro_read_in(dev, attr, channel, val);
+	case hwmon_power:
+		return smpmpro_read_power(dev, attr, channel, val);
+//	case hwmon_dimm_ce_thres:
+//		return smpmpro_read_dimm_ce_thres(dev, attr, channel, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int smpmpro_write_gpi(struct device *dev,
+				   struct device_attribute *da,
+				   const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
+	struct smpmpro_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 16, &val);
+
+	ret = i2c_smbus_write_word_swapped(client, attr->index, val);
+	if (ret < 0)
+		return -EPROTO;
+
+	return count;
+}
+
+static int smpmpro_write(struct device *dev, enum hwmon_sensor_types type,
+			  u32 attr, int channel, long val)
+{
+	return -EOPNOTSUPP;
+}
+
+static umode_t smpmpro_is_visible(const void *data,
+				   enum hwmon_sensor_types type,
+				   u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_temp:
+		return smpmpro_temp_is_visible(data, attr, channel);
+	case hwmon_in:
+		return smpmpro_in_is_visible(data, attr, channel);
+	case hwmon_power:
+		return smpmpro_power_is_visible(data, attr, channel);
+//	case hwmon_dimm_ce_thres:
+//		return smpmpro_dimm_ce_thres_is_visible(data, attr, channel);
+	default:
+		return 0;
+	}
+}
+
+static ssize_t show_label(struct device *dev, struct device_attribute *devattr,
+				 char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+
+	return sprintf(buf, "%s\n", label[index]);
+}
+
+static ssize_t show_gpi_label(struct device *dev,
+		struct device_attribute *devattr, char *buf)
+{
+	int index = to_sensor_dev_attr(devattr)->index;
+
+	return sprintf(buf, "%s\n", gpi_label[index]);
+}
+
+static const u32 smpmpro_temp_config[] = {
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	HWMON_T_INPUT,
+	0
+};
+
+static const struct hwmon_channel_info smpmpro_temp = {
+	.type = hwmon_temp,
+	.config = smpmpro_temp_config,
+};
+
+static const u32 smpmpro_in_config[] = {
+	HWMON_I_INPUT,
+	HWMON_I_INPUT,
+	HWMON_I_INPUT,
+	HWMON_I_INPUT,
+	0
+};
+
+static const struct hwmon_channel_info smpmpro_in = {
+	.type = hwmon_in,
+	.config = smpmpro_in_config,
+};
+
+static const u32 smpmpro_power_config[] = {
+	HWMON_P_INPUT,
+	HWMON_P_INPUT,
+	HWMON_P_INPUT,
+	HWMON_P_INPUT,
+	HWMON_P_INPUT,
+	HWMON_P_INPUT,
+	0
+};
+
+static const struct hwmon_channel_info smpmpro_power = {
+	.type = hwmon_power,
+	.config = smpmpro_power_config,
+};
+
+static const struct hwmon_channel_info *smpmpro_info[] = {
+	&smpmpro_temp,
+	&smpmpro_in,
+	&smpmpro_power,
+	NULL
+};
+
+static const struct hwmon_ops smpmpro_hwmon_ops = {
+	.is_visible = smpmpro_is_visible,
+	.read = smpmpro_read,
+	.write = smpmpro_write,
+};
+
+static const struct hwmon_chip_info smpmpro_chip_info = {
+	.ops = &smpmpro_hwmon_ops,
+	.info = smpmpro_info,
+};
+
+static SENSOR_DEVICE_ATTR(temp1_label, 0444, show_label, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_label, 0444, show_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp3_label, 0444, show_label, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp4_label, 0444, show_label, NULL, 5);
+static SENSOR_DEVICE_ATTR(temp5_label, 0444, show_label, NULL, 6);
+static SENSOR_DEVICE_ATTR(temp6_label, 0444, show_label, NULL, 7);
+static SENSOR_DEVICE_ATTR(temp7_label, 0444, show_label, NULL, 8);
+static SENSOR_DEVICE_ATTR(temp8_label, 0444, show_label, NULL, 9);
+static SENSOR_DEVICE_ATTR(temp9_label, 0444, show_label, NULL, 10);
+static SENSOR_DEVICE_ATTR(temp10_label, 0444, show_label, NULL, 11);
+static SENSOR_DEVICE_ATTR(temp11_label, 0444, show_label, NULL, 12);
+static SENSOR_DEVICE_ATTR(temp12_label, 0444, show_label, NULL, 13);
+static SENSOR_DEVICE_ATTR(temp13_label, 0444, show_label, NULL, 14);
+static SENSOR_DEVICE_ATTR(temp14_label, 0444, show_label, NULL, 15);
+
+static SENSOR_DEVICE_ATTR(in0_label, 0444, show_label, NULL, 5);
+static SENSOR_DEVICE_ATTR(in1_label, 0444, show_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(in2_label, 0444, show_label, NULL, 3);
+static SENSOR_DEVICE_ATTR(in3_label, 0444, show_label, NULL, 4);
+
+static SENSOR_DEVICE_ATTR(power1_label, 0444, show_label, NULL, 5);
+static SENSOR_DEVICE_ATTR(power2_label, 0444, show_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(power3_label, 0444, show_label, NULL, 3);
+static SENSOR_DEVICE_ATTR(power4_label, 0444, show_label, NULL, 4);
+static SENSOR_DEVICE_ATTR(power5_label, 0444, show_label, NULL, 16);
+static SENSOR_DEVICE_ATTR(power6_label, 0444, show_label, NULL, 2);
+
+static SENSOR_DEVICE_ATTR(gpi0_label, 0444, show_gpi_label, NULL, 0);
+static SENSOR_DEVICE_ATTR(gpi0_input, 0644, smpmpro_read_gpi, smpmpro_write_gpi, GPI_CTRL0_REG);
+static SENSOR_DEVICE_ATTR(gpi1_label, 0444, show_gpi_label, NULL, 1);
+static SENSOR_DEVICE_ATTR(gpi1_input, 0644, smpmpro_read_gpi, smpmpro_write_gpi, GPI_CTRL1_REG);
+static SENSOR_DEVICE_ATTR(gpi2_label, 0444, show_gpi_label, NULL, 2);
+static SENSOR_DEVICE_ATTR(gpi2_input, 0644, smpmpro_read_gpi, smpmpro_write_gpi, GPI_CTRL2_REG);
+static SENSOR_DEVICE_ATTR(gpi3_label, 0444, show_gpi_label, NULL, 3);
+static SENSOR_DEVICE_ATTR(gpi3_input, 0644, smpmpro_read_gpi, smpmpro_write_gpi, GPI_CTRL3_REG);
+static SENSOR_DEVICE_ATTR(gpi4_label, 0444, show_gpi_label, NULL, 4);
+static SENSOR_DEVICE_ATTR(gpi4_input, 0644, smpmpro_read_gpi,
+	       smpmpro_write_gpi, GPI_CE_UE_MASK_REG);
+static SENSOR_DEVICE_ATTR(gpi5_label, 0444, show_gpi_label, NULL, 5);
+static SENSOR_DEVICE_ATTR(gpi5_input, 0444, smpmpro_read_gpi, NULL, GPI_DATA_SET_REG);
+static SENSOR_DEVICE_ATTR(gpi6_label, 0444, show_gpi_label, NULL, 6);
+static SENSOR_DEVICE_ATTR(gpi6_input, 0444, smpmpro_read_gpi, NULL, GPI_DATA_SET0_REG);
+static SENSOR_DEVICE_ATTR(gpi7_label, 0444, show_gpi_label, NULL, 7);
+static SENSOR_DEVICE_ATTR(gpi7_input, 0444, smpmpro_read_gpi, NULL, GPI_DATA_SET1_REG);
+static SENSOR_DEVICE_ATTR(gpi8_label, 0444, show_gpi_label, NULL, 8);
+static SENSOR_DEVICE_ATTR(gpi8_input, 0444, smpmpro_read_gpi, NULL, GPI_DATA_SET2_REG);
+static SENSOR_DEVICE_ATTR(gpi9_label, 0444, show_gpi_label, NULL, 9);
+static SENSOR_DEVICE_ATTR(gpi9_input, 0444, smpmpro_read_gpi, NULL, GPI_DATA_SET3_REG);
+static SENSOR_DEVICE_ATTR(gpi10_label, 0444, show_gpi_label, NULL, 10);
+static SENSOR_DEVICE_ATTR(gpi10_input, 0444, smpmpro_read_gpi, NULL, GPI_CLUSTER_ERR_SET0_REG);
+static SENSOR_DEVICE_ATTR(gpi11_label, 0444, show_gpi_label, NULL, 11);
+static SENSOR_DEVICE_ATTR(gpi11_input, 0444, smpmpro_read_gpi, NULL, GPI_CLUSTER_ERR_SEL_REG);
+static SENSOR_DEVICE_ATTR(gpi12_label, 0444, show_gpi_label, NULL, 12);
+static SENSOR_DEVICE_ATTR(gpi12_input, 0444, smpmpro_read_gpi, NULL, GPI_MCU_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi13_label, 0444, show_gpi_label, NULL, 13);
+static SENSOR_DEVICE_ATTR(gpi13_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_PCIE_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi14_label, 0444, show_gpi_label, NULL, 14);
+static SENSOR_DEVICE_ATTR(gpi14_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_SYS_CACHE_ERR_SEL_REG);
+static SENSOR_DEVICE_ATTR(gpi15_label, 0444, show_gpi_label, NULL, 15);
+static SENSOR_DEVICE_ATTR(gpi15_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_SYS_CACHE_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi16_label, 0444, show_gpi_label, NULL, 16);
+static SENSOR_DEVICE_ATTR(gpi16_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_PCIE_ERR_SEL_REG);
+static SENSOR_DEVICE_ATTR(gpi17_label, 0444, show_gpi_label, NULL, 17);
+static SENSOR_DEVICE_ATTR(gpi17_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_VRD_FAULT_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi18_label, 0444, show_gpi_label, NULL, 18);
+static SENSOR_DEVICE_ATTR(gpi18_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_VRD_HOT_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi19_label, 0444, show_gpi_label, NULL, 19);
+static SENSOR_DEVICE_ATTR(gpi19_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_DIMM_HOT_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi20_label, 0444, show_gpi_label, NULL, 20);
+static SENSOR_DEVICE_ATTR(gpi20_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_BOOT_ERR1_REG);
+static SENSOR_DEVICE_ATTR(gpi21_label, 0444, show_gpi_label, NULL, 21);
+static SENSOR_DEVICE_ATTR(gpi21_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_BOOT_ERR2_REG);
+static SENSOR_DEVICE_ATTR(gpi22_label, 0444, show_gpi_label, NULL, 22);
+static SENSOR_DEVICE_ATTR(gpi22_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_WDT_STS_REG);
+static SENSOR_DEVICE_ATTR(gpi23_label, 0444, show_gpi_label, NULL, 23);
+static SENSOR_DEVICE_ATTR(gpi23_input, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_REG);
+
+static SENSOR_DEVICE_ATTR(gpi_core_cluster_sel, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, GPI_CORE_CLUSTER_SEL_REG);
+static SENSOR_DEVICE_ATTR(gpi_core_l1_err, 0444,
+	smpmpro_read_gpi, NULL, GPI_CORE_L1_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi_core_l2_err, 0444,
+	smpmpro_read_gpi, NULL, GPI_CORE_L2_ERR_REG);
+static SENSOR_DEVICE_ATTR(gpi_sys_cache_inst_sel, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, GPI_SYS_CACHE_INST_SEL_REG);
+static SENSOR_DEVICE_ATTR(gpi_sys_cache_err, 0444,
+	smpmpro_read_gpi, NULL, GPI_SYS_CACHE_ERR_REG);
+
+static SENSOR_DEVICE_ATTR(gpi_mcu_dimm_select, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, GPI_MCU_DIMM_SELECT_REG);
+static SENSOR_DEVICE_ATTR(gpi_mcu_dimm_err, 0444,
+	smpmpro_read_gpi, NULL, GPI_MCU_DIMM_ERR_REG);
+
+static SENSOR_DEVICE_ATTR(gpi_ras_err_smpro_type, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_SMPRO_TYPE_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_pmpro_type, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_PMPRO_TYPE_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_smpro_info_lo, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_SMPRO_INFO_LO_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_smpro_info_hi, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_SMPRO_INFO_HI_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_smpro_data_lo, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_SMPRO_DATA_LO_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_smpro_data_hi, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_SMPRO_DATA_HI_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_warn_smpro_info_lo, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_WARN_SMPRO_INFO_LO_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_warn_smpro_info_hi, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_WARN_SMPRO_INFO_HI_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_pmpro_info_lo, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_PMPRO_INFO_LO_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_pmpro_info_hi, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_PMPRO_INFO_HI_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_pmpro_data_lo, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_PMPRO_DATA_LO_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_err_pmpro_data_hi, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_ERR_PMPRO_DATA_HI_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_warn_pmpro_info_lo, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_WARN_PMPRO_INFO_LO_REG);
+static SENSOR_DEVICE_ATTR(gpi_ras_warn_pmpro_info_hi, 0444,
+	smpmpro_read_gpi, NULL, GPI_RAS_WARN_PMPRO_INFO_HI_REG);
+
+static SENSOR_DEVICE_ATTR(gpi_boot_stage_select, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, GPI_BOOT_STAGE_SELECT_REG);
+static SENSOR_DEVICE_ATTR(gpi_boot_stage_status_lo, 0444,
+	smpmpro_read_gpi, NULL, GPI_BOOT_STAGE_STATUS_LO_REG);
+static SENSOR_DEVICE_ATTR(gpi_boot_stage_cur_stage, 0444,
+	smpmpro_read_gpi, NULL, GPI_BOOT_STAGE_CUR_STAGE_REG);
+
+static SENSOR_DEVICE_ATTR(gpi_pcie_err_sel, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, GPI_PCIE_ERR_SEL_REG);
+static SENSOR_DEVICE_ATTR(gpi_pcie_err_type, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, GPI_PCIE_ERR_TYPE_REG);
+static SENSOR_DEVICE_ATTR(gpi_pcie_err_device, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, GPI_PCIE_ERR_DEVICE_REG);
+
+static SENSOR_DEVICE_ATTR(other_ce_err_cnt, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, OTHER_CE_ERR_CNT_REG);
+static SENSOR_DEVICE_ATTR(other_ce_err_len, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, OTHER_CE_ERR_LEN_REG);
+static SENSOR_DEVICE_ATTR(other_ce_err_data, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, OTHER_CE_ERR_DATA_REG);
+static SENSOR_DEVICE_ATTR(other_ue_err_cnt, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, OTHER_UE_ERR_CNT_REG);
+static SENSOR_DEVICE_ATTR(other_ue_err_len, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, OTHER_UE_ERR_LEN_REG);
+static SENSOR_DEVICE_ATTR(other_ue_err_data, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, OTHER_UE_ERR_DATA_REG);
+
+static SENSOR_DEVICE_ATTR(acpi_system_state, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, ACPI_SYSTEM_STATE_REG);
+static SENSOR_DEVICE_ATTR(acpi_cppc_cluster_sel, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, ACPI_CPPC_CLUSTER_SEL_REG);
+static SENSOR_DEVICE_ATTR(acpi_cppc_cluster_data, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, ACPI_CPPC_CLUSTER_DATA_REG);
+static SENSOR_DEVICE_ATTR(acpi_power_limit, 0644,
+	smpmpro_read_gpi, smpmpro_write_gpi, ACPI_POWER_LIMIT_REG);
+
+static struct attribute *smpmpro_attrs[] = {
+	&sensor_dev_attr_gpi0_label.dev_attr.attr,
+	&sensor_dev_attr_gpi0_input.dev_attr.attr,
+	&sensor_dev_attr_gpi1_label.dev_attr.attr,
+	&sensor_dev_attr_gpi1_input.dev_attr.attr,
+	&sensor_dev_attr_gpi2_label.dev_attr.attr,
+	&sensor_dev_attr_gpi2_input.dev_attr.attr,
+	&sensor_dev_attr_gpi3_label.dev_attr.attr,
+	&sensor_dev_attr_gpi3_input.dev_attr.attr,
+	&sensor_dev_attr_gpi4_label.dev_attr.attr,
+	&sensor_dev_attr_gpi4_input.dev_attr.attr,
+	&sensor_dev_attr_gpi5_label.dev_attr.attr,
+	&sensor_dev_attr_gpi5_input.dev_attr.attr,
+	&sensor_dev_attr_gpi6_label.dev_attr.attr,
+	&sensor_dev_attr_gpi6_input.dev_attr.attr,
+	&sensor_dev_attr_gpi7_label.dev_attr.attr,
+	&sensor_dev_attr_gpi7_input.dev_attr.attr,
+	&sensor_dev_attr_gpi8_label.dev_attr.attr,
+	&sensor_dev_attr_gpi8_input.dev_attr.attr,
+	&sensor_dev_attr_gpi9_label.dev_attr.attr,
+	&sensor_dev_attr_gpi9_input.dev_attr.attr,
+	&sensor_dev_attr_gpi10_label.dev_attr.attr,
+	&sensor_dev_attr_gpi10_input.dev_attr.attr,
+	&sensor_dev_attr_gpi11_label.dev_attr.attr,
+	&sensor_dev_attr_gpi11_input.dev_attr.attr,
+	&sensor_dev_attr_gpi12_label.dev_attr.attr,
+	&sensor_dev_attr_gpi12_input.dev_attr.attr,
+	&sensor_dev_attr_gpi13_label.dev_attr.attr,
+	&sensor_dev_attr_gpi13_input.dev_attr.attr,
+	&sensor_dev_attr_gpi14_label.dev_attr.attr,
+	&sensor_dev_attr_gpi14_input.dev_attr.attr,
+	&sensor_dev_attr_gpi15_label.dev_attr.attr,
+	&sensor_dev_attr_gpi15_input.dev_attr.attr,
+	&sensor_dev_attr_gpi16_label.dev_attr.attr,
+	&sensor_dev_attr_gpi16_input.dev_attr.attr,
+	&sensor_dev_attr_gpi17_label.dev_attr.attr,
+	&sensor_dev_attr_gpi17_input.dev_attr.attr,
+	&sensor_dev_attr_gpi18_label.dev_attr.attr,
+	&sensor_dev_attr_gpi18_input.dev_attr.attr,
+	&sensor_dev_attr_gpi19_label.dev_attr.attr,
+	&sensor_dev_attr_gpi19_input.dev_attr.attr,
+	&sensor_dev_attr_gpi20_label.dev_attr.attr,
+	&sensor_dev_attr_gpi20_input.dev_attr.attr,
+	&sensor_dev_attr_gpi21_label.dev_attr.attr,
+	&sensor_dev_attr_gpi21_input.dev_attr.attr,
+	&sensor_dev_attr_gpi22_label.dev_attr.attr,
+	&sensor_dev_attr_gpi22_input.dev_attr.attr,
+	&sensor_dev_attr_gpi23_label.dev_attr.attr,
+	&sensor_dev_attr_gpi23_input.dev_attr.attr,
+
+	&sensor_dev_attr_temp1_label.dev_attr.attr,
+	&sensor_dev_attr_temp2_label.dev_attr.attr,
+	&sensor_dev_attr_temp3_label.dev_attr.attr,
+	&sensor_dev_attr_temp4_label.dev_attr.attr,
+	&sensor_dev_attr_temp5_label.dev_attr.attr,
+	&sensor_dev_attr_temp6_label.dev_attr.attr,
+	&sensor_dev_attr_temp7_label.dev_attr.attr,
+	&sensor_dev_attr_temp8_label.dev_attr.attr,
+	&sensor_dev_attr_temp9_label.dev_attr.attr,
+	&sensor_dev_attr_temp10_label.dev_attr.attr,
+	&sensor_dev_attr_temp11_label.dev_attr.attr,
+	&sensor_dev_attr_temp12_label.dev_attr.attr,
+	&sensor_dev_attr_temp13_label.dev_attr.attr,
+	&sensor_dev_attr_temp14_label.dev_attr.attr,
+
+	&sensor_dev_attr_in0_label.dev_attr.attr,
+	&sensor_dev_attr_in1_label.dev_attr.attr,
+	&sensor_dev_attr_in2_label.dev_attr.attr,
+	&sensor_dev_attr_in3_label.dev_attr.attr,
+
+	&sensor_dev_attr_power1_label.dev_attr.attr,
+	&sensor_dev_attr_power2_label.dev_attr.attr,
+	&sensor_dev_attr_power3_label.dev_attr.attr,
+	&sensor_dev_attr_power4_label.dev_attr.attr,
+	&sensor_dev_attr_power5_label.dev_attr.attr,
+	&sensor_dev_attr_power6_label.dev_attr.attr,
+
+	&sensor_dev_attr_gpi_core_cluster_sel.dev_attr.attr,
+	&sensor_dev_attr_gpi_core_l1_err.dev_attr.attr,
+	&sensor_dev_attr_gpi_core_l2_err.dev_attr.attr,
+	&sensor_dev_attr_gpi_sys_cache_inst_sel.dev_attr.attr,
+	&sensor_dev_attr_gpi_sys_cache_err.dev_attr.attr,
+
+	&sensor_dev_attr_gpi_mcu_dimm_select.dev_attr.attr,
+	&sensor_dev_attr_gpi_mcu_dimm_err.dev_attr.attr,
+
+	&sensor_dev_attr_gpi_ras_err_smpro_type.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_pmpro_type.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_smpro_info_lo.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_smpro_info_hi.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_smpro_data_lo.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_smpro_data_hi.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_warn_smpro_info_lo.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_warn_smpro_info_hi.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_pmpro_info_lo.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_pmpro_info_hi.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_pmpro_data_lo.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_err_pmpro_data_hi.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_warn_pmpro_info_lo.dev_attr.attr,
+	&sensor_dev_attr_gpi_ras_warn_pmpro_info_hi.dev_attr.attr,
+
+	&sensor_dev_attr_gpi_boot_stage_select.dev_attr.attr,
+	&sensor_dev_attr_gpi_boot_stage_status_lo.dev_attr.attr,
+	&sensor_dev_attr_gpi_boot_stage_cur_stage.dev_attr.attr,
+
+	&sensor_dev_attr_gpi_pcie_err_sel.dev_attr.attr,
+	&sensor_dev_attr_gpi_pcie_err_type.dev_attr.attr,
+	&sensor_dev_attr_gpi_pcie_err_device.dev_attr.attr,
+
+	&sensor_dev_attr_other_ce_err_cnt.dev_attr.attr,
+	&sensor_dev_attr_other_ce_err_len.dev_attr.attr,
+	&sensor_dev_attr_other_ce_err_data.dev_attr.attr,
+	&sensor_dev_attr_other_ue_err_cnt.dev_attr.attr,
+	&sensor_dev_attr_other_ue_err_len.dev_attr.attr,
+	&sensor_dev_attr_other_ue_err_data.dev_attr.attr,
+
+	&sensor_dev_attr_acpi_system_state.dev_attr.attr,
+	&sensor_dev_attr_acpi_cppc_cluster_sel.dev_attr.attr,
+	&sensor_dev_attr_acpi_cppc_cluster_data.dev_attr.attr,
+	&sensor_dev_attr_acpi_power_limit.dev_attr.attr,
+
+	NULL
+};
+ATTRIBUTE_GROUPS(smpmpro);
+
+static int smpmpro_i2c_probe(struct i2c_client *client,
+			  const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = client->adapter;
+	struct device *dev = &client->dev;
+	struct smpmpro_data *data;
+	struct device *hwmon_dev;
+
+	if (!i2c_check_functionality(adapter,
+			I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
+		return -ENODEV;
+
+	data = devm_kzalloc(dev, sizeof(struct smpmpro_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->client = client;
+
+	/* Initialize the Altra SMPMPro chip */
+	smpmpro_init_device(client, data);
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
+							 data,
+							 &smpmpro_chip_info,
+							 smpmpro_groups);
+	return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const struct i2c_device_id smpmpro_i2c_id[] = {
+	{ "smpmpro", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, smpmpro_i2c_id);
+
+static struct i2c_driver smpmpro_driver = {
+	.class		= I2C_CLASS_HWMON,
+	.probe		= smpmpro_i2c_probe,
+	.driver = {
+		.name	= "smpmpro",
+	},
+	.id_table	= smpmpro_i2c_id,
+};
+
+module_i2c_driver(smpmpro_driver);
+
+MODULE_AUTHOR("Thinh Pham <thinh.pham@amperecomputing.com>");
+MODULE_AUTHOR("Dien Nguyen <dinguyen@amperecomputing.com>");
+MODULE_AUTHOR("Hoang Nguyen <hnguyen@amperecomputing.com>");
+MODULE_DESCRIPTION("Altra SMPMPro driver");
+MODULE_LICENSE("GPL");