2012-08-18 19:36:29

by Laxman Dewangan

[permalink] [raw]
Subject: [PATCH V2 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20

Tegra20 i2c controller does not support the continue transfer
which implements the I2C_M_NOSTART functionality of i2c
protocol mangling.
Removing the I2C_M_NOSTART functionality support for Tegra20.

Signed-off-by: Laxman Dewangan <[email protected]>
---
Changes from V1:
- make the hw feature structure as part of tegra device structure in
place of copying the member.
- Remove check for extra M_NOSTART flag if it is not supported in xfer function.

Please note that this should go on tegra common clock source tree to avoid conflicts.

drivers/i2c/busses/i2c-tegra.c | 61 ++++++++++++++++++++++++++++++---------
1 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 7149625..2f74236 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -27,6 +27,7 @@
#include <linux/slab.h>
#include <linux/i2c-tegra.h>
#include <linux/of_i2c.h>
+#include <linux/of_device.h>
#include <linux/module.h>

#include <asm/unaligned.h>
@@ -114,8 +115,18 @@ enum msg_end_type {
};

/**
+ * struct tegra_i2c_hw_feature : Different HW support on Tegra
+ * @has_continue_xfer_support: Continue transfer supports.
+ */
+
+struct tegra_i2c_hw_feature {
+ bool has_continue_xfer_support;
+};
+
+/**
* struct tegra_i2c_dev - per device i2c context
* @dev: device reference for power management
+ * @hw: Tegra i2c hw feature.
* @adapter: core i2c layer adapter information
* @div_clk: clock reference for div clock of i2c controller.
* @fast_clk: clock reference for fast clock of i2c controller.
@@ -133,6 +144,7 @@ enum msg_end_type {
*/
struct tegra_i2c_dev {
struct device *dev;
+ const struct tegra_i2c_hw_feature *hw;
struct i2c_adapter adapter;
struct clk *div_clk;
struct clk *fast_clk;
@@ -582,8 +594,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],

static u32 tegra_i2c_func(struct i2c_adapter *adap)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
- I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_NOSTART;
+ struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
+ u32 ret = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
+ I2C_FUNC_PROTOCOL_MANGLING;
+
+ if (i2c_dev->hw->has_continue_xfer_support)
+ ret |= I2C_FUNC_NOSTART;
+ return ret;
}

static const struct i2c_algorithm tegra_i2c_algo = {
@@ -591,6 +608,25 @@ static const struct i2c_algorithm tegra_i2c_algo = {
.functionality = tegra_i2c_func,
};

+static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
+ .has_continue_xfer_support = false,
+};
+
+static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
+ .has_continue_xfer_support = true,
+};
+
+#if defined(CONFIG_OF)
+/* Match table for of_platform binding */
+static const struct of_device_id tegra_i2c_of_match[] __devinitconst = {
+ { .compatible = "nvidia,tegra30-i2c", .data = &tegra30_i2c_hw, },
+ { .compatible = "nvidia,tegra20-i2c", .data = &tegra20_i2c_hw, },
+ { .compatible = "nvidia,tegra20-i2c-dvc", .data = &tegra20_i2c_hw, },
+ {},
+};
+MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
+#endif
+
static int __devinit tegra_i2c_probe(struct platform_device *pdev)
{
struct tegra_i2c_dev *i2c_dev;
@@ -659,11 +695,18 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev)
i2c_dev->bus_clk_rate = be32_to_cpup(prop);
}

- if (pdev->dev.of_node)
+ i2c_dev->hw = &tegra20_i2c_hw;
+
+ if (pdev->dev.of_node) {
+ const struct of_device_id *match;
+ match = of_match_device(of_match_ptr(tegra_i2c_of_match),
+ &pdev->dev);
+ i2c_dev->hw = match->data;
i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node,
"nvidia,tegra20-i2c-dvc");
- else if (pdev->id == 3)
+ } else if (pdev->id == 3) {
i2c_dev->is_dvc = 1;
+ }
init_completion(&i2c_dev->msg_complete);

