From 15e2fa02fdae240411df7a4e3298b988736a4499 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Mon, 21 Oct 2013 10:41:44 -0700 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/173796 Reviewed-by: Bill Richardson --- board/bolt/board.c | 7 +++++-- board/falco/board.c | 9 +++++---- board/peppy/board.c | 9 +++++---- board/slippy/board.c | 9 +++++---- common/chipset_baytrail.c | 15 ++++----------- common/chipset_haswell.c | 18 +++++++++++------- 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); -- cgit v1.2.1