Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asus G752VT, sometimes "failed to get rpm" #66

Open
rmanosuthi opened this issue Nov 23, 2018 · 9 comments
Open

Asus G752VT, sometimes "failed to get rpm" #66

rmanosuthi opened this issue Nov 23, 2018 · 9 comments

Comments

@rmanosuthi
Copy link

Hello,

I'm having a very strange problem with this module... sometimes the fans are recognized in sensors and /sys/class/hwmon/hwmonX but sometimes they aren't. NBFC reliably works with this laptop for reference.

Here's the dmesg log (I manually unloaded and reloaded it multiple times)

[   33.511056] asus-fan (init) - dmi sys info: 'ASUSTeK COMPUTER INC.'
[   33.511057] asus-fan (init) - dmi product: 'G752VT'
[   33.511197] asus-fan (init) - fan-id: 1 | failed to get rpm
[   33.511294] asus-fan (init) - fan-id: 2 | success getting rpm
[   33.511295] asus-fan (init) - temp-id: 1 | success getting temp
[   33.512375] asus-fan (init) - created hwmon device: hwmon4
[   33.512376] asus-fan (init) - finished init, found 2 fan(s) to control
[   44.059532] asus-fan (exit) - module unloaded---cleaning up
[   46.667862] asus-fan (init) - dmi sys info: 'ASUSTeK COMPUTER INC.'
[   46.667862] asus-fan (init) - dmi product: 'G752VT'
[   46.668043] asus-fan (init) - fan-id: 1 | failed to get rpm
[   46.668132] asus-fan (init) - fan-id: 2 | failed to get rpm
[   46.668132] asus-fan (init) - temp-id: 1 | success getting temp
[   46.669254] asus-fan (init) - created hwmon device: hwmon5
[   46.669254] asus-fan (init) - finished init, found 1 fan(s) to control
[   58.465439] asus-fan (exit) - module unloaded---cleaning up
[   60.387819] asus-fan (init) - dmi sys info: 'ASUSTeK COMPUTER INC.'
[   60.387819] asus-fan (init) - dmi product: 'G752VT'
[   60.387995] asus-fan (init) - fan-id: 1 | failed to get rpm
[   60.388081] asus-fan (init) - fan-id: 2 | failed to get rpm
[   60.388082] asus-fan (init) - temp-id: 1 | success getting temp
[   60.389486] asus-fan (init) - created hwmon device: hwmon6
[   60.389487] asus-fan (init) - finished init, found 1 fan(s) to control
[  102.007407] asus-fan (exit) - module unloaded---cleaning up
[  104.163909] asus-fan (init) - dmi sys info: 'ASUSTeK COMPUTER INC.'
[  104.163909] asus-fan (init) - dmi product: 'G752VT'
[  104.164084] asus-fan (init) - fan-id: 1 | failed to get rpm
[  104.164168] asus-fan (init) - fan-id: 2 | failed to get rpm
[  104.164168] asus-fan (init) - temp-id: 1 | success getting temp
[  104.165261] asus-fan (init) - created hwmon device: hwmon7
[  104.165262] asus-fan (init) - finished init, found 1 fan(s) to control
[  206.652514] asus-fan (exit) - module unloaded---cleaning up
[  211.473109] asus-fan (init) - dmi sys info: 'ASUSTeK COMPUTER INC.'
[  211.473109] asus-fan (init) - dmi product: 'G752VT'
[  211.473110] asus-fan (init) - forced loading of module: USE WITH CARE
[  211.473304] asus-fan (init) - created hwmon device: hwmon8
[  211.473305] asus-fan (init) - finished init, found 2 fan(s) to control
[  275.701564] asus-fan (exit) - module unloaded---cleaning up
[  278.071835] asus-fan (init) - dmi sys info: 'ASUSTeK COMPUTER INC.'
[  278.071836] asus-fan (init) - dmi product: 'G752VT'
[  278.071961] asus-fan (init) - fan-id: 1 | failed to get rpm
[  278.072047] asus-fan (init) - fan-id: 2 | success getting rpm
[  278.072048] asus-fan (init) - temp-id: 1 | success getting temp
[  278.073158] asus-fan (init) - created hwmon device: hwmon9
[  278.073159] asus-fan (init) - finished init, found 2 fan(s) to control
[  659.496634] asus-fan (exit) - module unloaded---cleaning up
[  662.941816] asus-fan (init) - dmi sys info: 'ASUSTeK COMPUTER INC.'
[  662.941817] asus-fan (init) - dmi product: 'G752VT'
[  662.941984] asus-fan (init) - fan-id: 1 | success getting rpm
[  662.942084] asus-fan (init) - fan-id: 2 | success getting rpm
[  662.942084] asus-fan (init) - temp-id: 1 | success getting temp
[  662.943190] asus-fan (init) - created hwmon device: hwmon10
[  662.943191] asus-fan (init) - finished init, found 2 fan(s) to control
^ yay, they now show up in sensors!

asus-wmi is not loaded. Even when it's loaded the same behaviour applies.

Here's my current kernel config since I'm on Gentoo
config-4.19.3-gentoo.txt

