From: "Brian S. Julin" <bri@abrij.org>
To: akpm@linux-foundation.org
Cc: lenb@kernel.org, linux-kernel@vger.kernel.org, greg@kroah.com,
linux-acpi@vger.kernel.org, mjg59@srcf.ucam.org
Subject: Re: [PATCH] misc: Add oqo-wmi driver for model 2 OQO backlight and rfkill control
Date: Wed, 3 Dec 2008 20:28:39 -0500 (EST) [thread overview]
Message-ID: <Pine.LNX.4.64.0812031931240.2166@sheavy.abrij.org> (raw)
In-Reply-To: <20081203150019.7e272c19.akpm@linux-foundation.org>
Yeah Matthew picked up the code before I did a final cleanup, at that
point I was still just waiting for someone(anyone) to test it. So the edges are
rough and there's still some painter's tape stuck to the walls. I guess I
can afford some time to shave the fuzzies off, but Matthew and I should
coordinate timewise if this all needs to be fixed before it's in a repository --
my opportunities to code are sparse, and his attention is probably a highly
contested resource.
Thanks for the review (and Sven in other reply, too) -- a few items I
probably would have missed cleaning. Some stuff is the wmi core
and sample drivers at least as they were when I coded, and those aren't
from me.
On Wed, 3 Dec 2008, Andrew Morton wrote:
>> +static int smread_s16(u8 hi_addr, u8 lo_addr)
>> +{
>> + s16 ret = -1;
>> + acpi_status status;
>> + u8 r;
>> +
>> + /* Keep some ACPI routines off the SMBUS */
>> + status = oqo_lock_smbus(1);
>> + if (ACPI_FAILURE(status))
>> + goto skip;
>
> Does this mean that we didn't take the lock? If so, will running
> oqo_lock_smbus(0) be correct?
That does this (in DSL) after unpacking arguments and running a switch statement:
Store (BUF1, VFLG)
Return (0x00)
...VFLG being a preallocated RAM value in the ACPI VM. That's all. Famous last
words but I really don't see it failing ever, or if it does the ACPI VM's in
deep doodoo anyways. At any rate it will not spin.
Really an ACPI doctor is needed to even explain why it's there at all --
there's a real mutex on the SMBUS, and this here is not it.
It seems to just be some sort of badly coded (in DSDT) advisory that keeps the
machine awake. I only even bothered to call it because it seemed like safer to
do so than not. We should really be the only writer, as ACPI routines merely
read it (and only a few of them.) The only access to it is through the WMI
interface. Can we just claim exclusivity on using that given this is an
omnibus driver?
> This driver does quite a lot of casting of things which the compiler
> will happily do for us. This could be viewed as having documentation
> benefit, but not much, and it is atypical.
Yep I know, I'm cast-and-paren trigger happy. Keeps me sane, and has saved
my butt a few times, but can be cleaned to match style.
P.S. I think it was Sven that pointed out a
label:
return
IIRC that was due to a cranky GCC warning FWIW.
--
Brian
next prev parent reply other threads:[~2008-12-04 1:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-27 19:05 [PATCH] misc: Add oqo-wmi driver for model 2 OQO backlight and rfkill control Matthew Garrett
2008-11-28 19:31 ` Len Brown
2008-11-29 3:35 ` Brian S. Julin
2008-11-29 4:50 ` Matthew Garrett
2008-12-03 19:55 ` Matthew Garrett
2008-12-03 22:55 ` Sven Wegener
2008-12-03 23:00 ` Andrew Morton
2008-12-04 1:28 ` Brian S. Julin [this message]
2009-01-09 6:13 ` Len Brown
2009-01-09 12:35 ` Matthew Garrett
2009-01-09 20:09 ` Len Brown
2009-01-09 22:34 ` (unknown) Len Brown
2009-01-09 22:34 ` Len Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.0812031931240.2166@sheavy.abrij.org \
--to=bri@abrij.org \
--cc=akpm@linux-foundation.org \
--cc=greg@kroah.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mjg59@srcf.ucam.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.