Re: [PATCH] net: phy: aquantia: clamp temperature value in aqr_hwmon_set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Feb 18, 2023 at 12:16:47AM +0100, Enrico Mioso wrote:
> React like otherdrivers do when an out-of-range value is passed from hwmon
> core.
> 

This is not an appropriate patch description.

> Signed-off-by: Enrico Mioso <mrkiko.rs@xxxxxxxxx>
> CC: Andrew Lunn <andrew@xxxxxxx>
> CC: Russell King <linux@xxxxxxxxxxxxxxx>
> ---
> I implemented this patch based on observing how other drivers are reacting,
> and after experiencing the hwmon core passing -INT MAX. Based on a

I don't understand "after experiencing the hwmon core passing -INT MAX".

> cursory look at the hwmon code, I don't believe this to be a bug. If this is
> instead the case, I hope I will be corrected and this patch rejected.

The bug, if anything, in the current code is that it should return -EINVAL.
-ERANGE is "Math result not representable" which doesn't make sense here.

Other than that, it is up to the driver author to decide if they want to
return an error in this situation or if they want to clamp the range.
We recommend to clamp because users won't normally know the valid range,
but, again, it is up to the driver author to decide if they want to burden
users with figuring out the valid range.

Guenter

> ---
>  drivers/net/phy/aquantia_hwmon.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/aquantia_hwmon.c b/drivers/net/phy/aquantia_hwmon.c
> index 19c4c280a6cd..6444055e720c 100644
> --- a/drivers/net/phy/aquantia_hwmon.c
> +++ b/drivers/net/phy/aquantia_hwmon.c
> @@ -70,8 +70,7 @@ static int aqr_hwmon_set(struct phy_device *phydev, int reg, long value)
>  {
>  	int temp;
>  
> -	if (value >= 128000 || value < -128000)
> -		return -ERANGE;
> +	clamp_val(value, -128000, 128000);
>  
>  	temp = value * 256 / 1000;
>  
> -- 
> 2.39.2
> 



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux