[PATCH V2 0/9] Some Tegra I2C Updates

[PATCH V2 0/9] Some Tegra I2C Updates

[Jon Hunter]

Add runtime-pm and pinctrl support for Tegra I2C driver.

The first 6 patches are trivial clean-up/simplification changes.

Changes since V1:
- Fixed cppcheck warning reported by Wolfram
- Added 3 more clean-up patches to fix some checkpatch issues

Jon Hunter (9):
  i2c: tegra: Fix lines over 80 characters
  i2c: tegra: Use BIT macro
  i2c: tegra: Fix missing blank lines after declarations
  i2c: tegra: Add missing new line characters
  i2c: tegra: Remove non device-tree support
  i2c: tegra: Use device name for adapter name
  i2c: tegra: Simplify I2C resume
  i2c: tegra: Add runtime power-management support
  i2c: tegra: Add pinctrl support

 drivers/i2c/busses/i2c-tegra.c | 193 ++++++++++++++++++++++++-----------------
 1 file changed, 114 insertions(+), 79 deletions(-)

-- 
2.1.4

[PATCH V2 1/9] i2c: tegra: Fix lines over 80 characters

[Jon Hunter]

Checkpatch warns about some lines over 80 characters in the Tegra I2C
driver and so fix these.

While we are at it, prefix the second instance of "STOP condition" in
the comment with a "the".

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index d9979da11485..b90bc326907d 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -193,7 +193,8 @@ struct tegra_i2c_dev {
 	bool is_multimaster_mode;
 };
 
-static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg)
+static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
+		       unsigned long reg)
 {
 	writel(val, i2c_dev->base + reg);
 }
@@ -643,9 +644,10 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 		return 0;
 
 	/*
-	 * NACK interrupt is generated before the I2C controller generates the
-	 * STOP condition on the bus. So wait for 2 clock periods before resetting
-	 * the controller so that STOP condition has been delivered properly.
+	 * NACK interrupt is generated before the I2C controller generates
+	 * the STOP condition on the bus. So wait for 2 clock periods
+	 * before resetting the controller so that the STOP condition has
+	 * been delivered properly.
 	 */
 	if (i2c_dev->msg_err == I2C_ERR_NO_ACK)
 		udelay(DIV_ROUND_UP(2 * 1000000, i2c_dev->bus_clk_rate));
-- 
2.1.4

[PATCH V2 2/9] i2c: tegra: Use BIT macro

[Jon Hunter]

Checkpatch warns about spacing around the '<<' operator in the Tegra I2C
driver and so fix these by converting the bit definitions that are using
this operator to use the BIT macro.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 66 +++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index b90bc326907d..98d13437eb42 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -36,21 +36,21 @@
 
 #define I2C_CNFG				0x000
 #define I2C_CNFG_DEBOUNCE_CNT_SHIFT		12
-#define I2C_CNFG_PACKET_MODE_EN			(1<<10)
-#define I2C_CNFG_NEW_MASTER_FSM			(1<<11)
-#define I2C_CNFG_MULTI_MASTER_MODE		(1<<17)
+#define I2C_CNFG_PACKET_MODE_EN			BIT(10)
+#define I2C_CNFG_NEW_MASTER_FSM			BIT(11)
+#define I2C_CNFG_MULTI_MASTER_MODE		BIT(17)
 #define I2C_STATUS				0x01C
 #define I2C_SL_CNFG				0x020
-#define I2C_SL_CNFG_NACK			(1<<1)
-#define I2C_SL_CNFG_NEWSL			(1<<2)
+#define I2C_SL_CNFG_NACK			BIT(1)
+#define I2C_SL_CNFG_NEWSL			BIT(2)
 #define I2C_SL_ADDR1				0x02c
 #define I2C_SL_ADDR2				0x030
 #define I2C_TX_FIFO				0x050
 #define I2C_RX_FIFO				0x054
 #define I2C_PACKET_TRANSFER_STATUS		0x058
 #define I2C_FIFO_CONTROL			0x05c
-#define I2C_FIFO_CONTROL_TX_FLUSH		(1<<1)
-#define I2C_FIFO_CONTROL_RX_FLUSH		(1<<0)
+#define I2C_FIFO_CONTROL_TX_FLUSH		BIT(1)
+#define I2C_FIFO_CONTROL_RX_FLUSH		BIT(0)
 #define I2C_FIFO_CONTROL_TX_TRIG_SHIFT		5
 #define I2C_FIFO_CONTROL_RX_TRIG_SHIFT		2
 #define I2C_FIFO_STATUS				0x060
@@ -60,26 +60,26 @@
 #define I2C_FIFO_STATUS_RX_SHIFT		0
 #define I2C_INT_MASK				0x064
 #define I2C_INT_STATUS				0x068
-#define I2C_INT_PACKET_XFER_COMPLETE		(1<<7)
-#define I2C_INT_ALL_PACKETS_XFER_COMPLETE	(1<<6)
-#define I2C_INT_TX_FIFO_OVERFLOW		(1<<5)
-#define I2C_INT_RX_FIFO_UNDERFLOW		(1<<4)
-#define I2C_INT_NO_ACK				(1<<3)
-#define I2C_INT_ARBITRATION_LOST		(1<<2)
-#define I2C_INT_TX_FIFO_DATA_REQ		(1<<1)
-#define I2C_INT_RX_FIFO_DATA_REQ		(1<<0)
+#define I2C_INT_PACKET_XFER_COMPLETE		BIT(7)
+#define I2C_INT_ALL_PACKETS_XFER_COMPLETE	BIT(6)
+#define I2C_INT_TX_FIFO_OVERFLOW		BIT(5)
+#define I2C_INT_RX_FIFO_UNDERFLOW		BIT(4)
+#define I2C_INT_NO_ACK				BIT(3)
+#define I2C_INT_ARBITRATION_LOST		BIT(2)
+#define I2C_INT_TX_FIFO_DATA_REQ		BIT(1)
+#define I2C_INT_RX_FIFO_DATA_REQ		BIT(0)
 #define I2C_CLK_DIVISOR				0x06c
 #define I2C_CLK_DIVISOR_STD_FAST_MODE_SHIFT	16
 #define I2C_CLK_MULTIPLIER_STD_FAST_MODE	8
 
 #define DVC_CTRL_REG1				0x000
-#define DVC_CTRL_REG1_INTR_EN			(1<<10)
+#define DVC_CTRL_REG1_INTR_EN			BIT(10)
 #define DVC_CTRL_REG2				0x004
 #define DVC_CTRL_REG3				0x008
-#define DVC_CTRL_REG3_SW_PROG			(1<<26)
-#define DVC_CTRL_REG3_I2C_DONE_INTR_EN		(1<<30)
+#define DVC_CTRL_REG3_SW_PROG			BIT(26)
+#define DVC_CTRL_REG3_I2C_DONE_INTR_EN		BIT(30)
 #define DVC_STATUS				0x00c
-#define DVC_STATUS_I2C_DONE_INTR		(1<<30)
+#define DVC_STATUS_I2C_DONE_INTR		BIT(30)
 
 #define I2C_ERR_NONE				0x00
 #define I2C_ERR_NO_ACK				0x01
@@ -89,26 +89,26 @@
 #define PACKET_HEADER0_HEADER_SIZE_SHIFT	28
 #define PACKET_HEADER0_PACKET_ID_SHIFT		16
 #define PACKET_HEADER0_CONT_ID_SHIFT		12
-#define PACKET_HEADER0_PROTOCOL_I2C		(1<<4)
-
-#define I2C_HEADER_HIGHSPEED_MODE		(1<<22)
-#define I2C_HEADER_CONT_ON_NAK			(1<<21)
-#define I2C_HEADER_SEND_START_BYTE		(1<<20)
-#define I2C_HEADER_READ				(1<<19)
-#define I2C_HEADER_10BIT_ADDR			(1<<18)
-#define I2C_HEADER_IE_ENABLE			(1<<17)
-#define I2C_HEADER_REPEAT_START			(1<<16)
-#define I2C_HEADER_CONTINUE_XFER		(1<<15)
+#define PACKET_HEADER0_PROTOCOL_I2C		BIT(4)
+
+#define I2C_HEADER_HIGHSPEED_MODE		BIT(22)
+#define I2C_HEADER_CONT_ON_NAK			BIT(21)
+#define I2C_HEADER_SEND_START_BYTE		BIT(20)
+#define I2C_HEADER_READ				BIT(19)
+#define I2C_HEADER_10BIT_ADDR			BIT(18)
+#define I2C_HEADER_IE_ENABLE			BIT(17)
+#define I2C_HEADER_REPEAT_START			BIT(16)
+#define I2C_HEADER_CONTINUE_XFER		BIT(15)
 #define I2C_HEADER_MASTER_ADDR_SHIFT		12
 #define I2C_HEADER_SLAVE_ADDR_SHIFT		1
 
 #define I2C_CONFIG_LOAD				0x08C
-#define I2C_MSTR_CONFIG_LOAD			(1 << 0)
-#define I2C_SLV_CONFIG_LOAD			(1 << 1)
-#define I2C_TIMEOUT_CONFIG_LOAD			(1 << 2)
+#define I2C_MSTR_CONFIG_LOAD			BIT(0)
+#define I2C_SLV_CONFIG_LOAD			BIT(1)
+#define I2C_TIMEOUT_CONFIG_LOAD			BIT(2)
 
 #define I2C_CLKEN_OVERRIDE			0x090
