linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/17] Improvements for Tegra I2C driver
@ 2020-09-01 21:10 Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 01/17] i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer Dmitry Osipenko
                   ` (16 more replies)
  0 siblings, 17 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Hello!

This series performs a small refactoring of the Tegra I2C driver code and
hardens the atomic-transfer mode.

Changelog:

v2: - Cleaned more messages in the "Clean up messages in the code" patch.

    - The error code of reset_control_reset() is checked now.

    - Added these new patches to clean up couple more things:

        i2c: tegra: Check errors for both positive and negative values
        i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load()
        i2c: tegra: Remove unnecessary whitespaces and newlines
        i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear()
        i2c: tegra: Improve driver module description

Dmitry Osipenko (17):
  i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer
  i2c: tegra: Add missing newline before returns
  i2c: tegra: Clean up messages in the code
  i2c: tegra: Don't ignore tegra_i2c_flush_fifos() error
  i2c: tegra: Use reset_control_reset()
  i2c: tegra: Improve formatting of function variables
  i2c: tegra: Use dev_err_probe()
  i2c: tegra: Runtime PM always available on Tegra
  i2c: tegra: Clean up probe function
  i2c: tegra: Drop '_timeout' from wait/poll function names
  i2c: tegra: Remove likely/unlikely from the code
  i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg()
  i2c: tegra: Check errors for both positive and negative values
  i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load()
  i2c: tegra: Remove unnecessary whitespaces and newlines
  i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear()
  i2c: tegra: Improve driver module description

 drivers/i2c/busses/i2c-tegra.c | 686 ++++++++++++++++++---------------
 1 file changed, 379 insertions(+), 307 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 01/17] i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 02/17] i2c: tegra: Add missing newline before returns Dmitry Osipenko
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

The tegra_i2c_flush_fifos() shouldn't sleep in atomic transfer and jiffies
are not updating if interrupts are disabled. Hence let's use proper delay
functions and use ktime API in order not to hang atomic transfer. Note
that this patch doesn't fix any known problem because normally FIFO is
flushed at the time of starting a new transfer.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 00d3e4d7a01e..aab58395fb71 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -470,7 +470,8 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
 
 static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
 {
-	unsigned long timeout = jiffies + HZ;
+	ktime_t ktime = ktime_get();
+	ktime_t ktimeout = ktime_add_ms(ktime, 1000);
 	unsigned int offset;
 	u32 mask, val;
 
@@ -489,13 +490,22 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
 	i2c_writel(i2c_dev, val, offset);
 
 	while (i2c_readl(i2c_dev, offset) & mask) {
-		if (time_after(jiffies, timeout)) {
-			dev_warn(i2c_dev->dev, "timeout waiting for fifo flush\n");
-			return -ETIMEDOUT;
-		}
-		usleep_range(1000, 2000);
+		if (ktime_after(ktime, ktimeout))
+			goto err_timeout;
+
+		if (i2c_dev->is_curr_atomic_xfer)
+			mdelay(1);
+		else
+			fsleep(1000);
+
+		ktime = ktime_get();
 	}
 	return 0;
+
+err_timeout:
+	dev_err(i2c_dev->dev, "fifo flush timed out\n");
+
+	return -ETIMEDOUT;
 }
 
 static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev *i2c_dev)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 02/17] i2c: tegra: Add missing newline before returns
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 01/17] i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 03/17] i2c: tegra: Clean up messages in the code Dmitry Osipenko
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Some places in the code are missing a newline before return, making
code more difficult to read and creating inconsistency of the code.
This patch adds the missing newlines.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index aab58395fb71..9bd91b6f32f4 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -317,6 +317,7 @@ static unsigned long tegra_i2c_reg_addr(struct tegra_i2c_dev *i2c_dev,
 		reg += (reg >= I2C_TX_FIFO) ? 0x10 : 0x40;
 	else if (i2c_dev->is_vi)
 		reg = 0xc00 + (reg << 2);
+
 	return reg;
 }
 
@@ -392,6 +393,7 @@ static int tegra_i2c_dma_submit(struct tegra_i2c_dev *i2c_dev, size_t len)
 	dma_desc->callback_param = i2c_dev;
 	dmaengine_submit(dma_desc);
 	dma_async_issue_pending(chan);
+
 	return 0;
 }
 
@@ -500,6 +502,7 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
 
 		ktime = ktime_get();
 	}
+
 	return 0;
 
 err_timeout:
@@ -707,6 +710,7 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
 	clk_disable(i2c_dev->slow_clk);
 disable_fast_clk:
 	clk_disable(i2c_dev->fast_clk);
+
 	return ret;
 }
 
@@ -1421,6 +1425,7 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap)
 
 	if (i2c_dev->hw->has_continue_xfer_support)
 		ret |= I2C_FUNC_NOSTART;
+
 	return ret;
 }
 
@@ -1888,6 +1893,7 @@ static int tegra_i2c_remove(struct platform_device *pdev)
 	clk_unprepare(i2c_dev->fast_clk);
 
 	tegra_i2c_release_dma(i2c_dev);
+
 	return 0;
 }
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 03/17] i2c: tegra: Clean up messages in the code
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 01/17] i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 02/17] i2c: tegra: Add missing newline before returns Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
       [not found]   ` <CAHp75Vf9ETJMibQGe4Nx7n4703GtgO1XBsE1yGwsk3TaSPTDHw@mail.gmail.com>
  2020-09-01 21:10 ` [PATCH v2 04/17] i2c: tegra: Don't ignore tegra_i2c_flush_fifos() error Dmitry Osipenko
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Use lowercase and consistent wording for all messages in the code.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 65 ++++++++++++++++------------------
 1 file changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 9bd91b6f32f4..0d358bc12973 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -377,7 +377,7 @@ static int tegra_i2c_dma_submit(struct tegra_i2c_dev *i2c_dev, size_t len)
 	enum dma_transfer_direction dir;
 	struct dma_chan *chan;
 
-	dev_dbg(i2c_dev->dev, "starting DMA for length: %zu\n", len);
+	dev_dbg(i2c_dev->dev, "starting dma for length: %zu\n", len);
 	reinit_completion(&i2c_dev->dma_complete);
 	dir = i2c_dev->msg_read ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV;
 	chan = i2c_dev->msg_read ? i2c_dev->rx_dma_chan : i2c_dev->tx_dma_chan;
@@ -385,7 +385,7 @@ static int tegra_i2c_dma_submit(struct tegra_i2c_dev *i2c_dev, size_t len)
 					       len, dir, DMA_PREP_INTERRUPT |
 					       DMA_CTRL_ACK);
 	if (!dma_desc) {
-		dev_err(i2c_dev->dev, "failed to get DMA descriptor\n");
+		dev_err(i2c_dev->dev, "failed to get dma descriptor\n");
 		return -EINVAL;
 	}
 
@@ -427,7 +427,7 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
 		return 0;
 
 	if (!IS_ENABLED(CONFIG_TEGRA20_APB_DMA)) {
-		dev_dbg(i2c_dev->dev, "Support for APB DMA not enabled!\n");
+		dev_dbg(i2c_dev->dev, "dma support not enabled\n");
 		return 0;
 	}
 
@@ -450,7 +450,7 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
 	dma_buf = dma_alloc_coherent(i2c_dev->dev, i2c_dev->dma_buf_size,
 				     &dma_phys, GFP_KERNEL | __GFP_NOWARN);
 	if (!dma_buf) {
-		dev_err(i2c_dev->dev, "failed to allocate the DMA buffer\n");
+		dev_err(i2c_dev->dev, "failed to allocate dma buffer\n");
 		err = -ENOMEM;
 		goto err_out;
 	}
@@ -462,8 +462,8 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
 err_out:
 	tegra_i2c_release_dma(i2c_dev);
 	if (err != -EPROBE_DEFER) {
-		dev_err(i2c_dev->dev, "cannot use DMA: %d\n", err);
-		dev_err(i2c_dev->dev, "falling back to PIO\n");
+		dev_err(i2c_dev->dev, "cannot use dma: %d\n", err);
+		dev_err(i2c_dev->dev, "falling back to pio\n");
 		return 0;
 	}
 
@@ -672,8 +672,7 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
 
 	ret = clk_enable(i2c_dev->fast_clk);
 	if (ret < 0) {
-		dev_err(i2c_dev->dev,
-			"Enabling fast clk failed, err %d\n", ret);
+		dev_err(dev, "failed to enable fast clock: %d\n", ret);
 		return ret;
 	}
 
@@ -685,8 +684,7 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
 
 	ret = clk_enable(i2c_dev->div_clk);
 	if (ret < 0) {
-		dev_err(i2c_dev->dev,
-			"Enabling div clk failed, err %d\n", ret);
+		dev_err(dev, "failed to enable div clock: %d\n", ret);
 		goto disable_slow_clk;
 	}
 
@@ -747,8 +745,7 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
 						I2C_CONFIG_LOAD_TIMEOUT);
 
 		if (err) {
-			dev_warn(i2c_dev->dev,
-				 "timeout waiting for config load\n");
+			dev_err(i2c_dev->dev, "failed to load config\n");
 			return err;
 		}
 	}
@@ -850,7 +847,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit)
 				   i2c_dev->bus_clk_rate * clk_multiplier);
 		if (err) {
 			dev_err(i2c_dev->dev,
-				"failed changing clock rate: %d\n", err);
+				"failed to set div-clk rate: %d\n", err);
 			return err;
 		}
 	}
@@ -1052,8 +1049,7 @@ static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev,
 		slv_config.device_fc = true;
 		ret = dmaengine_slave_config(chan, &slv_config);
 		if (ret < 0) {
-			dev_err(i2c_dev->dev, "DMA slave config failed: %d\n",
-				ret);
+			dev_err(i2c_dev->dev, "dma config failed: %d\n", ret);
 			dev_err(i2c_dev->dev, "falling back to PIO\n");
 			tegra_i2c_release_dma(i2c_dev);
 			i2c_dev->is_curr_dma_xfer = false;
@@ -1163,8 +1159,7 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap)
 
 	reg = i2c_readl(i2c_dev, I2C_BUS_CLEAR_STATUS);
 	if (!(reg & I2C_BC_STATUS)) {
-		dev_err(i2c_dev->dev,
-			"un-recovered arbitration lost\n");
+		dev_err(i2c_dev->dev, "un-recovered arbitration lost\n");
 		return -EIO;
 	}
 
@@ -1221,8 +1216,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
 			if (err < 0) {
 				dev_err(i2c_dev->dev,
-					"starting RX DMA failed, err %d\n",
-					err);
+					"starting rx dma failed: %d\n", err);
 				return err;
 			}
 
@@ -1281,8 +1275,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
 			if (err < 0) {
 				dev_err(i2c_dev->dev,
-					"starting TX DMA failed, err %d\n",
-					err);
+					"starting tx dma failed: %d\n", err);
 				return err;
 			}
 		} else {
@@ -1321,7 +1314,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 					 i2c_dev->tx_dma_chan);
 
 		if (!time_left && !completion_done(&i2c_dev->dma_complete)) {
-			dev_err(i2c_dev->dev, "DMA transfer timeout\n");
+			dev_err(i2c_dev->dev, "dma transfer timeout\n");
 			tegra_i2c_init(i2c_dev, true);
 			return -ETIMEDOUT;
 		}
@@ -1676,7 +1669,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (!res) {
-		dev_err(&pdev->dev, "no irq resource\n");
+		dev_err(dev, "no irq resource\n");
 		return -EINVAL;
 	}
 	irq = res->start;
@@ -1684,7 +1677,8 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	div_clk = devm_clk_get(&pdev->dev, "div-clk");
 	if (IS_ERR(div_clk)) {
 		if (PTR_ERR(div_clk) != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "missing controller clock\n");
+			dev_err(&pdev->dev, "failed to get div-clk: %ld\n",
+				PTR_ERR(div_clk));
 
 		return PTR_ERR(div_clk);
 	}
@@ -1705,7 +1699,9 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	i2c_dev->rst = devm_reset_control_get_exclusive(&pdev->dev, "i2c");
 	if (IS_ERR(i2c_dev->rst)) {
-		dev_err(&pdev->dev, "missing controller reset\n");
+		dev_err(dev, "failed to get reset control: %pe\n",
+			i2c_dev->rst);
+
 		return PTR_ERR(i2c_dev->rst);
 	}
 
@@ -1725,7 +1721,9 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	if (!i2c_dev->hw->has_single_clk_source) {
 		fast_clk = devm_clk_get(&pdev->dev, "fast-clk");
 		if (IS_ERR(fast_clk)) {
-			dev_err(&pdev->dev, "missing fast clock\n");
+			dev_err(dev, "failed to get fast clock\n: %ld\n",
+				PTR_ERR(fast_clk));
+
 			return PTR_ERR(fast_clk);
 		}
 		i2c_dev->fast_clk = fast_clk;
@@ -1746,7 +1744,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	ret = clk_prepare(i2c_dev->fast_clk);
 	if (ret < 0) {
-		dev_err(i2c_dev->dev, "Clock prepare failed %d\n", ret);
+		dev_err(dev, "failed to prepare fast clock: %d\n", ret);
 		return ret;
 	}
 
@@ -1770,7 +1768,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	ret = clk_prepare(i2c_dev->div_clk);
 	if (ret < 0) {
-		dev_err(i2c_dev->dev, "Clock prepare failed %d\n", ret);
+		dev_err(dev, "failed to prepare div-clk: %d\n", ret);
 		goto unprepare_slow_clk;
 	}
 
@@ -1787,13 +1785,13 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	if (!pm_runtime_enabled(&pdev->dev)) {
 		ret = tegra_i2c_runtime_resume(&pdev->dev);
 		if (ret < 0) {
-			dev_err(&pdev->dev, "runtime resume failed\n");
+			dev_err(dev, "runtime resume failed\n");
 			goto unprepare_div_clk;
 		}
 	} else {
 		ret = pm_runtime_get_sync(i2c_dev->dev);
 		if (ret < 0) {
-			dev_err(&pdev->dev, "runtime resume failed\n");
+			dev_err(dev, "runtime resume failed\n");
 			goto disable_rpm;
 		}
 	}
@@ -1801,8 +1799,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	if (i2c_dev->is_multimaster_mode) {
 		ret = clk_enable(i2c_dev->div_clk);
 		if (ret < 0) {
-			dev_err(i2c_dev->dev, "div_clk enable failed %d\n",
-				ret);
+			dev_err(dev, "failed to enable div-clk: %d\n", ret);
 			goto put_rpm;
 		}
 	}
@@ -1816,7 +1813,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	ret = tegra_i2c_init(i2c_dev, false);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to initialize i2c controller\n");
+		dev_err(dev, "failed to initialize i2c controller\n");
 		goto release_dma;
 	}
 
@@ -1825,7 +1822,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
 			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c_dev);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to request irq %i\n", i2c_dev->irq);
+		dev_err(dev, "failed to request irq %i\n", i2c_dev->irq);
 		goto release_dma;
 	}
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 04/17] i2c: tegra: Don't ignore tegra_i2c_flush_fifos() error
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 03/17] i2c: tegra: Clean up messages in the code Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 05/17] i2c: tegra: Use reset_control_reset() Dmitry Osipenko
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

The tegra_i2c_flush_fifos() may fail and transfer should be aborted in
this case.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 0d358bc12973..2da45ff32621 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1179,7 +1179,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 	bool dma;
 	u16 xfer_time = 100;
 
-	tegra_i2c_flush_fifos(i2c_dev);
+	err = tegra_i2c_flush_fifos(i2c_dev);
+	if (err)
+		return err;
 
 	i2c_dev->msg_buf = msg->buf;
 	i2c_dev->msg_buf_remaining = msg->len;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 05/17] i2c: tegra: Use reset_control_reset()
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 04/17] i2c: tegra: Don't ignore tegra_i2c_flush_fifos() error Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 06/17] i2c: tegra: Improve formatting of function variables Dmitry Osipenko
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Use a single reset_control_reset() instead of assert/deasset couple in
order to make code cleaner a tad. Note that the reset_control_reset()
uses 1 microsecond delay instead of 2 that was used previously, but this
shouldn't matter. In addition don't ignore potential error of the reset.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 2da45ff32621..aa815e48f77e 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -790,9 +790,9 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit)
 	u32 tsu_thd;
 	u8 tlow, thigh;
 
-	reset_control_assert(i2c_dev->rst);
-	udelay(2);
-	reset_control_deassert(i2c_dev->rst);
+	err = reset_control_reset(i2c_dev->rst);
+	if (WARN_ON_ONCE(err))
+		return err;
 
 	if (i2c_dev->is_dvc)
 		tegra_dvc_init(i2c_dev);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 06/17] i2c: tegra: Improve formatting of function variables
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 05/17] i2c: tegra: Use reset_control_reset() Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 07/17] i2c: tegra: Use dev_err_probe() Dmitry Osipenko
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Reorder the definition of variables in the code in order to make code
easier to read.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 51 ++++++++++++++--------------------
 1 file changed, 21 insertions(+), 30 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index aa815e48f77e..16f105283145 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -419,8 +419,8 @@ static void tegra_i2c_release_dma(struct tegra_i2c_dev *i2c_dev)
 static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
 {
 	struct dma_chan *chan;
-	u32 *dma_buf;
 	dma_addr_t dma_phys;
+	u32 *dma_buf;
 	int err;
 
 	if (!i2c_dev->hw->has_apb_dma || i2c_dev->is_vi)
@@ -513,11 +513,11 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
 
 static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev *i2c_dev)
 {
-	u32 val;
-	int rx_fifo_avail;
-	u8 *buf = i2c_dev->msg_buf;
 	size_t buf_remaining = i2c_dev->msg_buf_remaining;
+	u8 *buf = i2c_dev->msg_buf;
 	int words_to_transfer;
+	int rx_fifo_avail;
+	u32 val;
 
 	/*
 	 * Catch overflow due to message fully sent
@@ -574,11 +574,11 @@ static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev *i2c_dev)
 
 static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev *i2c_dev)
 {
-	u32 val;
-	int tx_fifo_avail;
-	u8 *buf = i2c_dev->msg_buf;
 	size_t buf_remaining = i2c_dev->msg_buf_remaining;
+	u8 *buf = i2c_dev->msg_buf;
 	int words_to_transfer;
+	int tx_fifo_avail;
+	u32 val;
 
 	if (i2c_dev->hw->has_mst_fifo) {
 		val = i2c_readl(i2c_dev, I2C_MST_FIFO_STATUS);
@@ -784,11 +784,8 @@ static void tegra_i2c_vi_init(struct tegra_i2c_dev *i2c_dev)
 
 static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool clk_reinit)
 {
-	u32 val;
+	u32 val, clk_divisor, clk_multiplier, tsu_thd, tlow, thigh;
 	int err;
-	u32 clk_divisor, clk_multiplier;
-	u32 tsu_thd;
-	u8 tlow, thigh;
 
 	err = reset_control_reset(i2c_dev->rst);
 	if (WARN_ON_ONCE(err))
@@ -896,9 +893,9 @@ static int tegra_i2c_disable_packet_mode(struct tegra_i2c_dev *i2c_dev)
 
 static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
 {
-	u32 status;
 	const u32 status_err = I2C_INT_NO_ACK | I2C_INT_ARBITRATION_LOST;
 	struct tegra_i2c_dev *i2c_dev = dev_id;
+	u32 status;
 
 	status = i2c_readl(i2c_dev, I2C_INT_STATUS);
 
@@ -1002,12 +999,11 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
 static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev,
 				       size_t len)
 {
-	u32 val, reg;
-	u8 dma_burst;
 	struct dma_slave_config slv_config = {0};
+	unsigned long reg_offset;
+	u32 val, reg, dma_burst;
 	struct dma_chan *chan;
 	int ret;
-	unsigned long reg_offset;
 
 	if (i2c_dev->hw->has_mst_fifo)
 		reg = I2C_MST_FIFO_CONTROL;
@@ -1132,9 +1128,9 @@ tegra_i2c_wait_completion_timeout(struct tegra_i2c_dev *i2c_dev,
 static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap)
 {
 	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
-	int err;
 	unsigned long time_left;
 	u32 reg;
+	int err;
 
 	reinit_completion(&i2c_dev->msg_complete);
 	reg = FIELD_PREP(I2C_BC_SCLK_THRESHOLD, 9) | I2C_BC_STOP_COND |
@@ -1170,14 +1166,12 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 			      struct i2c_msg *msg,
 			      enum msg_end_type end_state)
 {
-	u32 packet_header;
-	u32 int_mask;
-	unsigned long time_left;
-	size_t xfer_size;
+	unsigned long time_left, xfer_time = 100;
+	u32 packet_header, int_mask;
 	u32 *buffer = NULL;
-	int err = 0;
+	size_t xfer_size;
 	bool dma;
-	u16 xfer_time = 100;
+	int err;
 
 	err = tegra_i2c_flush_fifos(i2c_dev);
 	if (err)
@@ -1371,8 +1365,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 			  int num)
 {
 	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
-	int i;
-	int ret;
+	int i, ret;
 
 	ret = pm_runtime_get_sync(i2c_dev->dev);
 	if (ret < 0) {
@@ -1427,8 +1420,8 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap)
 static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev)
 {
 	struct device_node *np = i2c_dev->dev->of_node;
-	int ret;
 	bool multi_mode;
+	int ret;
 
 	ret = of_property_read_u32(np, "clock-frequency",
 				   &i2c_dev->bus_clk_rate);
@@ -1654,14 +1647,12 @@ MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
 static int tegra_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct clk *div_clk, *fast_clk;
 	struct tegra_i2c_dev *i2c_dev;
+	phys_addr_t base_phys;
 	struct resource *res;
-	struct clk *div_clk;
-	struct clk *fast_clk;
 	void __iomem *base;
-	phys_addr_t base_phys;
-	int irq;
-	int ret;
+	int irq, ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base_phys = res->start;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 07/17] i2c: tegra: Use dev_err_probe()
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 06/17] i2c: tegra: Improve formatting of function variables Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 08/17] i2c: tegra: Runtime PM always available on Tegra Dmitry Osipenko
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Use dev_err_probe() to replace the manual -EPROBE_DEFER handling, making
code cleaner.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 16f105283145..c142b424e46d 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1646,8 +1646,8 @@ MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
 
 static int tegra_i2c_probe(struct platform_device *pdev)
 {
+	struct clk *div_clk, *fast_clk, *slow_clk;
 	struct device *dev = &pdev->dev;
-	struct clk *div_clk, *fast_clk;
 	struct tegra_i2c_dev *i2c_dev;
 	phys_addr_t base_phys;
 	struct resource *res;
@@ -1668,13 +1668,9 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	irq = res->start;
 
 	div_clk = devm_clk_get(&pdev->dev, "div-clk");
-	if (IS_ERR(div_clk)) {
-		if (PTR_ERR(div_clk) != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "failed to get div-clk: %ld\n",
-				PTR_ERR(div_clk));
-
-		return PTR_ERR(div_clk);
-	}
+	if (IS_ERR(div_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(div_clk),
+				     "failed to get div-clk\n");
 
 	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
 	if (!i2c_dev)
@@ -1713,24 +1709,20 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	if (!i2c_dev->hw->has_single_clk_source) {
 		fast_clk = devm_clk_get(&pdev->dev, "fast-clk");
-		if (IS_ERR(fast_clk)) {
-			dev_err(dev, "failed to get fast clock\n: %ld\n",
-				PTR_ERR(fast_clk));
+		if (IS_ERR(fast_clk))
+			return dev_err_probe(&pdev->dev, PTR_ERR(fast_clk),
+					     "failed to get fast clock\n");
 
-			return PTR_ERR(fast_clk);
-		}
 		i2c_dev->fast_clk = fast_clk;
 	}
 
 	if (i2c_dev->is_vi) {
-		i2c_dev->slow_clk = devm_clk_get(dev, "slow");
-		if (IS_ERR(i2c_dev->slow_clk)) {
-			if (PTR_ERR(i2c_dev->slow_clk) != -EPROBE_DEFER)
-				dev_err(dev, "failed to get slow clock: %ld\n",
-					PTR_ERR(i2c_dev->slow_clk));
+		slow_clk = devm_clk_get(dev, "slow");
+		if (IS_ERR(slow_clk))
+			return dev_err_probe(&pdev->dev, PTR_ERR(slow_clk),
+					     "failed to get slow clock\n");
 
-			return PTR_ERR(i2c_dev->slow_clk);
-		}
+		i2c_dev->slow_clk = slow_clk;
 	}
 
 	platform_set_drvdata(pdev, i2c_dev);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 08/17] i2c: tegra: Runtime PM always available on Tegra
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (6 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 07/17] i2c: tegra: Use dev_err_probe() Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 09/17] i2c: tegra: Clean up probe function Dmitry Osipenko
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

The runtime PM is guaranteed to be always available on Tegra after commit
40b2bb1b132a ("ARM: tegra: enforce PM requirement"). Hence let's remove
all the RPM-availability checking and handling from the code.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index c142b424e46d..a5d9e3ce6320 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1767,18 +1767,10 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	if (!i2c_dev->is_vi)
 		pm_runtime_irq_safe(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-	if (!pm_runtime_enabled(&pdev->dev)) {
-		ret = tegra_i2c_runtime_resume(&pdev->dev);
-		if (ret < 0) {
-			dev_err(dev, "runtime resume failed\n");
-			goto unprepare_div_clk;
-		}
-	} else {
-		ret = pm_runtime_get_sync(i2c_dev->dev);
-		if (ret < 0) {
-			dev_err(dev, "runtime resume failed\n");
-			goto disable_rpm;
-		}
+	ret = pm_runtime_get_sync(i2c_dev->dev);
+	if (ret < 0) {
+		dev_err(dev, "runtime resume failed\n");
+		goto disable_rpm;
 	}
 
 	if (i2c_dev->is_multimaster_mode) {
@@ -1836,16 +1828,10 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 		clk_disable(i2c_dev->div_clk);
 
 put_rpm:
-	if (pm_runtime_enabled(&pdev->dev))
-		pm_runtime_put_sync(&pdev->dev);
-	else
-		tegra_i2c_runtime_suspend(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
 
 disable_rpm:
-	if (pm_runtime_enabled(&pdev->dev))
-		pm_runtime_disable(&pdev->dev);
-
-unprepare_div_clk:
+	pm_runtime_disable(&pdev->dev);
 	clk_unprepare(i2c_dev->div_clk);
 
 unprepare_slow_clk:
@@ -1867,8 +1853,6 @@ static int tegra_i2c_remove(struct platform_device *pdev)
 		clk_disable(i2c_dev->div_clk);
 
 	pm_runtime_disable(&pdev->dev);
-	if (!pm_runtime_status_suspended(&pdev->dev))
-		tegra_i2c_runtime_suspend(&pdev->dev);
 
 	clk_unprepare(i2c_dev->div_clk);
 	clk_unprepare(i2c_dev->slow_clk);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 09/17] i2c: tegra: Clean up probe function
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (7 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 08/17] i2c: tegra: Runtime PM always available on Tegra Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 10/17] i2c: tegra: Drop '_timeout' from wait/poll function names Dmitry Osipenko
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

The driver's probe function code is difficult to read and follow. This
patch splits probe function into several logical parts that are easy to
work with.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 398 ++++++++++++++++++++-------------
 1 file changed, 240 insertions(+), 158 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index a5d9e3ce6320..a8f6a32229c3 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -447,6 +447,9 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
 
 	i2c_dev->tx_dma_chan = chan;
 
+	i2c_dev->dma_buf_size = i2c_dev->adapter.quirks->max_write_len +
+				I2C_PACKET_HEADER_SIZE;
+
 	dma_buf = dma_alloc_coherent(i2c_dev->dev, i2c_dev->dma_buf_size,
 				     &dma_phys, GFP_KERNEL | __GFP_NOWARN);
 	if (!dma_buf) {
@@ -1417,19 +1420,27 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap)
 	return ret;
 }
 
-static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev)
+static int tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev)
 {
 	struct device_node *np = i2c_dev->dev->of_node;
+	u32 bus_clk_rate = I2C_MAX_STANDARD_MODE_FREQ;
 	bool multi_mode;
-	int ret;
 
-	ret = of_property_read_u32(np, "clock-frequency",
-				   &i2c_dev->bus_clk_rate);
-	if (ret)
-		i2c_dev->bus_clk_rate = I2C_MAX_STANDARD_MODE_FREQ; /* default clock rate */
+	of_property_read_u32(np, "clock-frequency", &bus_clk_rate);
+	i2c_dev->bus_clk_rate = bus_clk_rate;
 
 	multi_mode = of_property_read_bool(np, "multi-master");
 	i2c_dev->is_multimaster_mode = multi_mode;
+
+	i2c_dev->hw = of_device_get_match_data(i2c_dev->dev);
+
+	if (of_device_is_compatible(np, "nvidia,tegra20-i2c-dvc"))
+		i2c_dev->is_dvc = true;
+
+	if (of_device_is_compatible(np, "nvidia,tegra210-i2c-vi"))
+		i2c_dev->is_vi = true;
+
+	return 0;
 }
 
 static const struct i2c_algorithm tegra_i2c_algo = {
@@ -1644,221 +1655,292 @@ static const struct of_device_id tegra_i2c_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
 
-static int tegra_i2c_probe(struct platform_device *pdev)
+static int tegra_i2c_init_resources(struct tegra_i2c_dev *i2c_dev,
+				    struct platform_device *pdev)
 {
-	struct clk *div_clk, *fast_clk, *slow_clk;
-	struct device *dev = &pdev->dev;
-	struct tegra_i2c_dev *i2c_dev;
-	phys_addr_t base_phys;
 	struct resource *res;
-	void __iomem *base;
-	int irq, ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base_phys = res->start;
-	base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
-
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(dev, "no irq resource\n");
+	if (WARN_ON(!res))
 		return -EINVAL;
-	}
-	irq = res->start;
 
-	div_clk = devm_clk_get(&pdev->dev, "div-clk");
-	if (IS_ERR(div_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(div_clk),
-				     "failed to get div-clk\n");
+	i2c_dev->base_phys = res->start;
 
-	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
-	if (!i2c_dev)
-		return -ENOMEM;
+	i2c_dev->base = devm_ioremap_resource(i2c_dev->dev, res);
+	if (IS_ERR(i2c_dev->base))
+		return PTR_ERR(i2c_dev->base);
 
-	i2c_dev->base = base;
-	i2c_dev->base_phys = base_phys;
-	i2c_dev->div_clk = div_clk;
-	i2c_dev->adapter.algo = &tegra_i2c_algo;
-	i2c_dev->adapter.retries = 1;
-	i2c_dev->adapter.timeout = 6 * HZ;
-	i2c_dev->irq = irq;
-	i2c_dev->cont_id = pdev->id;
-	i2c_dev->dev = &pdev->dev;
+	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (WARN_ON(!res))
+		return -EINVAL;
 
-	i2c_dev->rst = devm_reset_control_get_exclusive(&pdev->dev, "i2c");
-	if (IS_ERR(i2c_dev->rst)) {
-		dev_err(dev, "failed to get reset control: %pe\n",
-			i2c_dev->rst);
+	i2c_dev->irq = res->start;
 
-		return PTR_ERR(i2c_dev->rst);
-	}
+	return 0;
+}
 
-	tegra_i2c_parse_dt(i2c_dev);
+static int tegra_i2c_init_clocks(struct tegra_i2c_dev *i2c_dev)
+{
+	const struct tegra_i2c_hw_feature *hw = i2c_dev->hw;
+	struct device *dev = i2c_dev->dev;
+	struct clk *clk;
+	int err, mode;
+
+	clk = devm_clk_get(dev, "div-clk");
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk),
+				     "failed to get div-clk\n");
 
-	i2c_dev->hw = of_device_get_match_data(&pdev->dev);
-	i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node,
-						  "nvidia,tegra20-i2c-dvc");
-	i2c_dev->is_vi = of_device_is_compatible(dev->of_node,
-						 "nvidia,tegra210-i2c-vi");
-	i2c_dev->adapter.quirks = i2c_dev->hw->quirks;
-	i2c_dev->dma_buf_size = i2c_dev->adapter.quirks->max_write_len +
-				I2C_PACKET_HEADER_SIZE;
-	init_completion(&i2c_dev->msg_complete);
-	init_completion(&i2c_dev->dma_complete);
+	i2c_dev->div_clk = clk;
 
-	if (!i2c_dev->hw->has_single_clk_source) {
-		fast_clk = devm_clk_get(&pdev->dev, "fast-clk");
-		if (IS_ERR(fast_clk))
-			return dev_err_probe(&pdev->dev, PTR_ERR(fast_clk),
-					     "failed to get fast clock\n");
+	if (!hw->has_single_clk_source) {
+		clk = devm_clk_get(dev, "fast-clk");
+		if (IS_ERR(clk))
+			return dev_err_probe(dev, PTR_ERR(clk),
+					     "failed to get fast-clk\n");
 
-		i2c_dev->fast_clk = fast_clk;
+		i2c_dev->fast_clk = clk;
 	}
 
 	if (i2c_dev->is_vi) {
-		slow_clk = devm_clk_get(dev, "slow");
-		if (IS_ERR(slow_clk))
-			return dev_err_probe(&pdev->dev, PTR_ERR(slow_clk),
-					     "failed to get slow clock\n");
+		clk = devm_clk_get(dev, "slow");
+		if (IS_ERR(clk))
+			return dev_err_probe(dev, PTR_ERR(clk),
+					     "failed to get slow clk\n");
 
-		i2c_dev->slow_clk = slow_clk;
+		i2c_dev->slow_clk = clk;
 	}
 
-	platform_set_drvdata(pdev, i2c_dev);
-
-	ret = clk_prepare(i2c_dev->fast_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to prepare fast clock: %d\n", ret);
-		return ret;
+	err = clk_prepare(i2c_dev->fast_clk);
+	if (err) {
+		dev_err(dev, "failed to prepare fast clk: %d\n", err);
+		return err;
 	}
 
-	ret = clk_prepare(i2c_dev->slow_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to prepare slow clock: %d\n", ret);
+	err = clk_prepare(i2c_dev->slow_clk);
+	if (err) {
+		dev_err(dev, "failed to prepare slow clk: %d\n", err);
 		goto unprepare_fast_clk;
 	}
 
-	if (i2c_dev->bus_clk_rate > I2C_MAX_FAST_MODE_FREQ &&
-	    i2c_dev->bus_clk_rate <= I2C_MAX_FAST_MODE_PLUS_FREQ)
-		i2c_dev->clk_divisor_non_hs_mode =
-				i2c_dev->hw->clk_divisor_fast_plus_mode;
-	else if (i2c_dev->bus_clk_rate > I2C_MAX_STANDARD_MODE_FREQ &&
-		 i2c_dev->bus_clk_rate <= I2C_MAX_FAST_MODE_FREQ)
-		i2c_dev->clk_divisor_non_hs_mode =
-				i2c_dev->hw->clk_divisor_fast_mode;
-	else
-		i2c_dev->clk_divisor_non_hs_mode =
-				i2c_dev->hw->clk_divisor_std_mode;
-
-	ret = clk_prepare(i2c_dev->div_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to prepare div-clk: %d\n", ret);
+	err = clk_prepare(i2c_dev->div_clk);
+	if (err) {
+		dev_err(dev, "failed to prepare div-clk: %d\n", err);
 		goto unprepare_slow_clk;
 	}
 
-	/*
-	 * VI I2C is in VE power domain which is not always on and not
-	 * an IRQ safe. So, IRQ safe device can't be attached to a non-IRQ
-	 * safe domain as it prevents powering off the PM domain.
-	 * Also, VI I2C device don't need to use runtime IRQ safe as it will
-	 * not be used for atomic transfers.
-	 */
-	if (!i2c_dev->is_vi)
-		pm_runtime_irq_safe(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
-	ret = pm_runtime_get_sync(i2c_dev->dev);
-	if (ret < 0) {
-		dev_err(dev, "runtime resume failed\n");
-		goto disable_rpm;
-	}
-
 	if (i2c_dev->is_multimaster_mode) {
-		ret = clk_enable(i2c_dev->div_clk);
-		if (ret < 0) {
-			dev_err(dev, "failed to enable div-clk: %d\n", ret);
-			goto put_rpm;
+		err = clk_enable(i2c_dev->div_clk);
+		if (err) {
+			dev_err(dev, "failed to enable div-clk: %d\n", err);
+			goto unprepare_div_clk;
 		}
 	}
 
-	if (i2c_dev->hw->supports_bus_clear)
-		i2c_dev->adapter.bus_recovery_info = &tegra_i2c_recovery_info;
+	switch (i2c_dev->bus_clk_rate) {
+	case I2C_MAX_FAST_MODE_FREQ ... I2C_MAX_FAST_MODE_PLUS_FREQ:
+		mode = hw->clk_divisor_fast_plus_mode;
+		break;
 
-	ret = tegra_i2c_init_dma(i2c_dev);
-	if (ret < 0)
-		goto disable_div_clk;
+	case I2C_MAX_STANDARD_MODE_FREQ ... I2C_MAX_FAST_MODE_FREQ - 1:
+		mode = hw->clk_divisor_fast_mode;
+		break;
 
-	ret = tegra_i2c_init(i2c_dev, false);
-	if (ret) {
-		dev_err(dev, "failed to initialize i2c controller\n");
-		goto release_dma;
+	default:
+		mode = hw->clk_divisor_std_mode;
+		break;
 	}
 
-	irq_set_status_flags(i2c_dev->irq, IRQ_NOAUTOEN);
+	i2c_dev->clk_divisor_non_hs_mode = mode;
 
-	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
-			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c_dev);
-	if (ret) {
-		dev_err(dev, "failed to request irq %i\n", i2c_dev->irq);
-		goto release_dma;
+	return 0;
+
+unprepare_div_clk:
+	clk_unprepare(i2c_dev->div_clk);
+unprepare_slow_clk:
+	clk_unprepare(i2c_dev->slow_clk);
+unprepare_fast_clk:
+	clk_unprepare(i2c_dev->fast_clk);
+
+	return err;
+}
+
+static void tegra_i2c_release_clocks(struct tegra_i2c_dev *i2c_dev)
+{
+	if (i2c_dev->is_multimaster_mode)
+		clk_disable(i2c_dev->div_clk);
+
+	clk_unprepare(i2c_dev->div_clk);
+	clk_unprepare(i2c_dev->slow_clk);
+	clk_unprepare(i2c_dev->fast_clk);
+}
+
+static int tegra_i2c_init_reset_control(struct tegra_i2c_dev *i2c_dev)
+{
+	struct device *dev = i2c_dev->dev;
+	struct reset_control *rst;
+
+	rst = devm_reset_control_get_exclusive(dev, "i2c");
+	if (IS_ERR(rst)) {
+		dev_err(dev, "failed to get reset control: %pe\n", rst);
+		return PTR_ERR(rst);
 	}
 
-	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
+	i2c_dev->rst = rst;
+
+	return 0;
+}
+
+static int tegra_i2c_init_adapter(struct tegra_i2c_dev *i2c_dev)
+{
+	i2c_dev->adapter.dev.of_node = i2c_dev->dev->of_node;
+	i2c_dev->adapter.dev.parent = i2c_dev->dev;
+	i2c_dev->adapter.retries = 1;
+	i2c_dev->adapter.timeout = 6 * HZ;
+	i2c_dev->adapter.quirks = i2c_dev->hw->quirks;
 	i2c_dev->adapter.owner = THIS_MODULE;
 	i2c_dev->adapter.class = I2C_CLASS_DEPRECATED;
-	strlcpy(i2c_dev->adapter.name, dev_name(&pdev->dev),
+	i2c_dev->adapter.algo = &tegra_i2c_algo;
+	i2c_dev->adapter.nr = i2c_dev->cont_id;
+
+	if (i2c_dev->hw->supports_bus_clear)
+		i2c_dev->adapter.bus_recovery_info = &tegra_i2c_recovery_info;
+
+	strlcpy(i2c_dev->adapter.name, dev_name(i2c_dev->dev),
 		sizeof(i2c_dev->adapter.name));
-	i2c_dev->adapter.dev.parent = &pdev->dev;
-	i2c_dev->adapter.nr = pdev->id;
-	i2c_dev->adapter.dev.of_node = pdev->dev.of_node;
 
-	ret = i2c_add_numbered_adapter(&i2c_dev->adapter);
-	if (ret)
-		goto release_dma;
+	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
 
-	pm_runtime_put(&pdev->dev);
+	return 0;
+}
+
+static int tegra_i2c_init_runtime_pm(struct tegra_i2c_dev *i2c_dev)
+{
+	/*
+	 * VI I2C is in VE power domain which is not always ON and not
+	 * IRQ-safe. Thus, IRQ-safe device shouldn't be attached to a
+	 * non IRQ-safe domain because this prevents powering off the power
+	 * domain.
+	 *
+	 * VI I2C device shouldn't be marked as IRQ-safe because VI I2C won't
+	 * be used for atomic transfers.
+	 */
+	if (!i2c_dev->is_vi)
+		pm_runtime_irq_safe(i2c_dev->dev);
+
+	pm_runtime_enable(i2c_dev->dev);
 
 	return 0;
+}
 
-release_dma:
-	tegra_i2c_release_dma(i2c_dev);
+static void tegra_i2c_release_runtime_pm(struct tegra_i2c_dev *i2c_dev)
+{
+	pm_runtime_disable(i2c_dev->dev);
+}
 
-disable_div_clk:
-	if (i2c_dev->is_multimaster_mode)
-		clk_disable(i2c_dev->div_clk);
+static int tegra_i2c_init_interrupt(struct tegra_i2c_dev *i2c_dev)
+{
+	irq_set_status_flags(i2c_dev->irq, IRQ_NOAUTOEN);
 
-put_rpm:
-	pm_runtime_put_sync(&pdev->dev);
+	return devm_request_irq(i2c_dev->dev, i2c_dev->irq, tegra_i2c_isr,
+				IRQF_NO_SUSPEND, dev_name(i2c_dev->dev),
+				i2c_dev);
+}
 
-disable_rpm:
-	pm_runtime_disable(&pdev->dev);
-	clk_unprepare(i2c_dev->div_clk);
+static int tegra_i2c_init_hardware(struct tegra_i2c_dev *i2c_dev)
+{
+	int ret;
 
-unprepare_slow_clk:
-	clk_unprepare(i2c_dev->slow_clk);
+	ret = pm_runtime_get_sync(i2c_dev->dev);
+	if (ret < 0) {
+		dev_err(i2c_dev->dev, "runtime resume failed: %d\n", ret);
+		return ret;
+	}
 
-unprepare_fast_clk:
-	clk_unprepare(i2c_dev->fast_clk);
+	ret = tegra_i2c_init(i2c_dev, false);
+	pm_runtime_put(i2c_dev->dev);
 
 	return ret;
 }
 
-static int tegra_i2c_remove(struct platform_device *pdev)
+static int tegra_i2c_probe(struct platform_device *pdev)
 {
-	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+	struct tegra_i2c_dev *i2c_dev;
+	int err;
 
-	i2c_del_adapter(&i2c_dev->adapter);
+	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
+	if (!i2c_dev)
+		return -ENOMEM;
 
-	if (i2c_dev->is_multimaster_mode)
-		clk_disable(i2c_dev->div_clk);
+	platform_set_drvdata(pdev, i2c_dev);
 
-	pm_runtime_disable(&pdev->dev);
+	init_completion(&i2c_dev->msg_complete);
+	init_completion(&i2c_dev->dma_complete);
 
-	clk_unprepare(i2c_dev->div_clk);
-	clk_unprepare(i2c_dev->slow_clk);
-	clk_unprepare(i2c_dev->fast_clk);
+	i2c_dev->cont_id = pdev->id;
+	i2c_dev->dev = &pdev->dev;
+
+	err = tegra_i2c_parse_dt(i2c_dev);
+	if (err)
+		return err;
+
+	err = tegra_i2c_init_resources(i2c_dev, pdev);
+	if (err)
+		return err;
+
+	err = tegra_i2c_init_adapter(i2c_dev);
+	if (err)
+		return err;
+
+	err = tegra_i2c_init_reset_control(i2c_dev);
+	if (err)
+		return err;
+
+	err = tegra_i2c_init_interrupt(i2c_dev);
+	if (err)
+		return err;
+
+	err = tegra_i2c_init_clocks(i2c_dev);
+	if (err)
+		return err;
+
+	err = tegra_i2c_init_runtime_pm(i2c_dev);
+	if (err)
+		goto release_clocks;
+
+	err = tegra_i2c_init_dma(i2c_dev);
+	if (err)
+		goto release_rpm;
+
+	err = tegra_i2c_init_hardware(i2c_dev);
+	if (err)
+		goto release_dma;
+
+	err = i2c_add_numbered_adapter(&i2c_dev->adapter);
+	if (err)
+		goto release_dma;
+
+	return 0;
+
+release_dma:
+	tegra_i2c_release_dma(i2c_dev);
+release_rpm:
+	tegra_i2c_release_runtime_pm(i2c_dev);
+release_clocks:
+	tegra_i2c_release_clocks(i2c_dev);
+
+	return err;
+}
+
+static int tegra_i2c_remove(struct platform_device *pdev)
+{
+	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&i2c_dev->adapter);
 
 	tegra_i2c_release_dma(i2c_dev);
+	tegra_i2c_release_runtime_pm(i2c_dev);
+	tegra_i2c_release_clocks(i2c_dev);
 
 	return 0;
 }
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 10/17] i2c: tegra: Drop '_timeout' from wait/poll function names
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (8 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 09/17] i2c: tegra: Clean up probe function Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 11/17] i2c: tegra: Remove likely/unlikely from the code Dmitry Osipenko
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Drop '_timeout' postfix from the wait/poll completion function names in
order to make the names shorter, making code cleaner a tad.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index a8f6a32229c3..3d6189e200ba 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1067,10 +1067,9 @@ static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev,
 	i2c_writel(i2c_dev, val, reg);
 }
 
-static unsigned long
-tegra_i2c_poll_completion_timeout(struct tegra_i2c_dev *i2c_dev,
-				  struct completion *complete,
-				  unsigned int timeout_ms)
+static unsigned long tegra_i2c_poll_completion(struct tegra_i2c_dev *i2c_dev,
+					       struct completion *complete,
+					       unsigned int timeout_ms)
 {
 	ktime_t ktime = ktime_get();
 	ktime_t ktimeout = ktime_add_ms(ktime, timeout_ms);
@@ -1094,16 +1093,14 @@ tegra_i2c_poll_completion_timeout(struct tegra_i2c_dev *i2c_dev,
 	return 0;
 }
 
-static unsigned long
-tegra_i2c_wait_completion_timeout(struct tegra_i2c_dev *i2c_dev,
-				  struct completion *complete,
-				  unsigned int timeout_ms)
+static unsigned long tegra_i2c_wait_completion(struct tegra_i2c_dev *i2c_dev,
+					       struct completion *complete,
+					       unsigned int timeout_ms)
 {
 	unsigned long ret;
 
 	if (i2c_dev->is_curr_atomic_xfer) {
-		ret = tegra_i2c_poll_completion_timeout(i2c_dev, complete,
-							timeout_ms);
+		ret = tegra_i2c_poll_completion(i2c_dev, complete, timeout_ms);
 	} else {
 		enable_irq(i2c_dev->irq);
 		ret = wait_for_completion_timeout(complete,
@@ -1121,8 +1118,7 @@ tegra_i2c_wait_completion_timeout(struct tegra_i2c_dev *i2c_dev,
 		 * needs to be checked after timeout.
 		 */
 		if (ret == 0)
-			ret = tegra_i2c_poll_completion_timeout(i2c_dev,
-								complete, 0);
+			ret = tegra_i2c_poll_completion(i2c_dev, complete, 0);
 	}
 
 	return ret;
@@ -1149,8 +1145,8 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap)
 	i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG);
 	tegra_i2c_unmask_irq(i2c_dev, I2C_INT_BUS_CLR_DONE);
 
-	time_left = tegra_i2c_wait_completion_timeout(
-			i2c_dev, &i2c_dev->msg_complete, 50);
+	time_left = tegra_i2c_wait_completion(i2c_dev, &i2c_dev->msg_complete,
+					      50);
 	if (time_left == 0) {
 		dev_err(i2c_dev->dev, "timed out for bus clear\n");
 		return -ETIMEDOUT;
@@ -1296,8 +1292,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 		i2c_readl(i2c_dev, I2C_INT_MASK));
 
 	if (dma) {
-		time_left = tegra_i2c_wait_completion_timeout(
-				i2c_dev, &i2c_dev->dma_complete, xfer_time);
+		time_left = tegra_i2c_wait_completion(i2c_dev,
+						      &i2c_dev->dma_complete,
+						      xfer_time);
 
 		/*
 		 * Synchronize DMA first, since dmaengine_terminate_sync()
@@ -1328,8 +1325,8 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 		}
 	}
 
-	time_left = tegra_i2c_wait_completion_timeout(
-			i2c_dev, &i2c_dev->msg_complete, xfer_time);
+	time_left = tegra_i2c_wait_completion(i2c_dev, &i2c_dev->msg_complete,
+					      xfer_time);
 
 	tegra_i2c_mask_irq(i2c_dev, int_mask);
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 11/17] i2c: tegra: Remove likely/unlikely from the code
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (9 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 10/17] i2c: tegra: Drop '_timeout' from wait/poll function names Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 12/17] i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg() Dmitry Osipenko
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

The likely/unlikely annotations should be used only in a hot paths of
performance-critical code. The I2C driver doesn't have such paths, and
thus, there is no justification for usage of likely/unlikely annotations
in the code. Hence remove them.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 3d6189e200ba..300a6576ff94 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -911,7 +911,7 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
 		goto err;
 	}
 
-	if (unlikely(status & status_err)) {
+	if (status & status_err) {
 		tegra_i2c_disable_packet_mode(i2c_dev);
 		if (status & I2C_INT_NO_ACK)
 			i2c_dev->msg_err |= I2C_ERR_NO_ACK;
@@ -1341,7 +1341,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 		i2c_dev->msg_err);
 
 	i2c_dev->is_curr_dma_xfer = false;
-	if (likely(i2c_dev->msg_err == I2C_ERR_NONE))
+	if (i2c_dev->msg_err == I2C_ERR_NONE)
 		return 0;
 
 	tegra_i2c_init(i2c_dev, true);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 12/17] i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg()
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (10 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 11/17] i2c: tegra: Remove likely/unlikely from the code Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 13/17] i2c: tegra: Check errors for both positive and negative values Dmitry Osipenko
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Factor out error recovery code from tegra_i2c_xfer_msg() in order to
make this function easier to read and follow.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 46 ++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 300a6576ff94..bc891d4b41ad 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1161,6 +1161,32 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap)
 	return -EAGAIN;
 }
 
+static int tegra_i2c_error_recover(struct tegra_i2c_dev *i2c_dev,
+				   struct i2c_msg *msg)
+{
+	if (i2c_dev->msg_err == I2C_ERR_NONE)
+		return 0;
+
+	tegra_i2c_init(i2c_dev, true);
+
+	/* start recovery upon arbitration loss in single master mode */
+	if (i2c_dev->msg_err == I2C_ERR_ARBITRATION_LOST) {
+		if (!i2c_dev->is_multimaster_mode)
+			return i2c_recover_bus(&i2c_dev->adapter);
+
+		return -EAGAIN;
+	}
+
+	if (i2c_dev->msg_err == I2C_ERR_NO_ACK) {
+		if (msg->flags & I2C_M_IGNORE_NAK)
+			return 0;
+
+		return -EREMOTEIO;
+	}
+
+	return -EIO;
+}
+
 static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 			      struct i2c_msg *msg,
 			      enum msg_end_type end_state)
@@ -1341,24 +1367,12 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 		i2c_dev->msg_err);
 
 	i2c_dev->is_curr_dma_xfer = false;
-	if (i2c_dev->msg_err == I2C_ERR_NONE)
-		return 0;
 
-	tegra_i2c_init(i2c_dev, true);
-	/* start recovery upon arbitration loss in single master mode */
-	if (i2c_dev->msg_err == I2C_ERR_ARBITRATION_LOST) {
-		if (!i2c_dev->is_multimaster_mode)
-			return i2c_recover_bus(&i2c_dev->adapter);
-		return -EAGAIN;
-	}
-
-	if (i2c_dev->msg_err == I2C_ERR_NO_ACK) {
-		if (msg->flags & I2C_M_IGNORE_NAK)
-			return 0;
-		return -EREMOTEIO;
-	}
+	err = tegra_i2c_error_recover(i2c_dev, msg);
+	if (err)
+		return err;
 
-	return -EIO;
+	return 0;
 }
 
 static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 13/17] i2c: tegra: Check errors for both positive and negative values
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (11 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 12/17] i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg() Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:10 ` [PATCH v2 14/17] i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load() Dmitry Osipenko
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

