[PATCH v1 0/2] drivers: provide devm_platform_request_irq()

[PATCH v1 0/2] drivers: provide devm_platform_request_irq()

[Dejin Zheng]

It will call devm_request_irq() after platform_get_irq() function
in many drivers, sometimes, it is not right of the error handling
for these two functions in some drivers. so provide this function
to simplify the driver.

the first patch will provide devm_platform_request_irq(), and the
other patch will convert to devm_platform_request_irq() in some
i2c bus dirver.

Dejin Zheng (2):
  drivers: provide devm_platform_request_irq()
  i2c: busses: convert to devm_platform_request_irq()

 drivers/base/platform.c            | 33 ++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-bcm-kona.c  | 16 +++------------
 drivers/i2c/busses/i2c-cadence.c   | 10 +++------
 drivers/i2c/busses/i2c-digicolor.c | 10 +++------
 drivers/i2c/busses/i2c-emev2.c     |  5 ++---
 drivers/i2c/busses/i2c-jz4780.c    |  5 ++---
 drivers/i2c/busses/i2c-meson.c     | 13 ++++--------
 drivers/i2c/busses/i2c-mxs.c       |  9 +++-----
 drivers/i2c/busses/i2c-pnx.c       |  9 ++------
 drivers/i2c/busses/i2c-rcar.c      |  9 +++-----
 drivers/i2c/busses/i2c-rk3x.c      | 14 +++----------
 drivers/i2c/busses/i2c-sirf.c      | 10 ++-------
 drivers/i2c/busses/i2c-stu300.c    |  4 ++--
 drivers/i2c/busses/i2c-synquacer.c | 12 +++--------
 include/linux/platform_device.h    |  4 ++++
 15 files changed, 72 insertions(+), 91 deletions(-)

-- 
2.25.0

[PATCH v1 1/2] drivers: provide devm_platform_request_irq()

[Dejin Zheng]

It will call devm_request_irq() after platform_get_irq() function
in many drivers, sometimes, it is not right of the error handling
for these two functions in some drivers. so provide this function
to simplify the driver.

Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
 drivers/base/platform.c         | 33 +++++++++++++++++++++++++++++++++
 include/linux/platform_device.h |  4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 823646ffb978..75c600e286a8 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -275,6 +275,39 @@ int platform_irq_count(struct platform_device *dev)
 }
 EXPORT_SYMBOL_GPL(platform_irq_count);
 