-#define I2C_MST_CORE_CLKEN_OVR			(1 << 0)
+#define I2C_MST_CORE_CLKEN_OVR			BIT(0)
 
 /*
  * msg_end_type: The bus control which need to be send at end of transfer.
-- 
2.1.4

[PATCH V2 3/9] i2c: tegra: Fix missing blank lines after declarations

[Jon Hunter]

Checkpatch warns about missing blank lines after declarations in the
Tegra I2C driver and so fix these.

Note that the initialisation of 'val' to zero in tegra_dvc_init() is
unnecessary and so remove this.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 98d13437eb42..7a7f899936a3 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -245,15 +245,17 @@ static void i2c_readsl(struct tegra_i2c_dev *i2c_dev, void *data,
 
 static void tegra_i2c_mask_irq(struct tegra_i2c_dev *i2c_dev, u32 mask)
 {
-	u32 int_mask = i2c_readl(i2c_dev, I2C_INT_MASK);
-	int_mask &= ~mask;
+	u32 int_mask;
+
+	int_mask = i2c_readl(i2c_dev, I2C_INT_MASK) & ~mask;
 	i2c_writel(i2c_dev, int_mask, I2C_INT_MASK);
 }
 
 static void tegra_i2c_unmask_irq(struct tegra_i2c_dev *i2c_dev, u32 mask)
 {
-	u32 int_mask = i2c_readl(i2c_dev, I2C_INT_MASK);
-	int_mask |= mask;
+	u32 int_mask;
+
+	int_mask = i2c_readl(i2c_dev, I2C_INT_MASK) | mask;
 	i2c_writel(i2c_dev, int_mask, I2C_INT_MASK);
 }
 
@@ -261,6 +263,7 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
 {
 	unsigned long timeout = jiffies + HZ;
 	u32 val = i2c_readl(i2c_dev, I2C_FIFO_CONTROL);
+
 	val |= I2C_FIFO_CONTROL_TX_FLUSH | I2C_FIFO_CONTROL_RX_FLUSH;
 	i2c_writel(i2c_dev, val, I2C_FIFO_CONTROL);
 
@@ -386,7 +389,8 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev *i2c_dev)
  */
 static void tegra_dvc_init(struct tegra_i2c_dev *i2c_dev)
 {
-	u32 val = 0;
+	u32 val;
+
 	val = dvc_readl(i2c_dev, DVC_CTRL_REG3);
 	val |= DVC_CTRL_REG3_SW_PROG;
 	val |= DVC_CTRL_REG3_I2C_DONE_INTR_EN;
@@ -400,6 +404,7 @@ static void tegra_dvc_init(struct tegra_i2c_dev *i2c_dev)
 static inline int tegra_i2c_clock_enable(struct tegra_i2c_dev *i2c_dev)
 {
 	int ret;
+
 	if (!i2c_dev->hw->has_single_clk_source) {
 		ret = clk_enable(i2c_dev->fast_clk);
 		if (ret < 0) {
@@ -461,11 +466,11 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 
 	if (!i2c_dev->is_dvc) {
 		u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG);
+
 		sl_cfg |= I2C_SL_CNFG_NACK | I2C_SL_CNFG_NEWSL;
 		i2c_writel(i2c_dev, sl_cfg, I2C_SL_CNFG);
 		i2c_writel(i2c_dev, 0xfc, I2C_SL_ADDR1);
 		i2c_writel(i2c_dev, 0x00, I2C_SL_ADDR2);
-
 	}
 
 	val = 7 << I2C_FIFO_CONTROL_TX_TRIG_SHIFT |
@@ -680,6 +685,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 
 	for (i = 0; i < num; i++) {
 		enum msg_end_type end_type = MSG_END_STOP;
+
 		if (i < (num - 1)) {
 			if (msgs[i + 1].flags & I2C_M_NOSTART)
 				end_type = MSG_END_CONTINUE;
@@ -956,6 +962,7 @@ unprepare_fast_clk:
 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);
 
 	if (i2c_dev->is_multimaster_mode)
-- 
2.1.4

[PATCH V2 4/9] i2c: tegra: Add missing new line characters

[Jon Hunter]

Add missing new line characters for the various error messages.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 7a7f899936a3..7f31a103c04a 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -833,7 +833,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	div_clk = devm_clk_get(&pdev->dev, "div-clk");
 	if (IS_ERR(div_clk)) {
-		dev_err(&pdev->dev, "missing controller clock");
+		dev_err(&pdev->dev, "missing controller clock\n");
 		return PTR_ERR(div_clk);
 	}
 
@@ -851,7 +851,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	i2c_dev->rst = devm_reset_control_get(&pdev->dev, "i2c");
 	if (IS_ERR(i2c_dev->rst)) {
-		dev_err(&pdev->dev, "missing controller reset");
+		dev_err(&pdev->dev, "missing controller reset\n");
 		return PTR_ERR(i2c_dev->rst);
 	}
 
@@ -871,7 +871,7 @@ 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");
+			dev_err(&pdev->dev, "missing fast clock\n");
 			return PTR_ERR(fast_clk);
 		}
 		i2c_dev->fast_clk = fast_clk;
@@ -919,7 +919,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	ret = tegra_i2c_init(i2c_dev);
 	if (ret) {
-		dev_err(&pdev->dev, "Failed to initialize i2c controller");
+		dev_err(&pdev->dev, "Failed to initialize i2c controller\n");
 		goto disable_div_clk;
 	}
 
-- 
2.1.4

[PATCH V2 5/9] i2c: tegra: Remove non device-tree support

[Jon Hunter]

Tegra has only supported device-tree for platform/board configuration
for quite some time now and so simplify the Tegra I2C driver by dropping
code for non device-tree platforms/boards.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 7f31a103c04a..e5c8c67a2491 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -857,15 +857,9 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	tegra_i2c_parse_dt(i2c_dev);
 
-	i2c_dev->hw = &tegra20_i2c_hw;
-
-	if (pdev->dev.of_node) {
-		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");
-	} else if (pdev->id == 3) {
-		i2c_dev->is_dvc = 1;
-	}
+	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");
 	init_completion(&i2c_dev->msg_complete);
 
 	if (!i2c_dev->hw->has_single_clk_source) {
-- 
2.1.4

[PATCH V2 6/9] i2c: tegra: Use device name for adapter name

[Jon Hunter]

All Tegra I2C devices have the name "Tegra I2C adapter" which is not
very useful when viewing the I2C adapter names via the sysfs. Therefore,
use the device name, which is unique for each I2C device, instead.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.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 e5c8c67a2491..bdb50258a9a8 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -927,7 +927,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
 	i2c_dev->adapter.owner = THIS_MODULE;
 	i2c_dev->adapter.class = I2C_CLASS_DEPRECATED;
-	strlcpy(i2c_dev->adapter.name, "Tegra I2C adapter",
+	strlcpy(i2c_dev->adapter.name, dev_name(&pdev->dev),
 		sizeof(i2c_dev->adapter.name));
 	i2c_dev->adapter.dev.parent = &pdev->dev;
 	i2c_dev->adapter.nr = pdev->id;
-- 
2.1.4

[PATCH V2 7/9] i2c: tegra: Simplify I2C resume

[Jon Hunter]

The I2C adapter is unlocked regardless of whether the tegra_i2c_init()
called during the resume is successful or not. However, if the
tegra_i2c_init() is not successful, then ->is_suspended is not set to
false. Simplify the resume code by only setting ->is_suspended to false
if tegra_i2c_init() is successful and return the error code from
tegra_i2c_init().

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index bdb50258a9a8..3c27012fa96c 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -989,17 +989,12 @@ static int tegra_i2c_resume(struct device *dev)
 	i2c_lock_adapter(&i2c_dev->adapter);
 
 	ret = tegra_i2c_init(i2c_dev);
-
-	if (ret) {
-		i2c_unlock_adapter(&i2c_dev->adapter);
-		return ret;
-	}
-
-	i2c_dev->is_suspended = false;
+	if (!ret)
+		i2c_dev->is_suspended = false;
 
 	i2c_unlock_adapter(&i2c_dev->adapter);
 
-	return 0;
+	return ret;
 }
 
 static SIMPLE_DEV_PM_OPS(tegra_i2c_pm, tegra_i2c_suspend, tegra_i2c_resume);
-- 
2.1.4

[PATCH V2 8/9] i2c: tegra: Add runtime power-management support

[Jon Hunter]

Update the Tegra I2C driver to use runtime PM and move the code in the
tegra_i2c_clock_enable/disable() functions to the PM runtime resume and
suspend callbacks, respectively.

Note that given that CONFIG_PM is not mandatory for Tegra, if CONFIG_PM
is not enabled and so runtime PM is not enabled, ensure that the I2C
clocks are turned on during probe and kept on by calling the resume
callback directly.

In the function tegra_i2c_init(), the variable 'err' does not need to be
initialised to zero in tegra_i2c_init() because it is initialised when
pm_runtime_get_sync() is called. Furthermore, to ensure we only return 0
from tegra_i2c_init(), it is necessary to re-initialise 'err' to 0 after
a successful call to pm_runtime_get_sync() because it can return a
positive value on success. However, alternatively re-initialise 'err' by
using the return value of the function tegra_i2c_flush_fifos() because
it can only be 0 or -ETIMEDOUT.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 60 ++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 3c27012fa96c..05e34dc29d5a 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/module.h>
 #include <linux/reset.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/unaligned.h>
 
@@ -401,8 +402,9 @@ static void tegra_dvc_init(struct tegra_i2c_dev *i2c_dev)
 	dvc_writel(i2c_dev, val, DVC_CTRL_REG1);
 }
 
-static inline int tegra_i2c_clock_enable(struct tegra_i2c_dev *i2c_dev)
+static int tegra_i2c_runtime_resume(struct device *dev)
 {
+	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
 	int ret;
 
 	if (!i2c_dev->hw->has_single_clk_source) {
@@ -413,32 +415,39 @@ static inline int tegra_i2c_clock_enable(struct tegra_i2c_dev *i2c_dev)
 			return ret;
 		}
 	}
+
 	ret = clk_enable(i2c_dev->div_clk);
 	if (ret < 0) {
 		dev_err(i2c_dev->dev,
 			"Enabling div clk failed, err %d\n", ret);
 		clk_disable(i2c_dev->fast_clk);
+		return ret;
 	}
-	return ret;
+
+	return 0;
 }
 
-static inline void tegra_i2c_clock_disable(struct tegra_i2c_dev *i2c_dev)
+static int tegra_i2c_runtime_suspend(struct device *dev)
 {
+	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
+
 	clk_disable(i2c_dev->div_clk);
 	if (!i2c_dev->hw->has_single_clk_source)
 		clk_disable(i2c_dev->fast_clk);
+
+	return 0;
 }
 
 static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 {
 	u32 val;
-	int err = 0;
+	int err;
 	u32 clk_divisor;
 	unsigned long timeout = jiffies + HZ;
 
-	err = tegra_i2c_clock_enable(i2c_dev);
+	err = pm_runtime_get_sync(i2c_dev->dev);
 	if (err < 0) {
-		dev_err(i2c_dev->dev, "Clock enable failed %d\n", err);
+		dev_err(i2c_dev->dev, "runtime resume failed %d\n", err);
 		return err;
 	}
 
@@ -477,8 +486,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 		0 << I2C_FIFO_CONTROL_RX_TRIG_SHIFT;
 	i2c_writel(i2c_dev, val, I2C_FIFO_CONTROL);
 
-	if (tegra_i2c_flush_fifos(i2c_dev))
-		err = -ETIMEDOUT;
+	err = tegra_i2c_flush_fifos(i2c_dev);
 
 	if (i2c_dev->is_multimaster_mode && i2c_dev->hw->has_slcg_override_reg)
 		i2c_writel(i2c_dev, I2C_MST_CORE_CLKEN_OVR, I2C_CLKEN_OVERRIDE);
@@ -502,7 +510,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 	}
 
 err:
-	tegra_i2c_clock_disable(i2c_dev);
+	pm_runtime_put(i2c_dev->dev);
 	return err;
 }
 
@@ -677,9 +685,9 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 	if (i2c_dev->is_suspended)
 		return -EBUSY;
 
-	ret = tegra_i2c_clock_enable(i2c_dev);
+	ret = pm_runtime_get_sync(i2c_dev->dev);
 	if (ret < 0) {
-		dev_err(i2c_dev->dev, "Clock enable failed %d\n", ret);
+		dev_err(i2c_dev->dev, "runtime resume failed %d\n", ret);
 		return ret;
 	}
 
@@ -696,7 +704,9 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 		if (ret)
 			break;
 	}