The driver's code is inconsistent in regards to the error values checking.
The correct way should be to check both positive and negative values.
This patch cleans up the error-checks in the code. Note that the
pm_runtime_get_sync() could return positive value on success, hence only
relevant parts of the code are changed by this patch.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index bc891d4b41ad..9a807caef4a6 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -674,19 +674,19 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
 		return ret;
 
 	ret = clk_enable(i2c_dev->fast_clk);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(dev, "failed to enable fast clock: %d\n", ret);
 		return ret;
 	}
 
 	ret = clk_enable(i2c_dev->slow_clk);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(dev, "failed to enable slow clock: %d\n", ret);
 		goto disable_fast_clk;
 	}
 
 	ret = clk_enable(i2c_dev->div_clk);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(dev, "failed to enable div clock: %d\n", ret);
 		goto disable_slow_clk;
 	}
@@ -1047,7 +1047,7 @@ static void tegra_i2c_config_fifo_trig(struct tegra_i2c_dev *i2c_dev,
 
 		slv_config.device_fc = true;
 		ret = dmaengine_slave_config(chan, &slv_config);
-		if (ret < 0) {
+		if (ret) {
 			dev_err(i2c_dev->dev, "dma config failed: %d\n", ret);
 			dev_err(i2c_dev->dev, "falling back to PIO\n");
 			tegra_i2c_release_dma(i2c_dev);
@@ -1235,7 +1235,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 						   xfer_size,
 						   DMA_FROM_DEVICE);
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
-			if (err < 0) {
+			if (err) {
 				dev_err(i2c_dev->dev,
 					"starting rx dma failed: %d\n", err);
 				return err;
@@ -1294,7 +1294,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 						   xfer_size,
 						   DMA_TO_DEVICE);
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
-			if (err < 0) {
+			if (err) {
 				dev_err(i2c_dev->dev,
 					"starting tx dma failed: %d\n", err);
 				return err;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 14/17] i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load()
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (12 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 13/17] i2c: tegra: Check errors for both positive and negative values Dmitry Osipenko
@ 2020-09-01 21:10 ` Dmitry Osipenko
  2020-09-01 21:11 ` [PATCH v2 15/17] i2c: tegra: Remove unnecessary whitespaces and newlines Dmitry Osipenko
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:10 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Improve coding style of the tegra_i2c_wait_for_config_load() function by
making code a bit more narrow, adhering to the common kernel coding style.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 9a807caef4a6..70691179f170 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -733,24 +733,23 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
 	u32 val;
 	int err;
 
-	if (i2c_dev->hw->has_config_load_reg) {
-		reg_offset = tegra_i2c_reg_addr(i2c_dev, I2C_CONFIG_LOAD);
-		addr = i2c_dev->base + reg_offset;
-		i2c_writel(i2c_dev, I2C_MSTR_CONFIG_LOAD, I2C_CONFIG_LOAD);
+	if (!i2c_dev->hw->has_config_load_reg)
+		return 0;
 
-		if (i2c_dev->is_curr_atomic_xfer)
-			err = readl_relaxed_poll_timeout_atomic(
-						addr, val, val == 0, 1000,
-						I2C_CONFIG_LOAD_TIMEOUT);
-		else
-			err = readl_relaxed_poll_timeout(
-						addr, val, val == 0, 1000,
-						I2C_CONFIG_LOAD_TIMEOUT);
+	reg_offset = tegra_i2c_reg_addr(i2c_dev, I2C_CONFIG_LOAD);
+	addr = i2c_dev->base + reg_offset;
+	i2c_writel(i2c_dev, I2C_MSTR_CONFIG_LOAD, I2C_CONFIG_LOAD);
 
-		if (err) {
-			dev_err(i2c_dev->dev, "failed to load config\n");
-			return err;
-		}
+	if (i2c_dev->is_curr_atomic_xfer)
+		err = readl_relaxed_poll_timeout_atomic(addr, val, val == 0, 1000,
+							I2C_CONFIG_LOAD_TIMEOUT);
+	else
+		err = readl_relaxed_poll_timeout(addr, val, val == 0, 1000,
+						 I2C_CONFIG_LOAD_TIMEOUT);
+
+	if (err) {
+		dev_err(i2c_dev->dev, "failed to load config\n");
+		return err;
 	}
 
 	return 0;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 15/17] i2c: tegra: Remove unnecessary whitespaces and newlines
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (13 preceding siblings ...)
  2020-09-01 21:10 ` [PATCH v2 14/17] i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load() Dmitry Osipenko
@ 2020-09-01 21:11 ` Dmitry Osipenko
  2020-09-01 21:11 ` [PATCH v2 16/17] i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear() Dmitry Osipenko
  2020-09-01 21:11 ` [PATCH v2 17/17] i2c: tegra: Improve driver module description Dmitry Osipenko
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:11 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Clean up couple places in the code by removing unnecessary whitespaces and
newlines.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 70691179f170..bdfccf055959 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1345,8 +1345,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 						i2c_dev->dma_phys,
 						xfer_size,
 						DMA_FROM_DEVICE);
-			memcpy(i2c_dev->msg_buf, i2c_dev->dma_buf,
-			       msg->len);
+			memcpy(i2c_dev->msg_buf, i2c_dev->dma_buf, msg->len);
 		}
 	}
 
@@ -2008,15 +2007,14 @@ static const struct dev_pm_ops tegra_i2c_pm = {
 };
 
 static struct platform_driver tegra_i2c_driver = {
-	.probe   = tegra_i2c_probe,
-	.remove  = tegra_i2c_remove,
-	.driver  = {
-		.name  = "tegra-i2c",
+	.probe = tegra_i2c_probe,
+	.remove = tegra_i2c_remove,
+	.driver = {
+		.name = "tegra-i2c",
 		.of_match_table = tegra_i2c_of_match,
-		.pm    = &tegra_i2c_pm,
+		.pm = &tegra_i2c_pm,
 	},
 };
-
 module_platform_driver(tegra_i2c_driver);
 
 MODULE_DESCRIPTION("nVidia Tegra2 I2C Bus Controller driver");
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 16/17] i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear()
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (14 preceding siblings ...)
  2020-09-01 21:11 ` [PATCH v2 15/17] i2c: tegra: Remove unnecessary whitespaces and newlines Dmitry Osipenko
@ 2020-09-01 21:11 ` Dmitry Osipenko
  2020-09-01 21:11 ` [PATCH v2 17/17] i2c: tegra: Improve driver module description Dmitry Osipenko
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:11 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Rename variable "reg" to "val" in order to better reflect the actual usage
of the variable in the code and to make naming consistent with the rest of
the code.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index bdfccf055959..628674ed8440 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1127,21 +1127,21 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap)
 {
 	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 	unsigned long time_left;
-	u32 reg;
+	u32 val;
 	int err;
 
 	reinit_completion(&i2c_dev->msg_complete);
-	reg = FIELD_PREP(I2C_BC_SCLK_THRESHOLD, 9) | I2C_BC_STOP_COND |
+	val = FIELD_PREP(I2C_BC_SCLK_THRESHOLD, 9) | I2C_BC_STOP_COND |
 	      I2C_BC_TERMINATE;
-	i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG);
+	i2c_writel(i2c_dev, val, I2C_BUS_CLEAR_CNFG);
 	if (i2c_dev->hw->has_config_load_reg) {
 		err = tegra_i2c_wait_for_config_load(i2c_dev);
 		if (err)
 			return err;
 	}
 
-	reg |= I2C_BC_ENABLE;
-	i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG);
+	val |= I2C_BC_ENABLE;
+	i2c_writel(i2c_dev, val, I2C_BUS_CLEAR_CNFG);
 	tegra_i2c_unmask_irq(i2c_dev, I2C_INT_BUS_CLR_DONE);
 
 	time_left = tegra_i2c_wait_completion(i2c_dev, &i2c_dev->msg_complete,
@@ -1151,8 +1151,8 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap)
 		return -ETIMEDOUT;
 	}
 
-	reg = i2c_readl(i2c_dev, I2C_BUS_CLEAR_STATUS);
-	if (!(reg & I2C_BC_STATUS)) {
+	val = i2c_readl(i2c_dev, I2C_BUS_CLEAR_STATUS);
+	if (!(val & I2C_BC_STATUS)) {
 		dev_err(i2c_dev->dev, "un-recovered arbitration lost\n");
 		return -EIO;
 	}
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 17/17] i2c: tegra: Improve driver module description
  2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
                   ` (15 preceding siblings ...)
  2020-09-01 21:11 ` [PATCH v2 16/17] i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear() Dmitry Osipenko
@ 2020-09-01 21:11 ` Dmitry Osipenko
  16 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:11 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang
  Cc: linux-i2c, linux-tegra, linux-kernel

Use proper spelling of "NVIDIA" and don't designate driver as Tegra2-only
since newer SoC generations are supported as well.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 628674ed8440..a3113faa2d0b 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -2017,6 +2017,6 @@ static struct platform_driver tegra_i2c_driver = {
 };
 module_platform_driver(tegra_i2c_driver);
 
-MODULE_DESCRIPTION("nVidia Tegra2 I2C Bus Controller driver");
+MODULE_DESCRIPTION("NVIDIA Tegra I2C Bus Controller driver");
 MODULE_AUTHOR("Colin Cross");
 MODULE_LICENSE("GPL v2");
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 03/17] i2c: tegra: Clean up messages in the code
       [not found]   ` <CAHp75Vf9ETJMibQGe4Nx7n4703GtgO1XBsE1yGwsk3TaSPTDHw@mail.gmail.com>
@ 2020-09-01 21:37     ` Dmitry Osipenko
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2020-09-01 21:37 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Thierry Reding, Jonathan Hunter, Laxman Dewangan, Wolfram Sang,
	linux-i2c, linux-tegra, linux-kernel

