[PATCH] fix sonypi/spicctrl set/get brightness support

Bastien Nocera hadess at hadess.net
Mon Feb 19 15:18:03 PST 2007


Hey David,

On Mon, 2007-02-19 at 18:02 -0500, David Zeuthen wrote:
> Hi,
> 
> Sorry for the lag! This looks good; it means things will work on Sony
> laptops without having to rely on on the spicctrl tool yes?

Yep.

> Some comments:
> 
>  - it would probably be good with a --with-sonypi build configuration
>    parameter and only build this for Linux for now. Would also remove
>    all the #ifdef __FreeBSD__ and #ifdef sun conditionals

At least FreeBSD has a similar driver called spic:
http://fxr.watson.org/fxr/source/i386/isa/spic.c

Should I name the callout "-sonypic" (for Sony Programmable I/O
Controller) instead?

>  - should probably use gtk-doc rather that Doxygen annotation if
>    at all. Presently we only build docs for libraries.

Copied from the pmu equivalent callout.

>  - Please avoid using C++ style comments

Yeah, quick FIXMEs...

>  - Do we need to open the device file all the time? I suppose so, as
>    the driver probably is single opener?

Huh, what were you thinking of? We can't keep it open in an addon, as
it's not really re-entrant (ie. doesn't like multiple openers).

>  - Perhaps we need range checking for setting the brightness?

Yeah, just in case.

>  - Perhaps improve on language in "Sets/Gets the bluetooth power" to be
>    more along the lines of "Turn internal bluetooth adapter on/off". 
>    I suspect that what it means?

Yep.

>  - I suspect the Bluetooth power control interface will follow and
>    will be just an .fdi file?

Not quite. It would also need code to detect the Bluetooth device (can I
do HAL calls from the callout?), and a way to pass the bluetooth device
USB IDs from the FDI file on a per-laptop model basis. Any ideas?

> Sorry again for the lag. 100 things going on here :-)

Sorry for the nag :)

-- 
Bastien Nocera <hadess at hadess.net> 



More information about the hal mailing list