-	tegra_i2c_clock_disable(i2c_dev);
+
+	pm_runtime_put(i2c_dev->dev);
+
 	return ret ?: i;
 }
 
@@ -902,12 +912,21 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 		goto unprepare_fast_clk;
 	}
 
+	pm_runtime_enable(&pdev->dev);
+	if (!pm_runtime_enabled(&pdev->dev)) {
+		ret = tegra_i2c_runtime_resume(&pdev->dev);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "runtime resume failed\n");
+			goto unprepare_div_clk;
+		}
+	}
+
 	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);
-			goto unprepare_div_clk;
+			goto disable_rpm;
 		}
 	}
 
@@ -943,6 +962,11 @@ disable_div_clk:
 	if (i2c_dev->is_multimaster_mode)
 		clk_disable(i2c_dev->div_clk);
 
+disable_rpm:
+	pm_runtime_disable(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		tegra_i2c_runtime_suspend(&pdev->dev);
+
 unprepare_div_clk:
 	clk_unprepare(i2c_dev->div_clk);
 
@@ -962,6 +986,10 @@ static int tegra_i2c_remove(struct platform_device *pdev)
 	if (i2c_dev->is_multimaster_mode)
 		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);
 	if (!i2c_dev->hw->has_single_clk_source)
 		clk_unprepare(i2c_dev->fast_clk);
