Re: [PATCH 3/6] hwmon: (k10temp) Check return value of amd_smn_read()

[Yazen Ghannam <yazen.ghannam@xxxxxxx>]

On 5/17/23 8:25 AM, Guenter Roeck wrote:

[...]

>>  static long get_raw_temp(struct k10temp_data *data)
>> @@ -213,9 +214,11 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
>>  				*val = 0;
>>  			break;
>>  		case 2 ... 13:		/* Tccd{1-12} */
>> -			amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
>> -				     ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
>> -						  &regval);
>> +			if (amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
>> +					 ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
>> +					 &regval))
>> +				return -EINVAL;
>> +
> 
> -EINVAL: Invalid Argument, supposed to be used for bad user input.
> I don't see how that would apply here. amd_smn_read() returns
> a valid error code. This error core should be returned to the caller,
> or there needs to be an explanation why this is not appropriate.
>

Understood. Will change it to return the amd_smn_read() error code.

>>  			*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
>>  			break;
>>  		default:
>> @@ -373,8 +376,10 @@ static void k10temp_get_ccd_support(struct pci_dev *pdev,
>>  	int i;
>>  
>>  	for (i = 0; i < limit; i++) {
>> -		amd_smn_read(amd_pci_dev_to_node_id(pdev),
>> -			     ZEN_CCD_TEMP(data->ccd_offset, i), &regval);
>> +		if (amd_smn_read(amd_pci_dev_to_node_id(pdev),
>> +				 ZEN_CCD_TEMP(data->ccd_offset, i), &regval))
>> +			continue;
>> +
> The reason for ignoring the error should be explained here.
>

Sure thing. I'll add a code comment above.

Thanks,
Yazen