+/**
+ * devm_platform_request_irq - get an irq and allocate an interrupt
+ *				line for a managed device
+ * @pdev: platform device
+ * @num: IRQ number index
+ * @irq: get an IRQ for a device if irq != NULL
+ * @handler: function to be called when the IRQ occurs
+ * @irqflags: interrupt type flags
+ * @devname: an ascii name for the claiming device, dev_name(dev) if NULL
+ * @dev_id: a cookie passed back to the handler function
+ *
+ * Return: zero on success, negative error number on failure.
+ */
+int devm_platform_request_irq(struct platform_device *pdev, unsigned int num,
+		unsigned int *irq, irq_handler_t handler,
+		unsigned long irqflags, const char *devname, void *dev_id)
+{
+	int tmp_irq, ret;
+
+	tmp_irq = platform_get_irq(pdev, num);
+	if (tmp_irq < 0)
+		return tmp_irq;
+
+	ret = devm_request_irq(&pdev->dev, tmp_irq, handler, irqflags,
+				devname, dev_id);
+	if (ret < 0)
+		dev_err(&pdev->dev, "can't request IRQ\n");
+	else if (irq != NULL)
+		*irq = tmp_irq;
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_platform_request_irq);
+
 /**
  * platform_get_resource_byname - get a resource for a device by name
  * @dev: platform device
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 77a2aada106d..d94652deea5c 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -11,6 +11,7 @@
 #define _PLATFORM_DEVICE_H_
 
 #include <linux/device.h>
+#include <linux/interrupt.h>
 
 #define PLATFORM_DEVID_NONE	(-1)
 #define PLATFORM_DEVID_AUTO	(-2)
@@ -70,6 +71,9 @@ devm_platform_ioremap_resource_byname(struct platform_device *pdev,
 extern int platform_get_irq(struct platform_device *, unsigned int);
 extern int platform_get_irq_optional(struct platform_device *, unsigned int);
 extern int platform_irq_count(struct platform_device *);
+extern int devm_platform_request_irq(struct platform_device *pdev,
+		unsigned int num, unsigned int *irq, irq_handler_t handler,
+		unsigned long irqflags, const char *devname, void *dev_id);
 extern struct resource *platform_get_resource_byname(struct platform_device *,
 						     unsigned int,
 						     const char *);
-- 
2.25.0

[PATCH v1 2/2] i2c: busses: convert to devm_platform_request_irq()

[Dejin Zheng]

Use devm_platform_request_irq() to simplify code, and it contains
platform_get_irq() and devm_request_irq().

Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
 drivers/i2c/busses/i2c-bcm-kona.c  | 16 +++-------------
 drivers/i2c/busses/i2c-cadence.c   | 10 +++-------
 drivers/i2c/busses/i2c-digicolor.c | 10 +++-------
 drivers/i2c/busses/i2c-emev2.c     |  5 ++---
 drivers/i2c/busses/i2c-jz4780.c    |  5 ++---
 drivers/i2c/busses/i2c-meson.c     | 13 ++++---------
 drivers/i2c/busses/i2c-mxs.c       |  9 +++------
 drivers/i2c/busses/i2c-pnx.c       |  9 ++-------
 drivers/i2c/busses/i2c-rcar.c      |  9 +++------
 drivers/i2c/busses/i2c-rk3x.c      | 14 +++-----------
 drivers/i2c/busses/i2c-sirf.c      | 10 ++--------
 drivers/i2c/busses/i2c-stu300.c    |  4 ++--
 drivers/i2c/busses/i2c-synquacer.c | 12 +++---------
 13 files changed, 35 insertions(+), 91 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm-kona.c b/drivers/i2c/busses/i2c-bcm-kona.c
index ed5e1275ae46..f45acb47552a 100644
--- a/drivers/i2c/busses/i2c-bcm-kona.c
+++ b/drivers/i2c/busses/i2c-bcm-kona.c
@@ -818,20 +818,10 @@ static int bcm_kona_i2c_probe(struct platform_device *pdev)
 	       ISR_NOACK_MASK,
 	       dev->base + ISR_OFFSET);
 
-	/* Get the interrupt number */
-	dev->irq = platform_get_irq(pdev, 0);
-	if (dev->irq < 0) {
-		rc = dev->irq;
-		goto probe_disable_clk;
-	}
-
-	/* register the ISR handler */
-	rc = devm_request_irq(&pdev->dev, dev->irq, bcm_kona_i2c_isr,
-			      IRQF_SHARED, pdev->name, dev);
-	if (rc) {
-		dev_err(dev->device, "failed to request irq %i\n", dev->irq);
+	rc = devm_platform_request_irq(pdev, 0, &dev->irq, bcm_kona_i2c_isr,
+					IRQF_SHARED, pdev->name, dev);
+	if (rc)
 		goto probe_disable_clk;
-	}
 
 	/* Enable the controller but leave it idle */
 	bcm_kona_i2c_send_cmd_to_ctrl(dev, BCM_CMD_NOACTION);
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 4b72398af505..9ffae4d231dc 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -1204,8 +1204,6 @@ static int cdns_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(id->membase))
 		return PTR_ERR(id->membase);
 
-	id->irq = platform_get_irq(pdev, 0);
-
 	id->adap.owner = THIS_MODULE;
 	id->adap.dev.of_node = pdev->dev.of_node;
 	id->adap.algo = &cdns_i2c_algo;
@@ -1256,12 +1254,10 @@ static int cdns_i2c_probe(struct platform_device *pdev)
 		goto err_clk_dis;
 	}
 
