[PATCH 0/3] i2c: ibm_iic: use devm and get irq in probe

[PATCH 0/3] i2c: ibm_iic: use devm and get irq in probe

[Rosen Penev]

Simplifies probe slightly and handles potential -EPROBE_DEFER a bit better.

Rosen Penev (3):
  i2c: ibm_iic: ioremap with platform pointer
  i2c: ibm_iic: get the irq early in probe
  i2c: ibm_iic: use devm for main allocation

 drivers/i2c/busses/i2c-ibm_iic.c | 44 ++++++++++++--------------------
 1 file changed, 17 insertions(+), 27 deletions(-)

-- 
2.55.0

[PATCH 1/3] i2c: ibm_iic: ioremap with platform pointer

[Rosen Penev]

devm_platform_ioremap_resource only needs a platform_device pointer and
calling it early allows handling potential -EPROBE_DEFER.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/i2c/busses/i2c-ibm_iic.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 7c70e8bda24e..043cb64a2544 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -684,21 +684,21 @@ static int iic_probe(struct platform_device *ofdev)
 	struct device_node *np = ofdev->dev.of_node;
 	struct ibm_iic_private *dev;
 	struct i2c_adapter *adap;
+	void __iomem *vaddr;
 	const u32 *freq;
 	int ret;
 
+	vaddr = devm_platform_ioremap_resource(ofdev, 0);
+	if (IS_ERR(vaddr))
+		return PTR_ERR(vaddr);
+
 	dev = kzalloc_obj(*dev);
 	if (!dev)
 		return -ENOMEM;
 
 	platform_set_drvdata(ofdev, dev);
 
-	dev->vaddr = of_iomap(np, 0);
-	if (dev->vaddr == NULL) {
-		dev_err(&ofdev->dev, "failed to iomap device\n");
-		ret = -ENXIO;
-		goto error_cleanup;
-	}
+	dev->vaddr = vaddr;
 
 	init_waitqueue_head(&dev->wq);
 
@@ -751,9 +751,6 @@ static int iic_probe(struct platform_device *ofdev)
 		free_irq(dev->irq, dev);
 	}
 
-	if (dev->vaddr)
-		iounmap(dev->vaddr);
-
 	kfree(dev);
 	return ret;
 }
@@ -772,7 +769,6 @@ static void iic_remove(struct platform_device *ofdev)
 		free_irq(dev->irq, dev);
 	}
 
-	iounmap(dev->vaddr);
 	kfree(dev);
 }
 
-- 
2.55.0

[PATCH 2/3] i2c: ibm_iic: get the irq early in probe

[Rosen Penev]

platform_get_irq() can return -EPROBE_DEFER, unlike
irq_of_map_and_parse(), which helps reduce work done during probe.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/i2c/busses/i2c-ibm_iic.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 043cb64a2544..2730c8410167 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -649,20 +649,11 @@ static inline u8 iic_clckdiv(unsigned int opb)
 }
 
 static int iic_request_irq(struct platform_device *ofdev,
-				     struct ibm_iic_private *dev)
+			     struct ibm_iic_private *dev, int irq)
 {
-	struct device_node *np = ofdev->dev.of_node;
-	int irq;
-
 	if (iic_force_poll)
 		return 0;
 
-	irq = irq_of_parse_and_map(np, 0);
-	if (!irq) {
-		dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n");
-		return 0;
-	}
-
 	/* Disable interrupts until we finish initialization, assumes
 	 *  level-sensitive IRQ setup...
 	 */
@@ -686,12 +677,19 @@ static int iic_probe(struct platform_device *ofdev)
 	struct i2c_adapter *adap;
 	void __iomem *vaddr;
 	const u32 *freq;
+	int irq;
 	int ret;
 
 	vaddr = devm_platform_ioremap_resource(ofdev, 0);
 	if (IS_ERR(vaddr))
 		return PTR_ERR(vaddr);
 
+	irq = platform_get_irq(ofdev, 0);
+	if (irq == -EPROBE_DEFER)
+		return irq;
+	if (irq < 0)
+		dev_warn(&ofdev->dev, "using polling mode\n");
+
 	dev = kzalloc_obj(*dev);
 	if (!dev)
 		return -ENOMEM;
@@ -702,9 +700,8 @@ static int iic_probe(struct platform_device *ofdev)
 
 	init_waitqueue_head(&dev->wq);
 
-	dev->irq = iic_request_irq(ofdev, dev);
-	if (!dev->irq)
-		dev_warn(&ofdev->dev, "using polling mode\n");
+	if (irq > 0)
+		dev->irq = iic_request_irq(ofdev, dev, irq);
 
 	/* Board specific settings */
 	if (iic_force_fast || of_get_property(np, "fast-mode", NULL))
-- 
2.55.0

[PATCH 3/3] i2c: ibm_iic: use devm for main allocation

[Rosen Penev]

Allows getting rid of kfree in failure and remove paths.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/i2c/busses/i2c-ibm_iic.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 2730c8410167..8fc5f314742f 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -690,7 +690,7 @@ static int iic_probe(struct platform_device *ofdev)
 	if (irq < 0)
 		dev_warn(&ofdev->dev, "using polling mode\n");
 
-	dev = kzalloc_obj(*dev);
+	dev = devm_kzalloc(&ofdev->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
@@ -748,7 +748,6 @@ static int iic_probe(struct platform_device *ofdev)
 		free_irq(dev->irq, dev);
 	}
 
-	kfree(dev);
 	return ret;
 }
 
