[PATCH 0/6] Add k10temp support for more client APUs

[PATCH 0/6] Add k10temp support for more client APUs

[Mario Limonciello]

Extend existing code to cover more missing client APUs.  Some logic changes
and failure handling will make this more scalable for other future APUs as
well.

Mario Limonciello (6):
  hwmon: (k10temp): Add additional missing Zen2 and Zen3 APUs
  x86/amd_nb: Rename PCI_DEVICE_ID_AMD_19H_DF_F3 to match specific model
  hwmon: (k10temp): Rework the temperature offset calculation
  hwmon: (k10temp): Show errors failing to read
  hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU
  hwmon: (k10temp): Add support for yellow carp

 arch/x86/kernel/amd_nb.c |  7 ++++++-
 drivers/hwmon/k10temp.c  | 44 ++++++++++++++++++++++++++++------------
 include/linux/pci_ids.h  |  3 ++-
 3 files changed, 39 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH 1/6] hwmon: (k10temp): Add additional missing Zen2 and Zen3 APUs

[Mario Limonciello]

These follow the rest of the existing codepaths for families
17h and 19h.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/hwmon/k10temp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index fe3d92152e35..1d3c8d319941 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -437,6 +437,8 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 			k10temp_get_ccd_support(pdev, data, 4);
 			break;
 		case 0x31:	/* Zen2 Threadripper */
+		case 0x60:	/* Renoir */
+		case 0x68:	/* Lucienne */
 		case 0x71:	/* Zen2 */
 			k10temp_get_ccd_support(pdev, data, 8);
 			break;
@@ -450,7 +452,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		switch (boot_cpu_data.x86_model) {
 		case 0x0 ... 0x1:	/* Zen3 SP3/TR */
 		case 0x21:		/* Zen3 Ryzen Desktop */
-		case 0x50:		/* Zen3 Ryzen APU */
+		case 0x50 ... 0x5f:	/* Green Sardine */
 			k10temp_get_ccd_support(pdev, data, 8);
 			break;
 		}
-- 
2.25.1

[PATCH 2/6] x86/amd_nb: Rename PCI_DEVICE_ID_AMD_19H_DF_F3 to match specific model

[Mario Limonciello]

Upcoming changes will introduce a different model in family 19h, so rename
to disambiguate.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/x86/kernel/amd_nb.c | 2 +-
 drivers/hwmon/k10temp.c  | 2 +-
 include/linux/pci_ids.h  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 23dda362dc0f..cf41fd7faad7 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -57,7 +57,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M60H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_DF_F3) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M01H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
 	{}
 };
diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index 1d3c8d319941..9e704338230d 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -493,7 +493,7 @@ static const struct pci_device_id k10temp_id_table[] = {
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M60H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
-	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_DF_F3) },
+	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_M01H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
 	{ PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) },
 	{}
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index fb1185155394..b2425a1574c7 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -554,7 +554,7 @@
 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493
 #define PCI_DEVICE_ID_AMD_17H_M60H_DF_F3 0x144b
 #define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443
-#define PCI_DEVICE_ID_AMD_19H_DF_F3	0x1653
+#define PCI_DEVICE_ID_AMD_19H_M01H_DF_F3 0x1653
 #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F3 0x166d
 #define PCI_DEVICE_ID_AMD_CNB17H_F3	0x1703
 #define PCI_DEVICE_ID_AMD_LANCE		0x2000
-- 
2.25.1

[PATCH 3/6] hwmon: (k10temp): Rework the temperature offset calculation

[Mario Limonciello]

Some of the existing assumptions made do not scale properly
to new silicon in upcoming changes.  This commit should cause
no functional changes to existing silicon.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/hwmon/k10temp.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index 9e704338230d..5c1aebf7a56d 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -65,10 +65,11 @@ static DEFINE_MUTEX(nb_smu_ind_mutex);
 #define F15H_M60H_HARDWARE_TEMP_CTRL_OFFSET	0xd8200c64
 #define F15H_M60H_REPORTED_TEMP_CTRL_OFFSET	0xd8200ca4
 
