[PATCH 0/6] Some Tegra I2C Updates

[PATCH 0/6] Some Tegra I2C Updates

[Jon Hunter]

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

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

Jon Hunter (6):
  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 | 97 +++++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 34 deletions(-)

-- 
2.1.4

[PATCH 1/6] 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>
---
 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 b126dbaa47e3..93b09396a10f 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -825,7 +825,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);
 	}
 
@@ -843,7 +843,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);
 	}
 
@@ -863,7 +863,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;
@@ -911,7 +911,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

Re: [PATCH 1/6] i2c: tegra: Add missing new line characters

[Laxman Dewangan]

On Thursday 11 August 2016 03:46 PM, Jon Hunter wrote:
> Add missing new line characters for the various error messages.
>
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>

Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[PATCH 2/6] 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 | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 93b09396a10f..412b914fa84b 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -851,13 +851,9 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	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

Re: [PATCH 2/6] i2c: tegra: Remove non device-tree support

[Laxman Dewangan]

On Thursday 11 August 2016 03:46 PM, Jon Hunter wrote:
> 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>
>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>

Re: [PATCH 2/6] i2c: tegra: Remove non device-tree support

[Wolfram Sang]

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

On Thu, Aug 11, 2016 at 11:16:56AM +0100, Jon Hunter wrote:
> 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

cppcheck rightfully says:

drivers/i2c/busses/i2c-tegra.c:854: style: Variable 'i2c_dev.hw' is reassigned a value before the old one has been used.

Dropping the patches...


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

Re: [PATCH 2/6] i2c: tegra: Remove non device-tree support

[Jon Hunter]

On 25/08/16 20:33, Wolfram Sang wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Aug 11, 2016 at 11:16:56AM +0100, Jon Hunter wrote:
>> 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> cppcheck rightfully says:
> 
> drivers/i2c/busses/i2c-tegra.c:854: style: Variable 'i2c_dev.hw' is reassigned a value before the old one has been used.
> 
> Dropping the patches...

Yes good catch. I will fix that.

Cheers
Jon

-- 
nvpublic

[PATCH 3/6] 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>
---
 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 412b914fa84b..57f9c1612533 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -921,7 +921,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

Re: [PATCH 3/6] i2c: tegra: Use device name for adapter name

[Laxman Dewangan]

On Thursday 11 August 2016 03:46 PM, Jon Hunter wrote:
> 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>

[PATCH 4/6] 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>
---
 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 57f9c1612533..4eaf3c4a531b 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -984,17 +984,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

Re: [PATCH 4/6] i2c: tegra: Simplify I2C resume

[Laxman Dewangan]

On Thursday 11 August 2016 03:46 PM, Jon Hunter wrote:
> 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

[PATCH 5/6] 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>
---
 drivers/i2c/busses/i2c-tegra.c | 61 ++++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 4eaf3c4a531b..4391a74d7616 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>
 
@@ -396,9 +397,11 @@ 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) {
 		ret = clk_enable(i2c_dev->fast_clk);
 		if (ret < 0) {
@@ -407,32 +410,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;
 	}
 
@@ -471,8 +481,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);
@@ -496,7 +505,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;
 }
 
@@ -670,9 +679,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;
 	}
 
@@ -688,7 +697,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;
 }
 
@@ -896,12 +907,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;
 		}
 	}
 
@@ -939,6 +959,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);
 
@@ -957,6 +982,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);
@@ -992,7 +1021,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

Re: [PATCH 5/6] i2c: tegra: Add runtime power-management support

[Laxman Dewangan]

On Thursday 11 August 2016 03:46 PM, Jon Hunter wrote:
> 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 5/6] i2c: tegra: Add runtime power-management support

[Wolfram Sang]

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


> @@ -407,32 +410,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;

You could have left the original 'return' instead of the 2 new ones, but
you decide.

> -	if (tegra_i2c_flush_fifos(i2c_dev))
> -		err = -ETIMEDOUT;
> +	err = tegra_i2c_flush_fifos(i2c_dev);

'err' is assigned but where is it checked?

I'll apply patches 1-4 meanwhile.


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

Re: [PATCH 5/6] i2c: tegra: Add runtime power-management support

[Jon Hunter]

On 25/08/16 20:26, Wolfram Sang wrote:
> * PGP Signed by an unknown key
> 
> 
>> @@ -407,32 +410,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;
> 
> You could have left the original 'return' instead of the 2 new ones, but
> you decide.

Yes I know, but I wanted to ensure for runtime-pm we only return 0 on
success. Yes clk_enable should only return 0 on success and a negative
error code otherwise, but I prefer this. So will leave as-is.

>> -	if (tegra_i2c_flush_fifos(i2c_dev))
>> -		err = -ETIMEDOUT;
>> +	err = tegra_i2c_flush_fifos(i2c_dev);
> 
> 'err' is assigned but where is it checked?

It will be returned by the function. This is no different to how it
works today if you look at the code. I did think about checking it right
after this call and returning but then I am changing the behaviour and
that should be another patch. I am not sure why it is like this in the
first place, but I did not wish to introduce any different behaviour here.

Cheers
Jon

-- 
nvpublic

Re: [PATCH 5/6] i2c: tegra: Add runtime power-management support

[Wolfram Sang]

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


> > You could have left the original 'return' instead of the 2 new ones, but
> > you decide.
> 
> Yes I know, but I wanted to ensure for runtime-pm we only return 0 on
> success. Yes clk_enable should only return 0 on success and a negative
> error code otherwise, but I prefer this. So will leave as-is.

OK.

> 
> >> -	if (tegra_i2c_flush_fifos(i2c_dev))
> >> -		err = -ETIMEDOUT;
> >> +	err = tegra_i2c_flush_fifos(i2c_dev);
> > 
> > 'err' is assigned but where is it checked?
> 
> It will be returned by the function. This is no different to how it
> works today if you look at the code. I did think about checking it right

I agree. I missed it before, thanks for the heads up.


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

[PATCH 6/6] 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>
---
 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 4391a74d7616..189795842aee 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>
@@ -402,6 +403,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) {
@@ -430,7 +435,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 6/6] i2c: tegra: Add pinctrl support

[Laxman Dewangan]

On Thursday 11 August 2016 03:47 PM, 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.
>
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 0/6] Some Tegra I2C Updates

[Jon Hunter]

Hi Wolfram,

On 11/08/16 11:16, Jon Hunter wrote:
> Add runtime-pm and pinctrl support for Tegra I2C driver.
> 
> The first 4 patches are trivial clean-up/simplification changes.
> 
> Jon Hunter (6):
>   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 | 97 +++++++++++++++++++++++++++---------------
>  1 file changed, 63 insertions(+), 34 deletions(-)

Any comments or can these be queued?

Cheers
Jon

-- 
nvpublic