-	ret = devm_request_irq(&pdev->dev, id->irq, cdns_i2c_isr, 0,
-				 DRIVER_NAME, id);
-	if (ret) {
-		dev_err(&pdev->dev, "cannot get irq %d\n", id->irq);
+	ret = devm_platform_request_irq(pdev, 0, &id->irq, cdns_i2c_isr, 0,
+					DRIVER_NAME, id);
+	if (ret)
 		goto err_clk_dis;
-	}
 
 	/*
 	 * Cadence I2C controller has a bug wherein it generates
diff --git a/drivers/i2c/busses/i2c-digicolor.c b/drivers/i2c/busses/i2c-digicolor.c
index 332f00437479..9ea965f41396 100644
--- a/drivers/i2c/busses/i2c-digicolor.c
+++ b/drivers/i2c/busses/i2c-digicolor.c
@@ -290,7 +290,7 @@ static int dc_i2c_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct dc_i2c *i2c;
-	int ret = 0, irq;
+	int ret = 0;
 
 	i2c = devm_kzalloc(&pdev->dev, sizeof(struct dc_i2c), GFP_KERNEL);
 	if (!i2c)
@@ -314,12 +314,8 @@ static int dc_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(i2c->regs))
 		return PTR_ERR(i2c->regs);
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	ret = devm_request_irq(&pdev->dev, irq, dc_i2c_irq, 0,
-			       dev_name(&pdev->dev), i2c);
+	ret = devm_platform_request_irq(pdev, 0, NULL, dc_i2c_irq, 0,
+					dev_name(&pdev->dev), i2c);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/i2c/busses/i2c-emev2.c b/drivers/i2c/busses/i2c-emev2.c
index 1a319352e51b..cae00a9ec86f 100644
--- a/drivers/i2c/busses/i2c-emev2.c
+++ b/drivers/i2c/busses/i2c-emev2.c
@@ -395,9 +395,8 @@ static int em_i2c_probe(struct platform_device *pdev)
 
 	em_i2c_reset(&priv->adap);
 
-	priv->irq = platform_get_irq(pdev, 0);
-	ret = devm_request_irq(&pdev->dev, priv->irq, em_i2c_irq_handler, 0,
-				"em_i2c", priv);
+	ret = devm_platform_request_irq(pdev, 0, &priv->irq,
+			em_i2c_irq_handler, 0, "em_i2c", priv);
 	if (ret)
 		goto err_clk;
 
diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index ba831df6661e..27de0309f211 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -825,9 +825,8 @@ static int jz4780_i2c_probe(struct platform_device *pdev)
 
 	jz4780_i2c_writew(i2c, JZ4780_I2C_INTM, 0x0);
 
-	i2c->irq = platform_get_irq(pdev, 0);
-	ret = devm_request_irq(&pdev->dev, i2c->irq, jz4780_i2c_irq, 0,
-			       dev_name(&pdev->dev), i2c);
+	ret = devm_platform_request_irq(pdev, 0, &i2c->irq, jz4780_i2c_irq, 0,
+					dev_name(&pdev->dev), i2c);
 	if (ret)
 		goto err;
 
diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index c5dec572fc48..2e5a855ff20a 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -398,7 +398,7 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct meson_i2c *i2c;
 	struct i2c_timings timings;
-	int irq, ret = 0;
+	int ret = 0;
 
 	i2c = devm_kzalloc(&pdev->dev, sizeof(struct meson_i2c), GFP_KERNEL);
 	if (!i2c)
@@ -425,15 +425,10 @@ static int meson_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(i2c->regs))
 		return PTR_ERR(i2c->regs);
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	ret = devm_request_irq(&pdev->dev, irq, meson_i2c_irq, 0, NULL, i2c);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "can't request IRQ\n");
+	ret = devm_platform_request_irq(pdev, 0, NULL, meson_i2c_irq,
+					0, NULL, i2c);
+	if (ret < 0)
 		return ret;
-	}
 
 	ret = clk_prepare(i2c->clk);
 	if (ret < 0) {
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 9587347447f0..cff82af3208a 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -802,7 +802,7 @@ static int mxs_i2c_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct mxs_i2c_dev *i2c;
 	struct i2c_adapter *adap;
-	int err, irq;
+	int err;
 
 	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
 	if (!i2c)
@@ -817,11 +817,8 @@ static int mxs_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(i2c->regs))
 		return PTR_ERR(i2c->regs);
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c);
+	err = devm_platform_request_irq(pdev, 0, NULL, mxs_i2c_isr, 0,
+					dev_name(dev), i2c);
 	if (err)
 		return err;
 
diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index 5d7207c10f1d..3e249373375f 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -718,13 +718,8 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	}
 	init_completion(&alg_data->mif.complete);
 
-	alg_data->irq = platform_get_irq(pdev, 0);
-	if (alg_data->irq < 0) {
-		ret = alg_data->irq;
-		goto out_clock;
-	}
-	ret = devm_request_irq(&pdev->dev, alg_data->irq, i2c_pnx_interrupt,
-			       0, pdev->name, alg_data);
+	ret = devm_platform_request_irq(pdev, 0, &alg_data->irq,
+				i2c_pnx_interrupt, 0, pdev->name, alg_data);
 	if (ret)
 		goto out_clock;
 
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index a45c4bf1ec01..bd59a13de707 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -984,13 +984,10 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	else
 		pm_runtime_put(dev);
 
-
-	priv->irq = platform_get_irq(pdev, 0);
-	ret = devm_request_irq(dev, priv->irq, rcar_i2c_irq, 0, dev_name(dev), priv);
-	if (ret < 0) {
-		dev_err(dev, "cannot get irq %d\n", priv->irq);
+	ret = devm_platform_request_irq(pdev, 0, &priv->irq, rcar_i2c_irq, 0,
+					dev_name(dev), priv);
+	if (ret < 0)
 		goto out_pm_disable;
-	}
 
 	platform_set_drvdata(pdev, priv);
 
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index bc698240c4aa..a8d47689de0a 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -1196,7 +1196,6 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
 	int ret = 0;
 	int bus_nr;
 	u32 value;
-	int irq;
 	unsigned long clk_rate;
 
 	i2c = devm_kzalloc(&pdev->dev, sizeof(struct rk3x_i2c), GFP_KERNEL);
@@ -1258,17 +1257,10 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
 		}
 	}
 
-	/* IRQ setup */
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	ret = devm_request_irq(&pdev->dev, irq, rk3x_i2c_irq,
-			       0, dev_name(&pdev->dev), i2c);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "cannot request IRQ\n");
+	ret = devm_platform_request_irq(pdev, 0, NULL, rk3x_i2c_irq,
+					0, dev_name(&pdev->dev), i2c);
+	if (ret < 0)
 		return ret;