-/* Common for Zen CPU families (Family 17h and 18h) */
-#define ZEN_REPORTED_TEMP_CTRL_OFFSET		0x00059800
+/* Common for Zen CPU families (Family 17h and 18h and 19h) */
+#define ZEN_REPORTED_TEMP_CTRL_BASE		0x00059800
 
-#define ZEN_CCD_TEMP(x)				(0x00059954 + ((x) * 4))
+#define ZEN_CCD_TEMP(offset, x)			(ZEN_REPORTED_TEMP_CTRL_BASE + \
+						 offset + ((x) * 4))
 #define ZEN_CCD_TEMP_VALID			BIT(11)
 #define ZEN_CCD_TEMP_MASK			GENMASK(10, 0)
 
@@ -103,6 +104,7 @@ struct k10temp_data {
 	u32 temp_adjust_mask;
 	u32 show_temp;
 	bool is_zen;
+	u32 ccd_offset;
 };
 
 #define TCTL_BIT	0
@@ -163,7 +165,7 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 *regval)
 static void read_tempreg_nb_zen(struct pci_dev *pdev, u32 *regval)
 {
 	amd_smn_read(amd_pci_dev_to_node_id(pdev),
-		     ZEN_REPORTED_TEMP_CTRL_OFFSET, regval);
+		     ZEN_REPORTED_TEMP_CTRL_BASE, regval);
 }
 
 static long get_raw_temp(struct k10temp_data *data)
@@ -226,7 +228,8 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
 			break;
 		case 2 ... 9:		/* Tccd{1-8} */
 			amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
-				     ZEN_CCD_TEMP(channel - 2), &regval);
+				     ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
+						  &regval);
 			*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
 			break;
 		default:
@@ -387,7 +390,7 @@ static void k10temp_get_ccd_support(struct pci_dev *pdev,
 
 	for (i = 0; i < limit; i++) {
 		amd_smn_read(amd_pci_dev_to_node_id(pdev),
-			     ZEN_CCD_TEMP(i), &regval);
+			     ZEN_CCD_TEMP(data->ccd_offset, i), &regval);
 		if (regval & ZEN_CCD_TEMP_VALID)
 			data->show_temp |= BIT(TCCD_BIT(i));
 	}
@@ -434,12 +437,14 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		case 0x8:	/* Zen+ */
 		case 0x11:	/* Zen APU */
 		case 0x18:	/* Zen+ APU */
+			data->ccd_offset = 0x154;
 			k10temp_get_ccd_support(pdev, data, 4);
 			break;
 		case 0x31:	/* Zen2 Threadripper */
 		case 0x60:	/* Renoir */
 		case 0x68:	/* Lucienne */
 		case 0x71:	/* Zen2 */
+			data->ccd_offset = 0x154;
 			k10temp_get_ccd_support(pdev, data, 8);
 			break;
 		}
@@ -453,6 +458,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		case 0x0 ... 0x1:	/* Zen3 SP3/TR */
 		case 0x21:		/* Zen3 Ryzen Desktop */
 		case 0x50 ... 0x5f:	/* Green Sardine */
+			data->ccd_offset = 0x154;
 			k10temp_get_ccd_support(pdev, data, 8);
 			break;
 		}
-- 
2.25.1

[PATCH 4/6] hwmon: (k10temp): Show errors failing to read

[Mario Limonciello]

