Re: [PATCH] staging: nvec: make i2c controller register writes robust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21/04/2024 11:46, Marc Dietrich wrote:
The i2c controller needs to read back the data written to its registers.
This way we can avoid the long delay in the interrupt handler.

Signed-off-by: Marc Dietrich <marvin24@xxxxxx>
---
  drivers/staging/nvec/nvec.c | 41 ++++++++++++++++++++++---------------
  1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 45df190c2f94..214839f51048 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -570,6 +570,22 @@ static void nvec_tx_set(struct nvec_chip *nvec)
  		(uint)nvec->tx->size, nvec->tx->data[1]);
  }

+/**
+ * i2c_writel - safely write to an I2C client controller register
+ * @val: value to be written
+ * @reg: register to write to
+ *
+ * A write to an I2C controller register needs to be read back to make sure
+ * that the value has arrived.
+ */
+static void i2c_writel(u32 val, void *reg)
+{
+	writel_relaxed(val, reg);
+
+	/* read back register to make sure that register writes completed */
+	readl_relaxed(reg);
+}

I thought the default behaviour of writel() should be to force writes
out of any CPU buffers. Are there any bus isuses here causing the code
to be necessary (and if so, why is there another buffer breaking the
writel behaviour?)

  /**
   * nvec_interrupt - Interrupt handler
   * @irq: The IRQ
@@ -604,7 +620,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
  	if ((status & RNW) == 0) {
  		received = readl(nvec->base + I2C_SL_RCVD);
  		if (status & RCVD)
-			writel(0, nvec->base + I2C_SL_RCVD);
+			i2c_writel(0, nvec->base + I2C_SL_RCVD);
  	}

  	if (status == (I2C_SL_IRQ | RCVD))
@@ -696,7 +712,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)

  	/* Send data if requested, but not on end of transmission */
  	if ((status & (RNW | END_TRANS)) == RNW)
-		writel(to_send, nvec->base + I2C_SL_RCVD);
+		i2c_writel(to_send, nvec->base + I2C_SL_RCVD);

  	/* If we have send the first byte */
  	if (status == (I2C_SL_IRQ | RNW | RCVD))
@@ -713,15 +729,6 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
  		status & RCVD ? " RCVD" : "",
  		status & RNW ? " RNW" : "");

-	/*
-	 * TODO: replace the udelay with a read back after each writel above
-	 * in order to work around a hardware issue, see i2c-tegra.c
-	 *
-	 * Unfortunately, this change causes an initialisation issue with the
-	 * touchpad, which needs to be fixed first.
-	 */
-	udelay(100);
-
  	return IRQ_HANDLED;
  }

@@ -737,15 +744,15 @@ static void tegra_init_i2c_slave(struct nvec_chip *nvec)

  	val = I2C_CNFG_NEW_MASTER_SFM | I2C_CNFG_PACKET_MODE_EN |
  	    (0x2 << I2C_CNFG_DEBOUNCE_CNT_SHIFT);
-	writel(val, nvec->base + I2C_CNFG);
+	i2c_writel(val, nvec->base + I2C_CNFG);

  	clk_set_rate(nvec->i2c_clk, 8 * 80000);

-	writel(I2C_SL_NEWSL, nvec->base + I2C_SL_CNFG);
-	writel(0x1E, nvec->base + I2C_SL_DELAY_COUNT);
+	i2c_writel(I2C_SL_NEWSL, nvec->base + I2C_SL_CNFG);
+	i2c_writel(0x1E, nvec->base + I2C_SL_DELAY_COUNT);

-	writel(nvec->i2c_addr >> 1, nvec->base + I2C_SL_ADDR1);
-	writel(0, nvec->base + I2C_SL_ADDR2);
+	i2c_writel(nvec->i2c_addr >> 1, nvec->base + I2C_SL_ADDR1);
+	i2c_writel(0, nvec->base + I2C_SL_ADDR2);

  	enable_irq(nvec->irq);
  }
@@ -754,7 +761,7 @@ static void tegra_init_i2c_slave(struct nvec_chip *nvec)
  static void nvec_disable_i2c_slave(struct nvec_chip *nvec)
  {
  	disable_irq(nvec->irq);
-	writel(I2C_SL_NEWSL | I2C_SL_NACK, nvec->base + I2C_SL_CNFG);
+	i2c_writel(I2C_SL_NEWSL | I2C_SL_NACK, nvec->base + I2C_SL_CNFG);
  	clk_disable_unprepare(nvec->i2c_clk);
  }
  #endif
--
2.43.0




--
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html





[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux