[PATCH] nvme-pci: add NVME_QUIRK_NO_SECONDARY_TEMP_THRESH for WD SN740

[PATCH] nvme-pci: add NVME_QUIRK_NO_SECONDARY_TEMP_THRESH for WD SN740

[Robson Cassiano]

The WD SN740 (15b7:5017) reports two secondary temperature sensors in
the SMART/Health log but returns the 0 and 0xffff (Kelvin) sentinels
for their temperature thresholds. nvme-hwmon exposes those as
temp2_min/max and temp3_min/max, producing -273.15 C and 65261.85 C in
sysfs.

Mark the controller with NVME_QUIRK_NO_SECONDARY_TEMP_THRESH so only
the sensor inputs are exposed.

Assisted-by: LLM
Signed-off-by: Robson Cassiano <linux@robsoncassiano.software>
---
Tested on the affected system, WD SN740 512 GB, firmware 73100004:

  # lspci -nn -s 01:00.0
  01:00.0 Non-Volatile memory controller [0108]: Sandisk Corp WD SN560/SN740/SN770/SN5000 NVMe SSD [15b7:5017] (rev 01)

Before, on 7.2.8:

  temp2_label    Sensor 1
  temp2_input    53850
  temp2_min      -273150
  temp2_max      65261850
  temp3_label    Sensor 2
  temp3_input    38850
  temp3_min      -273150
  temp3_max      65261850

After, on 7.3.0 with this patch:

  temp1_label    Composite
  temp1_input    45850
  temp1_min      -40150
  temp1_max      83850
  temp1_crit     87850
  temp2_label    Sensor 1
  temp2_input    59850
  temp3_label    Sensor 2
  temp3_input    45850

temp1_* are the Composite thresholds and stay exposed; the quirk only
suppresses the secondary sensor thresholds.

The quirk also covers the other models sharing 15b7:5017 (SN560, SN770,
SN5000); only the SN740 was tested.

OEM systems: Dell OptiPlex 3000, Lenovo IdeaCentre AIO 5 and Samsung
NP750QFG-KS1BR (the test system).

 drivers/nvme/host/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 77317e5..4f1f10b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -4170,6 +4170,8 @@ static const struct pci_device_id nvme_id_table[] = {
 	{ PCI_DEVICE(0x15b7, 0x5009),   /* Sandisk SN550 */
 		.driver_data = NVME_QUIRK_BROKEN_MSI |
 				NVME_QUIRK_NO_DEEPEST_PS },
+	{ PCI_DEVICE(0x15b7, 0x5017),   /* Sandisk SN560/SN740/SN770/SN5000 */
+		.driver_data = NVME_QUIRK_NO_SECONDARY_TEMP_THRESH, },
 	{ PCI_DEVICE(0x1987, 0x5012),	/* Phison E12 */
 		.driver_data = NVME_QUIRK_BOGUS_NID, },
 	{ PCI_DEVICE(0x1987, 0x5016),	/* Phison E16 */
-- 
2.56.0


base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

Re: [PATCH] nvme-pci: add NVME_QUIRK_NO_SECONDARY_TEMP_THRESH for WD SN740

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nvme-pci: false NVME_QUIRK_NO_SECONDARY_TEMP_THRESH for WD 15b7:5017
--

commit 94ec2f511365e9d402dee1ccba8357982a5690c8
Author: Robson Cassiano <linux@robsoncassiano.software>

nvme-pci: add NVME_QUIRK_NO_SECONDARY_TEMP_THRESH for WD SN740

This commit adds NVME_QUIRK_NO_SECONDARY_TEMP_THRESH for Sandisk/WD NVMe
controllers with PCI ID 15b7:5017 so that secondary temperature threshold
sysfs attributes are hidden when the controller reports 0 and 0xffff Kelvin.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 77317e5d00f9f..4f1f10bc13818 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -4170,6 +4170,8 @@ static const struct pci_device_id nvme_id_table[] = {
>  	{ PCI_DEVICE(0x15b7, 0x5009),   /* Sandisk SN550 */
>  		.driver_data = NVME_QUIRK_BROKEN_MSI |
>  				NVME_QUIRK_NO_DEEPEST_PS },
> +	{ PCI_DEVICE(0x15b7, 0x5017),   /* Sandisk SN560/SN740/SN770/SN5000 */
> +		.driver_data = NVME_QUIRK_NO_SECONDARY_TEMP_THRESH, },

[Severity: Medium]
Does setting NVME_QUIRK_NO_SECONDARY_TEMP_THRESH in nvme_id_table cause a
regression by hiding valid, host-writable temp2_min, temp2_max, temp3_min,
and temp3_max attributes on Sandisk SN560, SN740, SN770, and SN5000 drives?

Per the NVM Express Base Specification (Feature Identifier 04h, Temperature
Threshold), 0000h Kelvin (-273.15 C) is the mandatory power-on default for
the Under Temperature Threshold on all implemented sensors, and FFFFh Kelvin
(65261.85 C) is the mandatory power-on default for the Over Temperature
Threshold on all non-composite temperature sensors (TMPSEL > 0, since WCTEMP
in Identify Controller only applies to the Composite Temperature). When
commit 52deba0f02a98c ("nvme: hwmon: provide temperature min and max values
for each sensor") added secondary sensor threshold support, it explicitly
documented low = -273.1 C, high = +65261.8 C as the expected unconfigured
default output.

In drivers/nvme/host/hwmon.c:nvme_hwmon_is_visible(), secondary threshold
attributes are exposed as writable (0644) when this quirk is not set:

drivers/nvme/host/hwmon.c:nvme_hwmon_is_visible() {
	...
	case hwmon_temp_max:
	case hwmon_temp_min:
		if ((!channel && data->ctrl->wctemp) ||
		    (channel && data->log->temp_sensor[channel - 1] &&
		     !(data->ctrl->quirks &
		       NVME_QUIRK_NO_SECONDARY_TEMP_THRESH))) {
			if (data->ctrl->quirks &
			    NVME_QUIRK_NO_TEMP_THRESH_CHANGE)
				return 0444;
			return 0644;
		}
		break;
	...
}

This allows userspace to configure the secondary sensor thresholds via
nvme_hwmon_write() -> nvme_set_temp_thresh().

NVME_QUIRK_NO_SECONDARY_TEMP_THRESH (introduced in commit bd375feeaf3408 and
used in commit 340f4fc5508c29) is intended for broken controllers that fail
NVME_FEAT_TEMP_THRESH commands on secondary sensors with an error (-EIO /
Invalid Field in Command). Applying this quirk to a spec-compliant
controller simply because its unconfigured power-on defaults are 0 and
0xffff Kelvin prevents userspace from reading or configuring secondary
temperature thresholds on all 15b7:5017 devices and conflicts with
Documentation/nvme/feature-and-quirk-policy.rst.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/WnzOhxjlW7w5jwpSIQmUzrvduOStpoX2h3FS@robsoncassiano.software?part=1