Enabling Yellow Carp in past commit was initially not working "properly"
because extra IDs were needed, but this wasn't obvious because fail values
from `amd_smn_read` were ignored.
If errors are found, show a kernel warning.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/hwmon/k10temp.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index 5c1aebf7a56d..41d9c0c0a1f1 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -164,8 +164,10 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 *regval)
 
 static void read_tempreg_nb_zen(struct pci_dev *pdev, u32 *regval)
 {
-	amd_smn_read(amd_pci_dev_to_node_id(pdev),
+	int ret = amd_smn_read(amd_pci_dev_to_node_id(pdev),
 		     ZEN_REPORTED_TEMP_CTRL_BASE, regval);
+	if (ret)
+		dev_warn(&pdev->dev, "failed to read core temperature: %d\n", ret);
 }
 
 static long get_raw_temp(struct k10temp_data *data)
@@ -212,6 +214,7 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
 {
 	struct k10temp_data *data = dev_get_drvdata(dev);
 	u32 regval;
+	int ret;
 
 	switch (attr) {
 	case hwmon_temp_input:
@@ -227,10 +230,13 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
 				*val = 0;
 			break;
 		case 2 ... 9:		/* Tccd{1-8} */
-			amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
+			ret = amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
 				     ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
 						  &regval);
-			*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
+			if (ret)
+				dev_warn(dev, "failed to read CCD temperature: %d\n", ret);
+			else
+				*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
 			break;
 		default:
 			return -EOPNOTSUPP;
-- 
2.25.1

[PATCH 5/6] hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU

[Mario Limonciello]

Tdie is an offset calculation that should only be shown when temp_offset
is actually put into a table.  This is useless to show for all CPU/APU.
Show it only when necessary.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/hwmon/k10temp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index 41d9c0c0a1f1..e8ec0e36fc3b 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -435,7 +435,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	} else if (boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) {
 		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
 		data->read_tempreg = read_tempreg_nb_zen;
-		data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
 		data->is_zen = true;
 
 		switch (boot_cpu_data.x86_model) {
@@ -457,7 +456,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	} else if (boot_cpu_data.x86 == 0x19) {
 		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
 		data->read_tempreg = read_tempreg_nb_zen;
-		data->show_temp |= BIT(TDIE_BIT);
 		data->is_zen = true;
 
 		switch (boot_cpu_data.x86_model) {
@@ -478,6 +476,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 		if (boot_cpu_data.x86 == entry->model &&
 		    strstr(boot_cpu_data.x86_model_id, entry->id)) {
+			data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
 			data->temp_offset = entry->offset;
 			break;
 		}
-- 
2.25.1

[PATCH 6/6] hwmon: (k10temp): Add support for yellow carp

[Mario Limonciello]

Yellow carp matches same behavior as green sardine and other Zen3
products, but have different CCD offsets.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/x86/kernel/amd_nb.c | 5 +++++
 drivers/hwmon/k10temp.c  | 5 +++++
 include/linux/pci_ids.h  | 1 +
 3 files changed, 11 insertions(+)

diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index cf41fd7faad7..ba768fe1d0ae 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -24,7 +24,9 @@
 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494
 #define PCI_DEVICE_ID_AMD_17H_M60H_DF_F4 0x144c
 #define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444
+#define PCI_DEVICE_ID_AMD_19H_M40H_ROOT	0x14b5
 #define PCI_DEVICE_ID_AMD_19H_DF_F4	0x1654
+#define PCI_DEVICE_ID_AMD_19H_M40H_DF_F4 0x167d
 #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F4 0x166e
 
 /* Protect the PCI config register pairs used for SMN and DF indirect access. */
@@ -37,6 +39,7 @@ static const struct pci_device_id amd_root_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_ROOT) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_ROOT) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M60H_ROOT) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M40H_ROOT) },
 	{}
 };
 
@@ -58,6 +61,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M01H_DF_F3) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M40H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
 	{}
 };
@@ -74,6 +78,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M60H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_DF_F4) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M40H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
 	{}
diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index e8ec0e36fc3b..cd3d30b6237c 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -465,6 +465,10 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 			data->ccd_offset = 0x154;
 			k10temp_get_ccd_support(pdev, data, 8);
 			break;
+		case 0x40 ... 0x4f:	/* Yellow Carp */
+			data->ccd_offset = 0x300;
+			k10temp_get_ccd_support(pdev, data, 8);
+			break;
 		}
 	} else {
 		data->read_htcreg = read_htcreg_pci;
@@ -505,6 +509,7 @@ static const struct pci_device_id k10temp_id_table[] = {
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M60H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_M01H_DF_F3) },
+	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_M40H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
 	{ PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) },
 	{}
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index b2425a1574c7..f84ffa154db1 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -555,6 +555,7 @@
 #define PCI_DEVICE_ID_AMD_17H_M60H_DF_F3 0x144b
 #define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443
 #define PCI_DEVICE_ID_AMD_19H_M01H_DF_F3 0x1653
+#define PCI_DEVICE_ID_AMD_19H_M40H_DF_F3 0x167c
 #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F3 0x166d
 #define PCI_DEVICE_ID_AMD_CNB17H_F3	0x1703
 #define PCI_DEVICE_ID_AMD_LANCE		0x2000
