Clean up documentation of RCIN# open-drain workaround

On many of the Haswell boards, RCIN# was attached to PL6, which is not
an open-drain capable GPIO.  As a workaround, we toggle it to an input
to get it into a high-Z state.  Now that we understand the problem,
document it and remove the FIXME tag from the comments.

Baytrail systems map RCIN# to a different pin, so don't need this
workaround at all.

BUG=chrome-os-partner:20173
BRANCH=none
TEST=build all boards; pass unit tests

Change-Id: I545a90a523e2967fad40bd47cb47a51983a37bdb
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/173796
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

6 files changed, 35 insertions, 32 deletions

diff --git a/board/bolt/board.c b/board/bolt/board.c
index 3d88c762d8..2b770e4db5 100644
--- a/board/bolt/board.c
+++ b/board/bolt/board.c
@@ -108,8 +108,11 @@ const struct gpio_info gpio_list[] = {
 	{"PCH_NMI_L",            LM4_GPIO_F, (1<<2), GPIO_ODR_HIGH, NULL},
 	{"PCH_PWRBTN_L",         LM4_GPIO_H, (1<<0), GPIO_OUT_HIGH, NULL},
 	{"PCH_PWROK",            LM4_GPIO_F, (1<<5), GPIO_OUT_LOW, NULL},
-	/* FIXME: Why is PL6 act like it is inverted. Setting value to
-	 * 0 makes the signal high, and setting it to 1 makes the signal low. */
+	/*
+	 * PL6 is one of 4 pins on the EC which can't be used in open-drain
+	 * mode.  To work around this PCH_RCIN_L is set to an input. It will
+	 * only be set to an output when it needs to be driven to 0.
+	 */
 	{"PCH_RCIN_L",           LM4_GPIO_L, (1<<6), GPIO_INPUT, NULL},
 	{"PCH_SYSRST_L",         LM4_GPIO_F, (1<<1), GPIO_ODR_HIGH, NULL},
 	{"PCH_SMI_L",            LM4_GPIO_F, (1<<4), GPIO_ODR_HIGH, NULL},
diff --git a/board/falco/board.c b/board/falco/board.c
index c037441214..1200060db0 100644
--- a/board/falco/board.c
+++ b/board/falco/board.c
@@ -104,10 +104,11 @@ const struct gpio_info gpio_list[] = {
 	{"PCH_NMI_L",            LM4_GPIO_F, (1<<2), GPIO_OUT_HIGH, NULL},
 	{"PCH_PWRBTN_L",         LM4_GPIO_H, (1<<0), GPIO_OUT_HIGH, NULL},
 	{"PCH_PWROK",            LM4_GPIO_F, (1<<5), GPIO_OUT_LOW, NULL},
-	/* FIXME: Why does PL6 not honor open drain semantics? Setting it to 1
-	 * drives the pin low while setting it to 0 drives the pin high. To
-	 * work around this PCH_RCIN_L is set to an input. It will only
-	 * be set to an output when it needs to be driven to 0. */
+	/*
+	 * PL6 is one of 4 pins on the EC which can't be used in open-drain
+	 * mode.  To work around this PCH_RCIN_L is set to an input. It will
+	 * only be set to an output when it needs to be driven to 0.
+	 */
 	{"PCH_RCIN_L",           LM4_GPIO_L, (1<<6), GPIO_INPUT, NULL},
 	{"PCH_RSMRST_L",         LM4_GPIO_F, (1<<1), GPIO_OUT_LOW, NULL},
 	{"PCH_SMI_L",            LM4_GPIO_F, (1<<4), GPIO_ODR_HIGH, NULL},
diff --git a/board/peppy/board.c b/board/peppy/board.c
index 738ea662b0..a60316552b 100644
--- a/board/peppy/board.c
+++ b/board/peppy/board.c
@@ -102,10 +102,11 @@ const struct gpio_info gpio_list[] = {
 	{"PCH_NMI_L",            LM4_GPIO_F, (1<<2), GPIO_OUT_HIGH, NULL},
 	{"PCH_PWRBTN_L",         LM4_GPIO_H, (1<<0), GPIO_OUT_HIGH, NULL},
 	{"PCH_PWROK",            LM4_GPIO_F, (1<<5), GPIO_OUT_LOW, NULL},
-	/* FIXME: Why does PL6 not honor open drain semantics? Setting it to 1
-	 * drives the pin low while setting it to 0 drives the pin high. To
-	 * work around this PCH_RCIN_L is set to an input. It will only
-	 * be set to an output when it needs to be driven to 0. */
+	/*
+	 * PL6 is one of 4 pins on the EC which can't be used in open-drain
+	 * mode.  To work around this PCH_RCIN_L is set to an input. It will
+	 * only be set to an output when it needs to be driven to 0.
+	 */
 	{"PCH_RCIN_L",           LM4_GPIO_L, (1<<6), GPIO_INPUT, NULL},
 	{"PCH_RSMRST_L",         LM4_GPIO_F, (1<<1), GPIO_OUT_LOW, NULL},
 	{"PCH_SMI_L",            LM4_GPIO_F, (1<<4), GPIO_ODR_HIGH, NULL},
diff --git a/board/slippy/board.c b/board/slippy/board.c
index df731f3610..680b021a84 100644
--- a/board/slippy/board.c
+++ b/board/slippy/board.c
@@ -102,10 +102,11 @@ const struct gpio_info gpio_list[] = {
 	{"PCH_NMI_L",            LM4_GPIO_F, (1<<2), GPIO_OUT_HIGH, NULL},
 	{"PCH_PWRBTN_L",         LM4_GPIO_H, (1<<0), GPIO_OUT_HIGH, NULL},
 	{"PCH_PWROK",            LM4_GPIO_F, (1<<5), GPIO_OUT_LOW, NULL},
-	/* FIXME: Why does PL6 not honor open drain semantics? Setting it to 1
-	 * drives the pin low while setting it to 0 drives the pin high. To
-	 * work around this PCH_RCIN_L is set to an input. It will only
-	 * be set to an output when it needs to be driven to 0. */
+	/*
+	 * PL6 is one of 4 pins on the EC which can't be used in open-drain
+	 * mode.  To work around this PCH_RCIN_L is set to an input. It will
+	 * only be set to an output when it needs to be driven to 0.
+	 */
 	{"PCH_RCIN_L",           LM4_GPIO_L, (1<<6), GPIO_INPUT, NULL},
 	{"PCH_RSMRST_L",         LM4_GPIO_F, (1<<1), GPIO_OUT_LOW, NULL},
 	{"PCH_SMI_L",            LM4_GPIO_F, (1<<4), GPIO_ODR_HIGH, NULL},
diff --git a/common/chipset_baytrail.c b/common/chipset_baytrail.c
index c5f17f5a64..7de5aaac01 100644
--- a/common/chipset_baytrail.c
+++ b/common/chipset_baytrail.c
@@ -90,19 +90,12 @@ void chipset_reset(int cold_reset)
 		/*
 		 * Send a reset pulse to the PCH.  This just causes it to
 		 * assert INIT# to the CPU without dropping power or asserting
-		 * PLTRST# to reset the rest of the system.
+		 * PLTRST# to reset the rest of the system.  Pulse must be at
+		 * least 16 PCI clocks long = 500 ns.
 		 */
-
-		/*
-		 * Pulse must be at least 16 PCI clocks long = 500 ns. The gpio
-		 * pin used by the EC (PL6) does not behave in the correct
-		 * manner when configured as open drain. In order to mimic
-		 * open drain, the pin is initially configured as an input.
-		 * When it is needed to drive low, the flags are updated which
-		 * changes the pin to an output and drives the pin low.  */
-		gpio_set_flags(GPIO_PCH_RCIN_L, GPIO_OUT_LOW);
+		gpio_set_level(GPIO_PCH_RCIN_L, 0);
 		udelay(10);
-		gpio_set_flags(GPIO_PCH_RCIN_L, GPIO_INPUT);
+		gpio_set_level(GPIO_PCH_RCIN_L, 1);
 	}
 }
 
diff --git a/common/chipset_haswell.c b/common/chipset_haswell.c
index 6bc4b3283d..b656cb8797 100644
--- a/common/chipset_haswell.c
+++ b/common/chipset_haswell.c
@@ -91,16 +91,20 @@ void chipset_reset(int cold_reset)
 		/*
 		 * Send a RCIN# pulse to the PCH.  This just causes it to
 		 * assert INIT# to the CPU without dropping power or asserting
-		 * PLTRST# to reset the rest of the system.
+		 * PLTRST# to reset the rest of the system.  Pulse must be at
+		 * least 16 PCI clocks long = 500 ns.
 		 */
 
 		/*
-		 * Pulse must be at least 16 PCI clocks long = 500 ns. The gpio
-		 * pin used by the EC (PL6) does not behave in the correct
-		 * manner when configured as open drain. In order to mimic
-		 * open drain, the pin is initially configured as an input.
-		 * When it is needed to drive low, the flags are updated which
-		 * changes the pin to an output and drives the pin low.  */
+		 * The gpio pin used by the EC (PL6) does not behave in the
+		 * correct manner when configured as open drain. In order to
+		 * mimic open drain, the pin is initially configured as an
+		 * input.  When it is needed to drive low, the flags are
+		 * updated which changes the pin to an output and drives the
+		 * pin low.  Note that this logic will work fine even on boards
+		 * where RCIN# has been moved to a different pin, so there's no
+		 * need to #ifdef this behavior.  See crosbug.com/p/20173.
+		 */
 		gpio_set_flags(GPIO_PCH_RCIN_L, GPIO_OUT_LOW);
 		udelay(10);
 		gpio_set_flags(GPIO_PCH_RCIN_L, GPIO_INPUT);