[PATCH 0/4] i2c: tegra: Bug fixes, cleanups and M_NOSTART support

[PATCH 0/4] i2c: tegra: Bug fixes, cleanups and M_NOSTART support

[Laxman Dewangan]

This patch series having the:
- Handling of late register write due to Tegra PPSB design.
- support for I2C_M_NOSTART
- Use devm_* for all allocation.

Laxman Dewangan (4):
  i2c: tegra: make sure register writes completes
  i2c: tegra: add PROTOCOL_MANGLING as supported functionality.
  i2c: tegra: support for I2C_M_NOSTART functionality
  i2c: tegra: make all resource allocation through devm_*

 drivers/i2c/busses/i2c-tegra.c |  107 +++++++++++++++++++++-------------------
 1 files changed, 57 insertions(+), 50 deletions(-)

[PATCH 1/4] i2c: tegra: make sure register writes completes

[Laxman Dewangan]

The Tegra PPSB (an peripheral bus) queues writes transactions.
In order to guarantee that writes have completed before a
certain time, a read transaction to a register on the same
bus must be executed.
This is necessary in situations such as when clearing an
interrupt status or enable, so that when returning from an
interrupt handler, the HW has already de-asserted its
interrupt status output, which will avoid spurious interrupts.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 8b2e555..fa92396 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -430,6 +430,13 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
 	if (i2c_dev->is_dvc)
 		dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
 
+	/*
+	 * Register write get queued in the PPSB bus and write can
+	 * happen later. Read back register to make sure that register
+	 * write is completed.
+	 */
+	i2c_readl(i2c_dev, I2C_INT_STATUS);
+
 	if (status & I2C_INT_PACKET_XFER_COMPLETE) {
 		BUG_ON(i2c_dev->msg_buf_remaining);
 		complete(&i2c_dev->msg_complete);
@@ -444,6 +451,9 @@ err:
 	if (i2c_dev->is_dvc)
 		dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
 
+	/* Read back register to make sure that register writes completed */
+	i2c_readl(i2c_dev, I2C_INT_STATUS);
+
 	complete(&i2c_dev->msg_complete);
 	return IRQ_HANDLED;
 }
@@ -505,6 +515,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 	ret = wait_for_completion_timeout(&i2c_dev->msg_complete, TEGRA_I2C_TIMEOUT);
 	tegra_i2c_mask_irq(i2c_dev, int_mask);
 
+	/* Read back register to make sure that register writes completed */
+	i2c_readl(i2c_dev, I2C_INT_MASK);
+
 	if (WARN_ON(ret == 0)) {
 		dev_err(i2c_dev->dev, "i2c transfer timed out\n");
 
-- 
1.7.1.1

Re: [PATCH 1/4] i2c: tegra: make sure register writes completes

[Wolfram Sang]

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

On Tue, Jun 05, 2012 at 06:39:57PM +0530, Laxman Dewangan wrote:
> The Tegra PPSB (an peripheral bus) queues writes transactions.
> In order to guarantee that writes have completed before a
> certain time, a read transaction to a register on the same
> bus must be executed.
> This is necessary in situations such as when clearing an
> interrupt status or enable, so that when returning from an
> interrupt handler, the HW has already de-asserted its
> interrupt status output, which will avoid spurious interrupts.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-tegra.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 8b2e555..fa92396 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -430,6 +430,13 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
>  	if (i2c_dev->is_dvc)
>  		dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
>  
> +	/*
> +	 * Register write get queued in the PPSB bus and write can
> +	 * happen later. Read back register to make sure that register
> +	 * write is completed.
> +	 */
> +	i2c_readl(i2c_dev, I2C_INT_STATUS);

Does it make sense to put the read into i2c_writel?

> +
>  	if (status & I2C_INT_PACKET_XFER_COMPLETE) {
>  		BUG_ON(i2c_dev->msg_buf_remaining);
>  		complete(&i2c_dev->msg_complete);
> @@ -444,6 +451,9 @@ err:
>  	if (i2c_dev->is_dvc)
>  		dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
>  
> +	/* Read back register to make sure that register writes completed */
> +	i2c_readl(i2c_dev, I2C_INT_STATUS);
> +
>  	complete(&i2c_dev->msg_complete);
>  	return IRQ_HANDLED;
>  }
> @@ -505,6 +515,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>  	ret = wait_for_completion_timeout(&i2c_dev->msg_complete, TEGRA_I2C_TIMEOUT);
>  	tegra_i2c_mask_irq(i2c_dev, int_mask);
>  
> +	/* Read back register to make sure that register writes completed */
> +	i2c_readl(i2c_dev, I2C_INT_MASK);
> +

It definately makes sense to put this read into tegra_i2c_mask_irq()?

>  	if (WARN_ON(ret == 0)) {
>  		dev_err(i2c_dev->dev, "i2c transfer timed out\n");

Regards,

   Wolfram

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] i2c: tegra: make sure register writes completes

[Laxman Dewangan]

On Tuesday 12 June 2012 01:24 PM, Wolfram Sang wrote:
> * PGP Signed by an unknown key
>
> On Tue, Jun 05, 2012 at 06:39:57PM +0530, Laxman Dewangan wrote:
>> @@ -430,6 +430,13 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
>>   	if (i2c_dev->is_dvc)
>>   		dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
>>
>> +	/*
>> +	 * Register write get queued in the PPSB bus and write can
>> +	 * happen later. Read back register to make sure that register
>> +	 * write is completed.
>> +	 */
>> +	i2c_readl(i2c_dev, I2C_INT_STATUS);
> Does it make sense to put the read into i2c_writel?
>
We can not put in i2c_writel() as we also do fifo write using this and 
writing and reading back fifo can drainout the fifo.
hence putting this here seems more appropriate.
>> +
>>   	if (status&  I2C_INT_PACKET_XFER_COMPLETE) {
>>   		BUG_ON(i2c_dev->msg_buf_remaining);
>>   		complete(&i2c_dev->msg_complete);
>> @@ -444,6 +451,9 @@ err:
>>   	if (i2c_dev->is_dvc)
>>   		dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
>>
>> +	/* Read back register to make sure that register writes completed */
>> +	i2c_readl(i2c_dev, I2C_INT_STATUS);
>> +
>>   	complete(&i2c_dev->msg_complete);
>>   	return IRQ_HANDLED;
>>   }
>> @@ -505,6 +515,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>>   	ret = wait_for_completion_timeout(&i2c_dev->msg_complete, TEGRA_I2C_TIMEOUT);
>>   	tegra_i2c_mask_irq(i2c_dev, int_mask);
>>
>> +	/* Read back register to make sure that register writes completed */
>> +	i2c_readl(i2c_dev, I2C_INT_MASK);
>> +
> It definately makes sense to put this read into tegra_i2c_mask_irq()?
>
Ok, fine with me. I put the read back logic inside mask_irq() and 
unmask_irq().
Will send the fix in next patch.
>>   	if (WARN_ON(ret == 0)) {
>>   		dev_err(i2c_dev->dev, "i2c transfer timed out\n");
> Regards,
>
>     Wolfram
>

Re: [PATCH 1/4] i2c: tegra: make sure register writes completes

[Stephen Warren]

On 06/12/2012 04:16 AM, Laxman Dewangan wrote:
> On Tuesday 12 June 2012 01:24 PM, Wolfram Sang wrote:
>> * PGP Signed by an unknown key
>>
>> On Tue, Jun 05, 2012 at 06:39:57PM +0530, Laxman Dewangan wrote:
>>> @@ -430,6 +430,13 @@ static irqreturn_t tegra_i2c_isr(int irq, void
>>> *dev_id)
>>>       if (i2c_dev->is_dvc)
>>>           dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
>>>
>>> +    /*
>>> +     * Register write get queued in the PPSB bus and write can
>>> +     * happen later. Read back register to make sure that register
>>> +     * write is completed.
>>> +     */
>>> +    i2c_readl(i2c_dev, I2C_INT_STATUS);
>>
>> Does it make sense to put the read into i2c_writel?
>
> We can not put in i2c_writel() as we also do fifo write using this and
> writing and reading back fifo can drainout the fifo.
> hence putting this here seems more appropriate.

You can put it inside i2c_writel(), but make the read-back conditional
depending on which register was just written. The Tegra SD driver has
similar conditional code in readl/writel to WAR some HW quirks.

[PATCH 2/4] i2c: tegra: add PROTOCOL_MANGLING as supported functionality.

[Laxman Dewangan]

The Tegra i2c driver supports the I2C_M_IGNORE_NAK and hence
returning I2C_FUNC_PROTOCOL_MANGLING as supported functionality.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index fa92396..36d8725 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -572,7 +572,8 @@ 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;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR |
+		I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 static const struct i2c_algorithm tegra_i2c_algo = {
-- 
1.7.1.1

[PATCH 3/4] i2c: tegra: support for I2C_M_NOSTART functionality

[Laxman Dewangan]

Adding support for functionality I2C_M_NOSTART.
When multiple message transfer request made through i2c
and if any message is flagged with I2C_M_NOSTART then
it will not send the start/repeat-start and address of
that message i.e. sends data directly.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
This is rebased of earlier patch on same support.
At this time using the new flag I2C_FUNC_NOSTART.

 drivers/i2c/busses/i2c-tegra.c |   31 ++++++++++++++++++++++++++-----
 1 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 36d8725..4cc9594 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -97,8 +97,21 @@
 #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 I2C_HEADER_MASTER_ADDR_SHIFT		12
 #define I2C_HEADER_SLAVE_ADDR_SHIFT		1
+/*
+ * msg_end_type: The bus control which need to be send at end of transfer.
+ * @MSG_END_STOP: Send stop pulse at end of transfer.
+ * @MSG_END_REPEAT_START: Send repeat start at end of transfer.
+ * @MSG_END_CONTINUE: The following on message is coming and so do not send
+ *		stop or repeat start.
+ */
+enum msg_end_type {
+	MSG_END_STOP,
+	MSG_END_REPEAT_START,
+	MSG_END_CONTINUE,
+};
 
 /**
  * struct tegra_i2c_dev	- per device i2c context
@@ -459,7 +472,7 @@ err:
 }
 
 static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
-	struct i2c_msg *msg, int stop)
+	struct i2c_msg *msg, enum msg_end_type end_state)
 {
 	u32 packet_header;
 	u32 int_mask;
@@ -486,7 +499,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 	i2c_writel(i2c_dev, packet_header, I2C_TX_FIFO);
 
 	packet_header = I2C_HEADER_IE_ENABLE;
-	if (!stop)
+	if (end_state == MSG_END_CONTINUE)
+		packet_header |= I2C_HEADER_CONTINUE_XFER;
+	else if (end_state == MSG_END_REPEAT_START)
 		packet_header |= I2C_HEADER_REPEAT_START;
 	if (msg->flags & I2C_M_TEN) {
 		packet_header |= msg->addr;
@@ -561,8 +576,14 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 
 	clk_enable(i2c_dev->clk);
 	for (i = 0; i < num; i++) {
-		int stop = (i == (num - 1)) ? 1  : 0;
-		ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], stop);
+		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;
+			else
+				end_type = MSG_END_REPEAT_START;
+		}
+		ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], end_type);
 		if (ret)
 			break;
 	}
@@ -573,7 +594,7 @@ 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_PROTOCOL_MANGLING | I2C_FUNC_NOSTART;
 }
 
 static const struct i2c_algorithm tegra_i2c_algo = {
-- 
1.7.1.1

Re: [PATCH 0/4] i2c: tegra: Bug fixes, cleanups and M_NOSTART support

[Stephen Warren]

On 06/05/2012 07:09 AM, Laxman Dewangan wrote:
> This patch series having the:
> - Handling of late register write due to Tegra PPSB design.
> - support for I2C_M_NOSTART
> - Use devm_* for all allocation.

The series,

Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

Note that patch 4 touches context adjacent to Prashant's "i2c: tegra:
Add clk_prepare/clk_unprepare" patch, which I hope to take through the
Tegra tree since it's a requirement for the Tegra common clock
conversion. I don't think this will cause any significant conflict, but
perhaps it's worth resolving it explicitly.

Wolfram, perhaps we should put these 4 patches and Prashan'ts into their
own topic branch so that you can merge it into the I2C tree, and I can
merge it into the Tegra tree too? Or, I can take everything through
Tegra if you want, and ack it.

Laxman, do you expect any more I2C-related changes this kernel cycle?

Re: [PATCH 0/4] i2c: tegra: Bug fixes, cleanups and M_NOSTART support

[Laxman Dewangan]

On Tuesday 05 June 2012 09:44 PM, Stephen Warren wrote:
> On 06/05/2012 07:09 AM, Laxman Dewangan wrote:
>> This patch series having the:
>> - Handling of late register write due to Tegra PPSB design.
>> - support for I2C_M_NOSTART
>> - Use devm_* for all allocation.
> The series,
>
> Acked-by: Stephen Warren<swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>
> Note that patch 4 touches context adjacent to Prashant's "i2c: tegra:
> Add clk_prepare/clk_unprepare" patch, which I hope to take through the
> Tegra tree since it's a requirement for the Tegra common clock
> conversion. I don't think this will cause any significant conflict, but
> perhaps it's worth resolving it explicitly.
>
> Wolfram, perhaps we should put these 4 patches and Prashan'ts into their
> own topic branch so that you can merge it into the I2C tree, and I can
> merge it into the Tegra tree too? Or, I can take everything through
> Tegra if you want, and ack it.
>
> Laxman, do you expect any more I2C-related changes this kernel cycle?
I have two more changes in plan:
-I2c controller require two clock source and the fixes towards that. 
This can go on Tegra tree as changes are require in 
mach-tegra/tegra30_clock.c, tegra20_clock.c files also.

- Run time PM support but can be done later.

Re: [PATCH 0/4] i2c: tegra: Bug fixes, cleanups and M_NOSTART support

[Wolfram Sang]

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

On Tue, Jun 05, 2012 at 10:37:36PM +0530, Laxman Dewangan wrote:
> On Tuesday 05 June 2012 09:44 PM, Stephen Warren wrote:
> >On 06/05/2012 07:09 AM, Laxman Dewangan wrote:
> >>This patch series having the:
> >>- Handling of late register write due to Tegra PPSB design.
> >>- support for I2C_M_NOSTART
> >>- Use devm_* for all allocation.
> >The series,
> >
> >Acked-by: Stephen Warren<swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> >
> >Note that patch 4 touches context adjacent to Prashant's "i2c: tegra:
> >Add clk_prepare/clk_unprepare" patch, which I hope to take through the
> >Tegra tree since it's a requirement for the Tegra common clock
> >conversion. I don't think this will cause any significant conflict, but
> >perhaps it's worth resolving it explicitly.

Is it really a requirement? Just wondering if it will cause problems, if
Prashant's patch goes in via I2C after arm-soc has been merged. I am
fine with simply acking the patch, though.

> >Wolfram, perhaps we should put these 4 patches and Prashan'ts into their
> >own topic branch so that you can merge it into the I2C tree, and I can
> >merge it into the Tegra tree too? Or, I can take everything through
> >Tegra if you want, and ack it.

Laxman's patches should really go via I2C, I think. Can't we just fix
the conflict in arm-soc?

Regards,

   Wolfram

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/4] i2c: tegra: Bug fixes, cleanups and M_NOSTART support

[Stephen Warren]

On 06/12/2012 02:50 AM, Wolfram Sang wrote:
> On Tue, Jun 05, 2012 at 10:37:36PM +0530, Laxman Dewangan wrote:
>> On Tuesday 05 June 2012 09:44 PM, Stephen Warren wrote:
>>> On 06/05/2012 07:09 AM, Laxman Dewangan wrote:
>>>> This patch series having the: - Handling of late register
>>>> write due to Tegra PPSB design. - support for I2C_M_NOSTART -
>>>> Use devm_* for all allocation.
>>> The series,
>>> 
>>> Acked-by: Stephen Warren<swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>>> 
>>> Note that patch 4 touches context adjacent to Prashant's "i2c:
>>> tegra: Add clk_prepare/clk_unprepare" patch, which I hope to
>>> take through the Tegra tree since it's a requirement for the
>>> Tegra common clock conversion. I don't think this will cause
>>> any significant conflict, but perhaps it's worth resolving it
>>> explicitly.
> 
> Is it really a requirement? Just wondering if it will cause
> problems, if Prashant's patch goes in via I2C after arm-soc has
> been merged. I am fine with simply acking the patch, though.

AIUI, the clk_prepare patch is certainly a requirement for Tegra's
conversion to common clock; I believe that a clk_enable() without a
preceding clk_prepare() will fail since it's an invalid call sequence.
I'm not 100% sure yet, but I hope Prashant will post patches to
convert Tegra to common clock in time for 3.6, so having all the
driver clk_prepare in a branch prior to the common clock conversion is
required.

>>> Wolfram, perhaps we should put these 4 patches and Prashan'ts
>>> into their own topic branch so that you can merge it into the
>>> I2C tree, and I can merge it into the Tegra tree too? Or, I can
>>> take everything through Tegra if you want, and ack it.
> 
> Laxman's patches should really go via I2C, I think. Can't we just
> fix the conflict in arm-soc?

Yes, Laxman's changes should be able to go through I2C without a
problem. We can try this out without any cross-merged topic branches
and try resolving in arm-soc for now. If there turns out to be an
issue, we can always rebase the Tegra and/or I2C for-next branches to
fix it up.

[PATCH 4/4] i2c: tegra: make all resource allocation through devm_*

[Laxman Dewangan]

Use the devm_* for the memory region allocation, interrupt request,
clock handler request.
By doing this, it does not require to explicitly free it and hence
saving some code.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This is rebased of earlier similar patch and sending as
part of this series.

 drivers/i2c/busses/i2c-tegra.c |   62 +++++++++++-----------------------------
 1 files changed, 17 insertions(+), 45 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 4cc9594..6745631 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -607,7 +607,6 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev)
 	struct tegra_i2c_dev *i2c_dev;
 	struct tegra_i2c_platform_data *pdata = pdev->dev.platform_data;
 	struct resource *res;
-	struct resource *iomem;
 	struct clk *clk;
 	struct clk *i2c_clk;
 	const unsigned int *prop;
@@ -620,50 +619,41 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "no mem resource\n");
 		return -EINVAL;
 	}
-	iomem = request_mem_region(res->start, resource_size(res), pdev->name);
-	if (!iomem) {
-		dev_err(&pdev->dev, "I2C region already claimed\n");
-		return -EBUSY;
-	}
 
-	base = ioremap(iomem->start, resource_size(iomem));
+	base = devm_request_and_ioremap(&pdev->dev, res);
 	if (!base) {
-		dev_err(&pdev->dev, "Cannot ioremap I2C region\n");
-		return -ENOMEM;
+		dev_err(&pdev->dev, "Cannot request/ioremap I2C registers\n");
+		return -EADDRNOTAVAIL;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "no irq resource\n");
-		ret = -EINVAL;
-		goto err_iounmap;
+		return -EINVAL;
 	}
 	irq = res->start;
 
-	clk = clk_get(&pdev->dev, NULL);
+	clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "missing controller clock");
-		ret = PTR_ERR(clk);
-		goto err_release_region;
+		return PTR_ERR(clk);
 	}
 
-	i2c_clk = clk_get(&pdev->dev, "i2c");
+	i2c_clk = devm_clk_get(&pdev->dev, "i2c");
 	if (IS_ERR(i2c_clk)) {
 		dev_err(&pdev->dev, "missing bus clock");
-		ret = PTR_ERR(i2c_clk);
-		goto err_clk_put;
+		return PTR_ERR(i2c_clk);
 	}
 
-	i2c_dev = kzalloc(sizeof(struct tegra_i2c_dev), GFP_KERNEL);
+	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
 	if (!i2c_dev) {
-		ret = -ENOMEM;
-		goto err_i2c_clk_put;
+		dev_err(&pdev->dev, "Could not allocate struct tegra_i2c_dev");
+		return -ENOMEM;
 	}
 
 	i2c_dev->base = base;
 	i2c_dev->clk = clk;
 	i2c_dev->i2c_clk = i2c_clk;
-	i2c_dev->iomem = iomem;
 	i2c_dev->adapter.algo = &tegra_i2c_algo;
 	i2c_dev->irq = irq;
 	i2c_dev->cont_id = pdev->id;
@@ -692,13 +682,14 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev)
 	ret = tegra_i2c_init(i2c_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize i2c controller");
-		goto err_free;
+		return ret;
 	}
 
-	ret = request_irq(i2c_dev->irq, tegra_i2c_isr, 0, pdev->name, i2c_dev);
+	ret = devm_request_irq(&pdev->dev, i2c_dev->irq,
+			tegra_i2c_isr, 0, pdev->name, i2c_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request irq %i\n", i2c_dev->irq);
-		goto err_free;
+		return ret;
 	}
 
 	clk_enable(i2c_dev->i2c_clk);
@@ -716,38 +707,19 @@ 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");
-		goto err_free_irq;
+		clk_disable(i2c_dev->i2c_clk);
+		return ret;
 	}
 
 	of_i2c_register_devices(&i2c_dev->adapter);
 
 	return 0;
-err_free_irq:
-	free_irq(i2c_dev->irq, i2c_dev);
-err_free:
-	kfree(i2c_dev);
-err_i2c_clk_put:
-	clk_put(i2c_clk);
-err_clk_put:
-	clk_put(clk);
-err_release_region:
-	release_mem_region(iomem->start, resource_size(iomem));
-err_iounmap:
-	iounmap(base);
-	return ret;
 }
 
 static int __devexit tegra_i2c_remove(struct platform_device *pdev)
 {
 	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
 	i2c_del_adapter(&i2c_dev->adapter);
-	free_irq(i2c_dev->irq, i2c_dev);
-	clk_put(i2c_dev->i2c_clk);
-	clk_put(i2c_dev->clk);
-	release_mem_region(i2c_dev->iomem->start,
-		resource_size(i2c_dev->iomem));
-	iounmap(i2c_dev->base);
-	kfree(i2c_dev);
 	return 0;
 }
 
-- 
1.7.1.1