-	}
 
 	platform_set_drvdata(pdev, i2c);
 
diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-sirf.c
index d7f72ec331e8..a593c15bfbf5 100644
--- a/drivers/i2c/busses/i2c-sirf.c
+++ b/drivers/i2c/busses/i2c-sirf.c
@@ -274,7 +274,6 @@ static int i2c_sirfsoc_probe(struct platform_device *pdev)
 	struct clk *clk;
 	int bitrate;
 	int ctrl_speed;
-	int irq;
 
 	int err;
 	u32 regval;
@@ -314,13 +313,8 @@ static int i2c_sirfsoc_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		err = irq;
-		goto out;
-	}
-	err = devm_request_irq(&pdev->dev, irq, i2c_sirfsoc_irq, 0,
-		dev_name(&pdev->dev), siic);
+	err = devm_platform_request_irq(pdev, 0, NULL, i2c_sirfsoc_irq, 0,
+					dev_name(&pdev->dev), siic);
 	if (err)
 		goto out;
 
diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
index 64d739baf480..7893c532b8f2 100644
--- a/drivers/i2c/busses/i2c-stu300.c
+++ b/drivers/i2c/busses/i2c-stu300.c
@@ -881,8 +881,8 @@ static int stu300_probe(struct platform_device *pdev)
 	if (IS_ERR(dev->virtbase))
 		return PTR_ERR(dev->virtbase);
 
