Opened 8 years ago

Closed 7 years ago

#1555 closed defect (fixed)

Wake up from lid switch

Reported by: cjb Owned by: wad
Priority: blocker Milestone: Trial-2
Component: embedded controller Version:
Keywords: power Cc: cjb, rsmith, wad
Blocked By: Blocking:
Deployments affected: Action Needed:
Verified: no

Description

We get an EC SCI event when the laptop is flipped in or out of ebook mode. This is fine, but we also need a method to find out which mode we're in now -- for example, if we flip the screen into ebook mode and then power up, we'll never realise that we're in ebook mode because we won't catch an event. Likewise if a flip happens while OFW is booting, or before the power manager is listening to the input layer's stream and recording the state.

So, the EC should be able to tell us the state for ebook. Assigning to smithbone.

Change History (27)

comment:1 Changed 8 years ago by jg

  • Milestone changed from Untriaged to BTest-4

comment:2 Changed 8 years ago by jg

  • Keywords power added

comment:3 Changed 8 years ago by cjb

  • Priority changed from high to blocker

comment:4 Changed 8 years ago by jg

  • Component changed from embedded controller to kernel
  • Owner changed from rsmith to cjb

EC and firmware works.

comment:5 Changed 8 years ago by jg

  • Owner changed from cjb to dilinger

comment:6 Changed 8 years ago by jg

  • Cc cjb rsmith added

comment:7 Changed 8 years ago by kimquirk

  • Milestone changed from Trial-2 to BTest-4

I moved all bugs from B4 to Trial-2; this one belongs in B4 so I moved it manually.

comment:8 follow-up: Changed 8 years ago by dilinger

I'm looking for suggestions of how to export this to userspace. It seems like the ebook state is most closely related to the dcon, so the easiest way to do this would be to create /sys/platform/devices/dcon/ebook (containing a '1' or '0' value).

Of course, it's not *really* part of the dcon driver (and include EC commands in the dcon driver does not thrill me). So, another option is to create an EC-specific platform driver, which would allow us to have /sys/bus/platform/devices/ec/ebook . This would allow us more flexibility in the future; we could have an interface for issuing EC commands from userspace ('echo 2a > /sys/bus/platform/devices/ec/cmd'), as well as additional EC-related info (ie, /sys/bus/platform/devices/ec/dcon_power_rail).

We could also just do /proc/ebook or something lame like that.. Ideas?

comment:9 in reply to: ↑ 8 ; follow-up: Changed 8 years ago by jg

It's really analogous to the lid switch, rather than the dcon. It's a piece of physical information of the physical state of the machine.

So I don't think it's at all related to a dcon.

Probably worth a half hour google to see how this is getting implemented on tablet PC's on Linux.

Replying to dilinger:

I'm looking for suggestions of how to export this to userspace. It seems like the ebook state is most closely related to the dcon, so the easiest way to do this would be to create /sys/platform/devices/dcon/ebook (containing a '1' or '0' value).

Of course, it's not *really* part of the dcon driver (and include EC commands in the dcon driver does not thrill me). So, another option is to create an EC-specific platform driver, which would allow us to have /sys/bus/platform/devices/ec/ebook . This would allow us more flexibility in the future; we could have an interface for issuing EC commands from userspace ('echo 2a > /sys/bus/platform/devices/ec/cmd'), as well as additional EC-related info (ie, /sys/bus/platform/devices/ec/dcon_power_rail).

We could also just do /proc/ebook or something lame like that.. Ideas?

comment:10 Changed 8 years ago by dilinger

Ok, we now properly get the ebook_state mode during bootup, and apparently the input layer already tracks the state for us. Userspace apps can thus query the state via ioctl(EVIOCGSW), so there's really no need to have any additional infrastructure in the kernel for it. Patches have been committed to both master and stable; I vote we close this.

comment:11 Changed 8 years ago by cjb

HAL will only track switch state one-switch-per-inputdev, so I think we also need the device splitting out patch (which is in playground).

comment:12 in reply to: ↑ 9 Changed 8 years ago by rsmith

This would allow us more flexibility in the future; we could have an interface for >issuing EC commands from userspace ('echo 2a > /sys/bus/platform/devices/ec/cmd'),

Allowing a userspace process to issue the battery EEPROM query commands would be very handy in the reporting of battery usage info.

But if you do this you must filter the commands. Some of those commands like the battery info init, and set SCI mask would do bad things to a normal working system.

Perhaps only allow the ones that read info to be run via a non-privileged user.

comment:13 Changed 8 years ago by dilinger

  • Milestone changed from BTest-4 to Trial-2

We need to either split out devices, or (ideally) fix hal. There have been noises made about fixing hal, but we don't have a timeframe yet.

comment:14 Changed 8 years ago by kimquirk

  • Milestone changed from Trial-2 to BTest-4

Would like to fix this on the B4 boards.

comment:15 Changed 8 years ago by jg

  • Milestone changed from BTest-4 to Trial-2

comment:16 Changed 8 years ago by dilinger

Ok, this has been committed to master. Yell if it needs to go into stable.

comment:17 Changed 7 years ago by jg

What's the state of this, Andres?

comment:18 Changed 7 years ago by dilinger

  • Owner changed from dilinger to cjb

comment:19 Changed 7 years ago by wad

  • Cc wad added

This software bug has a related hardware component. The lid switch is communicated to the laptop via the Southbridge, which is powered down when the laptop is in suspend mode.

This will be corrected in CTest by running the lid switch to an EC input pin. A big question is whether it should also tie into the EC WAKEUP circuitry, through the addition of another transistor.

comment:20 Changed 7 years ago by cjb

  • Resolution set to fixed
  • Status changed from new to closed

Wad says this is fixed in the latest CTest schematics. Closing.

comment:21 Changed 7 years ago by cjb

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Summary changed from No way to determine "ebook mode" state. to Wake up from lid switch

.. reassigning to David Lin to add EC code to wake up from the pin for the lid.

comment:22 Changed 7 years ago by cjb

  • Owner changed from cjb to David.Lin
  • Status changed from reopened to new

comment:23 Changed 7 years ago by cjb

  • Component changed from kernel to embedded controller

comment:24 Changed 7 years ago by cjb

  • Resolution set to fixed
  • Status changed from new to closed

We're *already* waking up from lid switch. For some reason. I guess I'll close this bug.

comment:25 Changed 7 years ago by jg

  • Resolution fixed deleted
  • Status changed from closed to reopened

Will be looking into this a bit further.

comment:26 Changed 7 years ago by jg

  • Owner changed from David.Lin to wad
  • Status changed from reopened to new

comment:27 Changed 7 years ago by wad

  • Resolution set to fixed
  • Status changed from new to closed

Closing this for good. Adding the hardware capability to wakeup the EC
from lid switch events would be unduly complicated, given the different
power domains.

For the record, what we are eliminating is that there will be no way to
wakeup a unit from a complete power-down using just the lid. The power
button will have to be pressed.

Note: See TracTickets for help on using tickets.