platform_set_drvdata(pdev, i2c_dev);
@@ -751,16 +794,6 @@ static SIMPLE_DEV_PM_OPS(tegra_i2c_pm, tegra_i2c_suspend, tegra_i2c_resume);
#define TEGRA_I2C_PM NULL
#endif

-#if defined(CONFIG_OF)
-/* Match table for of_platform binding */
-static const struct of_device_id tegra_i2c_of_match[] __devinitconst = {
- { .compatible = "nvidia,tegra20-i2c", },
- { .compatible = "nvidia,tegra20-i2c-dvc", },
- {},
-};
-MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
-#endif
-
static struct platform_driver tegra_i2c_driver = {
.probe = tegra_i2c_probe,
.remove = __devexit_p(tegra_i2c_remove),
--
1.7.1.1


2012-08-18 19:36:50

by Laxman Dewangan

[permalink] [raw]
Subject: [PATCH V2 2/2] i2c: tegra: dynamically control fast clk

Tegra I2C driver enables the fast clock during initialization
and does not disable till driver removed.
Enable this clock before transfer and disable after transfer done.

Signed-off-by: Laxman Dewangan <[email protected]>
Reviewed-by: Wolfram Sang <[email protected]>
---

Changes from V1:
- Rebased again on patch 1.

drivers/i2c/busses/i2c-tegra.c | 35 ++++++++++++++++++++++++++++-------
1 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 2f74236..20c35fa 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -363,12 +363,36 @@ 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)
+{
+ int ret;
+ ret = clk_prepare_enable(i2c_dev->fast_clk);
+ if (ret < 0) {
+ dev_err(i2c_dev->dev,
+ "Enabling fast clk failed, err %d\n", ret);
+ return ret;
+ }
+ ret = clk_prepare_enable(i2c_dev->div_clk);
+ if (ret < 0) {
+ dev_err(i2c_dev->dev,
+ "Enabling div clk failed, err %d\n", ret);
+ clk_disable_unprepare(i2c_dev->fast_clk);
+ }
+ return ret;
+}
+
+static inline void tegra_i2c_clock_disable(struct tegra_i2c_dev *i2c_dev)
+{
+ clk_disable_unprepare(i2c_dev->div_clk);
+ clk_disable_unprepare(i2c_dev->fast_clk);
+}
+
static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
{
u32 val;
int err = 0;

- clk_prepare_enable(i2c_dev->div_clk);
+ tegra_i2c_clock_enable(i2c_dev);

tegra_periph_reset_assert(i2c_dev->div_clk);
udelay(2);
@@ -399,7 +423,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
if (tegra_i2c_flush_fifos(i2c_dev))
err = -ETIMEDOUT;

- clk_disable_unprepare(i2c_dev->div_clk);
+ tegra_i2c_clock_disable(i2c_dev);

if (i2c_dev->irq_disabled) {
i2c_dev->irq_disabled = 0;
@@ -575,7 +599,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
if (i2c_dev->is_suspended)
return -EBUSY;

- clk_prepare_enable(i2c_dev->div_clk);
+ tegra_i2c_clock_enable(i2c_dev);
for (i = 0; i < num; i++) {
enum msg_end_type end_type = MSG_END_STOP;
if (i < (num - 1)) {
@@ -588,7 +612,7 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
if (ret)
break;
}
- clk_disable_unprepare(i2c_dev->div_clk);
+ tegra_i2c_clock_disable(i2c_dev);
return ret ?: i;
}

@@ -724,8 +748,6 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev)
return ret;
}