-	dev->irq = platform_get_irq(pdev, 0);
-	ret = devm_request_irq(&pdev->dev, dev->irq, stu300_irh, 0, NAME, dev);
+	ret = devm_platform_request_irq(pdev, 0, &dev->irq, stu300_irh,
+					0, NAME, dev);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c
index c9a3dba6a75d..d9143373e688 100644
--- a/drivers/i2c/busses/i2c-synquacer.c
+++ b/drivers/i2c/busses/i2c-synquacer.c
@@ -577,16 +577,10 @@ static int synquacer_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(i2c->base))
 		return PTR_ERR(i2c->base);
 
-	i2c->irq = platform_get_irq(pdev, 0);
-	if (i2c->irq < 0)
-		return -ENODEV;
-
-	ret = devm_request_irq(&pdev->dev, i2c->irq, synquacer_i2c_isr,
-			       0, dev_name(&pdev->dev), i2c);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq);
+	ret = devm_platform_request_irq(pdev, 0, &i2c->irq, synquacer_i2c_isr,
+					0, dev_name(&pdev->dev), i2c);
+	if (ret < 0)
 		return ret;
-	}
 
 	i2c->state = STATE_IDLE;
 	i2c->dev = &pdev->dev;
-- 
2.25.0

Re: [PATCH v1 2/2] i2c: busses: convert to devm_platform_request_irq()

[Grygorii Strashko]

On 18/05/2020 18:53, Dejin Zheng wrote:
> Use devm_platform_request_irq() to simplify code, and it contains
> platform_get_irq() and devm_request_irq().
> 
> Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
> ---
>   drivers/i2c/busses/i2c-bcm-kona.c  | 16 +++-------------
>   drivers/i2c/busses/i2c-cadence.c   | 10 +++-------
>   drivers/i2c/busses/i2c-digicolor.c | 10 +++-------
>   drivers/i2c/busses/i2c-emev2.c     |  5 ++---
>   drivers/i2c/busses/i2c-jz4780.c    |  5 ++---
>   drivers/i2c/busses/i2c-meson.c     | 13 ++++---------
>   drivers/i2c/busses/i2c-mxs.c       |  9 +++------
>   drivers/i2c/busses/i2c-pnx.c       |  9 ++-------
>   drivers/i2c/busses/i2c-rcar.c      |  9 +++------
>   drivers/i2c/busses/i2c-rk3x.c      | 14 +++-----------
>   drivers/i2c/busses/i2c-sirf.c      | 10 ++--------
>   drivers/i2c/busses/i2c-stu300.c    |  4 ++--
>   drivers/i2c/busses/i2c-synquacer.c | 12 +++---------
>   13 files changed, 35 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bcm-kona.c b/drivers/i2c/busses/i2c-bcm-kona.c
> index ed5e1275ae46..f45acb47552a 100644
> --- a/drivers/i2c/busses/i2c-bcm-kona.c
> +++ b/drivers/i2c/busses/i2c-bcm-kona.c
> @@ -818,20 +818,10 @@ static int bcm_kona_i2c_probe(struct platform_device *pdev)
>   	       ISR_NOACK_MASK,
>   	       dev->base + ISR_OFFSET);
>   
> -	/* Get the interrupt number */
> -	dev->irq = platform_get_irq(pdev, 0);
> -	if (dev->irq < 0) {
> -		rc = dev->irq;
> -		goto probe_disable_clk;
> -	}
> -
> -	/* register the ISR handler */
> -	rc = devm_request_irq(&pdev->dev, dev->irq, bcm_kona_i2c_isr,
> -			      IRQF_SHARED, pdev->name, dev);
> -	if (rc) {
> -		dev_err(dev->device, "failed to request irq %i\n", dev->irq);
> +	rc = devm_platform_request_irq(pdev, 0, &dev->irq, bcm_kona_i2c_isr,
> +					IRQF_SHARED, pdev->name, dev);
> +	if (rc)
>   		goto probe_disable_clk;
> -	}
>   
>   	/* Enable the controller but leave it idle */
>   	bcm_kona_i2c_send_cmd_to_ctrl(dev, BCM_CMD_NOACTION);
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 4b72398af505..9ffae4d231dc 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -1204,8 +1204,6 @@ static int cdns_i2c_probe(struct platform_device *pdev)
>   	if (IS_ERR(id->membase))
>   		return PTR_ERR(id->membase);
>   
> -	id->irq = platform_get_irq(pdev, 0);
> -


