From 507a0cbff8815a9cb6a9b1b6521300516d6b5c2e Mon Sep 17 00:00:00 2001 From: Vadim Bendebury Date: Mon, 16 Mar 2015 14:12:40 -0700 Subject: i2c/ww_ring: various driver fixes and improvements When in development environment, some SP5 devices might not have the LED ring attached. They are still fully functional, but when booting up are generating massive amount of i2c error messages. This patch prevents accesses to non-existing lp55321 devices. When loading the program into the device the vendor recommends 1 ms delay when accessing the program control register. This patch separates these accesses into a function and add a delay after every access. Another fix - advance the program address when loading multipage programs. Set the global variable register 3c, not used by coreboot programs, to a fixed value. This will allow depthcharge to avoid re-initializing the controller when not necessary. BRANCH=storm BUG=chrome-os-partner:36059 TEST=booted firmware on an SP5 with no LED ring attached, no excessive error messages are generated, saw the default pattern displayed when the recovery button is pressed during reset. Change-Id: I6a2a27968684c40dae15317540a16405b1419e30 Signed-off-by: Patrick Georgi Original-Commit-Id: 5e0b4c84aca27460db594da1faf627ddee56f399 Original-Change-Id: I10f1f53cefb866d11ecf76ea48f74131d8b0ce77 Original-Signed-off-by: Vadim Bendebury Original-Reviewed-on: https://chromium-review.googlesource.com/260648 Original-Reviewed-by: Aaron Durbin Reviewed-on: http://review.coreboot.org/9867 Tested-by: build bot (Jenkins) Reviewed-by: Stefan Reinauer --- src/drivers/i2c/ww_ring/ww_ring.c | 99 +++++++++++++++++++++++++++++---------- 1 file changed, 74 insertions(+), 25 deletions(-) (limited to 'src/drivers/i2c/ww_ring') diff --git a/src/drivers/i2c/ww_ring/ww_ring.c b/src/drivers/i2c/ww_ring/ww_ring.c index b074576914..e1f255a858 100644 --- a/src/drivers/i2c/ww_ring/ww_ring.c +++ b/src/drivers/i2c/ww_ring/ww_ring.c @@ -27,6 +27,7 @@ */ #include +#include #include #include @@ -43,6 +44,7 @@ #define LP55231_ENGCTRL2_REG 0x01 #define LP55231_D1_CRT_CTRL_REG 0x26 #define LP55231_MISC_REG 0x36 +#define LP55231_VARIABLE_REG 0x3c #define LP55231_RESET_REG 0x3d #define LP55231_ENG1_PROG_START 0x4c #define LP55231_PROG_PAGE_REG 0x4f @@ -55,11 +57,22 @@ #define LP55231_ENGCTRL1_CHIP_EN 0x40 #define LP55231_ENGCTRL1_ALL_ENG_GO 0x2a +/* LP55231_ENGCTRL2_REG fields. */ +#define LP55231_ENGCTRL2_ALL_DISABLE 0 +#define LP55231_ENGCTRL2_ALL_LOAD 0x15 +#define LP55231_ENGCTRL2_ALL_RUN 0x2a + /* LP55231_MISC_REG fields. */ #define LP55231_MISC_AUTOINCR (1 << 6) #define LP55231_MISC_PUMP_1X (1 << 3) #define LP55231_MISC_INT_CLK (3 << 0) +/* + * LP55231_VARIABLE_REG cookie value. It indicates to depthcharge that the + * ring has been initialized by coreboot. + */ +#define LP55231_VARIABLE_COOKIE 0xb4 + /* Goes into LP55231_RESET_REG to reset the chip. */ #define LP55231_RESET_VALUE 0xff @@ -224,13 +237,25 @@ static int ledc_read(TiLp55231 *ledc, uint8_t addr, uint8_t *data) } /* - * Reset transaction is expected to result in a failing i2c command, - * no need to return a value. + * Reset transaction is expected to result in a failing i2c command. But even + * before trying it, read the reset register, which is supposed to always + * return 0. If this fails - there is no lp55231 at this address. + * + * Return 0 on success, -1 on failure to detect controller. */ -static void ledc_reset(TiLp55231 *ledc) +static int ledc_reset(TiLp55231 *ledc) { uint8_t data; + data = ~0; + ledc_read(ledc, LP55231_RESET_REG, &data); + if (data) { + printk(BIOS_WARNING, + "WW_RING: no controller found at address %#2.2x\n", + ledc->dev_addr); + return -1; + } + data = LP55231_RESET_VALUE; ledc_write(ledc, LP55231_RESET_REG, &data, 1); @@ -241,6 +266,7 @@ static void ledc_reset(TiLp55231 *ledc) * fails. */ ledc_read(ledc, LP55231_RESET_REG, &data); + return 0; } /* @@ -271,22 +297,32 @@ static void ledc_write_program(TiLp55231 *ledc, uint8_t load_addr, program, segment_size); count -= segment_size; + program += segment_size; page_offs = 0; page_num++; } } +static void ledc_write_engctrl2(TiLp55231 *ledc, uint8_t value) +{ + ledc_write(ledc, LP55231_ENGCTRL2_REG, &value, 1); + udelay(1500); +} + /* Run an lp55231 program on a controller. */ static void ledc_run_program(TiLp55231 *ledc, const TiLp55231Program *program_desc) { - uint8_t data; int i; + uint8_t data; + + /* All engines on hold. */ + data = LP55231_ENGCTRL1_CHIP_EN; + ledc_write(ledc, LP55231_ENGCTRL1_REG, &data, 1); + + ledc_write_engctrl2(ledc, LP55231_ENGCTRL2_ALL_DISABLE); + ledc_write_engctrl2(ledc, LP55231_ENGCTRL2_ALL_LOAD); - data = 0; - ledc_write(ledc, LP55231_ENGCTRL2_REG, &data, 1); - data = 0x15; - ledc_write(ledc, LP55231_ENGCTRL2_REG, &data, 1); ledc_write_program(ledc, program_desc->load_addr, program_desc->program_text, program_desc->program_size); @@ -295,10 +331,9 @@ static void ledc_run_program(TiLp55231 *ledc, ledc_write(ledc, LP55231_ENG1_PROG_START + i, program_desc->engine_start_addr + i, 1); - data = 0; - ledc_write(ledc, LP55231_ENGCTRL2_REG, &data, 1); - data = 0x2a; - ledc_write(ledc, LP55231_ENGCTRL2_REG, &data, 1); + data = LP55231_ENGCTRL1_CHIP_EN | LP55231_ENGCTRL1_ALL_ENG_GO; + ledc_write(ledc, LP55231_ENGCTRL1_REG, &data, 1); + ledc_write_engctrl2(ledc, LP55231_ENGCTRL2_ALL_RUN); } /* @@ -307,19 +342,15 @@ static void ledc_run_program(TiLp55231 *ledc, */ static int ledc_init_validate(TiLp55231 *ledc) { - const uint8_t ctrl1_reset[] = { - 0, - LP55231_ENGCTRL1_CHIP_EN, - LP55231_ENGCTRL1_CHIP_EN | LP55231_ENGCTRL1_ALL_ENG_GO - }; uint8_t data; int i; - ledc_reset(ledc); + if (ledc_reset(ledc)) + return -1; - /* Set up all engines to run. */ - for (i = 0; i < ARRAY_SIZE(ctrl1_reset); i++) - ledc_write(ledc, LP55231_ENGCTRL1_REG, ctrl1_reset + i, 1); + /* Enable the chip, keep engines in hold state. */ + data = LP55231_ENGCTRL1_CHIP_EN; + ledc_write(ledc, LP55231_ENGCTRL1_REG, &data, 1); /* * Internal clock, 3.3V output (pump 1x), autoincrement on multibyte @@ -334,7 +365,6 @@ static int ledc_init_validate(TiLp55231 *ledc) * value at reset. */ for (i = 0; i < 9; i++) { - data = 0; ledc_read(ledc, LP55231_D1_CRT_CTRL_REG + i, &data); if (data != LP55231_CRT_CTRL_DEFAULT) { printk(BIOS_WARNING, @@ -344,6 +374,13 @@ static int ledc_init_validate(TiLp55231 *ledc) } } + /* + * Signal Depthcharge that the controller has been initiazed by + * coreboot. + */ + data = LP55231_VARIABLE_COOKIE; + ledc_write(ledc, LP55231_VARIABLE_REG, &data, 1); + return 0; } @@ -364,9 +401,13 @@ int ww_ring_display_pattern(unsigned i2c_bus, enum VbScreenType_t screen_type) for (i = 0; i < ARRAY_SIZE(state_programs); i++) if (state_programs[i].vb_screen == screen_type) { int j; - for (j = 0; j < WW_RING_NUM_LED_CONTROLLERS; j++) + + for (j = 0; j < WW_RING_NUM_LED_CONTROLLERS; j++) { + if (!lp55231s[j].dev_addr) + continue; ledc_run_program(lp55231s + j, state_programs[i].programs[j]); + } return 0; } @@ -393,9 +434,17 @@ static void ww_ring_init(unsigned i2c_bus) if (!ledc_init_validate(ledc)) count++; + else + ledc->dev_addr = 0; /* Mark disabled. */ } printk(BIOS_INFO, "WW_RING: initialized %d out of %d\n", count, i); - if (count != i) - printk(BIOS_WARNING, "WW_RING: will keep going anyway\n"); + if (count != i) { + if (count) + printk(BIOS_WARNING, + "WW_RING: will keep going anyway\n"); + else + printk(BIOS_WARNING, + "WW_RING: LED ring not present\n"); + } } -- cgit v1.2.3