Re: [PATCH v4 1/7] hwmon: max31827: Make code cleaner

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Sep 19, 2023 at 12:34:49PM +0300, Daniel Matyas wrote:
> Used enums and while loops to replace switch for selecting and getting
> update interval from conversion rate bits.
> 
> Divided the write_alarm_val function into 2 functions. The new function
> is more generic: it can be used not only for alarm writes, but for any
> kind of writes which require the device to be in shutdown mode.
> 
> Signed-off-by: Daniel Matyas <daniel.matyas@analog.com>
> ---

Applied, with one change.

> @@ -333,39 +330,27 @@ static int max31827_write(struct device *dev, enum hwmon_sensor_types type,
>  			if (!st->enable)
>  				return -EINVAL;
>  
> -			switch (val) {
> -			case 125:
> -				val = MAX31827_CNV_8_HZ;
> -				break;
> -			case 250:
> -				val = MAX31827_CNV_4_HZ;
> -				break;
> -			case 1000:
> -				val = MAX31827_CNV_1_HZ;
> -				break;
> -			case 4000:
> -				val = MAX31827_CNV_1_DIV_4_HZ;
> -				break;
> -			case 16000:
> -				val = MAX31827_CNV_1_DIV_16_HZ;
> -				break;
> -			case 32000:
> -				val = MAX31827_CNV_1_DIV_32_HZ;
> -				break;
> -			case 64000:
> -				val = MAX31827_CNV_1_DIV_64_HZ;
> -				break;
> -			default:
> -				return -EINVAL;
> -			}
> +			/*
> +			 * Convert the desired conversion rate into register
> +			 * bits. res is already initialized with 1.
> +			 *
> +			 * This was inspired by lm73 driver.
> +			 */
> +			while (res < ARRAY_SIZE(max31827_conversions) &&
> +			       val < max31827_conversions[res])
> +				res++;
> +
> +			if (res == ARRAY_SIZE(max31827_conversions) ||
> +			    val != max31827_conversions[res])
> +				return -EOPNOTSUPP;

Changing the return value from -EINVAL to -EOPNOTSUPP was inappropriate
here. This needs to return -EINVAL because it is the result of an
invalid value provided by userspace, not the result of an unsupported
operation.

Guenter