Signed-off-by: Sven Schnelle svens@stackframe.org --- src/ec/acpi/ec.c | 10 ++++++++++ src/ec/acpi/ec.h | 2 ++ 2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/src/ec/acpi/ec.c b/src/ec/acpi/ec.c index 7a01b7e..e353260 100644 --- a/src/ec/acpi/ec.c +++ b/src/ec/acpi/ec.c @@ -113,6 +113,16 @@ int ec_write(u8 addr, u8 data) return send_ec_data(data); }
+void ec_set_bit(u8 addr, u8 bit) +{ + ec_write(addr, ec_read(addr) | (1 << bit)); +} + +void ec_clr_bit(u8 addr, u8 bit) +{ + ec_write(addr, ec_read(addr) & ~(1 << bit)); +} + struct chip_operations ec_acpi_ops = { CHIP_NAME("ACPI Embedded Controller") }; diff --git a/src/ec/acpi/ec.h b/src/ec/acpi/ec.h index 77ee637..cabbfea 100644 --- a/src/ec/acpi/ec.h +++ b/src/ec/acpi/ec.h @@ -44,6 +44,8 @@ int send_ec_data_nowait(u8 data); u8 recv_ec_data(void); u8 ec_read(u8 addr); int ec_write(u8 addr, u8 data); +void ec_set_bit(u8 addr, u8 bit); +void ec_clr_bit(u8 addr, u8 bit);
#endif
Signed-off-by: Sven Schnelle svens@stackframe.org --- src/ec/acpi/ec.c | 23 ++++++++++++++++------- src/ec/acpi/ec.h | 1 + 2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/ec/acpi/ec.c b/src/ec/acpi/ec.c index e353260..1d4ffb8 100644 --- a/src/ec/acpi/ec.c +++ b/src/ec/acpi/ec.c @@ -25,12 +25,15 @@ #include <delay.h> #include "ec.h"
+static int ec_cmd_reg = EC_SC; +static int ec_data_reg = EC_DATA; + int send_ec_command(u8 command) { int timeout;
timeout = 0x7ff; - while ((inb(EC_SC) & EC_IBF) && --timeout) { + while ((inb(ec_cmd_reg) & EC_IBF) && --timeout) { udelay(10); if ((timeout & 0xff) == 0) printk(BIOS_SPEW, "."); @@ -41,7 +44,7 @@ int send_ec_command(u8 command) // return -1; }
- outb(command, EC_SC); + outb(command, ec_cmd_reg); return 0; }
@@ -50,7 +53,7 @@ int send_ec_data(u8 data) int timeout;
timeout = 0x7ff; - while ((inb(EC_SC) & EC_IBF) && --timeout) { // wait for IBF = 0 + while ((inb(ec_cmd_reg) & EC_IBF) && --timeout) { // wait for IBF = 0 udelay(10); if ((timeout & 0xff) == 0) printk(BIOS_SPEW, "."); @@ -61,14 +64,14 @@ int send_ec_data(u8 data) // return -1; }
- outb(data, EC_DATA); + outb(data, ec_data_reg);
return 0; }
int send_ec_data_nowait(u8 data) { - outb(data, EC_DATA); + outb(data, ec_data_reg);
return 0; } @@ -80,7 +83,7 @@ u8 recv_ec_data(void)
timeout = 0x7fff; while (--timeout) { // Wait for OBF = 1 - if (inb(EC_SC) & EC_OBF) { + if (inb(ec_cmd_reg) & EC_OBF) { break; } udelay(10); @@ -92,7 +95,7 @@ u8 recv_ec_data(void) // return -1; }
- data = inb(EC_DATA); + data = inb(ec_data_reg); printk(BIOS_DEBUG, "recv_ec_data: 0x%02x\n", data);
return data; @@ -123,6 +126,12 @@ void ec_clr_bit(u8 addr, u8 bit) ec_write(addr, ec_read(addr) & ~(1 << bit)); }
+void ec_set_ports(u16 cmd_reg, u16 data_reg) +{ + ec_cmd_reg = cmd_reg; + ec_data_reg = data_reg; +} + struct chip_operations ec_acpi_ops = { CHIP_NAME("ACPI Embedded Controller") }; diff --git a/src/ec/acpi/ec.h b/src/ec/acpi/ec.h index cabbfea..b5a1197 100644 --- a/src/ec/acpi/ec.h +++ b/src/ec/acpi/ec.h @@ -46,6 +46,7 @@ u8 ec_read(u8 addr); int ec_write(u8 addr, u8 data); void ec_set_bit(u8 addr, u8 bit); void ec_clr_bit(u8 addr, u8 bit); +void ec_set_ports(u16 cmd_reg, u16 data_reg);
#endif
* Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/ec/acpi/ec.c | 23 ++++++++++++++++------- src/ec/acpi/ec.h | 1 + 2 files changed, 17 insertions(+), 7 deletions(-)
I wonder if you want two sets of access functions, like on the Getac P470 instead of changing the base address on the fly.
Stefan Reinauer stefan.reinauer@coreboot.org writes:
- Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/ec/acpi/ec.c | 23 ++++++++++++++++------- src/ec/acpi/ec.h | 1 + 2 files changed, 17 insertions(+), 7 deletions(-)
I wonder if you want two sets of access functions, like on the Getac P470 instead of changing the base address on the fly.
I like the Adress configuration more. Accessing the 0x1600/0x1604 register pair works the same way as the 0x62/0x66 pair, so i dont like to copy all the accessor function just because the base address changed. And the performance impact of looking up the addresses instead of having it as a literal in the outb asembler instruction is probably negligible.
Sven.
* Sven Schnelle svens@stackframe.org [110312 09:23]:
Stefan Reinauer stefan.reinauer@coreboot.org writes:
- Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/ec/acpi/ec.c | 23 ++++++++++++++++------- src/ec/acpi/ec.h | 1 + 2 files changed, 17 insertions(+), 7 deletions(-)
Acked-by: Stefan Reinauer stefan.reinauer@coreboot.org
Signed-off-by: Sven Schnelle svens@stackframe.org --- src/mainboard/lenovo/x60/mainboard.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/mainboard/lenovo/x60/mainboard.c b/src/mainboard/lenovo/x60/mainboard.c index b8e6a49..79e4a83 100644 --- a/src/mainboard/lenovo/x60/mainboard.c +++ b/src/mainboard/lenovo/x60/mainboard.c @@ -50,6 +50,11 @@ static void wlan_enable(void)
static void mainboard_enable(device_t dev) { + /* Enable 1600/1604 register pair */ + ec_set_bit(0x00, 0x05); + /* switch to just enabled registers for ACPI */ + ec_set_ports(0x1604, 0x1600); + backlight_enable(); trackpoint_enable(); /* FIXME: this should be ACPI's task
* Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/mainboard/lenovo/x60/mainboard.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/mainboard/lenovo/x60/mainboard.c b/src/mainboard/lenovo/x60/mainboard.c index b8e6a49..79e4a83 100644 --- a/src/mainboard/lenovo/x60/mainboard.c +++ b/src/mainboard/lenovo/x60/mainboard.c @@ -50,6 +50,11 @@ static void wlan_enable(void)
static void mainboard_enable(device_t dev) {
- /* Enable 1600/1604 register pair */
- ec_set_bit(0x00, 0x05);
- /* switch to just enabled registers for ACPI */
- ec_set_ports(0x1604, 0x1600);
I think you only need to use the high ports in SMM code to avoid race conditions.
Stefan
Stefan Reinauer stefan.reinauer@coreboot.org writes:
- Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/mainboard/lenovo/x60/mainboard.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/mainboard/lenovo/x60/mainboard.c b/src/mainboard/lenovo/x60/mainboard.c index b8e6a49..79e4a83 100644 --- a/src/mainboard/lenovo/x60/mainboard.c +++ b/src/mainboard/lenovo/x60/mainboard.c @@ -50,6 +50,11 @@ static void wlan_enable(void)
static void mainboard_enable(device_t dev) {
- /* Enable 1600/1604 register pair */
- ec_set_bit(0x00, 0x05);
- /* switch to just enabled registers for ACPI */
- ec_set_ports(0x1604, 0x1600);
I think you only need to use the high ports in SMM code to avoid race conditions.
Yes. But the original BIOS switches to those ports at the same time, and i want to do it the same way. Makes comparing bus cycles easier, and has no disadvantages IMHO.
* Sven Schnelle svens@stackframe.org [110312 09:16]:
Stefan Reinauer stefan.reinauer@coreboot.org writes:
- Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/mainboard/lenovo/x60/mainboard.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/mainboard/lenovo/x60/mainboard.c b/src/mainboard/lenovo/x60/mainboard.c index b8e6a49..79e4a83 100644 --- a/src/mainboard/lenovo/x60/mainboard.c +++ b/src/mainboard/lenovo/x60/mainboard.c @@ -50,6 +50,11 @@ static void wlan_enable(void)
static void mainboard_enable(device_t dev) {
- /* Enable 1600/1604 register pair */
- ec_set_bit(0x00, 0x05);
- /* switch to just enabled registers for ACPI */
- ec_set_ports(0x1604, 0x1600);
I think you only need to use the high ports in SMM code to avoid race conditions.
Yes. But the original BIOS switches to those ports at the same time, and i want to do it the same way. Makes comparing bus cycles easier, and has no disadvantages IMHO.
It does have a disadvantage if there is an SMI that accesses the EC while you access the EC in your code, too.
Also be sure to explicitly re-set the right address in SMM mode, otherwise SMM will still use the original registers as SMM uses an extra copy of the EC access code.
Last but not least, make sure that 0x1600 is blocked from use of the resource allocator for anything else.
* Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/mainboard/lenovo/x60/mainboard.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/mainboard/lenovo/x60/mainboard.c b/src/mainboard/lenovo/x60/mainboard.c index b8e6a49..79e4a83 100644 --- a/src/mainboard/lenovo/x60/mainboard.c +++ b/src/mainboard/lenovo/x60/mainboard.c @@ -50,6 +50,11 @@ static void wlan_enable(void)
static void mainboard_enable(device_t dev) {
- /* Enable 1600/1604 register pair */
- ec_set_bit(0x00, 0x05);
- /* switch to just enabled registers for ACPI */
I think the comment is misleading. ACPI has nothing to do with this.
- ec_set_ports(0x1604, 0x1600);
Stefan Reinauer stefan.reinauer@coreboot.org writes:
- Sven Schnelle svens@stackframe.org [110312 01:18]:
Signed-off-by: Sven Schnelle svens@stackframe.org
src/mainboard/lenovo/x60/mainboard.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/mainboard/lenovo/x60/mainboard.c b/src/mainboard/lenovo/x60/mainboard.c index b8e6a49..79e4a83 100644 --- a/src/mainboard/lenovo/x60/mainboard.c +++ b/src/mainboard/lenovo/x60/mainboard.c @@ -50,6 +50,11 @@ static void wlan_enable(void)
static void mainboard_enable(device_t dev) {
- /* Enable 1600/1604 register pair */
- ec_set_bit(0x00, 0x05);
- /* switch to just enabled registers for ACPI */
I think the comment is misleading. ACPI has nothing to do with this.
You're right. This is obviously wrong. Those are the EC registers, not ACPI ;)
Sven.
-
Stefan Reinauer -
Sven Schnelle