Re: [PATCH v2] platform: x86: asus-wmi: add fan control

[Matthew Garrett]

On Mon, Oct 14, 2013 at 06:27:33PM -0500, Felipe Contreras wrote:
> On Mon, Oct 14, 2013 at 6:22 PM, Matthew Garrett <mjg59@xxxxxxxxxxxxx> wrote:
> > The easiest is to just do it from userspace. I think Intel have some
> > code for doing this, but I haven't looked at the thermal code for years.
> 
> That defeats the purpose of the whole thermal binding infrastructure.

Not really, but I agree it's not ideal. If there's a mechanism to get 
the system temperature via WMI then you could easily construct your own 
thermal zone and associated cooling device, but otherwise you'd have to 
provide a mechanism for exporting either the CPU information from 
coretemp or the thermal zones from ACPI.

> >> > I don't think you can easily register multiple drivers for the same WMI
> >> > device.
> >>
> >> I don't mean this one, I mean the standalone one. Actually, the first
> >> one I sent doesn't require all this system memory stuff.
> >
> > Banging EC registers directly is the wrong thing to do. Going via WMI is
> > correct.
> 
> I'm not going to bother arguing against your absolutist rhetoric. The
> fact is one patch can be applied, the other can't. Besides, nobody
> said anything about banging EC registers directly.

I'm sorry, you're right - you're calling ACPI methods directly instead. 
This is still incorrect. The platform provides an exported interface, 
and you should use that exported interface.

As long as Corentin doesn't object, I'm happy to merge this driver in 
its current form (including virt_to_phys()) providing that it's wrapped 
in CONFIG_STAGING, and assuming that you'll do the supporting work in 
acpica. I'll pull it out again in 6 months or so if that hasn't been 
fixed up. Fair?

-- 
Matthew Garrett | mjg59@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/