Re: [PATCH] hwmon: Handle failure to register sensor with thermal zone correctly

[Dmitry Osipenko <digetx@gmail.com>]

21.02.2022 20:12, Guenter Roeck пишет:
> On 2/21/22 08:56, Dmitry Osipenko wrote:
>> 21.02.2022 19:44, Guenter Roeck пишет:
>>> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
>>> index 3501a3ead4ba..4bfe3791a5ba 100644
>>> --- a/drivers/hwmon/hwmon.c
>>> +++ b/drivers/hwmon/hwmon.c
>>> @@ -214,12 +214,14 @@ static int hwmon_thermal_add_sensor(struct
>>> device *dev, int index)
>>>         tzd = devm_thermal_zone_of_sensor_register(dev, index, tdata,
>>>                              &hwmon_thermal_ops);
>>> -    /*
>>> -     * If CONFIG_THERMAL_OF is disabled, this returns -ENODEV,
>>> -     * so ignore that error but forward any other error.
>>> -     */
>>> -    if (IS_ERR(tzd) && (PTR_ERR(tzd) != -ENODEV))
>>> -        return PTR_ERR(tzd);
>>> +    if (IS_ERR(tzd)) {
>>> +        if (PTR_ERR(tzd) != -ENODEV)
>>> +            return PTR_ERR(tzd);
>>> +        dev_warn(dev, "Failed to register temp%d_input with thermal
>>> zone\n",
>>> +             index + 1);
>>
>> Do we really need this warning? I suppose it should be okay if sensor
>> isn't attached to any device in a device-tree and just reports
>> temperature.
> 
> I'd rather leave it there for the time being. It will only affect
> devicetree
> systems (turns out there is already a check for of_node elsewhere). Thermal
> zone specification is not always easy and there may be a mismatch between
> what is reported by the driver and what the user (programmer) expects to
> see (which I think is what happens here). I don't want to silently
> ignore such problems without any notification.
> 
> We could make it dev_notice and/or change the message (instead of
> "Failed to
> ..." just say "temp%d_input not registered with thermal zone" , maybe ?).

I'd change it to:

dev_info(dev, "temp%d_input not attached to any thermal zone\n", index + 1);

I'd also add an info message to print out to which tzd attachment happened.