diff mbox series

[v2,5/6] i2c: mpc: use device managed APIs

Message ID 20210329015206.17437-6-chris.packham@alliedtelesis.co.nz
State Accepted
Commit 09aab7add7bf9a7368da94fd09529847255f5fd9
Headers show
Series i2c: mpc: Refactor to improve responsiveness | expand

Commit Message

Chris Packham March 29, 2021, 1:52 a.m. UTC 
Use device managed functions an clean up error handling.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/i2c/busses/i2c-mpc.c | 46 ++++++++++++++----------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

Comments

Andy Shevchenko April 12, 2021, 10:52 p.m. UTC | #1 
On Mon, Mar 29, 2021 at 4:54 AM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
>
> Use device managed functions an clean up error handling.

For the god sake how have you tested this?
The patch is broken.
Andy Shevchenko April 12, 2021, 10:55 p.m. UTC | #2 
On Tue, Apr 13, 2021 at 1:52 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>

> On Mon, Mar 29, 2021 at 4:54 AM Chris Packham

> <chris.packham@alliedtelesis.co.nz> wrote:

> >

> > Use device managed functions an clean up error handling.

>

> For the god sake how have you tested this?

> The patch is broken.


Looking into i2c-next and taking into account we are at rc7 I think we
must revert this.

-- 
With Best Regards,
Andy Shevchenko
Chris Packham April 12, 2021, 11:21 p.m. UTC | #3 
On 13/04/21 10:52 am, Andy Shevchenko wrote:
> On Mon, Mar 29, 2021 at 4:54 AM Chris Packham

> <chris.packham@alliedtelesis.co.nz> wrote:

>> Use device managed functions an clean up error handling.

> For the god sake how have you tested this?

> The patch is broken.

I've clearly missed the remove path in my testing. I was focused on the 
interrupt bevhaviour not the probe/remove which I'll make sure to check 
for the next round.
Chris Packham April 12, 2021, 11:26 p.m. UTC | #4 
On 13/04/21 11:21 am, Chris Packham wrote:
>

> On 13/04/21 10:52 am, Andy Shevchenko wrote:

>> On Mon, Mar 29, 2021 at 4:54 AM Chris Packham

>> <chris.packham@alliedtelesis.co.nz> wrote:

>>> Use device managed functions an clean up error handling.

>> For the god sake how have you tested this?

>> The patch is broken.

> I've clearly missed the remove path in my testing. I was focused on 

> the interrupt bevhaviour not the probe/remove which I'll make sure to 

> check for the next round.


Should I send a revert or leave it for Wolfram?
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 5b746a898e8e..46cdb36e2f9b 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -654,7 +654,6 @@  static int fsl_i2c_probe(struct platform_device *op)
 	u32 clock = MPC_I2C_CLOCK_LEGACY;
 	int result = 0;
 	int plen;
-	struct resource res;
 	struct clk *clk;
 	int err;
 
@@ -662,7 +661,7 @@  static int fsl_i2c_probe(struct platform_device *op)
 	if (!match)
 		return -EINVAL;
 
-	i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
+	i2c = devm_kzalloc(&op->dev, sizeof(*i2c), GFP_KERNEL);
 	if (!i2c)
 		return -ENOMEM;
 
@@ -670,24 +669,21 @@  static int fsl_i2c_probe(struct platform_device *op)
 
 	init_waitqueue_head(&i2c->queue);
 
-	i2c->base = of_iomap(op->dev.of_node, 0);
-	if (!i2c->base) {
+	i2c->base = devm_platform_ioremap_resource(op, 0);
+	if (IS_ERR(i2c->base)) {
 		dev_err(i2c->dev, "failed to map controller\n");
-		result = -ENOMEM;
-		goto fail_map;
+		return PTR_ERR(i2c->base);
 	}
 
-	i2c->irq = irq_of_parse_and_map(op->dev.of_node, 0);
-	if (i2c->irq < 0) {
-		result = i2c->irq;
-		goto fail_map;
-	}
+	i2c->irq = platform_get_irq(op, 0);
+	if (i2c->irq < 0)
+		return i2c->irq;
 
-	result = request_irq(i2c->irq, mpc_i2c_isr,
+	result = devm_request_irq(&op->dev, i2c->irq, mpc_i2c_isr,
 			IRQF_SHARED, "i2c-mpc", i2c);
 	if (result < 0) {
 		dev_err(i2c->dev, "failed to attach interrupt\n");
-		goto fail_request;
+		return result;
 	}
 
 	/*
@@ -699,7 +695,7 @@  static int fsl_i2c_probe(struct platform_device *op)
 		err = clk_prepare_enable(clk);
 		if (err) {
 			dev_err(&op->dev, "failed to enable clock\n");
-			goto fail_request;
+			return err;
 		} else {
 			i2c->clk_per = clk;
 		}
@@ -731,32 +727,26 @@  static int fsl_i2c_probe(struct platform_device *op)
 	}
 	dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 / HZ);
 
-	platform_set_drvdata(op, i2c);
-
 	i2c->adap = mpc_ops;
-	of_address_to_resource(op->dev.of_node, 0, &res);
 	scnprintf(i2c->adap.name, sizeof(i2c->adap.name),
-		  "MPC adapter at 0x%llx", (unsigned long long)res.start);
-	i2c_set_adapdata(&i2c->adap, i2c);
+		  "MPC adapter (%s)", of_node_full_name(op->dev.of_node));
 	i2c->adap.dev.parent = &op->dev;
+	i2c->adap.nr = op->id;
 	i2c->adap.dev.of_node = of_node_get(op->dev.of_node);
 	i2c->adap.bus_recovery_info = &fsl_i2c_recovery_info;
+	platform_set_drvdata(op, i2c);
+	i2c_set_adapdata(&i2c->adap, i2c);
 
-	result = i2c_add_adapter(&i2c->adap);
-	if (result < 0)
+	result = i2c_add_numbered_adapter(&i2c->adap);
+	if (result)
 		goto fail_add;
 
-	return result;
+	return 0;
 
  fail_add:
 	if (i2c->clk_per)
 		clk_disable_unprepare(i2c->clk_per);
-	free_irq(i2c->irq, i2c);
- fail_request:
-	irq_dispose_mapping(i2c->irq);
-	iounmap(i2c->base);
- fail_map:
-	kfree(i2c);
+
 	return result;
 };