Re: Call for testers: New acpi_ibm driver

Markus Brueffer wrote:
> On Tuesday 31 May 2005 03:55, [email protected] wrote:
> 
>>In message <[email protected]>, Markus Brueffer wrote:
>>>Changes:
>>>
>>>Relocation of the sysctl tree to hw.acpi.ibm and renaming of some
>>>sysctls
>>
>>I don't think it a good thing. New device specific tree
>>should not be introduced.
>
> 
> I tried to get the sysctls more in line with our other ACPI extras drivers, 
> which all reside under hw.acpi.<brand> and hence the rename of e.g. 
> brightness->lcd_brightness and the relocation of the tree. Is there some 
> policy where such stuff belongs to?

I do not have a problem with this matching other drivers.  However, 
takawata-san is the author so perhaps he has a reason for this opinion.

>>>Support for more hotkeys by the hotkey sysctl (see manpage)
>>>Improved support of ACPI events, disabled by default. You can enable it
>>>by setting the events sysctl to 1 (on my T41p there are now 24
>>>different keypress events that get reported)
>>>write support for: volume, mute, brightness and thinklight
>>
>>Is it ok to  touch EC? I know the two way of impliment it.
>>One is modifying RTC register, another is using SMI interface
>>via ACPI control method.
> 
> Writing to the EC directly is nothing more that accessing the locations via 
> acpi_SetInteger. While I would highly prefer the latter method, the problem 
> is, that several places of the EC in the DSDT aren't even named or only 
> named on some models.

Please do not read/write directly from EC space.  A better way is to use 
the EC device methods I added recently.  This uses proper locking for 
requests and will work concurrently with other EC users (say, cmbat):

     device_t dev;

     dev = devclass_get_device(devclass_find("acpi_ec"), 0);
     if (dev == NULL)
         error, EC not present -- check this during probe and fail
         if error

     error = ACPI_EC_WRITE(dev, address, buffer, len);

or ACPI_EC_READ().  "address" is from 0-255 (8 bit addresses in EC 
space) and buffer is a pointer to data to write or read.

> Consider e.g. setting volume and mute. Here is a 
> snippet of a T41p DSDT:
> 
>                 Device (EC)
>                 {
> [...]
>                     OperationRegion (ECOR, EmbeddedControl, 0x00, 0x0100)
>                     Field (ECOR, ByteAcc, NoLock, Preserve)
>                     {
>                         HDBM,   1,
>                             ,   1,
>                             ,   1,
> [...]
>                         HFSP,   8,
>                             ,   6,    <-- Volume
>                         HMUT,   1,    <-- Mute
>                         Offset (0x31),
>                         HBRV,   8,    <-- Brightness
>                         HWPM,   1,
> 
> Volume doesn't have a name on any Thinkpad I have seen so far, Mute and 
> Brightness only on some. But the offset for directly accessing the EC is 
> always the same.

The above method handles this.

-- 
Nate