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@chromium.org>
Original-Commit-Id: 5e0b4c84aca27460db594da1faf627ddee56f399
Original-Change-Id: I10f1f53cefb866d11ecf76ea48f74131d8b0ce77
Original-Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Original-Reviewed-on: https://chromium-review.googlesource.com/260648
Original-Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: http://review.coreboot.org/9867
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>

1 files 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 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 <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");
+	}
 }