@@ -997,7 +1025,11 @@ static int tegra_i2c_resume(struct device *dev)
 	return ret;
 }
 
-static SIMPLE_DEV_PM_OPS(tegra_i2c_pm, tegra_i2c_suspend, tegra_i2c_resume);
+static const struct dev_pm_ops tegra_i2c_pm = {
+	SET_RUNTIME_PM_OPS(tegra_i2c_runtime_suspend, tegra_i2c_runtime_resume,
+			   NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(tegra_i2c_suspend, tegra_i2c_resume)
+};
 #define TEGRA_I2C_PM	(&tegra_i2c_pm)
 #else
 #define TEGRA_I2C_PM	NULL
-- 
2.1.4

[PATCH V2 9/9] i2c: tegra: Add pinctrl support

[Jon Hunter]

On Tegra124/132 the pins for I2C6 are shared with the Display Port AUX
(DPAUX) channel and on Tegra210 the pins for I2C4 and I2C6 are shared
with DPAUX1 and DPAUX0, respectively. The multiplexing of the pins is
handled by a register in the DPAUX and so the Tegra DPAUX driver has
been updated to register a pinctrl device for managing these pins.

The pins for these particular I2C devices are bound to the I2C device
prior to probing. However, these I2C devices are in a different power
partition to the DPAUX devices that own the pins. Hence, it is desirable
to place the pins in the 'idle' state and allow the DPAUX power
partition to switch off, when these I2C devices is not in use.
Therefore, add calls to place the I2C pins in the 'default' and 'idle'
states when the I2C device is runtime resumed and suspended,
respectively.

Please note that the pinctrl functions that set the state of the pins
check to see if the devices has pins associated and will return zero
if they do not. Therefore, it is safe to call these pinctrl functions
even for I2C devices that do not have any pins associated.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 05e34dc29d5a..d86a993b75d6 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/module.h>
 #include <linux/reset.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/pm_runtime.h>
 
 #include <asm/unaligned.h>
@@ -407,6 +408,10 @@ static int tegra_i2c_runtime_resume(struct device *dev)
 	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
 	int ret;
 
+	ret = pinctrl_pm_select_default_state(i2c_dev->dev);
+	if (ret)
+		return ret;
+
 	if (!i2c_dev->hw->has_single_clk_source) {
 		ret = clk_enable(i2c_dev->fast_clk);
 		if (ret < 0) {
@@ -435,7 +440,7 @@ static int tegra_i2c_runtime_suspend(struct device *dev)
 	if (!i2c_dev->hw->has_single_clk_source)
 		clk_disable(i2c_dev->fast_clk);
 
-	return 0;
+	return pinctrl_pm_select_idle_state(i2c_dev->dev);
 }
 
 static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
-- 
2.1.4

Re: [PATCH V2 9/9] i2c: tegra: Add pinctrl support

[Stephen Warren]

On 08/26/2016 07:09 AM, Jon Hunter wrote:
> On Tegra124/132 the pins for I2C6 are shared with the Display Port AUX
> (DPAUX) channel and on Tegra210 the pins for I2C4 and I2C6 are shared
> with DPAUX1 and DPAUX0, respectively. The multiplexing of the pins is
> handled by a register in the DPAUX and so the Tegra DPAUX driver has
> been updated to register a pinctrl device for managing these pins.
>
> The pins for these particular I2C devices are bound to the I2C device
> prior to probing. However, these I2C devices are in a different power
> partition to the DPAUX devices that own the pins. Hence, it is desirable
> to place the pins in the 'idle' state and allow the DPAUX power
> partition to switch off, when these I2C devices is not in use.
> Therefore, add calls to place the I2C pins in the 'default' and 'idle'
> states when the I2C device is runtime resumed and suspended,
> respectively.
>
> Please note that the pinctrl functions that set the state of the pins
> check to see if the devices has pins associated and will return zero
> if they do not. Therefore, it is safe to call these pinctrl functions
> even for I2C devices that do not have any pins associated.

I think this should be handled by drivers/i2c/muxes/i2c-mux-pinctrl.c 
instead?

Re: [PATCH V2 0/9] Some Tegra I2C Updates

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On Fri, Aug 26, 2016 at 02:08:56PM +0100, Jon Hunter wrote:
> Add runtime-pm and pinctrl support for Tegra I2C driver.
> 
> The first 6 patches are trivial clean-up/simplification changes.
> 
> Changes since V1:
> - Fixed cppcheck warning reported by Wolfram
> - Added 3 more clean-up patches to fix some checkpatch issues

Thanks looks good. Now we only need to find out which one to apply
first, this one or "[v8,1/3] i2c: tegra: use readx_poll_timeout after
config_load reg programmed" and following. I cced you on the other mail.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V2 9/9] i2c: tegra: Add pinctrl support

[Jon Hunter]

On 26/08/16 16:55, Stephen Warren wrote:
> On 08/26/2016 07:09 AM, Jon Hunter wrote:
>> On Tegra124/132 the pins for I2C6 are shared with the Display Port AUX
>> (DPAUX) channel and on Tegra210 the pins for I2C4 and I2C6 are shared
>> with DPAUX1 and DPAUX0, respectively. The multiplexing of the pins is
>> handled by a register in the DPAUX and so the Tegra DPAUX driver has
>> been updated to register a pinctrl device for managing these pins.
>>
>> The pins for these particular I2C devices are bound to the I2C device
>> prior to probing. However, these I2C devices are in a different power
>> partition to the DPAUX devices that own the pins. Hence, it is desirable
>> to place the pins in the 'idle' state and allow the DPAUX power
>> partition to switch off, when these I2C devices is not in use.
>> Therefore, add calls to place the I2C pins in the 'default' and 'idle'
>> states when the I2C device is runtime resumed and suspended,
>> respectively.
>>
>> Please note that the pinctrl functions that set the state of the pins
>> check to see if the devices has pins associated and will return zero
>> if they do not. Therefore, it is safe to call these pinctrl functions
>> even for I2C devices that do not have any pins associated.
> 
> I think this should be handled by drivers/i2c/muxes/i2c-mux-pinctrl.c
> instead?

I remember having a look at i2c-mux some time back, but we did not have
requirement to share the pins dynamically at runtime between the DPAUX
and I2C devices.

The pins are just configured at probe time for either the DPAUX or I2C
device and then with this change when we are not active we can power
down the pins. However, the pins are always bound to either the DPAUX or
I2C.

Cheers
Jon

-- 
nvpublic

Re: [PATCH V2 0/9] Some Tegra I2C Updates

[Jon Hunter]

On 26/08/16 16:56, Wolfram Sang wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Aug 26, 2016 at 02:08:56PM +0100, Jon Hunter wrote:
>> Add runtime-pm and pinctrl support for Tegra I2C driver.
>>
>> The first 6 patches are trivial clean-up/simplification changes.
>>
>> Changes since V1:
>> - Fixed cppcheck warning reported by Wolfram
>> - Added 3 more clean-up patches to fix some checkpatch issues
> 
> Thanks looks good. Now we only need to find out which one to apply
> first, this one or "[v8,1/3] i2c: tegra: use readx_poll_timeout after
> config_load reg programmed" and following. I cced you on the other mail.

Yes I had not bothered basing mine on top as it seem to be stagnant.
However, if Shardar updates his series I am happy to rebase. I will be
out for a week and so I will pick this up when I get back.

Cheers!
Jon

-- 
nvpublic

Re: [PATCH V2 9/9] i2c: tegra: Add pinctrl support

[Stephen Warren]

On 08/26/2016 10:38 AM, Jon Hunter wrote:
>
> On 26/08/16 16:55, Stephen Warren wrote:
>> On 08/26/2016 07:09 AM, Jon Hunter wrote:
>>> On Tegra124/132 the pins for I2C6 are shared with the Display Port AUX
>>> (DPAUX) channel and on Tegra210 the pins for I2C4 and I2C6 are shared
>>> with DPAUX1 and DPAUX0, respectively. The multiplexing of the pins is
>>> handled by a register in the DPAUX and so the Tegra DPAUX driver has
>>> been updated to register a pinctrl device for managing these pins.
>>>
>>> The pins for these particular I2C devices are bound to the I2C device
>>> prior to probing. However, these I2C devices are in a different power
>>> partition to the DPAUX devices that own the pins. Hence, it is desirable
>>> to place the pins in the 'idle' state and allow the DPAUX power
>>> partition to switch off, when these I2C devices is not in use.
>>> Therefore, add calls to place the I2C pins in the 'default' and 'idle'
>>> states when the I2C device is runtime resumed and suspended,
>>> respectively.
>>>
>>> Please note that the pinctrl functions that set the state of the pins
>>> check to see if the devices has pins associated and will return zero
>>> if they do not. Therefore, it is safe to call these pinctrl functions
>>> even for I2C devices that do not have any pins associated.
>>
>> I think this should be handled by drivers/i2c/muxes/i2c-mux-pinctrl.c
>> instead?
>
> I remember having a look at i2c-mux some time back, but we did not have
> requirement to share the pins dynamically at runtime between the DPAUX
> and I2C devices.
>
> The pins are just configured at probe time for either the DPAUX or I2C
> device and then with this change when we are not active we can power
> down the pins. However, the pins are always bound to either the DPAUX or
> I2C.

Oh, so this isn't about 1 controller accessing 2 different physical 
buses at runtime by re-routing the SoC pinmux (which is the use-case 
i2c-mux-pinctrl handles), but simply about power-management.

If the I2C controller didn't need to set a different pinctrl state 
during idle, I would say that all the pin muxing should be set up at 
system boot time, just like every other part of the pinmux is. In that 
case, the fact that the pins could be routed to 1 of 2 I2C controllers 
(DPAUX0 vs. I2C4) is irrelevant to the patch; the routing is a static 
thing anyway.

Thinking some more, i2c-mux-pinctrl actually could handle this case, 
since it does define an idle pinmux state IIRC, or at least could be 
trivially extended to do so. That said, doing this directly in the I2C 
driver does seem better in this case, since the use-case is power about 
the power advantages for a single bus, rather than anything to do with 
multiple buses.

Hence I'm OK with the concept of this patch. I didn't review the code 
though.

Re: [PATCH V2 0/9] Some Tegra I2C Updates

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On Fri, Aug 26, 2016 at 02:08:56PM +0100, Jon Hunter wrote:
> Add runtime-pm and pinctrl support for Tegra I2C driver.
> 
> The first 6 patches are trivial clean-up/simplification changes.
> 
> Changes since V1:
> - Fixed cppcheck warning reported by Wolfram
> - Added 3 more clean-up patches to fix some checkpatch issues
> 
> Jon Hunter (9):
>   i2c: tegra: Fix lines over 80 characters
>   i2c: tegra: Use BIT macro
>   i2c: tegra: Fix missing blank lines after declarations
>   i2c: tegra: Add missing new line characters
>   i2c: tegra: Remove non device-tree support
>   i2c: tegra: Use device name for adapter name
>   i2c: tegra: Simplify I2C resume
>   i2c: tegra: Add runtime power-management support
>   i2c: tegra: Add pinctrl support
> 

Applied to for-next, thanks!

If you are in for some more cleanups ;)

    SPATCH
drivers/i2c/busses/i2c-tegra.c:529:2-23: WARNING: Assignment of bool to 0/1
drivers/i2c/busses/i2c-tegra.c:555:3-24: WARNING: Assignment of bool to 0/1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V2 0/9] Some Tegra I2C Updates

[Jon Hunter]

On 30/08/16 21:38, Wolfram Sang wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Aug 26, 2016 at 02:08:56PM +0100, Jon Hunter wrote:
>> Add runtime-pm and pinctrl support for Tegra I2C driver.
>>
>> The first 6 patches are trivial clean-up/simplification changes.
>>
>> Changes since V1:
>> - Fixed cppcheck warning reported by Wolfram
>> - Added 3 more clean-up patches to fix some checkpatch issues
>>
>> Jon Hunter (9):
>>   i2c: tegra: Fix lines over 80 characters
>>   i2c: tegra: Use BIT macro
>>   i2c: tegra: Fix missing blank lines after declarations
>>   i2c: tegra: Add missing new line characters
>>   i2c: tegra: Remove non device-tree support
>>   i2c: tegra: Use device name for adapter name
>>   i2c: tegra: Simplify I2C resume
>>   i2c: tegra: Add runtime power-management support
>>   i2c: tegra: Add pinctrl support
>>
> 
> Applied to for-next, thanks!
> 
> If you are in for some more cleanups ;)
> 
>     SPATCH
> drivers/i2c/busses/i2c-tegra.c:529:2-23: WARNING: Assignment of bool to 0/1
> drivers/i2c/busses/i2c-tegra.c:555:3-24: WARNING: Assignment of bool to 0/1