@daringer
Copy link
Owner

I've seen exactly the same behavior with my UV32VD zenbook and have to the slightest idea why this is happening, in fact, and I've also looked into this earlier, simply the rpm outputs are garbage, thus the module avoids it's initialization.

as a quick hack you might simply try removing the rpm checks (your log even shows that the first try for fan1 works but not for fan2, then both do not work, then again 1 and not 2, finally both work and the module inits.

Sooo, if you remove like this:

if (rpm == -1) {

and this:
if (rpm == -1) {

if-clause, you might overcome this error and the module should always load (because the check afterwards never fails in your log)...

pretty hacky, but make it an parameter for the module during insmod and I'll happily take it in with a pull-request...

@savisitor15
Copy link
Contributor

Happy to look into this one as I have the same line of laptop.

@daringer
Copy link
Owner

fix merged, thank you @savisitor15

@f-r-i-t-z
Copy link

The G752VT have the 0 RPM and DISABLE FAN. When we get -1, this mean it's disabled.

@savisitor15
Copy link
Contributor

savisitor15 commented Jun 30, 2019

If you're running the latest build you can simply give it the module parameter: force_rpm_override=1
It's like a force_load=1 but just slightly lighter.

the issue is the ACPI driver, it sometimes returns hex(ff) for the fan speed, even though it does actually report there being a fan.

@f-r-i-t-z
Copy link

@daringer YET about this device ASUS G752VT ( Maybe variants).
They work well, both fan's are recognized besides we need to use the <force_rpm_override=1>.
We have another "THING" about this device:
The GFX Temperature sensor use the <\_SB.PCI0.LPCB.EC0.TH0R> instead <\_SB.PCI0.LPCB.EC0.TH1R>.
No problem, I created a VARIANT IF/ELSE condition using by base the @HorstBaerbel asus-fan mod for their specific device and I get an OK when the module try to identify the GFX termal.
But I don't have READINGS for this TEMP.
I'm missing something or the
<
// try to read temprature
// size_t temp = temp1_input(asus_data.hwmon_dev, ); // @todo

Comments means we dont have YET this working?
Thank you!

@daringer daringer reopened this Jul 3, 2019
@f-r-i-t-z
Copy link

f-r-i-t-z commented Jul 3, 2019

More about my findings..
I'm trying to debug what is wrong ( OBS: I'm not versed in C*.* ):

  if (asus_data.variant == ASUS_FAN_HW_G752VT)
  {
    dbg_msg("|--> evaluate acpi request: \\_SB.PCI0.LPCB.EC0.TH0R");
    ret = acpi_evaluate_integer(NULL, "\\_SB.PCI0.LPCB.EC0.TH0R", NULL, &value);
    dbg_msg("|--> acpi request returned: %s", acpi_format_exception(ret));
    dbg_msg("|--> acpi request returned value: %d", value);
  }
  else
  {
    dbg_msg("|--> evaluate acpi request: \\_SB.PCI0.LPCB.EC0.TH1R");
    ret = acpi_evaluate_integer(NULL, "\\_SB.PCI0.LPCB.EC0.TH1R", NULL, &value);
    dbg_msg("|--> acpi request returned: %s", acpi_format_exception(ret));
    dbg_msg("|--> acpi request returned value: %d", value);
  }

  if (ret != AE_OK) {
    err_msg("read_temp", "failed reading temperature, errcode: %s",
     acpi_format_exception(ret));
    return ret;
  }
  size = sprintf((char *)&buf, "%llu\n", value);
  dbg_msg("|--> size: %d", size);
  return size;
}

static ssize_t temp1_label(struct device *dev, struct device_attribute *attr,
                           char *buf) {
  dbg_msg("|--> TEMP1_LABEL: %s", sprintf(buf, "%s\n", TEMP1_LABEL));
  return sprintf(buf, "%s\n", TEMP1_LABEL);
}

static ssize_t temp1_crit(struct device *dev, struct device_attribute *attr,
                          char *buf) {
  dbg_msg("|--> TEMP1_CRIT: %d", sprintf(buf, "%d\n", TEMP1_CRIT));
  return sprintf(buf, "%d\n", TEMP1_CRIT);
}

I GET this return.

[ 5825.433348] asus-fan (debug) - |--> TEMP1_LABEL: (efault)
[ 5825.433378] asus-fan (debug) - temp-id: 1 | get (acpi eval)
[ 5825.433379] asus-fan (debug) - |--> evaluate acpi request: \_SB.PCI0.LPCB.EC0.TH0R
[ 5825.433866] asus-fan (debug) - |--> acpi request returned: AE_OK
[ 5825.433869] asus-fan (debug) - |--> acpi request returned value: 58
[ 5825.433871] asus-fan (debug) - |--> size: 3
[ 5825.433940] asus-fan (debug) - |--> TEMP1_CRIT: 4

for some reason I cant get this values passed to;

    if (temp != AE_OK) {
      err_msg("init", "temperature read probe failed");
    } else {
      info_msg("init", "temp-id: 1 | success getting temp");
      hwmon_attrs[11] = &dev_attr_temp1_input.attr;
      hwmon_attrs[12] = &dev_attr_temp1_label.attr;
      hwmon_attrs[13] = &dev_attr_temp1_crit.attr;
    }

So!
Can you or someone help me to debug this in a better way?
I have a lot of free time and the device to test.

@daringer
Copy link
Owner

daringer commented Aug 4, 2019

ok, took some time, but at least have some ideas/clarifications:

  • line 976 is likely not causing problems, this is just my fail as I originally thought it is possible to read out the "critical temperature threshold", instead just the #define const is written
  • TEMP1_CRIT seems to be 4 (as you pasted) this feels broken, guess it is #defined as 105
  • The thing to do here is to if-else the asus_data.variant as you've already done, then the sprintf()-call will write the temperature as %llu into buf (which represents the output seen on cat .../hwmonX/temp1_input)
  • The last snipped you pasted just either ensures that input1_temp is available within the hwmon-files or leaves hwmon_attrs[11...13] equal to NULL in order to not show it.

Ok so far about the concepts here, to make it work I would propose trying the following:

  • remove the if() i.e., comment it out, thus the hwmon_attrs[...] lines will always be executed, here I would expect that all should work for your machine (at least from what is shown in your dmesg output)
  • once you verified that it should overall work for your setup you just have to cope with the replacement of the removed if(), in fact it should already be a disabled if(), as temp shall always be AE_OK as default ( to be seen here )

Just checked the current master for now @HorstBaerbel 's restructuring will ease these discussions massively... so running to review it

@f-r-i-t-z
Copy link

Hello @daringer ! I'm back from the cave after my first report...
The project still under development?

So! Now I'm using another model from Asus ROG. the GT 752VY ( basically the same from previous report) Only the vk and VSK variant are different.

The system still not report the correct temperature.
My actual findings:

Using the original code just changing the \\_SB.PCI0.LPCB.EC0.TH1R to \\_SB.PCI0.LPCB.EC0.TH0R and add the dbg_msg("|--> size: %llu\n", value);
dbg_msg("|--> TEMP1_LABEL: %s\n", TEMP1_LABEL);
dbg_msg("|--> TEMP1_CRIT %d\n", TEMP1_CRIT);
to receive some debug:

static ssize_t temp1_input(struct device *dev, struct device_attribute *attr,
                           char *buf) {
  acpi_status ret;
  unsigned long long int value;
  ssize_t size = 0;

  dbg_msg("temp-id: 1 | get (acpi eval)");

  // acpi call
  ret = acpi_evaluate_integer(NULL, "\\_SB.PCI0.LPCB.EC0.TH1R", NULL, &value);
  if (ret != AE_OK) {
    err_msg("read_temp", "failed reading temperature, errcode: %s",
     acpi_format_exception(ret));
    return ret;
  }
  size = sprintf((char *)&buf, "%llu\n", value);
	  dbg_msg("|--> size: %llu\n", size);
  return size;
}

static ssize_t temp1_label(struct device *dev, struct device_attribute *attr,
                           char *buf) {
	dbg_msg("|--> TEMP1_LABEL: %s\n", TEMP1_LABEL);
  return sprintf(buf, "%s\n", TEMP1_LABEL);
}

static ssize_t temp1_crit(struct device *dev, struct device_attribute *attr,
                          char *buf) {
	dbg_msg("|--> TEMP1_CRIT %d\n", TEMP1_CRIT);
  return sprintf(buf, "%d\n", TEMP1_CRIT);
}

I'm receiving this result:

asus-fan (debug) - |--> acpi request returned: AE_OK
asus-fan (debug) - fan-id: 1 | get RPM
asus-fan (debug) - fan-id: 1 | get control state
asus-fan (debug) - |--> TEMP1_CRIT 105
asus-fan (debug) - temp-id: 1 | get (acpi eval)
asus-fan (debug) - |--> size: 3
asus-fan (debug) - |--> TEMP1_LABEL: gfx_temp

and inside sensors:

CPU Fan:     1832 RPM  (min =   11 RPM, max =  256 RPM)
GFX Fan:     1672 RPM  (min =   11 RPM, max =  256 RPM)
gfx_tem:          N/A  (crit =  +0.1°C)

Why we are using size = sprintf((char *)&buf, "%llu\n", value); ? We get just the SIZE from the value and not the Temperature value by itself.
I'm missing something?

After change the line size = sprintf((char *)&buf, "%llu\n", value); to size = sprintf(buf, "%llu\n", value); the result change to

asus-fan (debug) - |--> TEMP1_CRIT 105
asus-fan (debug) - temp-id: 1 | get (acpi eval)
asus-fan (debug) - |--> ret: 0
asus-fan (debug) - |--> size: 73
asus-fan (debug) - |--> TEMP1_LABEL: gfx_temp

and inside sensors:

CPU Fan:     2823 RPM  (min =   11 RPM, max =  256 RPM)
GFX Fan:     1672 RPM  (min =   11 RPM, max =  256 RPM)
gfx_temp:      +0.1°C  (crit =  +0.1°C)

Thank you again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants