linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: new asus fan driver
@ 2013-11-05  8:59 Felipe Contreras
  2013-11-05 12:52 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Contreras @ 2013-11-05  8:59 UTC (permalink / raw)
  To: devel; +Cc: linux-kernel, Greg Kroah-Hartman, Matthew Garrett, Felipe Contreras

Simple driver to enable control of the fan in ASUS laptops. So far this
has only been tested in ASUS Zenbook Prime UX31A, but according to some
online reference [1], it should work in other models as well.

The implementation is very straight-forward, the only caveat is that the
fan speed needs to be saved after it has been manually changed because
it won't be reported properly until it goes back to 'auto' mode.

[1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Two incarnations of this code exists [1][2], one that used ACPI methods
directly, and one through WMI. Unfortunately the WMI version needs us to pass
physicall addresses which is not exactly clean, and that's the only reason the
code is proposed for staging.

Most likely this cannot graduate until acpica gets support to receive virtual
addresses.

 [1] http://article.gmane.org/gmane.linux.power-management.general/36774
 [2] http://article.gmane.org/gmane.linux.kernel/1576463

 drivers/staging/Kconfig                     |   2 +
 drivers/staging/Makefile                    |   1 +
 drivers/staging/asus-thermal/Kconfig        |   7 ++
 drivers/staging/asus-thermal/asus_thermal.c | 166 ++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+)
 create mode 100644 drivers/staging/asus-thermal/Kconfig
 create mode 100644 drivers/staging/asus-thermal/asus_thermal.c

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 3626dbc8..b245af5 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -148,4 +148,6 @@ source "drivers/staging/dgnc/Kconfig"
 
 source "drivers/staging/dgap/Kconfig"
 
+source "drivers/staging/asus-thermal/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index d1b4b80..70e4456 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -66,3 +66,4 @@ obj-$(CONFIG_USB_BTMTK)		+= btmtk_usb/
 obj-$(CONFIG_XILLYBUS)		+= xillybus/
 obj-$(CONFIG_DGNC)			+= dgnc/
 obj-$(CONFIG_DGAP)			+= dgap/
+obj-$(CONFIG_ASUS_THERMAL)	+= asus-thermal/
diff --git a/drivers/staging/asus-thermal/Kconfig b/drivers/staging/asus-thermal/Kconfig
new file mode 100644
index 0000000..8bf7db4
--- /dev/null
+++ b/drivers/staging/asus-thermal/Kconfig
@@ -0,0 +1,7 @@
+config ASUS_THERMAL
+	tristate "ASUS thermal driver"
+	depends on THERMAL
+	depends on X86
+	help
+	  Enables control of the fan in ASUS laptops.
+
diff --git a/drivers/staging/asus-thermal/asus_thermal.c b/drivers/staging/asus-thermal/asus_thermal.c
new file mode 100644
index 0000000..a84856e
--- /dev/null
+++ b/drivers/staging/asus-thermal/asus_thermal.c
@@ -0,0 +1,166 @@
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/acpi.h>
+#include <linux/thermal.h>
+#include <linux/dmi.h>
+
+MODULE_AUTHOR("Felipe Contreras <felipe.contreras@gmail.com>");
+MODULE_DESCRIPTION("ASUS fan driver");
+MODULE_LICENSE("GPL");
+
+#define ASUS_WMI_MGMT_GUID "97845ED0-4E6D-11DE-8A39-0800200C9A66"
+#define ASUS_WMI_METHODID_AGFN 0x4E464741
+#define ASUS_WMI_UNSUPPORTED_METHOD 0xFFFFFFFE
+
+static struct thermal_cooling_device *fan;
+static int fan_speed;
+
+struct bios_args {
+	u32 arg0;
+	u32 arg1;
+} __packed;
+
+struct fan_args {
+	u16 mfun;
+	u16 sfun;
+	u16 len;
+	u8 stas;
+	u8 err;
+	u8 fan;
+	u32 speed;
+} __packed;
+
+/*
+ * Copied from [1], where this code belongs.
+ * [1] drivers/platform/x86/asus-wmi.c
+ */
+static int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1,
+				    u32 *retval)
+{
+	struct bios_args args = {
+		.arg0 = arg0,
+		.arg1 = arg1,
+	};
+	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_status status;
+	union acpi_object *obj;
+	u32 tmp;
+
+	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 1, method_id,
+				     &input, &output);
+
+	if (ACPI_FAILURE(status))
+		goto exit;
+
+	obj = (union acpi_object *)output.pointer;
+	if (obj && obj->type == ACPI_TYPE_INTEGER)
+		tmp = (u32) obj->integer.value;
+	else
+		tmp = 0;
+
+	if (retval)
+		*retval = tmp;
+
+	kfree(obj);
+
+exit:
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	if (tmp == ASUS_WMI_UNSUPPORTED_METHOD)
+		return -ENODEV;
+
+	return 0;
+}
+
+static int fan_speed_helper(int write, int fan, unsigned long *speed)
+{
+	struct fan_args args = {
+		.len = sizeof(args),
+		.mfun = 0x13,
+		.sfun = write ? 0x07 : 0x06,
+		.fan = fan,
+		.speed = write ? *speed : 0,
+	};
+	int r;
+	u32 value;
+
+	if (!write && fan_speed >= 0) {
+		*speed = fan_speed;
+		return 0;
+	}
+
+	r = asus_wmi_evaluate_method(ASUS_WMI_METHODID_AGFN, virt_to_phys(&args), 0, &value);
+	if (r)
+		return r;
+	if (value || args.err)
+		return -EINVAL;
+
+	if (write)
+		fan_speed = fan > 0 ? *speed : -1;
+	else
+		*speed = args.speed;
+
+	return 0;
+}
+
+static int fan_set_auto(void)
+{
+	unsigned long speed = 0;
+	return fan_speed_helper(1, 0, &speed);
+}
+
+static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	*state = 0xff;
+	return 0;
+}
+
+static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	return fan_speed_helper(0, 1, state);
+}
+
+static int fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
+{
+	return fan_speed_helper(1, 1, &state);
+}
+
+static const struct thermal_cooling_device_ops fan_cooling_ops = {
+	.get_max_state = fan_get_max_state,
+	.get_cur_state = fan_get_cur_state,
+	.set_cur_state = fan_set_cur_state,
+};
+
+static int __init fan_init(void)
+{
+	struct thermal_cooling_device *cdev;
+	int r;
+
+	if (!wmi_has_guid(ASUS_WMI_MGMT_GUID))
+		return -ENODEV;
+
+	r = fan_set_auto();
+	if (r)
+		return r;
+
+	cdev = thermal_cooling_device_register("Fan", NULL, &fan_cooling_ops);
+	if (IS_ERR(cdev))
+		return PTR_ERR(cdev);
+	fan = cdev;
+	return 0;
+}
+
+static void __exit fan_exit(void)
+{
+	if (!fan)
+		return;
+	fan_set_auto();
+	thermal_cooling_device_unregister(fan);
+	fan = NULL;
+}
+
+module_init(fan_init);
+module_exit(fan_exit);
-- 
1.8.4.2+fc1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-05  8:59 [PATCH] staging: new asus fan driver Felipe Contreras
@ 2013-11-05 12:52 ` Greg Kroah-Hartman
  2013-11-05 14:29   ` Felipe Contreras
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-05 12:52 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: devel, linux-kernel, Matthew Garrett

On Tue, Nov 05, 2013 at 02:59:47AM -0600, Felipe Contreras wrote:
> Simple driver to enable control of the fan in ASUS laptops. So far this
> has only been tested in ASUS Zenbook Prime UX31A, but according to some
> online reference [1], it should work in other models as well.
> 
> The implementation is very straight-forward, the only caveat is that the
> fan speed needs to be saved after it has been manually changed because
> it won't be reported properly until it goes back to 'auto' mode.
> 
> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> 
> Two incarnations of this code exists [1][2], one that used ACPI methods
> directly, and one through WMI. Unfortunately the WMI version needs us to pass
> physicall addresses which is not exactly clean, and that's the only reason the
> code is proposed for staging.
> 
> Most likely this cannot graduate until acpica gets support to receive virtual
> addresses.

When is that going to happen?

