On Sat, 18 Feb 2023, Guenter Roeck wrote:
Date: Sat, 18 Feb 2023 15:42:34
From: Guenter Roeck <linux@xxxxxxxxxxxx>
To: Andrew Lunn <andrew@xxxxxxx>
Cc: Enrico Mioso <mrkiko.rs@xxxxxxxxx>, linux-hwmon@xxxxxxxxxxxxxxx,
Russell King <linux@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH] net: phy: aquantia: clamp temperature value in
aqr_hwmon_set
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
Thanks a lot for your review and feedback.
I will send a new patch then, simply changing -ERANGE to -EINVAL.
Onanother side - if I want to set those limits to -10 °C and +90 °C, may you suggest me a DTS snippet? Thanks,
Enrico
Enrico
