[coreboot-gerrit] New patch to review for coreboot: 934d297 i2c/ww_ring: various driver fixes and improvements
Patrick Georgi (pgeorgi@google.com)
gerrit at coreboot.org
Tue Apr 21 10:09:50 CEST 2015
Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/9867
-gerrit
commit 934d297612d09d3336f500ccc927d5b7721b5cb8
Author: Vadim Bendebury <vbendeb at chromium.org>
Date: Mon Mar 16 14:12:40 2015 -0700
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 <pgeorgi at chromium.org>
Original-Commit-Id: 5e0b4c84aca27460db594da1faf627ddee56f399
Original-Change-Id: I10f1f53cefb866d11ecf76ea48f74131d8b0ce77
Original-Signed-off-by: Vadim Bendebury <vbendeb at chromium.org>
Original-Reviewed-on: https://chromium-review.googlesource.com/260648
Original-Reviewed-by: Aaron Durbin <adurbin at chromium.org>
---
src/drivers/i2c/ww_ring/ww_ring.c | 99 +++++++++++++++++++++++++++++----------
1 file changed, 74 insertions(+), 25 deletions(-)
diff --git a/src/drivers/i2c/ww_ring/ww_ring.c b/src/drivers/i2c/ww_ring/ww_ring.c
index b074576..e1f255a 100644
--- a/src/drivers/i2c/ww_ring/ww_ring.c
+++ b/src/drivers/i2c/ww_ring/ww_ring.c
@@ -27,6 +27,7 @@
*/
#include <console/console.h>
+#include <delay.h>
#include <device/i2c.h>
#include <string.h>
@@ -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");
+ }
}
More information about the coreboot-gerrit
mailing list