lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHCvCEcGvrS=3p2Whj0Cmx9sx+aSzX2097LahQ=f3eRCCAN_bA@mail.gmail.com>
Date:   Mon, 29 Aug 2022 08:09:21 -0700
From:   Justin Ledford <justinledford@...gle.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
        linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: (max31790) add fanN_enable

The tach input isn't enabled in the device by default. So the only way
to start using the fan input sensors is to set the regulator mode
through the driver to RPM mode and then back to whatever mode you
actually want to use. The I2C interface to the device doesn't couple
the tach input to the regulator mode so I don't think it makes sense
for the driver to do this either.

On Mon, Aug 29, 2022 at 6:20 AM Guenter Roeck <linux@...ck-us.net> wrote:
>
> On Mon, Aug 29, 2022 at 02:43:51AM +0000, Justin Ledford wrote:
> > The MAX31790 has a tach input enable bit in each fan's configuration
> > register. This is only enabled by the driver if RPM mode is selected,
> > but the driver doesn't provide a way to independently enable tachometer
> > input regardless of the regulator mode.
> >
> > By adding the fanN_enable sysfs files, we can decouple the tach input
> > from the regulator mode. Also update the documentation.
> >
> > Signed-off-by: Justin Ledford <justinledford@...gle.com>
> > ---
> >  Documentation/hwmon/max31790.rst |  1 +
> >  drivers/hwmon/max31790.c         | 44 +++++++++++++++++++++++++++-----
> >  2 files changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/hwmon/max31790.rst b/Documentation/hwmon/max31790.rst
> > index 7b097c3b9b90..33c5c7330efc 100644
> > --- a/Documentation/hwmon/max31790.rst
> > +++ b/Documentation/hwmon/max31790.rst
> > @@ -38,6 +38,7 @@ Sysfs entries
> >  fan[1-12]_input    RO  fan tachometer speed in RPM
> >  fan[1-12]_fault    RO  fan experienced fault
> >  fan[1-6]_target    RW  desired fan speed in RPM
> > +fan[1-6]_enable    RW  enable or disable the tachometer input
> >  pwm[1-6]_enable    RW  regulator mode, 0=disabled (duty cycle=0%), 1=manual mode, 2=rpm mode
> >  pwm[1-6]           RW  read: current pwm duty cycle,
> >                         write: target pwm duty cycle (0-255)
> > diff --git a/drivers/hwmon/max31790.c b/drivers/hwmon/max31790.c
> > index 7e9362f6dc29..3ae02be4b41e 100644
> > --- a/drivers/hwmon/max31790.c
> > +++ b/drivers/hwmon/max31790.c
> > @@ -118,6 +118,12 @@ static struct max31790_data *max31790_update_device(struct device *dev)
> >                                       goto abort;
> >                               data->target_count[i] = rv;
> >                       }
> > +
> > +                     rv = i2c_smbus_read_byte_data(client,
> > +                                     MAX31790_REG_FAN_CONFIG(i));
> > +                     if (rv < 0)
> > +                             goto abort;
> > +                     data->fan_config[i] = rv;
>
> Why is this needed ?
>
> Guenter
>
> >               }
> >
> >               data->last_updated = jiffies;
> > @@ -202,6 +208,9 @@ static int max31790_read_fan(struct device *dev, u32 attr, int channel,
> >               }
> >               mutex_unlock(&data->update_lock);
> >               return 0;
> > +     case hwmon_fan_enable:
> > +             *val = !!(data->fan_config[channel] & MAX31790_FAN_CFG_TACH_INPUT_EN);
> > +             return 0;
> >       default:
> >               return -EOPNOTSUPP;
> >       }
> > @@ -214,7 +223,7 @@ static int max31790_write_fan(struct device *dev, u32 attr, int channel,
> >       struct i2c_client *client = data->client;
> >       int target_count;
> >       int err = 0;
> > -     u8 bits;
> > +     u8 bits, fan_config;
> >       int sr;
> >
> >       mutex_lock(&data->update_lock);
> > @@ -243,6 +252,23 @@ static int max31790_write_fan(struct device *dev, u32 attr, int channel,
> >                                       MAX31790_REG_TARGET_COUNT(channel),
> >                                       data->target_count[channel]);
> >               break;
> > +     case hwmon_fan_enable:
> > +             fan_config = data->fan_config[channel];
> > +             if (val == 0) {
> > +                     fan_config &= ~MAX31790_FAN_CFG_TACH_INPUT_EN;
> > +             } else if (val == 1) {
> > +                     fan_config |= MAX31790_FAN_CFG_TACH_INPUT_EN;
> > +             } else {
> > +                     err = -EINVAL;
> > +                     break;
> > +             }
> > +             if (fan_config != data->fan_config[channel]) {
> > +                     err = i2c_smbus_write_byte_data(client, MAX31790_REG_FAN_CONFIG(channel),
> > +                                                     fan_config);
> > +                     if (!err)
> > +                             data->fan_config[channel] = fan_config;
> > +             }
> > +             break;
> >       default:
> >               err = -EOPNOTSUPP;
> >               break;
> > @@ -270,6 +296,10 @@ static umode_t max31790_fan_is_visible(const void *_data, u32 attr, int channel)
> >                   !(fan_config & MAX31790_FAN_CFG_TACH_INPUT))
> >                       return 0644;
> >               return 0;
> > +     case hwmon_fan_enable:
> > +             if (channel < NR_CHANNEL)
> > +                     return 0644;
> > +             return 0;
> >       default:
> >               return 0;
> >       }
> > @@ -423,12 +453,12 @@ static umode_t max31790_is_visible(const void *data,
> >
> >  static const struct hwmon_channel_info *max31790_info[] = {
> >       HWMON_CHANNEL_INFO(fan,
> > -                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT,
> > -                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT,
> > -                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT,
> > -                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT,
> > -                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT,
> > -                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT,
> > +                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT | HWMON_F_ENABLE,
> > +                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT | HWMON_F_ENABLE,
> > +                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT | HWMON_F_ENABLE,
> > +                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT | HWMON_F_ENABLE,
> > +                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT | HWMON_F_ENABLE,
> > +                        HWMON_F_INPUT | HWMON_F_TARGET | HWMON_F_FAULT | HWMON_F_ENABLE,
> >                          HWMON_F_INPUT | HWMON_F_FAULT,
> >                          HWMON_F_INPUT | HWMON_F_FAULT,
> >                          HWMON_F_INPUT | HWMON_F_FAULT,
> > --
> > 2.37.2.672.g94769d06f0-goog
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