| Message ID | 1573746001-20979-2-git-send-email-akinobu.mita@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | nvme: hwmon: provide temperature threshold features | expand |
On Fri, Nov 15, 2019 at 12:40:00AM +0900, Akinobu Mita wrote: > According to the NVMe specification, the over temperature threshold and > under temperature threshold features shall be implemented for Composite > Temperature if a non-zero WCTEMP field value is reported in the Identify > Controller data structure. The features are also implemented for all > implemented temperature sensors (i.e., all Temperature Sensor fields that > report a non-zero value). > > This provides the over temperature threshold and under temperature > threshold for each sensor as temperature min and max values of hwmon > sysfs attributes. > > The WCTEMP is already provided as a temperature max value for Composite > Temperature, but this change isn't incompatible. Because the default > value of the over temperature threshold for Composite Temperature is > the WCTEMP. > > Now the alarm attribute for Composite Temperature indicates one of the > temperature is outside of a temperature threshold. Because there is only > a single bit in Critical Warning field that indicates a temperature is > outside of a threshold. > > Example output from the "sensors" command: > > nvme-pci-0100 > Adapter: PCI adapter > Composite: +33.9°C (low = -273.1°C, high = +69.8°C) > (crit = +79.8°C) > Sensor 1: +34.9°C (low = -273.1°C, high = +65261.8°C) > Sensor 2: +31.9°C (low = -273.1°C, high = +65261.8°C) > Sensor 5: +47.9°C (low = -273.1°C, high = +65261.8°C) > > This also adds helper macros for kelvin from/to milli Celsius conversion, > and replaces the repeated code in nvme-hwmon.c. > > Cc: Keith Busch <kbusch@kernel.org> > Cc: Jens Axboe <axboe@fb.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Sagi Grimberg <sagi@grimberg.me> > Cc: Jean Delvare <jdelvare@suse.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Tested-by: Guenter Roeck <linux@roeck-us.net> Tested with: INTEL SSDPEKKW512G7 Samsung SSD 970 EVO 500GB THNSN5256GPU7 NVMe TOSHIBA 256GB > --- > drivers/nvme/host/nvme-hwmon.c | 106 ++++++++++++++++++++++++++++++++++------- > include/linux/nvme.h | 6 +++ > 2 files changed, 96 insertions(+), 16 deletions(-) > > diff --git a/drivers/nvme/host/nvme-hwmon.c b/drivers/nvme/host/nvme-hwmon.c > index 5480cbb..97a84b4 100644 > --- a/drivers/nvme/host/nvme-hwmon.c > +++ b/drivers/nvme/host/nvme-hwmon.c > @@ -9,12 +9,57 @@ > > #include "nvme.h" > > +/* These macros should be moved to linux/temperature.h */ > +#define MILLICELSIUS_TO_KELVIN(t) DIV_ROUND_CLOSEST((t) + 273150, 1000) > +#define KELVIN_TO_MILLICELSIUS(t) ((t) * 1000L - 273150) > + > struct nvme_hwmon_data { > struct nvme_ctrl *ctrl; > struct nvme_smart_log log; > struct mutex read_lock; > }; > > +static int nvme_get_temp_thresh(struct nvme_ctrl *ctrl, int sensor, bool under, > + long *temp) > +{ > + unsigned int threshold = sensor << NVME_TEMP_THRESH_SELECT_SHIFT; > + u32 status; > + int ret; > + > + if (under) > + threshold |= NVME_TEMP_THRESH_TYPE_UNDER; > + > + ret = nvme_get_features(ctrl, NVME_FEAT_TEMP_THRESH, threshold, NULL, 0, > + &status); > + if (ret > 0) > + return -EIO; > + if (ret < 0) > + return ret; > + *temp = KELVIN_TO_MILLICELSIUS(status & NVME_TEMP_THRESH_MASK); > + > + return 0; > +} > + > +static int nvme_set_temp_thresh(struct nvme_ctrl *ctrl, int sensor, bool under, > + long temp) > +{ > + unsigned int threshold = sensor << NVME_TEMP_THRESH_SELECT_SHIFT; > + int ret; > + > + temp = MILLICELSIUS_TO_KELVIN(temp); > + threshold |= clamp_val(temp, 0, NVME_TEMP_THRESH_MASK); > + > + if (under) > + threshold |= NVME_TEMP_THRESH_TYPE_UNDER; > + > + ret = nvme_set_features(ctrl, NVME_FEAT_TEMP_THRESH, threshold, NULL, 0, > + NULL); > + if (ret > 0) > + return -EIO; > + > + return ret; > +} > + > static int nvme_hwmon_get_smart_log(struct nvme_hwmon_data *data) > { > int ret; > @@ -39,10 +84,11 @@ static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > */ > switch (attr) { > case hwmon_temp_max: > - *val = (data->ctrl->wctemp - 273) * 1000; > - return 0; > + return nvme_get_temp_thresh(data->ctrl, channel, false, val); > + case hwmon_temp_min: > + return nvme_get_temp_thresh(data->ctrl, channel, true, val); > case hwmon_temp_crit: > - *val = (data->ctrl->cctemp - 273) * 1000; > + *val = KELVIN_TO_MILLICELSIUS(data->ctrl->cctemp); > return 0; > default: > break; > @@ -59,7 +105,7 @@ static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > temp = get_unaligned_le16(log->temperature); > else > temp = le16_to_cpu(log->temp_sensor[channel - 1]); > - *val = (temp - 273) * 1000; > + *val = KELVIN_TO_MILLICELSIUS(temp); > break; > case hwmon_temp_alarm: > *val = !!(log->critical_warning & NVME_SMART_CRIT_TEMPERATURE); > @@ -73,6 +119,23 @@ static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > return err; > } > > +static int nvme_hwmon_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + struct nvme_hwmon_data *data = dev_get_drvdata(dev); > + > + switch (attr) { > + case hwmon_temp_max: > + return nvme_set_temp_thresh(data->ctrl, channel, false, val); > + case hwmon_temp_min: > + return nvme_set_temp_thresh(data->ctrl, channel, true, val); > + default: > + break; > + } > + > + return -EOPNOTSUPP; > +} > + > static const char * const nvme_hwmon_sensor_names[] = { > "Composite", > "Sensor 1", > @@ -105,8 +168,10 @@ static umode_t nvme_hwmon_is_visible(const void *_data, > return 0444; > break; > case hwmon_temp_max: > - if (!channel && data->ctrl->wctemp) > - return 0444; > + case hwmon_temp_min: > + if ((!channel && data->ctrl->wctemp) || > + (channel && data->log.temp_sensor[channel - 1])) > + return 0644; > break; > case hwmon_temp_alarm: > if (!channel) > @@ -126,16 +191,24 @@ static umode_t nvme_hwmon_is_visible(const void *_data, > static const struct hwmon_channel_info *nvme_hwmon_info[] = { > HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), > HWMON_CHANNEL_INFO(temp, > - HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_CRIT | > - HWMON_T_LABEL | HWMON_T_ALARM, > - HWMON_T_INPUT | HWMON_T_LABEL, > - HWMON_T_INPUT | HWMON_T_LABEL, > - HWMON_T_INPUT | HWMON_T_LABEL, > - HWMON_T_INPUT | HWMON_T_LABEL, > - HWMON_T_INPUT | HWMON_T_LABEL, > - HWMON_T_INPUT | HWMON_T_LABEL, > - HWMON_T_INPUT | HWMON_T_LABEL, > - HWMON_T_INPUT | HWMON_T_LABEL), > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_CRIT | HWMON_T_LABEL | HWMON_T_ALARM, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | > + HWMON_T_LABEL), > NULL > }; > > @@ -143,6 +216,7 @@ static const struct hwmon_ops nvme_hwmon_ops = { > .is_visible = nvme_hwmon_is_visible, > .read = nvme_hwmon_read, > .read_string = nvme_hwmon_read_string, > + .write = nvme_hwmon_write, > }; > > static const struct hwmon_chip_info nvme_hwmon_chip_info = { > diff --git a/include/linux/nvme.h b/include/linux/nvme.h > index 269b2e8..347a44f 100644 > --- a/include/linux/nvme.h > +++ b/include/linux/nvme.h > @@ -803,6 +803,12 @@ struct nvme_write_zeroes_cmd { > > /* Features */ > > +enum { > + NVME_TEMP_THRESH_MASK = 0xffff, > + NVME_TEMP_THRESH_SELECT_SHIFT = 16, > + NVME_TEMP_THRESH_TYPE_UNDER = 0x100000, > +}; > + > struct nvme_feat_auto_pst { > __le64 entries[32]; > }; > -- > 2.7.4 >
On Fri, Nov 15, 2019 at 12:40:00AM +0900, Akinobu Mita wrote: > +/* These macros should be moved to linux/temperature.h */ > +#define MILLICELSIUS_TO_KELVIN(t) DIV_ROUND_CLOSEST((t) + 273150, 1000) > +#define KELVIN_TO_MILLICELSIUS(t) ((t) * 1000L - 273150) Didn't we want to move this to a generic header? Otherwise this looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
2019年11月21日(木) 3:48 Christoph Hellwig <hch@lst.de>: > > On Fri, Nov 15, 2019 at 12:40:00AM +0900, Akinobu Mita wrote: > > +/* These macros should be moved to linux/temperature.h */ > > +#define MILLICELSIUS_TO_KELVIN(t) DIV_ROUND_CLOSEST((t) + 273150, 1000) > > +#define KELVIN_TO_MILLICELSIUS(t) ((t) * 1000L - 273150) > > Didn't we want to move this to a generic header? Yes. I start preparing for <linux/temperature.h>. Once the change is accepted, I'll move the macros to the header.
On Thu, Nov 21, 2019 at 10:48:21PM +0900, Akinobu Mita wrote: > 2019年11月21日(木) 3:48 Christoph Hellwig <hch@lst.de>: > > > > On Fri, Nov 15, 2019 at 12:40:00AM +0900, Akinobu Mita wrote: > > > +/* These macros should be moved to linux/temperature.h */ > > > +#define MILLICELSIUS_TO_KELVIN(t) DIV_ROUND_CLOSEST((t) + 273150, 1000) > > > +#define KELVIN_TO_MILLICELSIUS(t) ((t) * 1000L - 273150) > > > > Didn't we want to move this to a generic header? > > Yes. I start preparing for <linux/temperature.h>. > Once the change is accepted, I'll move the macros to the header. I agree, it is better to keep that change separate. Guenter
On Thu, Nov 21, 2019 at 06:27:12AM -0800, Guenter Roeck wrote: > On Thu, Nov 21, 2019 at 10:48:21PM +0900, Akinobu Mita wrote: > > 2019年11月21日(木) 3:48 Christoph Hellwig <hch@lst.de>: > > > > > > On Fri, Nov 15, 2019 at 12:40:00AM +0900, Akinobu Mita wrote: > > > > +/* These macros should be moved to linux/temperature.h */ > > > > +#define MILLICELSIUS_TO_KELVIN(t) DIV_ROUND_CLOSEST((t) + 273150, 1000) > > > > +#define KELVIN_TO_MILLICELSIUS(t) ((t) * 1000L - 273150) > > > > > > Didn't we want to move this to a generic header? > > > > Yes. I start preparing for <linux/temperature.h>. > > Once the change is accepted, I'll move the macros to the header. > > I agree, it is better to keep that change separate. I've added these two patches as-is for the next nvme-5.5 pull request.
diff --git a/drivers/nvme/host/nvme-hwmon.c b/drivers/nvme/host/nvme-hwmon.c index 5480cbb..97a84b4 100644 --- a/drivers/nvme/host/nvme-hwmon.c +++ b/drivers/nvme/host/nvme-hwmon.c @@ -9,12 +9,57 @@ #include "nvme.h" +/* These macros should be moved to linux/temperature.h */ +#define MILLICELSIUS_TO_KELVIN(t) DIV_ROUND_CLOSEST((t) + 273150, 1000) +#define KELVIN_TO_MILLICELSIUS(t) ((t) * 1000L - 273150) + struct nvme_hwmon_data { struct nvme_ctrl *ctrl; struct nvme_smart_log log; struct mutex read_lock; }; +static int nvme_get_temp_thresh(struct nvme_ctrl *ctrl, int sensor, bool under, + long *temp) +{ + unsigned int threshold = sensor << NVME_TEMP_THRESH_SELECT_SHIFT; + u32 status; + int ret; + + if (under) + threshold |= NVME_TEMP_THRESH_TYPE_UNDER; + + ret = nvme_get_features(ctrl, NVME_FEAT_TEMP_THRESH, threshold, NULL, 0, + &status); + if (ret > 0) + return -EIO; + if (ret < 0) + return ret; + *temp = KELVIN_TO_MILLICELSIUS(status & NVME_TEMP_THRESH_MASK); + + return 0; +} + +static int nvme_set_temp_thresh(struct nvme_ctrl *ctrl, int sensor, bool under, + long temp) +{ + unsigned int threshold = sensor << NVME_TEMP_THRESH_SELECT_SHIFT; + int ret; + + temp = MILLICELSIUS_TO_KELVIN(temp); + threshold |= clamp_val(temp, 0, NVME_TEMP_THRESH_MASK); + + if (under) + threshold |= NVME_TEMP_THRESH_TYPE_UNDER; + + ret = nvme_set_features(ctrl, NVME_FEAT_TEMP_THRESH, threshold, NULL, 0, + NULL); + if (ret > 0) + return -EIO; + + return ret; +} + static int nvme_hwmon_get_smart_log(struct nvme_hwmon_data *data) { int ret; @@ -39,10 +84,11 @@ static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types type, */ switch (attr) { case hwmon_temp_max: - *val = (data->ctrl->wctemp - 273) * 1000; - return 0; + return nvme_get_temp_thresh(data->ctrl, channel, false, val); + case hwmon_temp_min: + return nvme_get_temp_thresh(data->ctrl, channel, true, val); case hwmon_temp_crit: - *val = (data->ctrl->cctemp - 273) * 1000; + *val = KELVIN_TO_MILLICELSIUS(data->ctrl->cctemp); return 0; default: break; @@ -59,7 +105,7 @@ static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types type, temp = get_unaligned_le16(log->temperature); else temp = le16_to_cpu(log->temp_sensor[channel - 1]); - *val = (temp - 273) * 1000; + *val = KELVIN_TO_MILLICELSIUS(temp); break; case hwmon_temp_alarm: *val = !!(log->critical_warning & NVME_SMART_CRIT_TEMPERATURE); @@ -73,6 +119,23 @@ static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types type, return err; } +static int nvme_hwmon_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + struct nvme_hwmon_data *data = dev_get_drvdata(dev); + + switch (attr) { + case hwmon_temp_max: + return nvme_set_temp_thresh(data->ctrl, channel, false, val); + case hwmon_temp_min: + return nvme_set_temp_thresh(data->ctrl, channel, true, val); + default: + break; + } + + return -EOPNOTSUPP; +} + static const char * const nvme_hwmon_sensor_names[] = { "Composite", "Sensor 1", @@ -105,8 +168,10 @@ static umode_t nvme_hwmon_is_visible(const void *_data, return 0444; break; case hwmon_temp_max: - if (!channel && data->ctrl->wctemp) - return 0444; + case hwmon_temp_min: + if ((!channel && data->ctrl->wctemp) || + (channel && data->log.temp_sensor[channel - 1])) + return 0644; break; case hwmon_temp_alarm: if (!channel) @@ -126,16 +191,24 @@ static umode_t nvme_hwmon_is_visible(const void *_data, static const struct hwmon_channel_info *nvme_hwmon_info[] = { HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ), HWMON_CHANNEL_INFO(temp, - HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_CRIT | - HWMON_T_LABEL | HWMON_T_ALARM, - HWMON_T_INPUT | HWMON_T_LABEL, - HWMON_T_INPUT | HWMON_T_LABEL, - HWMON_T_INPUT | HWMON_T_LABEL, - HWMON_T_INPUT | HWMON_T_LABEL, - HWMON_T_INPUT | HWMON_T_LABEL, - HWMON_T_INPUT | HWMON_T_LABEL, - HWMON_T_INPUT | HWMON_T_LABEL, - HWMON_T_INPUT | HWMON_T_LABEL), + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_CRIT | HWMON_T_LABEL | HWMON_T_ALARM, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MIN | + HWMON_T_LABEL), NULL }; @@ -143,6 +216,7 @@ static const struct hwmon_ops nvme_hwmon_ops = { .is_visible = nvme_hwmon_is_visible, .read = nvme_hwmon_read, .read_string = nvme_hwmon_read_string, + .write = nvme_hwmon_write, }; static const struct hwmon_chip_info nvme_hwmon_chip_info = { diff --git a/include/linux/nvme.h b/include/linux/nvme.h index 269b2e8..347a44f 100644 --- a/include/linux/nvme.h +++ b/include/linux/nvme.h @@ -803,6 +803,12 @@ struct nvme_write_zeroes_cmd { /* Features */ +enum { + NVME_TEMP_THRESH_MASK = 0xffff, + NVME_TEMP_THRESH_SELECT_SHIFT = 16, + NVME_TEMP_THRESH_TYPE_UNDER = 0x100000, +}; + struct nvme_feat_auto_pst { __le64 entries[32]; };
According to the NVMe specification, the over temperature threshold and under temperature threshold features shall be implemented for Composite Temperature if a non-zero WCTEMP field value is reported in the Identify Controller data structure. The features are also implemented for all implemented temperature sensors (i.e., all Temperature Sensor fields that report a non-zero value). This provides the over temperature threshold and under temperature threshold for each sensor as temperature min and max values of hwmon sysfs attributes. The WCTEMP is already provided as a temperature max value for Composite Temperature, but this change isn't incompatible. Because the default value of the over temperature threshold for Composite Temperature is the WCTEMP. Now the alarm attribute for Composite Temperature indicates one of the temperature is outside of a temperature threshold. Because there is only a single bit in Critical Warning field that indicates a temperature is outside of a threshold. Example output from the "sensors" command: nvme-pci-0100 Adapter: PCI adapter Composite: +33.9°C (low = -273.1°C, high = +69.8°C) (crit = +79.8°C) Sensor 1: +34.9°C (low = -273.1°C, high = +65261.8°C) Sensor 2: +31.9°C (low = -273.1°C, high = +65261.8°C) Sensor 5: +47.9°C (low = -273.1°C, high = +65261.8°C) This also adds helper macros for kelvin from/to milli Celsius conversion, and replaces the repeated code in nvme-hwmon.c. Cc: Keith Busch <kbusch@kernel.org> Cc: Jens Axboe <axboe@fb.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Sagi Grimberg <sagi@grimberg.me> Cc: Jean Delvare <jdelvare@suse.com> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- drivers/nvme/host/nvme-hwmon.c | 106 ++++++++++++++++++++++++++++++++++------- include/linux/nvme.h | 6 +++ 2 files changed, 96 insertions(+), 16 deletions(-)