Re: [PATCH][ACPI] Panasonic Hotkey Driver

[David B. <dbr...@li...>]

Hiroshi Miura wrote:

>At Sat, 21 Aug 2004 01:44:03 -0700,
>David Bronaugh wrote:
>  
>
>>OK, this all makes sense now. It seems like the best way to do screen 
>>brightness setting would be to read fields 2 and 3 from SINF into 
>>internal fields (pcc_max_bright and pcc_min_bright?) to check user input 
>>and make sure it is not outside these bounds.
>>    
>>
>
>these are ok?
>
>pcc_ac_max_bright
>pcc_ac_min_bright
>pcc_dc_max_bright
>pcc_dc_min_bright
>  
>
In my patch, I took off the "pcc" prefix -- I felt it was unnecessary because the name the driver uses in /proc/acpi is "pcc".

I also modified the names slightly; if you don't like them, you can change them.

>>I don't think it is necessary to adjust (or read) fields 5 6 and 7 -- 
>>could you confirm on your R3 that these are not necessary (try the 
>>things I tried on my R1N)? If you can confirm this, we can simplify the 
>>driver to only expose 1 field for the 'brightness' control.
>>    
>>
>
>I think it is necessary to adjuct field 5 6 and 7.
>This sinf is kept through hibernate/power off and on.
>BIOS show this field when boot or resume and set brightness according as power state.
>
>
>  
>
Yes, I noticed that the SINF does persist over reboots.

The attached patch is against Hiroshi's v0.5 code, and does the following:
 - gives access (via a simple interface) to all the fields in SINF that 
are available on the person's laptop
 - cleans up the coding style to bring it closer to 
Documentation/CodingStyle guidelines
 - cleans up a few messy spots in the code
 - corrects typographic errors
 - documents some of the stranger SINF fields in the code
 - makes an attempt at cleaning up error messages
 - centralizes data structure definitions at the top of the file
 - removes the global 'sinf' variable (I left the 'num_sifr' variable alone)
 - moves the call to acpi_pcc_proc_init to avoid a possible use of 
uninitialized data via the /proc interface

I didn't decide to put code in the driver to make the R1N look like all 
the others; this could be a TODO (though I like having access to all 235 
brightness levels).

I also haven't really cleaned up the error messages enough -- hopefully 
this can happen later. Nor have I unified all the naming conventions -- 
the driver still has an identity crisis (it isn't sure if it's ACPI_PCC, 
pcc_acpi, or HOTKEY).

Someone should run scripts/Lintain on the code, too, to clean up 
inconsistencies.

I have also attached a separate patch which cleans up the in-kernel 
help. Later today (or maybe tomorrow; I'm getting tired) I'll send 
scripts which handle all the hotkey events properly; then these can be 
put up on the website.

David Bronaugh