02.09.2020 00:23, Andy Shevchenko пишет:
> 
> 
> On Wednesday, September 2, 2020, Dmitry Osipenko <digetx@gmail.com
> <mailto:digetx@gmail.com>> wrote:
> 
>     Use lowercase and consistent wording for all messages in the code.
> 
> 
> Why? DMA is an abbreviation and reads in capital letters. FWIW, I don’t
> like bending English grammar.

The idea is to make messages to look less "noisy" in a log. Although, I
don't have strong preference in regards to the style and keeping the
capitalization of abbreviations would be okay too. All I really care
about is to make all messages to use one common style.

I'll consider returning the capitalization in the next revision, thank
you for the input!

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-09-01 21:37 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 21:10 [PATCH v2 00/17] Improvements for Tegra I2C driver Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 01/17] i2c: tegra: Make tegra_i2c_flush_fifos() usable in atomic transfer Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 02/17] i2c: tegra: Add missing newline before returns Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 03/17] i2c: tegra: Clean up messages in the code Dmitry Osipenko
     [not found]   ` <CAHp75Vf9ETJMibQGe4Nx7n4703GtgO1XBsE1yGwsk3TaSPTDHw@mail.gmail.com>
2020-09-01 21:37     ` Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 04/17] i2c: tegra: Don't ignore tegra_i2c_flush_fifos() error Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 05/17] i2c: tegra: Use reset_control_reset() Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 06/17] i2c: tegra: Improve formatting of function variables Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 07/17] i2c: tegra: Use dev_err_probe() Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 08/17] i2c: tegra: Runtime PM always available on Tegra Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 09/17] i2c: tegra: Clean up probe function Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 10/17] i2c: tegra: Drop '_timeout' from wait/poll function names Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 11/17] i2c: tegra: Remove likely/unlikely from the code Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 12/17] i2c: tegra: Factor out error recovery from tegra_i2c_xfer_msg() Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 13/17] i2c: tegra: Check errors for both positive and negative values Dmitry Osipenko
2020-09-01 21:10 ` [PATCH v2 14/17] i2c: tegra: Improve coding style of tegra_i2c_wait_for_config_load() Dmitry Osipenko
2020-09-01 21:11 ` [PATCH v2 15/17] i2c: tegra: Remove unnecessary whitespaces and newlines Dmitry Osipenko
2020-09-01 21:11 ` [PATCH v2 16/17] i2c: tegra: Rename variable in tegra_i2c_issue_bus_clear() Dmitry Osipenko
2020-09-01 21:11 ` [PATCH v2 17/17] i2c: tegra: Improve driver module description Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).