Stefan Reinauer wrote:
+++ b/src/ec/google/chromeec/ec.c
..
+/* an internal API to send a command to the EC and wait for response. */ +struct chromeec_command {
- u8 cmd_code; /* command code in, status out */
- u8 cmd_version; /* command version */
- const void* cmd_data_in; /* command data, if any */
- void* cmd_data_out; /* command response, if any */
- u16 cmd_size_in; /* size of command data */
- u16 cmd_size_out; /* expected size of command response in,
* actual received size out */+};
No need for __packed since the struct only goes to lower layer code which packs the actual command?
+int google_chromeec_kbbacklight(int percent) +{
- struct chromeec_command cec_cmd;
- struct ec_params_pwm_set_keyboard_backlight cmd_backlight;
- struct ec_response_pwm_get_keyboard_backlight rsp_backlight;
- /* if they were dumb, help them out */
- percent = percent % 101;
Please:
percent = MAX(percent, 0); percent = MIN(percent, 100);
The first one can of course be skipped if the parameter is unsigned.
+void google_chromeec_post(u8 postcode) +{
- /* backlight is a percent. postcode is a u8.
* Convert the u8 to %.*/- postcode = (postcode/4) + (postcode/8);
- google_chromeec_kbbacklight(postcode);
+}
Cute.
+u32 google_chromeec_get_events_b(void) +{
- return google_chromeec_get_mask(EC_CMD_HOST_EVENT_GET_B);
+}
What's "b" ? That's seriously undescriptive. From ACPI?
+static +int google_chromeec_hello(void) +{
- struct chromeec_command cec_cmd;
- struct ec_params_hello cmd_hello;
- struct ec_response_hello rsp_hello;
- cmd_hello.in_data = 0x10203040;
- cec_cmd.cmd_code = EC_CMD_HELLO;
- cec_cmd.cmd_version = 0;
- cec_cmd.cmd_data_in = &cmd_hello.in_data;
- cec_cmd.cmd_data_out = &rsp_hello.out_data;
- cec_cmd.cmd_size_in = sizeof(cmd_hello.in_data);
- cec_cmd.cmd_size_out = sizeof(rsp_hello.out_data);
Statically initializing the struct would look a little nicer:
struct chromeec_command cec_cmd = { .cmd_code = EC_CMD_HELLO, .cmd_version = 0, ... };
+static void google_chromeec_init(device_t dev) +{
..
- if (cec_cmd.cmd_code ||
(recovery_mode_enabled() &&(lpcv_cmd.current_image != EC_IMAGE_RO))) {struct ec_params_reboot_ec reboot_ec;/* Reboot the EC and make it come back in RO mode */reboot_ec.cmd = EC_REBOOT_COLD;reboot_ec.flags = 0;cec_cmd.cmd_code = EC_CMD_REBOOT_EC;cec_cmd.cmd_version = 0;cec_cmd.cmd_data_in = &reboot_ec;cec_cmd.cmd_size_in = sizeof(reboot_ec);cec_cmd.cmd_size_out = 0; /* ignore response, if any */printk(BIOS_DEBUG, "Rebooting with EC in RO mode:\n");google_chromeec_command(&cec_cmd);udelay(1000);hard_reset();hlt();- }
+}
How about die() at the end instead of hlt()? I believe hlt returns.
All in all, lovely work! I guess you had lots of fun while debugging.
//Peter