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 02:45:43AM +0100, Andrew Lunn wrote:
> > 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);
> 
> It could be argued that value < -128000 should return
> -EUNOBTAINABLE. I've had trouble getting DRAMS to work at -40C, even

Hmm, I don't see that one anywhere.

> those listed as industrial. Setting an alarm for -128C is pointless.
> 
> +128C is also a bit questionable. The aQuantia PHYs do run hot, you
> often see a heat sink, and they are supposed to support up to 108C. So
> an alarm for 128C probably also does not work.
> 

It has been an endless argument if limits should be clamped to the
limits supported by the chip registers or to the limits supported by
the chip according to its datasheet. I personally see that as the
responsibility of the person actually configuring the limits, not the
driver. After all, there could be chip variants supporting different
limits, and who knows what chip variants are going to be available
in the future. Anyway, as I said, this has been an endless argument,
and it is another detail that I usually let driver developers decide
because they often feel passionate about it.

Guenter

> Anyway, as Guenter suggested, please change -ERANGE to -EINVAL.
> 
>      Andrew



[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