Re: [PATCH 3/3] ideapad: add backlight driver

2011-06-22 Thread Matthew Garrett
On Wed, Jun 22, 2011 at 04:07:38PM +0800, Ike Panhc wrote:

 -static void ideapad_sync_rfk_state(struct acpi_device *adevice)
 +static void ideapad_sync_rfk_state(struct ideapad_private *priv)
  {

This should be a separate patch.

 -static void __devexit ideapad_unregister_rfkill(struct acpi_device *adevice,
 - int dev)
 +static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)

This also seems unrelated.

 +static int ideapad_backlight_get_brightness(struct backlight_device 
 *blightdev)
 +{
 + unsigned long now;
 +
 + if (read_ec_data(ideapad_handle, 0x12, now))
 + return -EAGAIN;

Description says you're using commands on the VPC2004 device, but it 
looks like you're just poking the embedded controller? Are you sure the 
EC offsets are stable?

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe platform-driver-x86 
in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] ideapad: add backlight driver

2011-06-22 Thread Ike Panhc
On 06/23/2011 03:40 AM, Matthew Garrett wrote:
 +static int ideapad_backlight_get_brightness(struct backlight_device 
 *blightdev)
 +{
 +unsigned long now;
 +
 +if (read_ec_data(ideapad_handle, 0x12, now))
 +return -EAGAIN;
 
 Description says you're using commands on the VPC2004 device, but it 
 looks like you're just poking the embedded controller? Are you sure the 
 EC offsets are stable?
 

Yes, they are stable, though on some machine, some functions are not fully 
implement.
For example, on s10-3, the brightness control is no function but bl_power works 
fine.
on B550, the brightness control works fine but only get acpi notify when 
bl_power
turning on.

function read_ec_data will access VPCW and VPCR methods that exist in every 
VPC2004
devices. In this case, VPCW will write 0x12 to VCMD and then VPCR read the 
current
brightness value from VDAT.

So far as I have seen, there are two design for VDAT and VCMD. One way (eg, 
ideapad
B550) is they are EC registers and embedded controller will do all things. The 
other
way (eg, ideapad Y530) is they are variables in DSDT, and CPU/DSDT will correct 
informations into VDAT.

Both of the design are ok for ideapad-laptop because we only touch VPCW/VPCR 
methods
and let DSDT or EC do the rest.

I put DSDTs all I have at http://people.ubuntu.com/~ikepanhc/DSDTs  
--
To unsubscribe from this list: send the line unsubscribe platform-driver-x86 
in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html