[Skiboot-stable] [PATCH] npu2-opencapi: Don't drive reset signal permanently

[Frederic Barrat]

[ Upstream commit 53408440edb30de7ad18f12db285f15a0863fbc3 ]

A problem was found with the way we manage the I2C signal to reset
adapters. Skiboot currently always drives the value of the opencapi
reset signal. We set the I2C pin for reset in output mode and keep it
in output mode permanently. And since the reset signal is inverted, it
is explicitly set to high by the I2C controller pretty much all the
time.

When the opencapi card is powered off, for example on a reboot,
actively driving the I2C reset pin to high keeps applying a voltage to
part of the FPGA, which can leak current, send the FPGA in a bad state
since it's unexpected or even damage the card. To prevent damaging
adapters, the recommendation from the hardware team is to switch back
the pin to input mode at the end of a reset cycle. There are pull-up
resistors on the planar of all the platforms to make sure the reset
signal is high "naturally". When the slot is powered off, the reset
pin won't be kept high by the i2c controller any more.

Signed-off-by: Frederic Barrat <fbarrat at linux.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
---
 hw/npu2-opencapi.c | 46 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c
index f855cdfc..4d15240e 100644
--- a/hw/npu2-opencapi.c
+++ b/hw/npu2-opencapi.c
@@ -918,19 +918,53 @@ err:
 static void deassert_adapter_reset(struct npu2_dev *dev)
 {
 	uint8_t pin, data;
-	int rc;
+	int rc, rc2;
 
 	pin = get_reset_pin(dev);
 
+	/*
+	 * All we need to do here is deassert the reset signal by
+	 * setting the reset pin to high. However, we cannot leave the
+	 * pin in output mode, as it can cause troubles with the
+	 * opencapi adapter: when the slot is powered off (on a reboot
+	 * for example), if the i2c controller is actively setting the
+	 * reset signal to high, it maintains voltage on part of the
+	 * fpga and can leak current. It can lead the fpga to be in an
+	 * unspecified state and potentially cause damage.
+	 *
+	 * The circumvention is to set the pin back to input
+	 * mode. There are pullup resistors on the planar on all
+	 * platforms to make sure the signal will "naturally" be high,
+	 * without the i2c controller actively setting it, so we won't
+	 * have problems when the slot is powered off. And it takes
+	 * the adapter out of reset.
+	 *
+	 * To summarize:
+	 * 1. set the pin to input mode. That is enough to raise the
+	 *    signal
+	 * 2. set the value of the pin to high. The pin is input mode,
+	 *    so it won't really do anything. But it's more coherent
+	 *    and avoids bad surprises on the next call to
+	 *    assert_adapter_reset()
+	 */
 	lock(&dev->npu->i2c_lock);
-	dev->npu->i2c_pin_wr_state |= pin;
-	data = dev->npu->i2c_pin_wr_state;
+	dev->npu->i2c_pin_mode |= pin;
+	data = dev->npu->i2c_pin_mode;
 
 	rc = i2c_request_send(dev->npu->i2c_port_id_ocapi,
-			platform.ocapi->i2c_reset_addr, SMBUS_WRITE,
-			0x1, 1,
-			&data, sizeof(data), 120);
+			      platform.ocapi->i2c_reset_addr, SMBUS_WRITE,
+			      0x3, 1,
+			      &data, sizeof(data), 120);
+
+	dev->npu->i2c_pin_wr_state |= pin;
+	data = dev->npu->i2c_pin_wr_state;
+	rc2 = i2c_request_send(dev->npu->i2c_port_id_ocapi,
+			       platform.ocapi->i2c_reset_addr, SMBUS_WRITE,
+			       0x1, 1,
+			       &data, sizeof(data), 120);
 	unlock(&dev->npu->i2c_lock);
+	if (!rc)
+		rc = rc2;
 	if (rc) {
 		/**
 		 * @fwts-label OCAPIDeviceResetFailed
-- 
2.21.1