Message-ID: <CAMP44s2vHKuNyqRndWty+-Ua47KEL8fwfeq+sakC7MJKv5KrTw@mail.gmail.com>
Date:	Tue, 5 Nov 2013 20:52:38 -0600
From:	Felipe Contreras <felipe.contreras@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	"open list:STAGING SUBSYSTEM" <devel@...verdev.osuosl.org>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] staging: new asus fan driver

On Tue, Nov 5, 2013 at 8:19 PM, Greg Kroah-Hartman
<gregkh@...uxfoundation.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@...uxfoundation.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@...uxfoundation.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@...il.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists