SB-TSI hwmon driver v4
Subsystems (3)
v4 cover SB-TSI hwmon driver v4
v4: addressed comments in v3; converted DT binding to required schema
v3: addressed comment in v2 and improved documentation
v2: rewrote using devm_hwmon_device_register_with_info() API and addressed
comments received in v1
v1: first version
Kun Yi (3):
hwmon: (sbtsi) Add basic support for SB-TSI sensors
hwmon: (sbtsi) Add documentation
dt-bindings: (hwmon/sbtsi_tmep) Add SB-TSI hwmon driver bindings
.../devicetree/bindings/hwmon/amd,sbtsi.yaml | 54 ++++ Documentation/hwmon/index.rst | 1 + Documentation/hwmon/sbtsi_temp.rst | 40 +++ drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/sbtsi_temp.c | 265 ++++++++++++++++++ 6 files changed, 371 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/amd,sbtsi.yaml create mode 100644 Documentation/hwmon/sbtsi_temp.rst create mode 100644 drivers/hwmon/sbtsi_temp.c
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
PATCH v4 1/3 hwmon: (sbtsi) Add basic support for SB-TSI sensors
SB Temperature Sensor Interface (SB-TSI) is an SMBus compatible interface that reports AMD SoC's Ttcl (normalized temperature), and resembles a typical 8-pin remote temperature sensor's I2C interface to BMC.
This commit adds basic support using this interface to read CPU temperature, and read/write high/low CPU temp thresholds.
To instantiate this driver on an AMD CPU with SB-TSI
support, the i2c bus number would be the bus connected from the board management controller (BMC) to the CPU. The i2c address is specified in Section 6.3.1 of the spec [1]: The SB-TSI address is normally 98h for socket 0 and 90h for socket 1, but it could vary based on hardware address select pins.
[1]: https://www.amd.com/system/files/TechDocs/56255_OSRR.pdf
Test status: tested reading temp1_input, and reading/writing temp1_max/min.
--- drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/sbtsi_temp.c | 265 +++++++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+) create mode 100644 drivers/hwmon/sbtsi_temp.c
drivers/hwmon/Kconfig +10 -0
@@ -1522,6 +1522,16 @@ config SENSORS_SL28CPLD This driver can also be built as a module. If so, the module will be called sl28cpld-hwmon. +config SENSORS_SBTSI + tristate "Emulated SB-TSI temperature sensor" + depends on I2C + help + If you say yes here you get support for emulated temperature + sensors on AMD SoCs with SB-TSI interface connected to a BMC device. + + This driver can also be built as a module. If so, the module will + be called sbtsi_temp. + config SENSORS_SHT15 tristate "Sensiron humidity and temperature sensors. SHT15 and compat." depends on GPIOLIB || COMPILE_TEST
drivers/hwmon/Makefile +1 -0
@@ -160,6 +160,7 @@ obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o +obj-$(CONFIG_SENSORS_SBTSI) += sbtsi_temp.o obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o obj-$(CONFIG_SENSORS_SCH5636) += sch5636.o
drivers/hwmon/sbtsi_temp.c new file +265 -0
265 lines not on this page · raw message
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
Guenter Roeck
maintainerRe: [PATCH linux hwmon-next v4 1/3] hwmon: (sbtsi) Add basic support for SB-TSI sensors
Quoted 139 lines · Kun Yi
SB Temperature Sensor Interface (SB-TSI) is an SMBus compatible interface that reports AMD SoC's Ttcl (normalized temperature), and resembles a typical 8-pin remote temperature sensor's I2C interface to BMC.
This commit adds basic support using this interface to read CPU temperature, and read/write high/low CPU temp thresholds.
To instantiate this driver on an AMD CPU with SB-TSI
support, the i2c bus number would be the bus connected from the board management controller (BMC) to the CPU. The i2c address is specified in Section 6.3.1 of the spec [1]: The SB-TSI address is normally 98h for socket 0 and 90h for socket 1, but it could vary based on hardware address select pins.[1]: https://www.amd.com/system/files/TechDocs/56255_OSRR.pdf
Test status: tested reading
temp1_input, and reading/writing temp1_max/min.Signed-off-by Kun Yi <[email protected]>--- drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/sbtsi_temp.c | 265 +++++++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+) create mode 100644 drivers/hwmon/sbtsi_temp.cdrivers/hwmon/Kconfig +10
-0@@ -1522,6 +1522,16 @@ config SENSORS_SL28CPLD This driver can also be built as a module. If so, the module will be called sl28cpld-hwmon. +config SENSORS_SBTSI + tristate "Emulated SB-TSI temperature sensor" + depends on I2C + help + If you say yes here you get support for emulated temperature + sensors on AMD SoCs with SB-TSI interface connected to a BMC device. + + This driver can also be built as a module. If so, the module will + be called sbtsi_temp. + config SENSORS_SHT15 tristate "Sensiron humidity and temperature sensors. SHT15 and compat." depends on GPIOLIB || COMPILE_TESTdrivers/hwmon/Makefile +1
-0@@ -160,6 +160,7 @@ obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o +obj-$(CONFIG_SENSORS_SBTSI) += sbtsi_temp.o obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o obj-$(CONFIG_SENSORS_SCH5636) += sch5636.odrivers/hwmon/sbtsi_temp.c new file +72
-0@@ -0,0 +1,265 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* sbtsi_temp.c - hwmon driver for a SBI Temperature Sensor Interface (SB-TSI) +* compliant AMD SoC temperature device. +* +* Copyright (c) 2020, Google Inc. +* Copyright (c) 2020, Kun Yi <[email protected]> +*/ + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/of.h> + +/* +* SB-TSI registers only support SMBus byte data access. "_INT" registers are +* the integer part of a temperature value or limit, and "_DEC" registers are +* corresponding decimal parts. +*/ +#define SBTSI_REG_TEMP_INT 0x01/* RO */ +#define SBTSI_REG_STATUS 0x02/* RO */ +#define SBTSI_REG_CONFIG 0x03/* RO */ +#define SBTSI_REG_TEMP_HIGH_INT 0x07/* RW */ +#define SBTSI_REG_TEMP_LOW_INT 0x08/* RW */ +#define SBTSI_REG_TEMP_DEC 0x10/* RW */ +#define SBTSI_REG_TEMP_HIGH_DEC 0x13/* RW */ +#define SBTSI_REG_TEMP_LOW_DEC 0x14/* RW */ + +#define SBTSI_CONFIG_READ_ORDER_SHIFT 5 + +#define SBTSI_TEMP_MIN 0 +#define SBTSI_TEMP_MAX 255875 + +/* Each client has this additional data */ +struct sbtsi_data { +struct i2c_client *client; +struct mutex lock; +}; + +/* +* From SB-TSI spec: CPU temperature readings and limit registers encode the +* temperature in increments of 0.125 from 0 to 255.875. The "high byte" +* register encodes the base-2 of the integer portion, and the upper 3 bits of +* the "low byte" encode in base-2 the decimal portion. +* +* e.g. INT=0x19, DEC=0x20 represents 25.125 degrees Celsius +* +* Therefore temperature in millidegree Celsius = +* (INT + DEC / 256) * 1000 = (INT * 8 + DEC / 32) * 125 +*/ +static inline int sbtsi_reg_to_mc (s32 integer,s32 decimal) +{ +return ((integer <<3 ) + (decimal >>5 )) *125 ; +} + +/* +* Inversely, given temperature in millidegree Celsius +* INT = (TEMP / 125) / 8 +* DEC = ((TEMP / 125) % 8) * 32 +* Caller have to make sure temp doesn't exceed 255875, the max valid value. +*/ +static inline void sbtsi_mc_to_reg (s32 temp,u8 *integer,u8 *decimal) +{ + temp /=125 ; + *integer = temp >>3 ; + *decimal = (temp &0x7 ) <<5 ; +} +
drivers/hwmon/sbtsi_temp.c line 82
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+static int sbtsi_read (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long *val) +{ +struct sbtsi_data *data =dev_get_drvdata (dev); +s32 temp_int, temp_dec; +int err, reg_int, reg_dec; +bool reading_input; + +if (type != hwmon_temp) +return -EINVAL ;
Unnecessary check.
Quoted 36 lines
drivers/hwmon/sbtsi_temp.c new file +36
-0+ + reading_input = false; +switch (attr) { +case hwmon_temp_input: + reading_input = true; +/* +* ReadOrder bit specifies the reading order of integer and +* decimal part of CPU temp for atomic reads. If bit == 0, +* reading integer part triggers latching of the decimal part, +* so integer part should be read first. If bit == 1, read +* order should be reversed. +*/ + err =i2c_smbus_read_byte_data (data->client,SBTSI_REG_CONFIG ); +if (err <0 ) +return err; + +mutex_lock (&data->lock); +if (err &BIT (SBTSI_CONFIG_READ_ORDER_SHIFT )) { + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + }else { + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + } +mutex_unlock (&data->lock); +break ; +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ;
drivers/hwmon/sbtsi_temp.c line 128
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+ reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } + +if (!reading_input) { + temp_int =i2c_smbus_read_byte_data (data->client, reg_int); + temp_dec =i2c_smbus_read_byte_data (data->client, reg_dec); + }
This adds unnecessary complexity. I would suggest to move the calls to the case statements and drop reading_input as well as reg_int, reg_dec.
Also, I just realized that the mutex is needed here as well, since the values might be written while being read, which could result in inconsistent read values. Example: Limit is changed from 100 to 99.75. Since the integer value is written first, it is possible that 99.0 is returned.
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+ +if (temp_int <0 ) +return temp_int; +if (temp_dec <0 ) +return temp_dec; + + *val =sbtsi_reg_to_mc (temp_int, temp_dec); + +return 0 ; +}
drivers/hwmon/sbtsi_temp.c line 148
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+ +static int sbtsi_write (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long val) +{ +struct sbtsi_data *data =dev_get_drvdata (dev); +int reg_int, reg_dec, err; +u8 temp_int, temp_dec; + +if (type != hwmon_temp) +return -EINVAL ;
This check is really unnecessary.
Quoted 73 lines
drivers/hwmon/sbtsi_temp.c new file +73
-0+ +switch (attr) { +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ; + reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } + + val =clamp_val (val,SBTSI_TEMP_MIN ,SBTSI_TEMP_MAX ); +mutex_lock (&data->lock); +sbtsi_mc_to_reg (val, &temp_int, &temp_dec); + err =i2c_smbus_write_byte_data (data->client, reg_int, temp_int); +if (err) + goto exit; + + err =i2c_smbus_write_byte_data (data->client, reg_dec, temp_dec); +exit: +mutex_unlock (&data->lock); +return err; +} + +static umode_t sbtsi_is_visible (const void *data, +enum hwmon_sensor_types type, +u32 attr,int channel) +{ +switch (type) { +case hwmon_temp: +switch (attr) { +case hwmon_temp_input: +return 0444 ; +case hwmon_temp_min: +return 0644 ; +case hwmon_temp_max: +return 0644 ; + } +break ; +default : +break ; + } +return 0 ; +} + +static const struct hwmon_channel_info *sbtsi_info[] = { +HWMON_CHANNEL_INFO (chip, +HWMON_C_REGISTER_TZ ), +HWMON_CHANNEL_INFO (temp, +HWMON_T_INPUT |HWMON_T_MIN |HWMON_T_MAX ), +NULL +}; + +static const struct hwmon_ops sbtsi_hwmon_ops = { + .is_visible = sbtsi_is_visible, + .read = sbtsi_read, + .write = sbtsi_write, +}; + +static const struct hwmon_chip_info sbtsi_chip_info = { + .ops = &sbtsi_hwmon_ops, + .info = sbtsi_info, +}; + +static int sbtsi_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct device *dev = &client->dev; +struct device *hwmon_dev; +struct sbtsi_data *data;
drivers/hwmon/sbtsi_temp.c line 231
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+ + data =devm_kzalloc (dev,sizeof (struct sbtsi_data ),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + + data->client = client; +mutex_init (&data->lock); + + hwmon_dev = + devm_hwmon_device_register_with_info(dev, client->name, data,
Line length limit is now 100 columns, so the above line split is unnecessary.
Quoted 37 lines
drivers/hwmon/sbtsi_temp.c new file +34
-0+ &sbtsi_chip_info, NULL); + +return PTR_ERR_OR_ZERO (hwmon_dev); +} + +static const struct i2c_device_id sbtsi_id[] = { + {"sbtsi" ,0 }, + {} +}; +MODULE_DEVICE_TABLE (i2c, sbtsi_id); + +static const struct of_device_id __maybe_unused sbtsi_of_match[] = { + { + .compatible ="amd,sbtsi" , + }, + { }, +}; +MODULE_DEVICE_TABLE (of, sbtsi_of_match); + +static struct i2c_driver sbtsi_driver = { + .class =I2C_CLASS_HWMON , + .driver = { + .name ="sbtsi" , + .of_match_table =of_match_ptr (sbtsi_of_match), + }, + .probe = sbtsi_probe, + .id_table = sbtsi_id, +}; + +module_i2c_driver (sbtsi_driver); + +MODULE_AUTHOR ("Kun Yi <[email protected]>" ); +MODULE_DESCRIPTION ("Hwmon driver for AMD SB-TSI emulated sensor" ); +MODULE_LICENSE ("GPL" );Signature · 1 line
2.29.2.454.gaff20da3a2-goog
RE: [PATCH linux hwmon-next v4 1/3] hwmon: (sbtsi) Add basic support for SB-TSI sensors
[AMD Official Use Only - Approved for External Use]
Hi Kun Yi,
Quoted 2 lines
Test status: tested reading
temp1_input, and reading/writing temp1_max/min.
Tested the above at our end from an arm based BMC connected to the 2P Rome machine, it works fine.
Regards,
Naveenk
-----Original Message-----
From: Guenter Roeck <[email protected]>
Sent: Thursday, December 3, 2020 7:08 AM
To: Kun Yi <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Venkatesh, Supreeth <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH linux hwmon-next v4 1/3] hwmon: (sbtsi) Add basic support for SB-TSI sensors
[CAUTION: External Email]
Quoted 145 lines · Kun Yi
SB Temperature Sensor Interface (SB-TSI) is an SMBus compatible interface that reports AMD SoC's Ttcl (normalized temperature), and resembles a typical 8-pin remote temperature sensor's I2C interface to BMC.
This commit adds basic support using this interface to read CPU temperature, and read/write high/low CPU temp thresholds.
To instantiate this driver on an AMD CPU with SB-TSI support, the i2c bus number would be the bus connected from the board management controller (BMC) to the CPU. The i2c address is specified in Section 6.3.1 of the spec [1]: The SB-TSI address is normally 98h for socket 0 and 90h for socket 1, but it could vary based on hardware address select pins.
[1]:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
amd.com%2Fsystem%2Ffiles%2FTechDocs%2F56255_OSRR.pdf&data=04%7C01% 7CNaveenKrishna.Chatradhi%40amd.com%7C1a8cb0ebe4bb43bc759808d8972c562f %7C3dd8961fe4884e608e11a82d994e183d%7C0%7C1%7C637425563976853035%7CUnk nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw iLCJXVCI6Mn0%3D%7C1000&sdata=oH5YmE000EUVGVxruzI%2FBRnWOKptlG%2Bpf yKferIwYME%3D&reserved=0Test status: tested reading
temp1_input, and reading/writing temp1_max/min.Signed-off-by Kun Yi <[email protected]>--- drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/sbtsi_temp.c | 265 +++++++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+) create mode 100644 drivers/hwmon/sbtsi_temp.cdrivers/hwmon/Kconfig index renamed from drivers/hwmon/Kconfig +0
-0
3c059fc23cd6..3d6a809700ae100644drivers/hwmon/Kconfig +10
-0@@ -1522,6 +1522,16 @@ config SENSORS_SL28CPLD This driver can also be built as a module. If so, the module will be called sl28cpld-hwmon. +config SENSORS_SBTSI + tristate "Emulated SB-TSI temperature sensor" + depends on I2C + help + If you say yes here you get support for emulated temperature + sensors on AMD SoCs with SB-TSI interface connected to a BMC device. + + This driver can also be built as a module. If so, the module will + be called sbtsi_temp. + config SENSORS_SHT15 tristate "Sensiron humidity and temperature sensors. SHT15 and compat." depends on GPIOLIB || COMPILE_TEST diff --gita/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index
01ca5d3fbad4..ee8c037919da100644drivers/hwmon/Makefile +1
-0@@ -160,6 +160,7 @@ obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o +obj-$(CONFIG_SENSORS_SBTSI) += sbtsi_temp.o obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o obj-$(CONFIG_SENSORS_SCH5636) += sch5636.odrivers/hwmon/sbtsi_temp.c new file +71
-0@@ -0,0 +1,265 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* sbtsi_temp.c - hwmon driver for a SBI Temperature Sensor Interface (SB-TSI) +* compliant AMD SoC temperature device. +* +* Copyright (c) 2020, Google Inc. +* Copyright (c) 2020, Kun Yi <[email protected]> */ + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/of.h> + +/* +* SB-TSI registers only support SMBus byte data access. "_INT" +registers are +* the integer part of a temperature value or limit, and "_DEC" +registers are +* corresponding decimal parts. +*/ +#define SBTSI_REG_TEMP_INT 0x01/* RO */ +#define SBTSI_REG_STATUS 0x02/* RO */ +#define SBTSI_REG_CONFIG 0x03/* RO */ +#define SBTSI_REG_TEMP_HIGH_INT 0x07/* RW */ +#define SBTSI_REG_TEMP_LOW_INT 0x08/* RW */ +#define SBTSI_REG_TEMP_DEC 0x10/* RW */ +#define SBTSI_REG_TEMP_HIGH_DEC 0x13/* RW */ +#define SBTSI_REG_TEMP_LOW_DEC 0x14/* RW */ + +#define SBTSI_CONFIG_READ_ORDER_SHIFT 5 + +#define SBTSI_TEMP_MIN 0 +#define SBTSI_TEMP_MAX 255875 + +/* Each client has this additional data */ struct sbtsi_data { +struct i2c_client *client; +struct mutex lock; +}; + +/* +* From SB-TSI spec: CPU temperature readings and limit registers +encode the +* temperature in increments of 0.125 from 0 to 255.875. The "high byte" +* register encodes the base-2 of the integer portion, and the upper +3 bits of +* the "low byte" encode in base-2 the decimal portion. +* +* e.g. INT=0x19, DEC=0x20 represents 25.125 degrees Celsius +* +* Therefore temperature in millidegree Celsius = +* (INT + DEC / 256) * 1000 = (INT * 8 + DEC / 32) * 125 +*/ +static inline int sbtsi_reg_to_mc (s32 integer,s32 decimal) { +return ((integer <<3 ) + (decimal >>5 )) *125 ; } + +/* +* Inversely, given temperature in millidegree Celsius +* INT = (TEMP / 125) / 8 +* DEC = ((TEMP / 125) % 8) * 32 +* Caller have to make sure temp doesn't exceed 255875, the max valid value. +*/ +static inline void sbtsi_mc_to_reg (s32 temp,u8 *integer,u8 +*decimal) { + temp /=125 ; + *integer = temp >>3 ; + *decimal = (temp &0x7 ) <<5 ; +}
drivers/hwmon/sbtsi_temp.c line 81
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+ +static int sbtsi_read (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long *val) { +struct sbtsi_data *data =dev_get_drvdata (dev); +s32 temp_int, temp_dec; +int err, reg_int, reg_dec; +bool reading_input; + +if (type != hwmon_temp) +return -EINVAL ;
Unnecessary check.
Quoted 36 lines
drivers/hwmon/sbtsi_temp.c new file +36
-0+ + reading_input = false; +switch (attr) { +case hwmon_temp_input: + reading_input = true; +/* +* ReadOrder bit specifies the reading order of integer and +* decimal part of CPU temp for atomic reads. If bit == 0, +* reading integer part triggers latching of the decimal part, +* so integer part should be read first. If bit == 1, read +* order should be reversed. +*/ + err =i2c_smbus_read_byte_data (data->client,SBTSI_REG_CONFIG ); +if (err <0 ) +return err; + +mutex_lock (&data->lock); +if (err &BIT (SBTSI_CONFIG_READ_ORDER_SHIFT )) { + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + }else { + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + } +mutex_unlock (&data->lock); +break ; +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ;
drivers/hwmon/sbtsi_temp.c line 128
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+ reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } + +if (!reading_input) { + temp_int =i2c_smbus_read_byte_data (data->client, reg_int); + temp_dec =i2c_smbus_read_byte_data (data->client, reg_dec); + }
This adds unnecessary complexity. I would suggest to move the calls to the case statements and drop reading_input as well as reg_int, reg_dec.
Also, I just realized that the mutex is needed here as well, since the values might be written while being read, which could result in inconsistent read values. Example: Limit is changed from 100 to 99.75. Since the integer value is written first, it is possible that 99.0 is returned.
Quoted 9 lines
drivers/hwmon/sbtsi_temp.c new file +9
-0+ +if (temp_int <0 ) +return temp_int; +if (temp_dec <0 ) +return temp_dec; + + *val =sbtsi_reg_to_mc (temp_int, temp_dec); + +return 0 ;
drivers/hwmon/sbtsi_temp.c line 148
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+} + +static int sbtsi_write (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long val) { +struct sbtsi_data *data =dev_get_drvdata (dev); +int reg_int, reg_dec, err; +u8 temp_int, temp_dec; + +if (type != hwmon_temp) +return -EINVAL ;
This check is really unnecessary.
Quoted 73 lines
drivers/hwmon/sbtsi_temp.c new file +73
-0+ +switch (attr) { +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ; + reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } + + val =clamp_val (val,SBTSI_TEMP_MIN ,SBTSI_TEMP_MAX ); +mutex_lock (&data->lock); +sbtsi_mc_to_reg (val, &temp_int, &temp_dec); + err =i2c_smbus_write_byte_data (data->client, reg_int, temp_int); +if (err) + goto exit; + + err =i2c_smbus_write_byte_data (data->client, reg_dec, +temp_dec); +exit: +mutex_unlock (&data->lock); +return err; +} + +static umode_t sbtsi_is_visible (const void *data, +enum hwmon_sensor_types type, +u32 attr,int channel) { +switch (type) { +case hwmon_temp: +switch (attr) { +case hwmon_temp_input: +return 0444 ; +case hwmon_temp_min: +return 0644 ; +case hwmon_temp_max: +return 0644 ; + } +break ; +default : +break ; + } +return 0 ; +} + +static const struct hwmon_channel_info *sbtsi_info[] = { +HWMON_CHANNEL_INFO (chip, +HWMON_C_REGISTER_TZ ), +HWMON_CHANNEL_INFO (temp, +HWMON_T_INPUT |HWMON_T_MIN |HWMON_T_MAX ), +NULL +}; + +static const struct hwmon_ops sbtsi_hwmon_ops = { + .is_visible = sbtsi_is_visible, + .read = sbtsi_read, + .write = sbtsi_write, +}; + +static const struct hwmon_chip_info sbtsi_chip_info = { + .ops = &sbtsi_hwmon_ops, + .info = sbtsi_info, +}; + +static int sbtsi_probe (struct i2c_client *client, +const struct i2c_device_id *id) { +struct device *dev = &client->dev; +struct device *hwmon_dev; +struct sbtsi_data *data; +
drivers/hwmon/sbtsi_temp.c
Quoted 10 lines
drivers/hwmon/sbtsi_temp.c new file +10
-0+ data =devm_kzalloc (dev,sizeof (struct sbtsi_data ),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + + data->client = client; +mutex_init (&data->lock); + + hwmon_dev = + devm_hwmon_device_register_with_info(dev, client->name, + data,
Line length limit is now 100 columns, so the above line split is unnecessary.
Quoted 36 lines
drivers/hwmon/sbtsi_temp.c new file +33
-1+ &sbtsi_chip_info, + NULL); + +return PTR_ERR_OR_ZERO (hwmon_dev); } + +static const struct i2c_device_id sbtsi_id[] = { + {"sbtsi" ,0 }, + {} +}; +MODULE_DEVICE_TABLE (i2c, sbtsi_id); + +static const struct of_device_id __maybe_unused sbtsi_of_match[] = { + { + .compatible ="amd,sbtsi" , + }, + { }, +}; +MODULE_DEVICE_TABLE (of, sbtsi_of_match); + +static struct i2c_driver sbtsi_driver = { + .class =I2C_CLASS_HWMON , + .driver = { + .name ="sbtsi" , + .of_match_table =of_match_ptr (sbtsi_of_match), + }, + .probe = sbtsi_probe, + .id_table = sbtsi_id, +}; + +module_i2c_driver (sbtsi_driver); + +MODULE_AUTHOR("Kun Yi <[email protected]>"); MODULE_DESCRIPTION("Hwmon +driver for AMD SB-TSI emulated sensor"); MODULE_LICENSE("GPL"); --2.29.2.454.gaff20da3a2-goog
Re: [PATCH linux hwmon-next v4 1/3] hwmon: (sbtsi) Add basic support for SB-TSI sensors
In reply to Guenter RoeckThanks Guenter for reviewing!
Quoted 145 lines · Guenter Roeck
Quoted 143 lines · Kun Yi
SB Temperature Sensor Interface (SB-TSI) is an SMBus compatible interface that reports AMD SoC's Ttcl (normalized temperature), and resembles a typical 8-pin remote temperature sensor's I2C interface to BMC.
This commit adds basic support using this interface to read CPU temperature, and read/write high/low CPU temp thresholds.
To instantiate this driver on an AMD CPU with SB-TSI
support, the i2c bus number would be the bus connected from the board management controller (BMC) to the CPU. The i2c address is specified in Section 6.3.1 of the spec [1]: The SB-TSI address is normally 98h for socket 0 and 90h for socket 1, but it could vary based on hardware address select pins.[1]: https://www.amd.com/system/files/TechDocs/56255_OSRR.pdf
Test status: tested reading
temp1_input, and reading/writing temp1_max/min.Signed-off-by Kun Yi <[email protected]>--- drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/sbtsi_temp.c | 265 +++++++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+) create mode 100644 drivers/hwmon/sbtsi_temp.cdrivers/hwmon/Kconfig +10
-0@@ -1522,6 +1522,16 @@ config SENSORS_SL28CPLD This driver can also be built as a module. If so, the module will be called sl28cpld-hwmon. +config SENSORS_SBTSI + tristate "Emulated SB-TSI temperature sensor" + depends on I2C + help + If you say yes here you get support for emulated temperature + sensors on AMD SoCs with SB-TSI interface connected to a BMC device. + + This driver can also be built as a module. If so, the module will + be called sbtsi_temp. + config SENSORS_SHT15 tristate "Sensiron humidity and temperature sensors. SHT15 and compat." depends on GPIOLIB || COMPILE_TESTdrivers/hwmon/Makefile +1
-0@@ -160,6 +160,7 @@ obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o +obj-$(CONFIG_SENSORS_SBTSI) += sbtsi_temp.o obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o obj-$(CONFIG_SENSORS_SCH5636) += sch5636.odrivers/hwmon/sbtsi_temp.c new file +76
-0@@ -0,0 +1,265 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* sbtsi_temp.c - hwmon driver for a SBI Temperature Sensor Interface (SB-TSI) +* compliant AMD SoC temperature device. +* +* Copyright (c) 2020, Google Inc. +* Copyright (c) 2020, Kun Yi <[email protected]> +*/ + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/of.h> + +/* +* SB-TSI registers only support SMBus byte data access. "_INT" registers are +* the integer part of a temperature value or limit, and "_DEC" registers are +* corresponding decimal parts. +*/ +#define SBTSI_REG_TEMP_INT 0x01/* RO */ +#define SBTSI_REG_STATUS 0x02/* RO */ +#define SBTSI_REG_CONFIG 0x03/* RO */ +#define SBTSI_REG_TEMP_HIGH_INT 0x07/* RW */ +#define SBTSI_REG_TEMP_LOW_INT 0x08/* RW */ +#define SBTSI_REG_TEMP_DEC 0x10/* RW */ +#define SBTSI_REG_TEMP_HIGH_DEC 0x13/* RW */ +#define SBTSI_REG_TEMP_LOW_DEC 0x14/* RW */ + +#define SBTSI_CONFIG_READ_ORDER_SHIFT 5 + +#define SBTSI_TEMP_MIN 0 +#define SBTSI_TEMP_MAX 255875 + +/* Each client has this additional data */ +struct sbtsi_data { +struct i2c_client *client; +struct mutex lock; +}; + +/* +* From SB-TSI spec: CPU temperature readings and limit registers encode the +* temperature in increments of 0.125 from 0 to 255.875. The "high byte" +* register encodes the base-2 of the integer portion, and the upper 3 bits of +* the "low byte" encode in base-2 the decimal portion. +* +* e.g. INT=0x19, DEC=0x20 represents 25.125 degrees Celsius +* +* Therefore temperature in millidegree Celsius = +* (INT + DEC / 256) * 1000 = (INT * 8 + DEC / 32) * 125 +*/ +static inline int sbtsi_reg_to_mc (s32 integer,s32 decimal) +{ +return ((integer <<3 ) + (decimal >>5 )) *125 ; +} + +/* +* Inversely, given temperature in millidegree Celsius +* INT = (TEMP / 125) / 8 +* DEC = ((TEMP / 125) % 8) * 32 +* Caller have to make sure temp doesn't exceed 255875, the max valid value. +*/ +static inline void sbtsi_mc_to_reg (s32 temp,u8 *integer,u8 *decimal) +{ + temp /=125 ; + *integer = temp >>3 ; + *decimal = (temp &0x7 ) <<5 ; +} + +static int sbtsi_read (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long *val) +{ +struct sbtsi_data *data =dev_get_drvdata (dev);
Quoted 8 lines
drivers/hwmon/sbtsi_temp.c line 82
Quoted 6 lines
drivers/hwmon/sbtsi_temp.c new file +6
-0+s32 temp_int, temp_dec; +int err, reg_int, reg_dec; +bool reading_input; + +if (type != hwmon_temp) +return -EINVAL ;Unnecessary check.
Removed.
Quoted 43 lines
Quoted 43 lines
drivers/hwmon/sbtsi_temp.c new file +42
-0+ + reading_input = false; +switch (attr) { +case hwmon_temp_input: + reading_input = true; +/* +* ReadOrder bit specifies the reading order of integer and +* decimal part of CPU temp for atomic reads. If bit == 0, +* reading integer part triggers latching of the decimal part, +* so integer part should be read first. If bit == 1, read +* order should be reversed. +*/ + err =i2c_smbus_read_byte_data (data->client,SBTSI_REG_CONFIG ); +if (err <0 ) +return err; + +mutex_lock (&data->lock); +if (err &BIT (SBTSI_CONFIG_READ_ORDER_SHIFT )) { + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + }else { + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + } +mutex_unlock (&data->lock); +break ; +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ; + reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } +
Quoted 8 lines
drivers/hwmon/sbtsi_temp.c line 128
Quoted 4 lines
drivers/hwmon/sbtsi_temp.c new file +4
-0+if (!reading_input) { + temp_int =i2c_smbus_read_byte_data (data->client, reg_int); + temp_dec =i2c_smbus_read_byte_data (data->client, reg_dec); + }This adds unnecessary complexity. I would suggest to move the calls to the case statements and drop
reading_inputas well asreg_int,reg_dec.
Done.
Quoted 6 lines
Also, I just realized that the mutex is needed here as well, since the values might be written while being read, which could result in inconsistent read values. Example: Limit is changed from 100 to 99.75. Since the integer value is written first, it is possible that 99.0 is returned.
That's a good catch. Modified.
Quoted 15 lines
Quoted 15 lines
drivers/hwmon/sbtsi_temp.c new file +14
-0+ +if (temp_int <0 ) +return temp_int; +if (temp_dec <0 ) +return temp_dec; + + *val =sbtsi_reg_to_mc (temp_int, temp_dec); + +return 0 ; +} + +static int sbtsi_write (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long val) +{
Quoted 8 lines
drivers/hwmon/sbtsi_temp.c line 148
Quoted 6 lines
drivers/hwmon/sbtsi_temp.c new file +6
-0+struct sbtsi_data *data =dev_get_drvdata (dev); +int reg_int, reg_dec, err; +u8 temp_int, temp_dec; + +if (type != hwmon_temp) +return -EINVAL ;This check is really unnecessary.
Removed.
Quoted 78 lines
Quoted 78 lines
drivers/hwmon/sbtsi_temp.c new file +77
-0+ +switch (attr) { +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ; + reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } + + val =clamp_val (val,SBTSI_TEMP_MIN ,SBTSI_TEMP_MAX ); +mutex_lock (&data->lock); +sbtsi_mc_to_reg (val, &temp_int, &temp_dec); + err =i2c_smbus_write_byte_data (data->client, reg_int, temp_int); +if (err) + goto exit; + + err =i2c_smbus_write_byte_data (data->client, reg_dec, temp_dec); +exit: +mutex_unlock (&data->lock); +return err; +} + +static umode_t sbtsi_is_visible (const void *data, +enum hwmon_sensor_types type, +u32 attr,int channel) +{ +switch (type) { +case hwmon_temp: +switch (attr) { +case hwmon_temp_input: +return 0444 ; +case hwmon_temp_min: +return 0644 ; +case hwmon_temp_max: +return 0644 ; + } +break ; +default : +break ; + } +return 0 ; +} + +static const struct hwmon_channel_info *sbtsi_info[] = { +HWMON_CHANNEL_INFO (chip, +HWMON_C_REGISTER_TZ ), +HWMON_CHANNEL_INFO (temp, +HWMON_T_INPUT |HWMON_T_MIN |HWMON_T_MAX ), +NULL +}; + +static const struct hwmon_ops sbtsi_hwmon_ops = { + .is_visible = sbtsi_is_visible, + .read = sbtsi_read, + .write = sbtsi_write, +}; + +static const struct hwmon_chip_info sbtsi_chip_info = { + .ops = &sbtsi_hwmon_ops, + .info = sbtsi_info, +}; + +static int sbtsi_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct device *dev = &client->dev; +struct device *hwmon_dev; +struct sbtsi_data *data; + + data =devm_kzalloc (dev,sizeof (struct sbtsi_data ),GFP_KERNEL ); +if (!data) +return -ENOMEM ;
Quoted 8 lines
drivers/hwmon/sbtsi_temp.c line 231
Quoted 6 lines
drivers/hwmon/sbtsi_temp.c new file +6
-0+ + data->client = client; +mutex_init (&data->lock); + + hwmon_dev = + devm_hwmon_device_register_with_info(dev, client->name, data,Line length limit is now 100 columns, so the above line split is unnecessary.
Wow I didn't know.. Thanks! Adjusted a couple other lines based on the 100 cols.
Quoted 38 lines
Quoted 38 lines
drivers/hwmon/sbtsi_temp.c new file +34
-1+ &sbtsi_chip_info, NULL); + +return PTR_ERR_OR_ZERO (hwmon_dev); +} + +static const struct i2c_device_id sbtsi_id[] = { + {"sbtsi" ,0 }, + {} +}; +MODULE_DEVICE_TABLE (i2c, sbtsi_id); + +static const struct of_device_id __maybe_unused sbtsi_of_match[] = { + { + .compatible ="amd,sbtsi" , + }, + { }, +}; +MODULE_DEVICE_TABLE (of, sbtsi_of_match); + +static struct i2c_driver sbtsi_driver = { + .class =I2C_CLASS_HWMON , + .driver = { + .name ="sbtsi" , + .of_match_table =of_match_ptr (sbtsi_of_match), + }, + .probe = sbtsi_probe, + .id_table = sbtsi_id, +}; + +module_i2c_driver (sbtsi_driver); + +MODULE_AUTHOR ("Kun Yi <[email protected]>" ); +MODULE_DESCRIPTION ("Hwmon driver for AMD SB-TSI emulated sensor" ); +MODULE_LICENSE ("GPL" ); --2.29.2.454.gaff20da3a2-goog
Signature · 2 lines
Regards, Kun
Re: [PATCH linux hwmon-next v4 1/3] hwmon: (sbtsi) Add basic support for SB-TSI sensors
In reply to Kun YiQuoted 17 lines · Kun Yi
[CAUTION: External Email]
SB Temperature Sensor Interface (SB-TSI) is an SMBus compatible interface that reports AMD SoC's Ttcl (normalized temperature), and resembles a typical 8-pin remote temperature sensor's I2C interface to BMC.
This commit adds basic support using this interface to read CPU temperature, and read/write high/low CPU temp thresholds.
To instantiate this driver on an AMD CPU with SB-TSI
support, the i2c bus number would be the bus connected from the board management controller (BMC) to the CPU. The i2c address is specified in Section 6.3.1 of the spec [1]: The SB-TSI address is normally 98h for socket 0 and 90h for socket 1, but it could vary based on hardware address select pins.
Quoted 6 lines
Test status: tested reading
temp1_input, and reading/writing temp1_max/min.Signed-off-by Kun Yi <[email protected]>
Series
Quoted 314 lines
--- drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/sbtsi_temp.c | 265 +++++++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+) create mode 100644 drivers/hwmon/sbtsi_temp.cdrivers/hwmon/Kconfig +10
-0@@ -1522,6 +1522,16 @@ config SENSORS_SL28CPLD This driver can also be built as a module. If so, the module will be called sl28cpld-hwmon. +config SENSORS_SBTSI + tristate "Emulated SB-TSI temperature sensor" + depends on I2C + help + If you say yes here you get support for emulated temperature + sensors on AMD SoCs with SB-TSI interface connected to a BMC device. + + This driver can also be built as a module. If so, the module will + be called sbtsi_temp. + config SENSORS_SHT15 tristate "Sensiron humidity and temperature sensors. SHT15 and compat." depends on GPIOLIB || COMPILE_TESTdrivers/hwmon/Makefile +1
-0@@ -160,6 +160,7 @@ obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o +obj-$(CONFIG_SENSORS_SBTSI) += sbtsi_temp.o obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o obj-$(CONFIG_SENSORS_SCH5636) += sch5636.odrivers/hwmon/sbtsi_temp.c new file +265
-1@@ -0,0 +1,265 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* sbtsi_temp.c - hwmon driver for a SBI Temperature Sensor Interface (SB-TSI) +* compliant AMD SoC temperature device. +* +* Copyright (c) 2020, Google Inc. +* Copyright (c) 2020, Kun Yi <[email protected]> +*/ + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/of.h> + +/* +* SB-TSI registers only support SMBus byte data access. "_INT" registers are +* the integer part of a temperature value or limit, and "_DEC" registers are +* corresponding decimal parts. +*/ +#define SBTSI_REG_TEMP_INT 0x01/* RO */ +#define SBTSI_REG_STATUS 0x02/* RO */ +#define SBTSI_REG_CONFIG 0x03/* RO */ +#define SBTSI_REG_TEMP_HIGH_INT 0x07/* RW */ +#define SBTSI_REG_TEMP_LOW_INT 0x08/* RW */ +#define SBTSI_REG_TEMP_DEC 0x10/* RW */ +#define SBTSI_REG_TEMP_HIGH_DEC 0x13/* RW */ +#define SBTSI_REG_TEMP_LOW_DEC 0x14/* RW */ + +#define SBTSI_CONFIG_READ_ORDER_SHIFT 5 + +#define SBTSI_TEMP_MIN 0 +#define SBTSI_TEMP_MAX 255875 + +/* Each client has this additional data */ +struct sbtsi_data { +struct i2c_client *client; +struct mutex lock; +}; + +/* +* From SB-TSI spec: CPU temperature readings and limit registers encode the +* temperature in increments of 0.125 from 0 to 255.875. The "high byte" +* register encodes the base-2 of the integer portion, and the upper 3 bits of +* the "low byte" encode in base-2 the decimal portion. +* +* e.g. INT=0x19, DEC=0x20 represents 25.125 degrees Celsius +* +* Therefore temperature in millidegree Celsius = +* (INT + DEC / 256) * 1000 = (INT * 8 + DEC / 32) * 125 +*/ +static inline int sbtsi_reg_to_mc (s32 integer,s32 decimal) +{ +return ((integer <<3 ) + (decimal >>5 )) *125 ; +} + +/* +* Inversely, given temperature in millidegree Celsius +* INT = (TEMP / 125) / 8 +* DEC = ((TEMP / 125) % 8) * 32 +* Caller have to make sure temp doesn't exceed 255875, the max valid value. +*/ +static inline void sbtsi_mc_to_reg (s32 temp,u8 *integer,u8 *decimal) +{ + temp /=125 ; + *integer = temp >>3 ; + *decimal = (temp &0x7 ) <<5 ; +} + +static int sbtsi_read (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long *val) +{ +struct sbtsi_data *data =dev_get_drvdata (dev); +s32 temp_int, temp_dec; +int err, reg_int, reg_dec; +bool reading_input; + +if (type != hwmon_temp) +return -EINVAL ; + + reading_input = false; +switch (attr) { +case hwmon_temp_input: + reading_input = true; +/* +* ReadOrder bit specifies the reading order of integer and +* decimal part of CPU temp for atomic reads. If bit == 0, +* reading integer part triggers latching of the decimal part, +* so integer part should be read first. If bit == 1, read +* order should be reversed. +*/ + err =i2c_smbus_read_byte_data (data->client,SBTSI_REG_CONFIG ); +if (err <0 ) +return err; + +mutex_lock (&data->lock); +if (err &BIT (SBTSI_CONFIG_READ_ORDER_SHIFT )) { + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + }else { + temp_int =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_INT ); + temp_dec =i2c_smbus_read_byte_data (data->client, +SBTSI_REG_TEMP_DEC ); + } +mutex_unlock (&data->lock); +break ; +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ; + reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } + +if (!reading_input) { + temp_int =i2c_smbus_read_byte_data (data->client, reg_int); + temp_dec =i2c_smbus_read_byte_data (data->client, reg_dec); + } + +if (temp_int <0 ) +return temp_int; +if (temp_dec <0 ) +return temp_dec; + + *val =sbtsi_reg_to_mc (temp_int, temp_dec); + +return 0 ; +} + +static int sbtsi_write (struct device *dev,enum hwmon_sensor_types type, +u32 attr,int channel,long val) +{ +struct sbtsi_data *data =dev_get_drvdata (dev); +int reg_int, reg_dec, err; +u8 temp_int, temp_dec; + +if (type != hwmon_temp) +return -EINVAL ; + +switch (attr) { +case hwmon_temp_max: + reg_int =SBTSI_REG_TEMP_HIGH_INT ; + reg_dec =SBTSI_REG_TEMP_HIGH_DEC ; +break ; +case hwmon_temp_min: + reg_int =SBTSI_REG_TEMP_LOW_INT ; + reg_dec =SBTSI_REG_TEMP_LOW_DEC ; +break ; +default : +return -EINVAL ; + } + + val =clamp_val (val,SBTSI_TEMP_MIN ,SBTSI_TEMP_MAX ); +mutex_lock (&data->lock); +sbtsi_mc_to_reg (val, &temp_int, &temp_dec); + err =i2c_smbus_write_byte_data (data->client, reg_int, temp_int); +if (err) + goto exit; + + err =i2c_smbus_write_byte_data (data->client, reg_dec, temp_dec); +exit: +mutex_unlock (&data->lock); +return err; +} + +static umode_t sbtsi_is_visible (const void *data, +enum hwmon_sensor_types type, +u32 attr,int channel) +{ +switch (type) { +case hwmon_temp: +switch (attr) { +case hwmon_temp_input: +return 0444 ; +case hwmon_temp_min: +return 0644 ; +case hwmon_temp_max: +return 0644 ; + } +break ; +default : +break ; + } +return 0 ; +} + +static const struct hwmon_channel_info *sbtsi_info[] = { +HWMON_CHANNEL_INFO (chip, +HWMON_C_REGISTER_TZ ), +HWMON_CHANNEL_INFO (temp, +HWMON_T_INPUT |HWMON_T_MIN |HWMON_T_MAX ), +NULL +}; + +static const struct hwmon_ops sbtsi_hwmon_ops = { + .is_visible = sbtsi_is_visible, + .read = sbtsi_read, + .write = sbtsi_write, +}; + +static const struct hwmon_chip_info sbtsi_chip_info = { + .ops = &sbtsi_hwmon_ops, + .info = sbtsi_info, +}; + +static int sbtsi_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct device *dev = &client->dev; +struct device *hwmon_dev; +struct sbtsi_data *data; + + data =devm_kzalloc (dev,sizeof (struct sbtsi_data ),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + + data->client = client; +mutex_init (&data->lock); + + hwmon_dev = +devm_hwmon_device_register_with_info (dev, client->name, data, + &sbtsi_chip_info,NULL ); + +return PTR_ERR_OR_ZERO (hwmon_dev); +} + +static const struct i2c_device_id sbtsi_id[] = { + {"sbtsi" ,0 }, + {} +}; +MODULE_DEVICE_TABLE (i2c, sbtsi_id); + +static const struct of_device_id __maybe_unused sbtsi_of_match[] = { + { + .compatible ="amd,sbtsi" , + }, + { }, +}; +MODULE_DEVICE_TABLE (of, sbtsi_of_match); + +static struct i2c_driver sbtsi_driver = { + .class =I2C_CLASS_HWMON , + .driver = { + .name ="sbtsi" , + .of_match_table =of_match_ptr (sbtsi_of_match), + }, + .probe = sbtsi_probe, + .id_table = sbtsi_id, +}; + +module_i2c_driver (sbtsi_driver); + +MODULE_AUTHOR ("Kun Yi <[email protected]>" ); +MODULE_DESCRIPTION ("Hwmon driver for AMD SB-TSI emulated sensor" ); +MODULE_LICENSE ("GPL" ); --2.29.2.454.gaff20da3a2-goog
PATCH v4 2/3 hwmon: (sbtsi) Add documentation
In reply to Kun YiDocument the SB-TSI sensor interface driver.
--- Documentation/hwmon/index.rst | 1 + Documentation/hwmon/sbtsi_temp.rst | 40 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 Documentation/hwmon/sbtsi_temp.rst
Documentation/hwmon/index.rst +1 -0
@@ -151,6 +151,7 @@ Hardware Monitoring Kernel Drivers pxe1610 pwm-fan raspberrypi-hwmon + sbtsi_temp sch5627 sch5636 scpi-hwmon
Documentation/hwmon/sbtsi_temp.rst new file +40 -0
@@ -0,0 +1,40 @@ +Kernel driver sbtsi_temp +================== + +Supported hardware: + + * Sideband interface (SBI) Temperature Sensor Interface (SB-TSI) + compliant AMD SoC temperature device. + + Prefix: 'sbtsi_temp' + + Addresses scanned: This driver doesn't support address scanning. + + To instantiate this driver on an AMD CPU with SB-TSI + support, the i2c bus number would be the bus connected from the board + management controller (BMC) to the CPU. The i2c address is specified in + Section 6.3.1 of the SoC register reference: The SB-TSI address is normally + 98h for socket 0 and 90h for socket 1, but it could vary based on hardware + address select pins. + + Datasheet: The SB-TSI interface and protocol is available as part of + the open source SoC register reference at: + + https://www.amd.com/system/files/TechDocs/56255_OSRR.pdf + + The Advanced Platform Management Link (APML) Specification is + available at: + + http://developer.amd.com/wordpress/media/2012/10/41918.pdf + +Author: Kun Yi <[email protected]> + +Description +----------- + +The SBI temperature sensor interface (SB-TSI) is an emulation of the software +and physical interface of a typical 8-pin remote temperature sensor (RTS) on +AMD SoCs. It implements one temperature sensor with readings and limit +registers encode the temperature in increments of 0.125 from 0 to 255.875. +Limits can be set through the writable thresholds, and if reached will trigger +corresponding alert signals.
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
PATCH v4 3/3 dt-bindings: (hwmon/sbtsi_tmep) Add SB-TSI hwmon driver bindings
In reply to Kun YiDocument device tree bindings for AMD SB-TSI emulated temperature sensor.
--- .../devicetree/bindings/hwmon/amd,sbtsi.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/amd,sbtsi.yaml
Documentation/devicetree/bindings/hwmon/amd,sbtsi.yaml new file +54 -0
@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/amd,sbtsi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: > + Sideband interface Temperature Sensor Interface (SB-TSI) compliant + AMD SoC temperature device + +maintainers: + - Kun Yi <[email protected]> + - Supreeth Venkatesh <[email protected]> + +description: | + SB Temperature Sensor Interface (SB-TSI) is an SMBus compatible + interface that reports AMD SoC's Ttcl (normalized temperature), + and resembles a typical 8-pin remote temperature sensor's I2C interface + to BMC. The emulated thermal sensor can report temperatures in increments + of 0.125 degrees, ranging from 0 to 255.875. + +properties: + compatible: + enum: + - amd,sbtsi + + reg: + maxItems: 1 + description: | + I2C bus address of the device as specified in Section 6.3.1 of the + SoC register reference. The SB-TSI address is normally 98h for socket + 0 and 90h for socket 1, but it could vary based on hardware address + select pins. + \[open source SoC register reference\] + https://www.amd.com/system/files/TechDocs/56255_OSRR.pdf + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + sbtsi@4c { + compatible = "amd,sbtsi"; + reg = <0x4c>; + }; + }; +...
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
Re: [PATCH linux hwmon-next v4 3/3] dt-bindings: (hwmon/sbtsi_tmep) Add SB-TSI hwmon driver bindings
Quoted 9 lines · Kun Yi
Document device tree bindings for AMD SB-TSI emulated temperature sensor.
Signed-off-by Kun Yi <[email protected]>--- .../devicetree/bindings/hwmon/amd,sbtsi.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/amd,sbtsi.yaml