| Message ID | 20210826184057.26428-5-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add k10temp support for more client APUs | expand |
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), > ®val); > - *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; >
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), ®val); - *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;
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(-)