RE: [PATCH 2/2] hwmon: (emc2305) fix pwm never being able to set lower

[Xingjiang Qiao <nanpuyue@gmail.com>]

Even when the cooling state is not set by the thermal subsystem, no lower state can be set by hwmon.



> On Tue, Dec 6, 2022 at 2:27 AM Michael Shych <michaelsh@nvidia.com> wrote:
> Hi,
> 
> This is intention - to set low limit through 'hwmon'.
> Thus, if it is set through 'hwom', cooling state is limited and not allowed to be set to a lower value.
> If user doesn't want this feature, he is not supposed to use 'hwmon' for fan speed setting.
> 
> Regards,
>    Michael.
> 
> > -----Original Message-----
> > From: Xingjiang Qiao <nanpuyue@gmail.com>
> > Sent: Monday, December 5, 2022 7:36 PM
> > To: Guenter Roeck <linux@roeck-us.net>; Jean Delvare
> > <jdelvare@suse.com>
> > Cc: Michael Shych <michaelsh@nvidia.com>; Xingjiang Qiao
> > <nanpuyue@gmail.com>; linux-hwmon@vger.kernel.org
> > Subject: [PATCH 2/2] hwmon: (emc2305) fix pwm never being able to set
> > lower
> >
> > There are fields 'last_hwmon_state' and 'last_thermal_state' in the structure
> > 'emc2305_cdev_data', which respectively store the cooling state set by the
> > 'hwmon' and 'thermal' subsystem, and the driver author hopes that if the
> > state set by 'hwmon' is lower than the value set by 'thermal', the driver will
> > just save it without actually setting the pwm. Currently, the
> > 'last_thermal_state' also be updated by 'hwmon', which will cause the
> > cooling state to never be set to a lower value. This patch fixes that.
> >
> > Signed-off-by: Xingjiang Qiao <nanpuyue@gmail.com>
> > ---
> >  drivers/hwmon/emc2305.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c index
> > 4df84065cbfb..f51760f8aa85 100644
> > --- a/drivers/hwmon/emc2305.c
> > +++ b/drivers/hwmon/emc2305.c
> > @@ -398,11 +398,9 @@ emc2305_write(struct device *dev, enum
> > hwmon_sensor_types type, u32 attr, int ch
> >                                * Update PWM only in case requested state
> > is not less than the
> >                                * last thermal state.
> >                                */
> > -                             if (data-
> > >cdev_data[cdev_idx].last_hwmon_state >=
> > +                             if (data-
> > >cdev_data[cdev_idx].last_hwmon_state <
> >                                   data-
> > >cdev_data[cdev_idx].last_thermal_state)
> > -                                     return emc2305_set_cur_state(data-
> > >cdev_data[cdev_idx].cdev,
> > -                                                     data-
> > >cdev_data[cdev_idx].last_hwmon_state);
> > -                             return 0;
> > +                                     return 0;
> >                       }
> >                       return emc2305_set_pwm(dev, val, channel);
> >               default:
> > --
> > 2.38.1