> 
>  [1] http://article.gmane.org/gmane.linux.power-management.general/36774
>  [2] http://article.gmane.org/gmane.linux.kernel/1576463
> 
>  drivers/staging/Kconfig                     |   2 +
>  drivers/staging/Makefile                    |   1 +
>  drivers/staging/asus-thermal/Kconfig        |   7 ++
>  drivers/staging/asus-thermal/asus_thermal.c | 166 ++++++++++++++++++++++++++++

I need a TODO file for a staging driver listing who is responsible for
it, and what is needed to be done to it in order to get it out of
staging.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-05 12:52 ` Greg Kroah-Hartman
@ 2013-11-05 14:29   ` Felipe Contreras
  2013-11-05 15:16     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Contreras @ 2013-11-05 14:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: open list:STAGING SUBSYSTEM, Linux Kernel Mailing List, Matthew Garrett

On Tue, Nov 5, 2013 at 6:52 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Nov 05, 2013 at 02:59:47AM -0600, Felipe Contreras wrote:
>> Simple driver to enable control of the fan in ASUS laptops. So far this
>> has only been tested in ASUS Zenbook Prime UX31A, but according to some
>> online reference [1], it should work in other models as well.
>>
>> The implementation is very straight-forward, the only caveat is that the
>> fan speed needs to be saved after it has been manually changed because
>> it won't be reported properly until it goes back to 'auto' mode.
>>
>> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> Two incarnations of this code exists [1][2], one that used ACPI methods
>> directly, and one through WMI. Unfortunately the WMI version needs us to pass
>> physicall addresses which is not exactly clean, and that's the only reason the
>> code is proposed for staging.
>>
>> Most likely this cannot graduate until acpica gets support to receive virtual
>> addresses.
>
> When is that going to happen?

I don't know. Presumably when I get it done, which might be never, or
a few days from now. Most likely it will take some time.

>>  [1] http://article.gmane.org/gmane.linux.power-management.general/36774
>>  [2] http://article.gmane.org/gmane.linux.kernel/1576463
>>
>>  drivers/staging/Kconfig                     |   2 +
>>  drivers/staging/Makefile                    |   1 +
>>  drivers/staging/asus-thermal/Kconfig        |   7 ++
>>  drivers/staging/asus-thermal/asus_thermal.c | 166 ++++++++++++++++++++++++++++
>
> I need a TODO file for a staging driver listing who is responsible for
> it, and what is needed to be done to it in order to get it out of
> staging.