In many cases It is two strictly different steps
1) Request resource, including IRQ mapping and differed probe handling.
    It should be done as early as possible to avoid unnecessary initialization steps
    when resource (irq) is not ready,  and so avoid boot time increasing.
2) Actually request and enable IRQ, which, in many case, should be done late in probe
    when driver and HW are actually ready to handle IRQs.

here, for example, between this point

>   	id->adap.owner = THIS_MODULE;
>   	id->adap.dev.of_node = pdev->dev.of_node;
>   	id->adap.algo = &cdns_i2c_algo;
> @@ -1256,12 +1254,10 @@ static int cdns_i2c_probe(struct platform_device *pdev)
>   		goto err_clk_dis;
>   	}
>   
> -	ret = devm_request_irq(&pdev->dev, id->irq, cdns_i2c_isr, 0,
> -				 DRIVER_NAME, id);
> -	if (ret) {
> -		dev_err(&pdev->dev, "cannot get irq %d\n", id->irq);

and this point the following happens:
  - devm_clk_get() can fail and cause probe defer
  - clk_prepare_enable()
  - pm_runtime_.. - pm init
  - cdns_i2c_.. - hw int

> +	ret = devm_platform_request_irq(pdev, 0, &id->irq, cdns_i2c_isr, 0,
> +					DRIVER_NAME, id);

and now platform_get_irq(), which can fail due to IRQ controller not ready,
and all above has to be reverted.

> +	if (ret)
>   		goto err_clk_dis;
> -	}
[...]

A bit risky optimization, especially for bulk changes.

-- 
Best regards,
grygorii

Re: [PATCH v1 2/2] i2c: busses: convert to devm_platform_request_irq()

[Dejin Zheng]