@@ -765,8 +764,6 @@ static void iic_remove(struct platform_device *ofdev)
 		iic_interrupt_mode(dev, 0);
 		free_irq(dev->irq, dev);
 	}
-
-	kfree(dev);
 }
 
 static const struct of_device_id ibm_iic_match[] = {
-- 
2.55.0

Re: [PATCH 1/3] i2c: ibm_iic: ioremap with platform pointer

[Markus Elfring]

> devm_platform_ioremap_resource only needs a platform_device pointer and
> calling it early allows handling potential -EPROBE_DEFER.

I find the change description improvable.

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n94

Regards,
Markus

Re: [PATCH 3/3] i2c: ibm_iic: use devm for main allocation

[Markus Elfring]

> Allows getting rid of kfree in failure and remove paths.

Will another improved change description become more desirable?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n94

Regards,
Markus

Re: [PATCH 1/3] i2c: ibm_iic: ioremap with platform pointer

[Andi Shyti]

Hi Rosen,

On Fri, Aug 21, 2026 at 04:02:14PM -0700, Rosen Penev wrote:
> devm_platform_ioremap_resource only needs a platform_device pointer and
> calling it early allows handling potential -EPROBE_DEFER.

Please improve the commit message as suggested by Markus. Please
use the imperative form, it's not just a formality, but it helps
understand what you actually did (I'm using the imperative form
here sothat you clearly understand me).

Besides devm_platform_ioremap_resource() does not return
EPROBE_DEFER.

> Signed-off-by: Rosen Penev <rosenp@gmail.com>

...

> @@ -751,9 +751,6 @@ static int iic_probe(struct platform_device *ofdev)
>  		free_irq(dev->irq, dev);
>  	}
>  
> -	if (dev->vaddr)
> -		iounmap(dev->vaddr);

I think now you can remove linux/of_address.h

Thanks,
Andi

> -
>  	kfree(dev);
>  	return ret;
>  }
> @@ -772,7 +769,6 @@ static void iic_remove(struct platform_device *ofdev)
>  		free_irq(dev->irq, dev);
>  	}
>  
> -	iounmap(dev->vaddr);
>  	kfree(dev);
>  }
>  
> -- 
> 2.55.0
> 

Re: [PATCH 2/3] i2c: ibm_iic: get the irq early in probe

[Andi Shyti]

Hi Rosen,

On Fri, Aug 21, 2026 at 04:02:15PM -0700, Rosen Penev wrote:
> platform_get_irq() can return -EPROBE_DEFER, unlike
> irq_of_map_and_parse(), which helps reduce work done during probe.

same comment for the commit log goes here.

> Signed-off-by: Rosen Penev <rosenp@gmail.com>

...

> @@ -686,12 +677,19 @@ static int iic_probe(struct platform_device *ofdev)
>  	struct i2c_adapter *adap;
>  	void __iomem *vaddr;
>  	const u32 *freq;
> +	int irq;
>  	int ret;
>  
>  	vaddr = devm_platform_ioremap_resource(ofdev, 0);
>  	if (IS_ERR(vaddr))
>  		return PTR_ERR(vaddr);
>  
> +	irq = platform_get_irq(ofdev, 0);
> +	if (irq == -EPROBE_DEFER)
> +		return irq;

if iic_force_poll is set true, there is no reason to return if
platform_get_irq() fails with EPROBE_DEFER. Check for
iic_force_poll first.

Andi

> +	if (irq < 0)
> +		dev_warn(&ofdev->dev, "using polling mode\n");
> +

Re: [PATCH 3/3] i2c: ibm_iic: use devm for main allocation

[Andi Shyti]

Hi Rosen,

On Fri, Aug 21, 2026 at 04:02:16PM -0700, Rosen Penev wrote:
> Allows getting rid of kfree in failure and remove paths.

the patch is good, but as you are going to resend, the comment
for the commit log is still valid.

Thanks,
Andi