On 9/3/20 6:47 AM, Lars Povlsen wrote:
> If the temperature is read before the internal calibration is
> completed, the driver returns -EIO. Instead it should return -EAGAIN
> to encourage repeating the operation.
>
> Signed-off-by: Lars Povlsen <[email protected]>
> ---
> drivers/hwmon/sparx5-temp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
> index 1a2b1026b026..98be48e3a22a 100644
> --- a/drivers/hwmon/sparx5-temp.c
> +++ b/drivers/hwmon/sparx5-temp.c
> @@ -56,7 +56,7 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
> case hwmon_temp_input:
> stat = readl_relaxed(hwmon->base + TEMP_STAT);
> if (!(stat & TEMP_STAT_VALID))
> - return -EIO;
> + return -EAGAIN;
The problem is that this may result in a hard loop and effectively hang the system,
or at least the calling process. It may be better to return -ENODATA in this situation.
Guenter
> value = stat & TEMP_STAT_TEMP;
> /*
> * From register documentation:
> --
> 2.27.0
>
Guenter Roeck writes:
> On 9/3/20 6:47 AM, Lars Povlsen wrote:
>> If the temperature is read before the internal calibration is
>> completed, the driver returns -EIO. Instead it should return -EAGAIN
>> to encourage repeating the operation.
>>
>> Signed-off-by: Lars Povlsen <[email protected]>
>> ---
>> drivers/hwmon/sparx5-temp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
>> index 1a2b1026b026..98be48e3a22a 100644
>> --- a/drivers/hwmon/sparx5-temp.c
>> +++ b/drivers/hwmon/sparx5-temp.c
>> @@ -56,7 +56,7 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
>> case hwmon_temp_input:
>> stat = readl_relaxed(hwmon->base + TEMP_STAT);
>> if (!(stat & TEMP_STAT_VALID))
>> - return -EIO;
>> + return -EAGAIN;
>
> The problem is that this may result in a hard loop and effectively hang the system,
> or at least the calling process. It may be better to return -ENODATA in this situation.
>
Guenther,
Thank you for your response.
The motivation for the patch was as I tried to use thermal support to
control a fan, and I noticed an initial quip from
thermal_core.c:update_temperature() - it will throw a dev_warn() for
anything else than an "EAGAIN". The EAGAIN also seems to be used by
several other hwmon drivers in similar situations.
The bottom line is that I would like to get rid of the warning
message. Changing the error code to ENODATA does not fix what I am
trying to accomplish.
Do you have any suggestions on how to achieve the objective?
Sincerely,
---Lars
> Guenter
>
>> value = stat & TEMP_STAT_TEMP;
>> /*
>> * From register documentation:
>> --
>> 2.27.0
>>
--
Lars Povlsen,
Microchip
On 9/4/20 1:33 AM, Lars Povlsen wrote:
>
> Guenter Roeck writes:
>
>> On 9/3/20 6:47 AM, Lars Povlsen wrote:
>>> If the temperature is read before the internal calibration is
>>> completed, the driver returns -EIO. Instead it should return -EAGAIN
>>> to encourage repeating the operation.
>>>
>>> Signed-off-by: Lars Povlsen <[email protected]>
>>> ---
>>> drivers/hwmon/sparx5-temp.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/sparx5-temp.c b/drivers/hwmon/sparx5-temp.c
>>> index 1a2b1026b026..98be48e3a22a 100644
>>> --- a/drivers/hwmon/sparx5-temp.c
>>> +++ b/drivers/hwmon/sparx5-temp.c
>>> @@ -56,7 +56,7 @@ static int s5_read(struct device *dev, enum hwmon_sensor_types type,
>>> case hwmon_temp_input:
>>> stat = readl_relaxed(hwmon->base + TEMP_STAT);
>>> if (!(stat & TEMP_STAT_VALID))
>>> - return -EIO;
>>> + return -EAGAIN;
>>
>> The problem is that this may result in a hard loop and effectively hang the system,
>> or at least the calling process. It may be better to return -ENODATA in this situation.
>>
>
> Guenther,
>
> Thank you for your response.
>
> The motivation for the patch was as I tried to use thermal support to
> control a fan, and I noticed an initial quip from
> thermal_core.c:update_temperature() - it will throw a dev_warn() for
> anything else than an "EAGAIN". The EAGAIN also seems to be used by
> several other hwmon drivers in similar situations.
>
> The bottom line is that I would like to get rid of the warning
> message. Changing the error code to ENODATA does not fix what I am
> trying to accomplish.
>
Sigh. I know that -EAGAIN is used, but it has its own problems - userspace
is inclined to retry directly, causing hard loops.
Ok, lets go with -EAGAIN then. Can't be helped. I'll apply your patch.
Guenter
> Do you have any suggestions on how to achieve the objective?
>
> Sincerely,
>
> ---Lars
>
>> Guenter
>>
>>> value = stat & TEMP_STAT_TEMP;
>>> /*
>>> * From register documentation:
>>> --
>>> 2.27.0
>>>
>