On Mon, May 18, 2020 at 08:34:01PM +0300, Grygorii Strashko wrote:
> 
> 
> On 18/05/2020 18:53, Dejin Zheng wrote:
> > Use devm_platform_request_irq() to simplify code, and it contains
> > platform_get_irq() and devm_request_irq().
> > 
> > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
> > ---
> >   drivers/i2c/busses/i2c-bcm-kona.c  | 16 +++-------------
> >   drivers/i2c/busses/i2c-cadence.c   | 10 +++-------
> >   drivers/i2c/busses/i2c-digicolor.c | 10 +++-------
> >   drivers/i2c/busses/i2c-emev2.c     |  5 ++---
> >   drivers/i2c/busses/i2c-jz4780.c    |  5 ++---
> >   drivers/i2c/busses/i2c-meson.c     | 13 ++++---------
> >   drivers/i2c/busses/i2c-mxs.c       |  9 +++------
> >   drivers/i2c/busses/i2c-pnx.c       |  9 ++-------
> >   drivers/i2c/busses/i2c-rcar.c      |  9 +++------
> >   drivers/i2c/busses/i2c-rk3x.c      | 14 +++-----------
> >   drivers/i2c/busses/i2c-sirf.c      | 10 ++--------
> >   drivers/i2c/busses/i2c-stu300.c    |  4 ++--
> >   drivers/i2c/busses/i2c-synquacer.c | 12 +++---------
> >   13 files changed, 35 insertions(+), 91 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-bcm-kona.c b/drivers/i2c/busses/i2c-bcm-kona.c
> > index ed5e1275ae46..f45acb47552a 100644
> > --- a/drivers/i2c/busses/i2c-bcm-kona.c
> > +++ b/drivers/i2c/busses/i2c-bcm-kona.c
> > @@ -818,20 +818,10 @@ static int bcm_kona_i2c_probe(struct platform_device *pdev)
> >   	       ISR_NOACK_MASK,
> >   	       dev->base + ISR_OFFSET);
> > -	/* Get the interrupt number */
> > -	dev->irq = platform_get_irq(pdev, 0);
> > -	if (dev->irq < 0) {
> > -		rc = dev->irq;
> > -		goto probe_disable_clk;
> > -	}
> > -
> > -	/* register the ISR handler */
> > -	rc = devm_request_irq(&pdev->dev, dev->irq, bcm_kona_i2c_isr,
> > -			      IRQF_SHARED, pdev->name, dev);
> > -	if (rc) {
> > -		dev_err(dev->device, "failed to request irq %i\n", dev->irq);
> > +	rc = devm_platform_request_irq(pdev, 0, &dev->irq, bcm_kona_i2c_isr,
> > +					IRQF_SHARED, pdev->name, dev);
> > +	if (rc)
> >   		goto probe_disable_clk;
> > -	}
> >   	/* Enable the controller but leave it idle */
> >   	bcm_kona_i2c_send_cmd_to_ctrl(dev, BCM_CMD_NOACTION);
> > diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> > index 4b72398af505..9ffae4d231dc 100644
> > --- a/drivers/i2c/busses/i2c-cadence.c
> > +++ b/drivers/i2c/busses/i2c-cadence.c
> > @@ -1204,8 +1204,6 @@ static int cdns_i2c_probe(struct platform_device *pdev)
> >   	if (IS_ERR(id->membase))
> >   		return PTR_ERR(id->membase);
> > -	id->irq = platform_get_irq(pdev, 0);
> > -
> 
> 
> In many cases It is two strictly different steps
> 1) Request resource, including IRQ mapping and differed probe handling.
>    It should be done as early as possible to avoid unnecessary initialization steps
>    when resource (irq) is not ready,  and so avoid boot time increasing.
> 2) Actually request and enable IRQ, which, in many case, should be done late in probe
>    when driver and HW are actually ready to handle IRQs.
> 
> here, for example, between this point
> 
> >   	id->adap.owner = THIS_MODULE;
> >   	id->adap.dev.of_node = pdev->dev.of_node;
> >   	id->adap.algo = &cdns_i2c_algo;
> > @@ -1256,12 +1254,10 @@ static int cdns_i2c_probe(struct platform_device *pdev)
> >   		goto err_clk_dis;
> >   	}
> > -	ret = devm_request_irq(&pdev->dev, id->irq, cdns_i2c_isr, 0,
> > -				 DRIVER_NAME, id);
> > -	if (ret) {
> > -		dev_err(&pdev->dev, "cannot get irq %d\n", id->irq);
> 
> and this point the following happens:
>  - devm_clk_get() can fail and cause probe defer
>  - clk_prepare_enable()
>  - pm_runtime_.. - pm init
>  - cdns_i2c_.. - hw int
> 
> > +	ret = devm_platform_request_irq(pdev, 0, &id->irq, cdns_i2c_isr, 0,
> > +					DRIVER_NAME, id);
> 
> and now platform_get_irq(), which can fail due to IRQ controller not ready,
> and all above has to be reverted.
> 
> > +	if (ret)
> >   		goto err_clk_dis;
> > -	}
> [...]
> 
> A bit risky optimization, especially for bulk changes.
>
Grygorii, Thanks for your comments, you are right, abandon these two patches.

BTW, The gmail will prevent me sending messages to many recipients, so
remove some recipients. and still sending to i2c and kernel mail list.

BR,
Dejin

> -- 
> Best regards,
> grygorii