On Mon, 2022-09-19 at 13:18 +0100, Hans de Goede wrote:
> Hi Luke,
>
> On 9/16/22 01:46, Luke D. Jones wrote:
> > The previously added patch to add support for pwm change for TUF
> > laptops
> > also is usuable for more than TUF. The same method `0x00110014` is
> > used to read the fan RPM.
> >
> > Add two extra attributes for reading fan2 plus fan2 label.
> >
> > Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx>
> > ---
> > drivers/platform/x86/asus-wmi.c | 36
> > +++++++++++++++++++++++++++++++--
> > 1 file changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c
> > b/drivers/platform/x86/asus-wmi.c
> > index ae46af731de9..7fe6ce25da0a 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -72,6 +72,7 @@ module_param(fnlock_default, bool, 0444);
> >
> > #define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)
> >
> > +#define ASUS_GPU_FAN_DESC "gpu_fan"
> > #define ASUS_FAN_DESC "cpu_fan"
> > #define ASUS_FAN_MFUN 0x13
> > #define ASUS_FAN_SFUN_READ 0x06
> > @@ -2078,6 +2079,30 @@ static ssize_t asus_hwmon_temp1(struct
> > device *dev,
> > }
> >
> > /* GPU fan on modern ROG laptops */
> > +static ssize_t fan2_input_show(struct device *dev,
> > + struct device_attribute
> > *attr,
> > + char *buf)
> > +{
> > + struct asus_wmi *asus = dev_get_drvdata(dev);
> > + int value;
> > + int ret;
> > +
> > + ret = asus_wmi_get_devstate(asus,
> > ASUS_WMI_DEVID_GPU_FAN_CTRL, &value);
> > + if (ret < 0)
> > + return ret;
> > +
> > + value &= 0xffff;
> > +
> > + return sysfs_emit(buf, "%d\n", value < 0 ? -1 : value *
> > 100);
>
> As already mentioned since you & with 0xffff above the sign bit can
> never be
> set, so the value is never less then < 0, so I have simplified this
> to:
>
> return sysfs_emit(buf, "%d\n", value * 100);
>
> while merging.
Thanks Hans really appreciate it, sorry i missed that. My workload is a
bit too large at the moment :)
Cheers,
Luke.
>
> > +}
> > +
> > +static ssize_t fan2_label_show(struct device *dev,
> > + struct device_attribute
> > *attr,
> > + char *buf)
> > +{
> > + return sprintf(buf, "%s\n", ASUS_GPU_FAN_DESC);
> > +}
>
> And here I have done s/sprintf/sysfs_emit/ with those changes
> I've applied this patch to my review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> Note it will show up in my review-hans branch once I've pushed my
> local branch there, which might take a while.
>
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
>
> Regards,
>
> Hans
>
>
>
> > +
> > static ssize_t pwm2_enable_show(struct device *dev,
> > struct device_attribute *attr,
> > char *buf)
> > @@ -2127,9 +2152,12 @@ static ssize_t pwm2_enable_store(struct
> > device *dev,
> > /* Fan1 */
> > static DEVICE_ATTR_RW(pwm1);
> > static DEVICE_ATTR_RW(pwm1_enable);
> > -static DEVICE_ATTR_RW(pwm2_enable);
> > static DEVICE_ATTR_RO(fan1_input);
> > static DEVICE_ATTR_RO(fan1_label);
> > +/* Fan2 - GPU fan */
> > +static DEVICE_ATTR_RW(pwm2_enable);
> > +static DEVICE_ATTR_RO(fan2_input);
> > +static DEVICE_ATTR_RO(fan2_label);
> >
> > /* Temperature */
> > static DEVICE_ATTR(temp1_input, S_IRUGO, asus_hwmon_temp1, NULL);
> > @@ -2140,6 +2168,8 @@ static struct attribute *hwmon_attributes[] =
> > {
> > &dev_attr_pwm2_enable.attr,
> > &dev_attr_fan1_input.attr,
> > &dev_attr_fan1_label.attr,
> > + &dev_attr_fan2_input.attr,
> > + &dev_attr_fan2_label.attr,
> >
> > &dev_attr_temp1_input.attr,
> > NULL
> > @@ -2160,7 +2190,9 @@ static umode_t
> > asus_hwmon_sysfs_is_visible(struct kobject *kobj,
> > || attr == &dev_attr_pwm1_enable.attr) {
> > if (asus->fan_type == FAN_TYPE_NONE)
> > return 0;
> > - } else if (attr == &dev_attr_pwm2_enable.attr) {
> > + } else if (attr == &dev_attr_fan2_input.attr
> > + || attr == &dev_attr_fan2_label.attr
> > + || attr == &dev_attr_pwm2_enable.attr) {
> > if (asus->gpu_fan_type == FAN_TYPE_NONE)
> > return 0;
> > } else if (attr == &dev_attr_temp1_input.attr) {
>