-- 
2.25.1

Re: [PATCH 4/6] hwmon: (k10temp): Show errors failing to read

[Guenter Roeck]

On 8/26/21 11:40 AM, Mario Limonciello wrote:
> Enabling Yellow Carp in past commit was initially not working "properly"
> because extra IDs were needed, but this wasn't obvious because fail values
> from `amd_smn_read` were ignored.
> If errors are found, show a kernel warning.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/hwmon/k10temp.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index 5c1aebf7a56d..41d9c0c0a1f1 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -164,8 +164,10 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 *regval)
>   
>   static void read_tempreg_nb_zen(struct pci_dev *pdev, u32 *regval)
>   {
> -	amd_smn_read(amd_pci_dev_to_node_id(pdev),
> +	int ret = amd_smn_read(amd_pci_dev_to_node_id(pdev),
>   		     ZEN_REPORTED_TEMP_CTRL_BASE, regval);
> +	if (ret)
> +		dev_warn(&pdev->dev, "failed to read core temperature: %d\n", ret);
>   }

It would be much better to change the code to return the error to the caller
(and thus to userspace) instead of polluting the kernel log and (presumably)
reporting a random value to userspace.

Guenter

>   
>   static long get_raw_temp(struct k10temp_data *data)
> @@ -212,6 +214,7 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
>   {
>   	struct k10temp_data *data = dev_get_drvdata(dev);
>   	u32 regval;
> +	int ret;
>   
>   	switch (attr) {
>   	case hwmon_temp_input:
> @@ -227,10 +230,13 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
>   				*val = 0;
>   			break;
>   		case 2 ... 9:		/* Tccd{1-8} */
> -			amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
> +			ret = amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
>   				     ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
>   						  &regval);
> -			*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
> +			if (ret)
> +				dev_warn(dev, "failed to read CCD temperature: %d\n", ret);

This is a static error. Just return the error to userspace, please,
and don't pollute the kernel log.

Guenter

> +			else
> +				*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
>   			break;
>   		default:
>   			return -EOPNOTSUPP;
> 

Re: [PATCH 3/6] hwmon: (k10temp): Rework the temperature offset calculation

[Guenter Roeck]

On 8/26/21 11:40 AM, Mario Limonciello wrote:
> Some of the existing assumptions made do not scale properly
> to new silicon in upcoming changes.  This commit should cause
> no functional changes to existing silicon.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/hwmon/k10temp.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index 9e704338230d..5c1aebf7a56d 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -65,10 +65,11 @@ static DEFINE_MUTEX(nb_smu_ind_mutex);
>   #define F15H_M60H_HARDWARE_TEMP_CTRL_OFFSET	0xd8200c64
>   #define F15H_M60H_REPORTED_TEMP_CTRL_OFFSET	0xd8200ca4
>   
> -/* Common for Zen CPU families (Family 17h and 18h) */
> -#define ZEN_REPORTED_TEMP_CTRL_OFFSET		0x00059800
> +/* Common for Zen CPU families (Family 17h and 18h and 19h) */
> +#define ZEN_REPORTED_TEMP_CTRL_BASE		0x00059800
>   
> -#define ZEN_CCD_TEMP(x)				(0x00059954 + ((x) * 4))
> +#define ZEN_CCD_TEMP(offset, x)			(ZEN_REPORTED_TEMP_CTRL_BASE + \
> +						 offset + ((x) * 4))

(offset)

>   #define ZEN_CCD_TEMP_VALID			BIT(11)
>   #define ZEN_CCD_TEMP_MASK			GENMASK(10, 0)
>   
> @@ -103,6 +104,7 @@ struct k10temp_data {
>   	u32 temp_adjust_mask;
>   	u32 show_temp;
>   	bool is_zen;
> +	u32 ccd_offset;
>   };
>   
>   #define TCTL_BIT	0
> @@ -163,7 +165,7 @@ static void read_tempreg_nb_f15(struct pci_dev *pdev, u32 *regval)
>   static void read_tempreg_nb_zen(struct pci_dev *pdev, u32 *regval)
>   {
>   	amd_smn_read(amd_pci_dev_to_node_id(pdev),
> -		     ZEN_REPORTED_TEMP_CTRL_OFFSET, regval);
> +		     ZEN_REPORTED_TEMP_CTRL_BASE, regval);
>   }
>   
>   static long get_raw_temp(struct k10temp_data *data)
> @@ -226,7 +228,8 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
>   			break;
>   		case 2 ... 9:		/* Tccd{1-8} */
>   			amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
> -				     ZEN_CCD_TEMP(channel - 2), &regval);
> +				     ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
> +						  &regval);
>   			*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
>   			break;
>   		default:
> @@ -387,7 +390,7 @@ static void k10temp_get_ccd_support(struct pci_dev *pdev,
>   
>   	for (i = 0; i < limit; i++) {
>   		amd_smn_read(amd_pci_dev_to_node_id(pdev),
> -			     ZEN_CCD_TEMP(i), &regval);
> +			     ZEN_CCD_TEMP(data->ccd_offset, i), &regval);
>   		if (regval & ZEN_CCD_TEMP_VALID)
>   			data->show_temp |= BIT(TCCD_BIT(i));
>   	}
> @@ -434,12 +437,14 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   		case 0x8:	/* Zen+ */
>   		case 0x11:	/* Zen APU */
>   		case 0x18:	/* Zen+ APU */
> +			data->ccd_offset = 0x154;
>   			k10temp_get_ccd_support(pdev, data, 4);
>   			break;
>   		case 0x31:	/* Zen2 Threadripper */
>   		case 0x60:	/* Renoir */
>   		case 0x68:	/* Lucienne */
>   		case 0x71:	/* Zen2 */
> +			data->ccd_offset = 0x154;
>   			k10temp_get_ccd_support(pdev, data, 8);
>   			break;
>   		}
> @@ -453,6 +458,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   		case 0x0 ... 0x1:	/* Zen3 SP3/TR */
>   		case 0x21:		/* Zen3 Ryzen Desktop */
>   		case 0x50 ... 0x5f:	/* Green Sardine */
> +			data->ccd_offset = 0x154;
>   			k10temp_get_ccd_support(pdev, data, 8);
>   			break;
>   		}
> 