Thanks! I have sent a patch to fix these.

Cheers
Jon

-- 
nvpublic

Re: [PATCH V2 9/9] i2c: tegra: Add pinctrl support

[Linus Walleij]

On Fri, Aug 26, 2016 at 3:09 PM, Jon Hunter <jonathanh@nvidia.com> wrote:

> On Tegra124/132 the pins for I2C6 are shared with the Display Port AUX
> (DPAUX) channel and on Tegra210 the pins for I2C4 and I2C6 are shared
> with DPAUX1 and DPAUX0, respectively. The multiplexing of the pins is
> handled by a register in the DPAUX and so the Tegra DPAUX driver has
> been updated to register a pinctrl device for managing these pins.
>
> The pins for these particular I2C devices are bound to the I2C device
> prior to probing. However, these I2C devices are in a different power
> partition to the DPAUX devices that own the pins. Hence, it is desirable
> to place the pins in the 'idle' state and allow the DPAUX power
> partition to switch off, when these I2C devices is not in use.
> Therefore, add calls to place the I2C pins in the 'default' and 'idle'
> states when the I2C device is runtime resumed and suspended,
> respectively.
>
> Please note that the pinctrl functions that set the state of the pins
> check to see if the devices has pins associated and will return zero
> if they do not. Therefore, it is safe to call these pinctrl functions
> even for I2C devices that do not have any pins associated.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>

This is exactly how I imagined these states being used,
so obviously:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij