> On Mon, Jul 04, 2022 at 04:06:05PM +0000, Ibrahim Tilki wrote:
> > MAX31760 is a presicion fan speed controller with nonvolatile lookup table.
> > Device has one internal and one external temperature sensor support.
> > Controls two fans and measures their speeds.
> > Generates hardware alerts when programmable max and critical temperatures are exceeded.
> >
> > Driver creates following sysfs attributes for configuring lookup table:
> > pwm1_auto_point[1-48]_{pwm,temp,temp_hyst}
> >
> > Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@xxxxxxxxxx>
> > Reviewed-by: Nurettin Bolucu <Nurettin.Bolucu@xxxxxxxxxx>
> > ---
> > drivers/hwmon/Kconfig | 10 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/max31760.c | 614
> > +++++++++++++++++++++++++++++++++++++++
>
> Please add doucmentation describing the supported sysfs attributes.
>
Driver does not create a sysfs attribute other than the standard sysfs attributes defined here:
https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface
Should I still add documentation?
> > 3 files changed, 625 insertions(+)
> > create mode 100644 drivers/hwmon/max31760.c
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index
> > 590d3d550..59cc44a5c 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
...
> > + case hwmon_pwm:
> > + switch (attr) {
> > + case hwmon_pwm_input:
> > + if (val < 0 || val > 255)
> > + return -EINVAL;
> > +
> > + return regmap_write(state->regmap, REG_PWMR, val);
> > + case hwmon_pwm_enable:
> > + if (val == 0)
> > + return -EOPNOTSUPP;
> > +
> > + if (val == 1)
> > + return regmap_set_bits(state->regmap, REG_CR2, CR2_DFC);
> > +
>
> Accepting all other values ?
>
Standard sysfs documentation defines pwm_enable as follows:
- 0: no fan speed control (i.e. fan at full speed)
- 1: manual fan speed control enabled (using pwm[1-*])
- 2+: automatic fan speed control enabled
So I assumed I should accept all other values as automatic fan speed control.
If that is not the case, what is the correct way to handle this attribute?
> > + return regmap_clear_bits(state->regmap, REG_CR2, CR2_DFC);
> > + case hwmon_pwm_freq:
> > + pwm_index = find_closest(val, max31760_pwm_freq,
> > + ARRAY_SIZE(max31760_pwm_freq));
> > +
> > + return regmap_update_bits(state->regmap,
> > + REG_CR1, CR1_DRV,
> > + FIELD_PREP(CR1_DRV, pwm_index));
...