Re: [PATCH 2/6] x86/amd_nb: Rename PCI_DEVICE_ID_AMD_19H_DF_F3 to match specific model

[Guenter Roeck]

On 8/26/21 11:40 AM, Mario Limonciello wrote:
> Upcoming changes will introduce a different model in family 19h, so rename
> to disambiguate.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   arch/x86/kernel/amd_nb.c | 2 +-
>   drivers/hwmon/k10temp.c  | 2 +-
>   include/linux/pci_ids.h  | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index 23dda362dc0f..cf41fd7faad7 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -57,7 +57,7 @@ static const struct pci_device_id amd_nb_misc_ids[] = {
>   	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M60H_DF_F3) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
> -	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_DF_F3) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M01H_DF_F3) },

I'll leave this up to the PCI/x86 maintainers to decide (personally I don't
really care one way or the other), but we already do have more than one
family 19h entry (M50 below), so this rename isn't really necessary.

If the rename is done, it might make sense to rename PCI_DEVICE_ID_AMD_17H_DF_F3
as well for consistency.

Guenter

>   	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
>   	{}
>   };
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index 1d3c8d319941..9e704338230d 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -493,7 +493,7 @@ static const struct pci_device_id k10temp_id_table[] = {
>   	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) },
>   	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M60H_DF_F3) },
>   	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
> -	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_DF_F3) },
> +	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_M01H_DF_F3) },
>   	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_19H_M50H_DF_F3) },
>   	{ PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) },
>   	{}
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index fb1185155394..b2425a1574c7 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -554,7 +554,7 @@
>   #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493
>   #define PCI_DEVICE_ID_AMD_17H_M60H_DF_F3 0x144b
>   #define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443
> -#define PCI_DEVICE_ID_AMD_19H_DF_F3	0x1653
> +#define PCI_DEVICE_ID_AMD_19H_M01H_DF_F3 0x1653
>   #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F3 0x166d
>   #define PCI_DEVICE_ID_AMD_CNB17H_F3	0x1703
>   #define PCI_DEVICE_ID_AMD_LANCE		0x2000
>