- clk_prepare_enable(i2c_dev->fast_clk);
-
i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
i2c_dev->adapter.owner = THIS_MODULE;
i2c_dev->adapter.class = I2C_CLASS_HWMON;
@@ -739,7 +761,6 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev)
ret = i2c_add_numbered_adapter(&i2c_dev->adapter);
if (ret) {
dev_err(&pdev->dev, "Failed to add I2C adapter\n");
- clk_disable_unprepare(i2c_dev->fast_clk);
return ret;
}

--
1.7.1.1

2012-08-19 06:33:58

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20

On Sun, Aug 19, 2012 at 12:47:46AM +0530, Laxman Dewangan wrote:
> Tegra20 i2c controller does not support the continue transfer
> which implements the I2C_M_NOSTART functionality of i2c
> protocol mangling.
> Removing the I2C_M_NOSTART functionality support for Tegra20.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> ---
> Changes from V1:
> - make the hw feature structure as part of tegra device structure in
> place of copying the member.
> - Remove check for extra M_NOSTART flag if it is not supported in xfer function.
>
> Please note that this should go on tegra common clock source tree to avoid conflicts.

Looks good to me regarding the I2C part:

Reviewed-by: Wolfram Sang <[email protected]>

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (882.00 B)
signature.asc (198.00 B)
Digital signature
Download all attachments

2012-08-20 17:15:35

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20

On 08/18/2012 01:17 PM, Laxman Dewangan wrote:
> Tegra20 i2c controller does not support the continue transfer
> which implements the I2C_M_NOSTART functionality of i2c
> protocol mangling.
> Removing the I2C_M_NOSTART functionality support for Tegra20.

Thanks, applied the series to Tegra's for-3.7/drivers-i2c branch.

Note that I had to fix up patch 1 to remove const on the tegra*_i2c_hw
variable declarations to avoid compiler warnings when assigning pointers
into tegra_i2c_of_match[].data.

2012-08-20 17:29:16

by Laxman Dewangan

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20

On Monday 20 August 2012 10:45 PM, Stephen Warren wrote:
> On 08/18/2012 01:17 PM, Laxman Dewangan wrote:
>> Tegra20 i2c controller does not support the continue transfer
>> which implements the I2C_M_NOSTART functionality of i2c
>> protocol mangling.
>> Removing the I2C_M_NOSTART functionality support for Tegra20.
> Thanks, applied the series to Tegra's for-3.7/drivers-i2c branch.
>
> Note that I had to fix up patch 1 to remove const on the tegra*_i2c_hw
> variable declarations to avoid compiler warnings when assigning pointers
> into tegra_i2c_of_match[].data.
Thanks for correction but
I did not get this warning and when saw the structure, it is declared as
const type only..
struct of_device_id
{
char name[32];
char type[32];
char compatible[128];
#ifdef __KERNEL__
const void *data;
#else
kernel_ulong_t data;
#endif
};

2012-08-20 17:36:25

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] i2c: tegra: I2_M_NOSTART functionality not supported in Tegra20

On 08/20/2012 11:10 AM, Laxman Dewangan wrote:
> On Monday 20 August 2012 10:45 PM, Stephen Warren wrote:
>> On 08/18/2012 01:17 PM, Laxman Dewangan wrote:
>>> Tegra20 i2c controller does not support the continue transfer
>>> which implements the I2C_M_NOSTART functionality of i2c
>>> protocol mangling.
>>> Removing the I2C_M_NOSTART functionality support for Tegra20.
>> Thanks, applied the series to Tegra's for-3.7/drivers-i2c branch.
>>
>> Note that I had to fix up patch 1 to remove const on the tegra*_i2c_hw
>> variable declarations to avoid compiler warnings when assigning pointers
>> into tegra_i2c_of_match[].data.
>
> Thanks for correction but
> I did not get this warning and when saw the structure, it is declared as
> const type only..

Ah. This is something new in linux-next but not in 3.6-rc2. It was
introduced by:

98d7bbb of: add const to struct *of_device_id.data

I'll revert the modifications I made to the patch, and ignore the
warnings in Tegra's for-next; they won't be present in 3.7 presumably.