Done.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-05 14:29   ` Felipe Contreras
@ 2013-11-05 15:16     ` Greg Kroah-Hartman
  2013-11-05 19:54       ` Felipe Contreras
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-05 15:16 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: open list:STAGING SUBSYSTEM, Linux Kernel Mailing List, Matthew Garrett

On Tue, Nov 05, 2013 at 08:29:40AM -0600, Felipe Contreras wrote:
> On Tue, Nov 5, 2013 at 6:52 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Nov 05, 2013 at 02:59:47AM -0600, Felipe Contreras wrote:
> >> Simple driver to enable control of the fan in ASUS laptops. So far this
> >> has only been tested in ASUS Zenbook Prime UX31A, but according to some
> >> online reference [1], it should work in other models as well.
> >>
> >> The implementation is very straight-forward, the only caveat is that the
> >> fan speed needs to be saved after it has been manually changed because
> >> it won't be reported properly until it goes back to 'auto' mode.
> >>
> >> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
> >>
> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> ---
> >>
> >> Two incarnations of this code exists [1][2], one that used ACPI methods
> >> directly, and one through WMI. Unfortunately the WMI version needs us to pass
> >> physicall addresses which is not exactly clean, and that's the only reason the
> >> code is proposed for staging.
> >>
> >> Most likely this cannot graduate until acpica gets support to receive virtual
> >> addresses.
> >
> > When is that going to happen?
> 
> I don't know. Presumably when I get it done, which might be never, or
> a few days from now. Most likely it will take some time.

Then why not just merge this to the proper place, instead of relying on
staging?  I don't want to have to have a staging driver that is waiting
on external things to get it merged, instead of issues with the driver
itself, as you are now putting the burden of maintenance on me, for no
good reason other than you being too "lazy" to get other stuff done :)

So, I really don't want this driver, sorry, please merge it to the
"proper" place in the kernel itself, fixing up the ACPI core to do so,
or just living with the driver as-is if you don't want to do that
work...

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-05 15:16     ` Greg Kroah-Hartman
@ 2013-11-05 19:54       ` Felipe Contreras
  2013-11-06  2:19         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Contreras @ 2013-11-05 19:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: open list:STAGING SUBSYSTEM, Linux Kernel Mailing List, Matthew Garrett

On Tue, Nov 5, 2013 at 9:16 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Nov 05, 2013 at 08:29:40AM -0600, Felipe Contreras wrote:
>> On Tue, Nov 5, 2013 at 6:52 AM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > On Tue, Nov 05, 2013 at 02:59:47AM -0600, Felipe Contreras wrote:
>> >> Simple driver to enable control of the fan in ASUS laptops. So far this
>> >> has only been tested in ASUS Zenbook Prime UX31A, but according to some
>> >> online reference [1], it should work in other models as well.
>> >>
>> >> The implementation is very straight-forward, the only caveat is that the
>> >> fan speed needs to be saved after it has been manually changed because
>> >> it won't be reported properly until it goes back to 'auto' mode.
>> >>
>> >> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
>> >>
>> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> >> ---
>> >>
>> >> Two incarnations of this code exists [1][2], one that used ACPI methods
>> >> directly, and one through WMI. Unfortunately the WMI version needs us to pass
>> >> physicall addresses which is not exactly clean, and that's the only reason the
>> >> code is proposed for staging.
>> >>
>> >> Most likely this cannot graduate until acpica gets support to receive virtual
>> >> addresses.
>> >
>> > When is that going to happen?
>>
>> I don't know. Presumably when I get it done, which might be never, or
>> a few days from now. Most likely it will take some time.
>
> Then why not just merge this to the proper place, instead of relying on
> staging?  I don't want to have to have a staging driver that is waiting
> on external things to get it merged, instead of issues with the driver
> itself, as you are now putting the burden of maintenance on me, for no
> good reason other than you being too "lazy" to get other stuff done :)

*I* am too lazy? I am doing all the work. Nobody else is doing
anything about it.

Linux is throttling the CPU speed of this machine without trying to to
increase the fan speed first, that is not ideal.

So I do the first step of writing the fan driver and I get told it
shouldn't be a separate driver, it should be part of wmi, only to be
told later by the same person that it doesn't belong in wmi when I put
it there. Then I get told virt_to_physical() is not good, and it
should move to staging, and then I'm told you don't want it.

And at no point in time has anybody helped an iota to actually solve
the throttling problem.

So much for collaborative effort.

> So, I really don't want this driver, sorry, please merge it to the
> "proper" place in the kernel itself, fixing up the ACPI core to do so,
> or just living with the driver as-is if you don't want to do that
> work...

Forget it, I was hoping other people could use the driver while
somebody explains to me how to plug it o the thermal framework so it
gets activated when the machine gets hot, but clearly that's not going
to happen.

I have done my fair share of collaboration.

I'll just keep the driver for myself and figure out how to solve the
throttling by myself, which apparently I have to do anyway.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-05 19:54       ` Felipe Contreras
@ 2013-11-06  2:19         ` Greg Kroah-Hartman
  2013-11-06  2:52           ` Felipe Contreras
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-06  2:19 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: open list:STAGING SUBSYSTEM, Matthew Garrett, Linux Kernel Mailing List

On Tue, Nov 05, 2013 at 01:54:51PM -0600, Felipe Contreras wrote:
> On Tue, Nov 5, 2013 at 9:16 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Nov 05, 2013 at 08:29:40AM -0600, Felipe Contreras wrote:
> >> On Tue, Nov 5, 2013 at 6:52 AM, Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >> > On Tue, Nov 05, 2013 at 02:59:47AM -0600, Felipe Contreras wrote:
> >> >> Simple driver to enable control of the fan in ASUS laptops. So far this
> >> >> has only been tested in ASUS Zenbook Prime UX31A, but according to some
> >> >> online reference [1], it should work in other models as well.
> >> >>
> >> >> The implementation is very straight-forward, the only caveat is that the
> >> >> fan speed needs to be saved after it has been manually changed because
> >> >> it won't be reported properly until it goes back to 'auto' mode.
> >> >>
> >> >> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
> >> >>
> >> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> >> ---
> >> >>
> >> >> Two incarnations of this code exists [1][2], one that used ACPI methods
> >> >> directly, and one through WMI. Unfortunately the WMI version needs us to pass
> >> >> physicall addresses which is not exactly clean, and that's the only reason the
> >> >> code is proposed for staging.
> >> >>
> >> >> Most likely this cannot graduate until acpica gets support to receive virtual
> >> >> addresses.
> >> >
> >> > When is that going to happen?
> >>
> >> I don't know. Presumably when I get it done, which might be never, or
> >> a few days from now. Most likely it will take some time.
> >
> > Then why not just merge this to the proper place, instead of relying on
> > staging?  I don't want to have to have a staging driver that is waiting
> > on external things to get it merged, instead of issues with the driver
> > itself, as you are now putting the burden of maintenance on me, for no
> > good reason other than you being too "lazy" to get other stuff done :)
> 
> *I* am too lazy? I am doing all the work. Nobody else is doing
> anything about it.
> 
> Linux is throttling the CPU speed of this machine without trying to to
> increase the fan speed first, that is not ideal.
> 
> So I do the first step of writing the fan driver and I get told it
> shouldn't be a separate driver, it should be part of wmi, only to be
> told later by the same person that it doesn't belong in wmi when I put
> it there. Then I get told virt_to_physical() is not good, and it
> should move to staging, and then I'm told you don't want it.

Who told you to put it in staging?  That is the person that needs to be
corrected here, it's not your issue, sorry, I didn't realize the
backstory here.

And I would push back on those people, if it solves your problem, and
there's no other good solution, then it should go into the "real" part
of the kernel.  Who has rejected this driver and for what reason?

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-06  2:19         ` Greg Kroah-Hartman
@ 2013-11-06  2:52           ` Felipe Contreras
  2013-11-06  8:55             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Contreras @ 2013-11-06  2:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: open list:STAGING SUBSYSTEM, Matthew Garrett, Linux Kernel Mailing List

On Tue, Nov 5, 2013 at 8:19 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Nov 05, 2013 at 01:54:51PM -0600, Felipe Contreras wrote:
>> On Tue, Nov 5, 2013 at 9:16 AM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > On Tue, Nov 05, 2013 at 08:29:40AM -0600, Felipe Contreras wrote:
>> >> On Tue, Nov 5, 2013 at 6:52 AM, Greg Kroah-Hartman
>> >> <gregkh@linuxfoundation.org> wrote:
>> >> > On Tue, Nov 05, 2013 at 02:59:47AM -0600, Felipe Contreras wrote:
>> >> >> Simple driver to enable control of the fan in ASUS laptops. So far this
>> >> >> has only been tested in ASUS Zenbook Prime UX31A, but according to some
>> >> >> online reference [1], it should work in other models as well.
>> >> >>
>> >> >> The implementation is very straight-forward, the only caveat is that the
>> >> >> fan speed needs to be saved after it has been manually changed because
>> >> >> it won't be reported properly until it goes back to 'auto' mode.
>> >> >>
>> >> >> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
>> >> >>
>> >> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> >> >> ---
>> >> >>
>> >> >> Two incarnations of this code exists [1][2], one that used ACPI methods
>> >> >> directly, and one through WMI. Unfortunately the WMI version needs us to pass
>> >> >> physicall addresses which is not exactly clean, and that's the only reason the
>> >> >> code is proposed for staging.
>> >> >>
>> >> >> Most likely this cannot graduate until acpica gets support to receive virtual
>> >> >> addresses.
>> >> >
>> >> > When is that going to happen?
>> >>
>> >> I don't know. Presumably when I get it done, which might be never, or
>> >> a few days from now. Most likely it will take some time.
>> >
>> > Then why not just merge this to the proper place, instead of relying on
>> > staging?  I don't want to have to have a staging driver that is waiting
>> > on external things to get it merged, instead of issues with the driver
>> > itself, as you are now putting the burden of maintenance on me, for no
>> > good reason other than you being too "lazy" to get other stuff done :)
>>
>> *I* am too lazy? I am doing all the work. Nobody else is doing
>> anything about it.
>>
>> Linux is throttling the CPU speed of this machine without trying to to
>> increase the fan speed first, that is not ideal.
>>
>> So I do the first step of writing the fan driver and I get told it
>> shouldn't be a separate driver, it should be part of wmi, only to be
>> told later by the same person that it doesn't belong in wmi when I put
>> it there. Then I get told virt_to_physical() is not good, and it
>> should move to staging, and then I'm told you don't want it.
>
> Who told you to put it in staging?  That is the person that needs to be
> corrected here, it's not your issue, sorry, I didn't realize the
> backstory here.

Matthew Garrett. He said I should wrap my wmi fan code [1], so that it
compiles only when staging. I was doing that when I realized how ugly
it would all be, and there doesn't seem to be any precedent for that,
so I just wrote it as a separate driver.

> And I would push back on those people, if it solves your problem, and
> there's no other good solution, then it should go into the "real" part
> of the kernel.  Who has rejected this driver and for what reason?

Matthew Garrett. Because as it's described in the TODO, this code is
passing a physical address of our structs to the ACPI code, but that's
what the DSDT expects, there's no way around it. Doing virt_to_phys()
and passing that to the ACPI code might be a little ugly, but as far
as I know there's checks already in place.

Matthew said the acpica code is the one that should be doing the
virt_to_phys() conversion, which is probably true, but since nobody is
going to do that, the task is left for me.

Before going into this daunting task, I would rather have the CPU
throttling fixed by hooking the fan to a thermal zone, which was my
motivation of starting it in the first place, but I have not received
any help as to how to do that, I was hoping having the driver land
somewhere (e.g. staging) might help to get the first part out of the
way, and concentrate on how to actually make it useful.

[1] http://article.gmane.org/gmane.linux.power-management.general/39522

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-06  2:52           ` Felipe Contreras
@ 2013-11-06  8:55             ` Greg Kroah-Hartman
  2013-11-06 14:30               ` Matthew Garrett
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2013-11-06  8:55 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: open list:STAGING SUBSYSTEM, Matthew Garrett, Linux Kernel Mailing List

On Tue, Nov 05, 2013 at 08:52:38PM -0600, Felipe Contreras wrote:
> On Tue, Nov 5, 2013 at 8:19 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Nov 05, 2013 at 01:54:51PM -0600, Felipe Contreras wrote:
> >> On Tue, Nov 5, 2013 at 9:16 AM, Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >> > On Tue, Nov 05, 2013 at 08:29:40AM -0600, Felipe Contreras wrote:
> >> >> On Tue, Nov 5, 2013 at 6:52 AM, Greg Kroah-Hartman
> >> >> <gregkh@linuxfoundation.org> wrote:
> >> >> > On Tue, Nov 05, 2013 at 02:59:47AM -0600, Felipe Contreras wrote:
> >> >> >> Simple driver to enable control of the fan in ASUS laptops. So far this
> >> >> >> has only been tested in ASUS Zenbook Prime UX31A, but according to some
> >> >> >> online reference [1], it should work in other models as well.
> >> >> >>
> >> >> >> The implementation is very straight-forward, the only caveat is that the
> >> >> >> fan speed needs to be saved after it has been manually changed because
> >> >> >> it won't be reported properly until it goes back to 'auto' mode.
> >> >> >>
> >> >> >> [1] http://forum.notebookreview.com/asus/705656-fan-control-asus-prime-ux31-ux31a-ux32a-ux32vd.html
> >> >> >>
> >> >> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> >> >> ---
> >> >> >>
> >> >> >> Two incarnations of this code exists [1][2], one that used ACPI methods
> >> >> >> directly, and one through WMI. Unfortunately the WMI version needs us to pass
> >> >> >> physicall addresses which is not exactly clean, and that's the only reason the
> >> >> >> code is proposed for staging.
> >> >> >>
> >> >> >> Most likely this cannot graduate until acpica gets support to receive virtual
> >> >> >> addresses.
> >> >> >
> >> >> > When is that going to happen?
> >> >>
> >> >> I don't know. Presumably when I get it done, which might be never, or
> >> >> a few days from now. Most likely it will take some time.
> >> >
> >> > Then why not just merge this to the proper place, instead of relying on
> >> > staging?  I don't want to have to have a staging driver that is waiting
> >> > on external things to get it merged, instead of issues with the driver
> >> > itself, as you are now putting the burden of maintenance on me, for no
> >> > good reason other than you being too "lazy" to get other stuff done :)
> >>
> >> *I* am too lazy? I am doing all the work. Nobody else is doing
> >> anything about it.
> >>
> >> Linux is throttling the CPU speed of this machine without trying to to
> >> increase the fan speed first, that is not ideal.
> >>
> >> So I do the first step of writing the fan driver and I get told it
> >> shouldn't be a separate driver, it should be part of wmi, only to be
> >> told later by the same person that it doesn't belong in wmi when I put
> >> it there. Then I get told virt_to_physical() is not good, and it
> >> should move to staging, and then I'm told you don't want it.
> >
> > Who told you to put it in staging?  That is the person that needs to be
> > corrected here, it's not your issue, sorry, I didn't realize the
> > backstory here.
> 
> Matthew Garrett. He said I should wrap my wmi fan code [1], so that it
> compiles only when staging. I was doing that when I realized how ugly
> it would all be, and there doesn't seem to be any precedent for that,
> so I just wrote it as a separate driver.
> 
> > And I would push back on those people, if it solves your problem, and
> > there's no other good solution, then it should go into the "real" part
> > of the kernel.  Who has rejected this driver and for what reason?
> 
> Matthew Garrett. Because as it's described in the TODO, this code is
> passing a physical address of our structs to the ACPI code, but that's
> what the DSDT expects, there's no way around it. Doing virt_to_phys()
> and passing that to the ACPI code might be a little ugly, but as far
> as I know there's checks already in place.

Crap, I just spent the day, and a car-ride with Matthew, today, I could
have discussed this with him...

Matthew, please don't push drivers to the staging tree that have no
problems on their own, and don't require work within them to get out of
staging, as that doesn't help much, as I don't want to take code that
has to wait for external things to happen before it can move out of
staging.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] staging: new asus fan driver
  2013-11-06  8:55             ` Greg Kroah-Hartman
@ 2013-11-06 14:30               ` Matthew Garrett
  0 siblings, 0 replies; 9+ messages in thread
From: Matthew Garrett @ 2013-11-06 14:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Felipe Contreras, open list:STAGING SUBSYSTEM, Linux Kernel Mailing List

On Wed, Nov 06, 2013 at 12:55:56AM -0800, Greg Kroah-Hartman wrote:

> Matthew, please don't push drivers to the staging tree that have no
> problems on their own, and don't require work within them to get out of
> staging, as that doesn't help much, as I don't want to take code that
> has to wait for external things to happen before it can move out of
> staging.

I'm not willing to commit to supporting this code with the 
virt_to_phys() hack in place, but I also don't want to block Felipe from 
having an incentive to fix up the ACPI core so we can do this cleanly. 
What I suggested was adding it to the asus-wmi driver and wrapping the 
functionality with CONFIG_STAGING in order to indicate that there were 
no promises that this feature would remain, and then giving Felipe a few 
months to do the ACPI work. If it got done in a reasonable time then 
we'd keep the code, and if not I'd drop it. I certainly didn't suggest 
writing a separate driver, let alone putting it in staging.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-11-06 14:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-05  8:59 [PATCH] staging: new asus fan driver Felipe Contreras
2013-11-05 12:52 ` Greg Kroah-Hartman
2013-11-05 14:29   ` Felipe Contreras
2013-11-05 15:16     ` Greg Kroah-Hartman
2013-11-05 19:54       ` Felipe Contreras
2013-11-06  2:19         ` Greg Kroah-Hartman
2013-11-06  2:52           ` Felipe Contreras
2013-11-06  8:55             ` Greg Kroah-Hartman
2013-11-06 14:30               ` Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).