LinuxLists.cc - [PATCH 0/7] mfd: Use devm_mfd_add_devices and devm_regmap_add_irq

[permalink] [raw]

Subject: [PATCH 1/7] mfd: as3722: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

Use devm_mfd_add_devices() for adding MFD child devices and
devm_regmap_add_irq_chip() for IRQ chip registration.

This reduces the error code path and .remove callback for removing
MFD child devices and deleting IRQ chip data.

Signed-off-by: Laxman Dewangan <[email protected]>
---
drivers/mfd/as3722.c | 31 +++++++++----------------------
1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
index e1f597f..f87342c 100644
--- a/drivers/mfd/as3722.c
+++ b/drivers/mfd/as3722.c
@@ -385,9 +385,10 @@ static int as3722_i2c_probe(struct i2c_client *i2c,
return ret;

irq_flags = as3722->irq_flags | IRQF_ONESHOT;
- ret = regmap_add_irq_chip(as3722->regmap, as3722->chip_irq,
- irq_flags, -1, &as3722_irq_chip,
- &as3722->irq_data);
+ ret = devm_regmap_add_irq_chip(as3722->dev, as3722->regmap,
+ as3722->chip_irq,
+ irq_flags, -1, &as3722_irq_chip,
+ &as3722->irq_data);
if (ret < 0) {
dev_err(as3722->dev, "Failed to add regmap irq: %d\n", ret);
return ret;
@@ -395,33 +396,20 @@ static int as3722_i2c_probe(struct i2c_client *i2c,

ret = as3722_configure_pullups(as3722);
if (ret < 0)
- goto scrub;
+ return ret;

- ret = mfd_add_devices(&i2c->dev, -1, as3722_devs,
- ARRAY_SIZE(as3722_devs), NULL, 0,
- regmap_irq_get_domain(as3722->irq_data));
+ ret = devm_mfd_add_devices(&i2c->dev, -1, as3722_devs,
+ ARRAY_SIZE(as3722_devs), NULL, 0,
+ regmap_irq_get_domain(as3722->irq_data));
if (ret) {
dev_err(as3722->dev, "Failed to add MFD devices: %d\n", ret);
- goto scrub;
+ return ret;
}

device_init_wakeup(as3722->dev, true);

dev_dbg(as3722->dev, "AS3722 core driver initialized successfully\n");
return 0;
-
-scrub:
- regmap_del_irq_chip(as3722->chip_irq, as3722->irq_data);
- return ret;
-}
-
-static int as3722_i2c_remove(struct i2c_client *i2c)
-{
- struct as3722 *as3722 = i2c_get_clientdata(i2c);
-
- mfd_remove_devices(as3722->dev);
- regmap_del_irq_chip(as3722->chip_irq, as3722->irq_data);
- return 0;
}

static int __maybe_unused as3722_i2c_suspend(struct device *dev)
@@ -470,7 +458,6 @@ static struct i2c_driver as3722_i2c_driver = {
.pm = &as3722_pm_ops,
},
.probe = as3722_i2c_probe,
- .remove = as3722_i2c_remove,
.id_table = as3722_i2c_id,
};

--
2.1.4

2016-04-21 12:38:12

[permalink] [raw]

Subject: [PATCH 2/7] mfd: lp8788: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

Use devm_mfd_add_devices() for adding MFD child devices and
devm_regmap_add_irq_chip() for IRQ chip registration.

This reduces the error code path and .remove callback for removing
MFD child devices and deleting IRQ chip data.

Signed-off-by: Laxman Dewangan <[email protected]>
CC: Milo Kim <[email protected]>
---
drivers/mfd/lp8788-irq.c | 12 +++---------
drivers/mfd/lp8788.c | 10 ----------
2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/lp8788-irq.c b/drivers/mfd/lp8788-irq.c
index 792d51b..aacd39f 100644
--- a/drivers/mfd/lp8788-irq.c
+++ b/drivers/mfd/lp8788-irq.c
@@ -175,9 +175,9 @@ int lp8788_irq_init(struct lp8788 *lp, int irq)
lp->irqdm = irqd->domain;
mutex_init(&irqd->irq_lock);

- ret = request_threaded_irq(irq, NULL, lp8788_irq_handler,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- "lp8788-irq", irqd);
+ ret = devm_request_threaded_irq(lp->dev, irq, NULL, lp8788_irq_handler,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "lp8788-irq", irqd);
if (ret) {
dev_err(lp->dev, "failed to create a thread for IRQ_N\n");
return ret;
@@ -187,9 +187,3 @@ int lp8788_irq_init(struct lp8788 *lp, int irq)

return 0;
}
-
-void lp8788_irq_exit(struct lp8788 *lp)
-{
- if (lp->irq)
- free_irq(lp->irq, lp->irqdm);
-}
diff --git a/drivers/mfd/lp8788.c b/drivers/mfd/lp8788.c
index acf6165..37fea46 100644
--- a/drivers/mfd/lp8788.c
+++ b/drivers/mfd/lp8788.c
@@ -203,15 +203,6 @@ static int lp8788_probe(struct i2c_client *cl, const struct i2c_device_id *id)
ARRAY_SIZE(lp8788_devs), NULL, 0, NULL);
}

-static int lp8788_remove(struct i2c_client *cl)
-{
- struct lp8788 *lp = i2c_get_clientdata(cl);
-
- mfd_remove_devices(lp->dev);
- lp8788_irq_exit(lp);
- return 0;
-}
-
static const struct i2c_device_id lp8788_ids[] = {
{"lp8788", 0},
{ }
@@ -223,7 +214,6 @@ static struct i2c_driver lp8788_driver = {
.name = "lp8788",
},
.probe = lp8788_probe,
- .remove = lp8788_remove,
.id_table = lp8788_ids,
};

--
2.1.4

2016-04-21 12:38:16

[permalink] [raw]

Subject: [PATCH 3/7] mfd: max77686: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

Use devm_mfd_add_devices() for adding MFD child devices and
devm_regmap_add_irq_chip() for IRQ chip registration.

This reduces the error code path and .remove callback for removing
MFD child devices and deleting IRQ chip data.

Signed-off-by: Laxman Dewangan <[email protected]>
CC: Chanwoo Choi <[email protected]>
CC: Krzysztof Kozlowski <[email protected]>
---
drivers/mfd/max77686.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 7a0457e..7b68ed7 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -230,38 +230,24 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
return -ENODEV;
}

- ret = regmap_add_irq_chip(max77686->regmap, max77686->irq,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
- IRQF_SHARED, 0, irq_chip,
- &max77686->irq_data);
+ ret = devm_regmap_add_irq_chip(&i2c->dev, max77686->regmap,
+ max77686->irq,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
+ IRQF_SHARED, 0, irq_chip,
+ &max77686->irq_data);
if (ret < 0) {
dev_err(&i2c->dev, "failed to add PMIC irq chip: %d\n", ret);
return ret;
}

- ret = mfd_add_devices(max77686->dev, -1, cells, n_devs, NULL, 0, NULL);
+ ret = devm_mfd_add_devices(max77686->dev, -1, cells, n_devs, NULL,
+ 0, NULL);
if (ret < 0) {
dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
- goto err_del_irqc;
+ return ret;
}

return 0;
-
-err_del_irqc:
- regmap_del_irq_chip(max77686->irq, max77686->irq_data);
-
- return ret;
-}
-
-static int max77686_i2c_remove(struct i2c_client *i2c)
-{
- struct max77686_dev *max77686 = i2c_get_clientdata(i2c);
-
- mfd_remove_devices(max77686->dev);
-
- regmap_del_irq_chip(max77686->irq, max77686->irq_data);
-
- return 0;
}

static const struct i2c_device_id max77686_i2c_id[] = {
@@ -317,7 +303,6 @@ static struct i2c_driver max77686_i2c_driver = {
.of_match_table = of_match_ptr(max77686_pmic_dt_match),
},
.probe = max77686_i2c_probe,
- .remove = max77686_i2c_remove,
.id_table = max77686_i2c_id,
};

--
2.1.4

2016-04-21 12:38:20

[permalink] [raw]

Subject: [PATCH 4/7] mfd: rc5t583: Use devm_mfd_add_devices and devm_request_threaded_irq

Use devm_mfd_add_devices() for adding MFD child devices and
devm_request_threaded_irq() for IRQ registration.

This reduces the need of remove callback for removing MFD child
devices and unregistering IRQ.

Signed-off-by: Laxman Dewangan <[email protected]>
---
drivers/mfd/rc5t583-irq.c | 11 ++---------
drivers/mfd/rc5t583.c | 24 +++---------------------
2 files changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/mfd/rc5t583-irq.c b/drivers/mfd/rc5t583-irq.c
index 3f8812d..f8dde59 100644
--- a/drivers/mfd/rc5t583-irq.c
+++ b/drivers/mfd/rc5t583-irq.c
@@ -389,17 +389,10 @@ int rc5t583_irq_init(struct rc5t583 *rc5t583, int irq, int irq_base)
irq_clear_status_flags(__irq, IRQ_NOREQUEST);
}

- ret = request_threaded_irq(irq, NULL, rc5t583_irq, IRQF_ONESHOT,
- "rc5t583", rc5t583);
+ ret = devm_request_threaded_irq(rc5t583->dev, irq, NULL, rc5t583_irq,
+ IRQF_ONESHOT, "rc5t583", rc5t583);
if (ret < 0)
dev_err(rc5t583->dev,
"Error in registering interrupt error: %d\n", ret);
return ret;
}
-
-int rc5t583_irq_exit(struct rc5t583 *rc5t583)
-{
- if (rc5t583->chip_irq)
- free_irq(rc5t583->chip_irq, rc5t583);
- return 0;
-}
diff --git a/drivers/mfd/rc5t583.c b/drivers/mfd/rc5t583.c
index fc2b2d9..d12243d 100644
--- a/drivers/mfd/rc5t583.c
+++ b/drivers/mfd/rc5t583.c
@@ -252,7 +252,6 @@ static int rc5t583_i2c_probe(struct i2c_client *i2c,
struct rc5t583 *rc5t583;
struct rc5t583_platform_data *pdata = dev_get_platdata(&i2c->dev);
int ret;
- bool irq_init_success = false;

if (!pdata) {
dev_err(&i2c->dev, "Err: Platform data not found\n");
@@ -284,32 +283,16 @@ static int rc5t583_i2c_probe(struct i2c_client *i2c,
/* Still continue with warning, if irq init fails */
if (ret)
dev_warn(&i2c->dev, "IRQ init failed: %d\n", ret);
- else
- irq_init_success = true;
}

- ret = mfd_add_devices(rc5t583->dev, -1, rc5t583_subdevs,
- ARRAY_SIZE(rc5t583_subdevs), NULL, 0, NULL);
+ ret = devm_mfd_add_devices(rc5t583->dev, -1, rc5t583_subdevs,
+ ARRAY_SIZE(rc5t583_subdevs), NULL, 0, NULL);
if (ret) {
dev_err(&i2c->dev, "add mfd devices failed: %d\n", ret);
- goto err_add_devs;
+ return ret;
}

return 0;
-
-err_add_devs:
- if (irq_init_success)
- rc5t583_irq_exit(rc5t583);
- return ret;
-}
-
-static int rc5t583_i2c_remove(struct i2c_client *i2c)
-{
- struct rc5t583 *rc5t583 = i2c_get_clientdata(i2c);
-
- mfd_remove_devices(rc5t583->dev);
- rc5t583_irq_exit(rc5t583);
- return 0;
}

static const struct i2c_device_id rc5t583_i2c_id[] = {
@@ -324,7 +307,6 @@ static struct i2c_driver rc5t583_i2c_driver = {
.name = "rc5t583",
},
.probe = rc5t583_i2c_probe,
- .remove = rc5t583_i2c_remove,
.id_table = rc5t583_i2c_id,
};

--
2.1.4

2016-04-21 12:38:33

[permalink] [raw]

Subject: [PATCH 6/7] mfd: tps65910: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

Use devm_mfd_add_devices() for adding MFD child devices and
devm_regmap_add_irq_chip() for IRQ chip registration.

This reduces the error code path and .remove callback for removing
MFD child devices and deleting IRQ chip data.

Signed-off-by: Laxman Dewangan <[email protected]>
CC: Tony Lindgren <[email protected]>
CC: [email protected]
---
drivers/mfd/tps65910.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 8086e5d..11cab15 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -252,9 +252,10 @@ static int tps65910_irq_init(struct tps65910 *tps65910, int irq,
}

tps65910->chip_irq = irq;
- ret = regmap_add_irq_chip(tps65910->regmap, tps65910->chip_irq,
- IRQF_ONESHOT, pdata->irq_base,
- tps6591x_irqs_chip, &tps65910->irq_data);
+ ret = devm_regmap_add_irq_chip(tps65910->dev, tps65910->regmap,
+ tps65910->chip_irq,
+ IRQF_ONESHOT, pdata->irq_base,
+ tps6591x_irqs_chip, &tps65910->irq_data);
if (ret < 0) {
dev_warn(tps65910->dev, "Failed to add irq_chip %d\n", ret);
tps65910->chip_irq = 0;
@@ -262,13 +263,6 @@ static int tps65910_irq_init(struct tps65910 *tps65910, int irq,
return ret;
}

-static int tps65910_irq_exit(struct tps65910 *tps65910)
-{
- if (tps65910->chip_irq > 0)
- regmap_del_irq_chip(tps65910->chip_irq, tps65910->irq_data);
- return 0;
-}
-
static bool is_volatile_reg(struct device *dev, unsigned int reg)
{
struct tps65910 *tps65910 = dev_get_drvdata(dev);
@@ -516,22 +510,12 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
regmap_irq_get_domain(tps65910->irq_data));
if (ret < 0) {
dev_err(&i2c->dev, "mfd_add_devices failed: %d\n", ret);
- tps65910_irq_exit(tps65910);
return ret;
}

return ret;
}

-static int tps65910_i2c_remove(struct i2c_client *i2c)
-{
- struct tps65910 *tps65910 = i2c_get_clientdata(i2c);
-
- tps65910_irq_exit(tps65910);
-
- return 0;
-}
-
static const struct i2c_device_id tps65910_i2c_id[] = {
{ "tps65910", TPS65910 },
{ "tps65911", TPS65911 },
@@ -546,7 +530,6 @@ static struct i2c_driver tps65910_i2c_driver = {
.of_match_table = of_match_ptr(tps65910_of_match),
},
.probe = tps65910_i2c_probe,
- .remove = tps65910_i2c_remove,
.id_table = tps65910_i2c_id,
};

--
2.1.4

2016-04-21 12:38:25

[permalink] [raw]

Subject: [PATCH 5/7] mfd: sec: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

Use devm_mfd_add_devices() for adding MFD child devices and
devm_regmap_add_irq_chip() for IRQ chip registration.

This reduces the error code path and .remove callback for removing
MFD child devices and deleting IRQ chip data.

Signed-off-by: Laxman Dewangan <[email protected]>
CC: Sangbeom Kim <[email protected]>
CC: Krzysztof Kozlowski <[email protected]>
CC: [email protected]
---
drivers/mfd/sec-core.c | 20 +++-----------------
drivers/mfd/sec-irq.c | 14 +++++---------
2 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 400e1d7..ca6b80d 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -481,29 +481,16 @@ static int sec_pmic_probe(struct i2c_client *i2c,
/* If this happens the probe function is problem */
BUG();
}
- ret = mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs, NULL,
- 0, NULL);
+ ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
+ NULL, 0, NULL);
if (ret)
- goto err_mfd;
+ return ret;

device_init_wakeup(sec_pmic->dev, sec_pmic->wakeup);
sec_pmic_configure(sec_pmic);
sec_pmic_dump_rev(sec_pmic);

return ret;
-
-err_mfd:
- sec_irq_exit(sec_pmic);
- return ret;
-}
-
-static int sec_pmic_remove(struct i2c_client *i2c)
-{
- struct sec_pmic_dev *sec_pmic = i2c_get_clientdata(i2c);
-
- mfd_remove_devices(sec_pmic->dev);
- sec_irq_exit(sec_pmic);
- return 0;
}

static void sec_pmic_shutdown(struct i2c_client *i2c)
@@ -583,7 +570,6 @@ static struct i2c_driver sec_pmic_driver = {
.of_match_table = of_match_ptr(sec_dt_match),
},
.probe = sec_pmic_probe,
- .remove = sec_pmic_remove,
.shutdown = sec_pmic_shutdown,
.id_table = sec_pmic_id,
};
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index d77de43..5eb59c233d5 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -483,10 +483,11 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
return -EINVAL;
}

- ret = regmap_add_irq_chip(sec_pmic->regmap_pmic, sec_pmic->irq,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- sec_pmic->irq_base, sec_irq_chip,
- &sec_pmic->irq_data);
+ ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic,
+ sec_pmic->irq,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ sec_pmic->irq_base, sec_irq_chip,
+ &sec_pmic->irq_data);
if (ret != 0) {
dev_err(sec_pmic->dev, "Failed to register IRQ chip: %d\n", ret);
return ret;
@@ -500,8 +501,3 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)

return 0;
}
-
-void sec_irq_exit(struct sec_pmic_dev *sec_pmic)
-{
- regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
-}
--
2.1.4

2016-04-21 12:38:46

[permalink] [raw]

Subject: [PATCH 7/7] mfd: wl1273-core: Use devm_mfd_add_devices() for mfd_device registration

Use devm_mfd_add_devices() for MFD devices registration and get
rid of .remove callback to remove MFD child-devices. This is done
by managed device framework.

Signed-off-by: Laxman Dewangan <[email protected]>
---
drivers/mfd/wl1273-core.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/mfd/wl1273-core.c b/drivers/mfd/wl1273-core.c
index f7c52d9..7080465 100644
--- a/drivers/mfd/wl1273-core.c
+++ b/drivers/mfd/wl1273-core.c
@@ -170,15 +170,6 @@ static int wl1273_fm_set_volume(struct wl1273_core *core, unsigned int volume)
return 0;
}

-static int wl1273_core_remove(struct i2c_client *client)
-{
- dev_dbg(&client->dev, "%s\n", __func__);
-
- mfd_remove_devices(&client->dev);
-
- return 0;
-}
-
static int wl1273_core_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -237,8 +228,8 @@ static int wl1273_core_probe(struct i2c_client *client,
dev_dbg(&client->dev, "%s: number of children: %d.\n",
__func__, children);

- r = mfd_add_devices(&client->dev, -1, core->cells,
- children, NULL, 0, NULL);
+ r = devm_mfd_add_devices(&client->dev, -1, core->cells,
+ children, NULL, 0, NULL);
if (r)
goto err;

@@ -258,7 +249,6 @@ static struct i2c_driver wl1273_core_driver = {
},
.probe = wl1273_core_probe,
.id_table = wl1273_driver_id_table,
- .remove = wl1273_core_remove,
};

static int __init wl1273_core_init(void)
--
2.1.4

2016-04-21 23:15:51

by Kim, Milo

[permalink] [raw]

Subject: Re: [PATCH 2/7] mfd: lp8788: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

Hi Laxman,

On 4/21/2016 9:25 PM, Laxman Dewangan wrote:
> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_regmap_add_irq_chip() for IRQ chip registration.

This patch doesn't include the code regarding devm_mfd_add_devices().
Could you check it again? Or am I missing any previous patches?

>
> This reduces the error code path and .remove callback for removing
> MFD child devices and deleting IRQ chip data.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> CC: Milo Kim <[email protected]>
> ---
> drivers/mfd/lp8788-irq.c | 12 +++---------
> drivers/mfd/lp8788.c | 10 ----------
> 2 files changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mfd/lp8788-irq.c b/drivers/mfd/lp8788-irq.c
> index 792d51b..aacd39f 100644
> --- a/drivers/mfd/lp8788-irq.c
> +++ b/drivers/mfd/lp8788-irq.c
> @@ -175,9 +175,9 @@ int lp8788_irq_init(struct lp8788 *lp, int irq)
> lp->irqdm = irqd->domain;
> mutex_init(&irqd->irq_lock);
>
> - ret = request_threaded_irq(irq, NULL, lp8788_irq_handler,
> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> - "lp8788-irq", irqd);
> + ret = devm_request_threaded_irq(lp->dev, irq, NULL, lp8788_irq_handler,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> + "lp8788-irq", irqd);
> if (ret) {
> dev_err(lp->dev, "failed to create a thread for IRQ_N\n");
> return ret;
> @@ -187,9 +187,3 @@ int lp8788_irq_init(struct lp8788 *lp, int irq)
>
> return 0;
> }
> -
> -void lp8788_irq_exit(struct lp8788 *lp)
> -{
> - if (lp->irq)
> - free_irq(lp->irq, lp->irqdm);
> -}
> diff --git a/drivers/mfd/lp8788.c b/drivers/mfd/lp8788.c
> index acf6165..37fea46 100644
> --- a/drivers/mfd/lp8788.c
> +++ b/drivers/mfd/lp8788.c
> @@ -203,15 +203,6 @@ static int lp8788_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> ARRAY_SIZE(lp8788_devs), NULL, 0, NULL);
> }
>
> -static int lp8788_remove(struct i2c_client *cl)
> -{
> - struct lp8788 *lp = i2c_get_clientdata(cl);
> -
> - mfd_remove_devices(lp->dev);
> - lp8788_irq_exit(lp);
> - return 0;
> -}
> -
> static const struct i2c_device_id lp8788_ids[] = {
> {"lp8788", 0},
> { }
> @@ -223,7 +214,6 @@ static struct i2c_driver lp8788_driver = {
> .name = "lp8788",
> },
> .probe = lp8788_probe,
> - .remove = lp8788_remove,
> .id_table = lp8788_ids,
> };
>
>

Best regards,
Milo

2016-04-22 09:02:21

[permalink] [raw]

Subject: Re: [PATCH 2/7] mfd: lp8788: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Friday 22 April 2016 04:45 AM, Kim, Milo wrote:
> Hi Laxman,
>
> On 4/21/2016 9:25 PM, Laxman Dewangan wrote:
>> Use devm_mfd_add_devices() for adding MFD child devices and
>> devm_regmap_add_irq_chip() for IRQ chip registration.
>
> This patch doesn't include the code regarding devm_mfd_add_devices().
> Could you check it again? Or am I missing any previous patches?

Sigh.. yaah, it is missed. Dont know how I missed it in my grep result.
I will recycle the patch. But I like to get review other patches also
for V2. External code review is more stronger then self code review.

2016-04-25 10:56:32

[permalink] [raw]

Subject: Re: [PATCH 0/7] mfd: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On 04/21/2016 02:25 PM, Laxman Dewangan wrote:
> This series is an effort to reduce the code in error path and need of
> remove callback by using the devm_mfd_add_devices() and
> devm_regmap_add_irq_chip() or devm_request_threaded_irq APIs for interrupt
> support.
>
> This is tested with build and comiplation only with arm64 defconfig and enabling
> the required MFD CONFIGs.

Please compile it also on ARMv7. The changes look trivial but one
compilation without testing might not be sufficient.

Best regards,
Krzysztof

2016-04-25 10:57:08

[permalink] [raw]

Subject: Re: [PATCH 3/7] mfd: max77686: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On 04/21/2016 02:25 PM, Laxman Dewangan wrote:
> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_regmap_add_irq_chip() for IRQ chip registration.
>
> This reduces the error code path and .remove callback for removing
> MFD child devices and deleting IRQ chip data.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> CC: Chanwoo Choi <[email protected]>
> CC: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/mfd/max77686.c | 31 ++++++++-----------------------
> 1 file changed, 8 insertions(+), 23 deletions(-)

Switching existing code to devm-like interface doesn't bring huge
benefits but looks okay and I'm fine with it:

Tested-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2016-04-25 12:45:14

[permalink] [raw]

Subject: Re: [PATCH 5/7] mfd: sec: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On 04/21/2016 02:25 PM, Laxman Dewangan wrote:
> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_regmap_add_irq_chip() for IRQ chip registration.
>
> This reduces the error code path and .remove callback for removing
> MFD child devices and deleting IRQ chip data.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> CC: Sangbeom Kim <[email protected]>
> CC: Krzysztof Kozlowski <[email protected]>
> CC: [email protected]
> ---
> drivers/mfd/sec-core.c | 20 +++-----------------
> drivers/mfd/sec-irq.c | 14 +++++---------
> 2 files changed, 8 insertions(+), 26 deletions(-)

Tested-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2016-04-25 13:04:16

[permalink] [raw]

Subject: Re: [PATCH 0/7] mfd: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Monday 25 April 2016 04:26 PM, Krzysztof Kozlowski wrote:
> On 04/21/2016 02:25 PM, Laxman Dewangan wrote:
>> This series is an effort to reduce the code in error path and need of
>> remove callback by using the devm_mfd_add_devices() and
>> devm_regmap_add_irq_chip() or devm_request_threaded_irq APIs for interrupt
>> support.
>>
>> This is tested with build and comiplation only with arm64 defconfig and enabling
>> the required MFD CONFIGs.
> Please compile it also on ARMv7. The changes look trivial but one
> compilation without testing might not be sufficient.
Sure. Thank you very much for testing.

I did build test with the ARMv7 for all patches and testing on actual
platform with Jetson-TX1 platform + max77620 which have similar change
with bind/unbind. This is what I learn from you for testing the remove
callback/unbinding without making =m on RTC patches and this really
helps me on my lots of testing.

2016-04-28 10:14:42

[permalink] [raw]

Subject: Re: [PATCH 3/7] mfd: max77686: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Thursday 28 April 2016 02:31 PM, Lee Jones wrote:
> On Mon, 25 Apr 2016, Krzysztof Kozlowski wrote:
>
>> On 04/21/2016 02:25 PM, Laxman Dewangan wrote:
>>> Use devm_mfd_add_devices() for adding MFD child devices and
>>> devm_regmap_add_irq_chip() for IRQ chip registration.
>>>
>>> This reduces the error code path and .remove callback for removing
>>> MFD child devices and deleting IRQ chip data.
>>>
>>> Signed-off-by: Laxman Dewangan <[email protected]>
>>> CC: Chanwoo Choi <[email protected]>
>>> CC: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> drivers/mfd/max77686.c | 31 ++++++++-----------------------
>>> 1 file changed, 8 insertions(+), 23 deletions(-)
>> Switching existing code to devm-like interface doesn't bring huge
>> benefits but looks okay and I'm fine with it:
> This is pretty much my view, but it get's Laxman's patch count up. ;)

Yaah. :-)

There is some other motivation of doing this:
* I got the review comment about the resource leak and sequencing in my
max77620. It was silly mistake done by me and it causes recycle of
patch. To avoid this in future, devm_ was better option.
* Spent lots of time on unbinding test during my RTC patch. Although fix
was not related to the devm_ but gave the impression that something we
are doing on probe. devm_ looks straight forward.
- Some of code quality tools suggest to avoid goto statement. Only
possible if we dont have any code in error path i.e. return from any place.
- If we have devm_ apis for few resource and some does not support then
difficult to use them as this affect the sequence of deallocation.
Existing devm_ can be used effectively only if we have all resource
allocation using devm_.
- Reducing code size always better.

2016-04-28 10:39:46

[permalink] [raw]

Subject: Re: [PATCH 3/7] mfd: max77686: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On 04/28/2016 12:02 PM, Laxman Dewangan wrote:
>
> On Thursday 28 April 2016 02:31 PM, Lee Jones wrote:
>> On Mon, 25 Apr 2016, Krzysztof Kozlowski wrote:
>>
>>> On 04/21/2016 02:25 PM, Laxman Dewangan wrote:
>>>> Use devm_mfd_add_devices() for adding MFD child devices and
>>>> devm_regmap_add_irq_chip() for IRQ chip registration.
>>>>
>>>> This reduces the error code path and .remove callback for removing
>>>> MFD child devices and deleting IRQ chip data.
>>>>
>>>> Signed-off-by: Laxman Dewangan <[email protected]>
>>>> CC: Chanwoo Choi <[email protected]>
>>>> CC: Krzysztof Kozlowski <[email protected]>
>>>> ---
>>>> drivers/mfd/max77686.c | 31 ++++++++-----------------------
>>>> 1 file changed, 8 insertions(+), 23 deletions(-)
>>> Switching existing code to devm-like interface doesn't bring huge
>>> benefits but looks okay and I'm fine with it:
>> This is pretty much my view, but it get's Laxman's patch count up. ;)
>
> Yaah. :-)

The patch was applied so discussion seems useful only for future work. I
don't agree with some of your motivation.

> There is some other motivation of doing this:
> * I got the review comment about the resource leak and sequencing in my
> max77620. It was silly mistake done by me and it causes recycle of
> patch. To avoid this in future, devm_ was better option.

This is new code. You made error in new code... this is not an argument
to change something in existing well tested code.

> * Spent lots of time on unbinding test during my RTC patch. Although fix
> was not related to the devm_ but gave the impression that something we
> are doing on probe. devm_ looks straight forward.

Nope, the opposite. Usage of devm-like interface hides the order of
cleaning up. With explicit clean it is easy to spot the reverse work of
probe and to find the differences (like when the code is not in the
exact reverse order).

> - Some of code quality tools suggest to avoid goto statement. Only
> possible if we dont have any code in error path i.e. return from any place.

The 'goto' with one exit point is a part of kernel coding style. Of
course lack of error paths is easier to read... usually.

> - If we have devm_ apis for few resource and some does not support then
> difficult to use them as this affect the sequence of deallocation.
> Existing devm_ can be used effectively only if we have all resource
> allocation using devm_.

Ekhm? I don't understand.

> - Reducing code size always better.

Usually... but (again) introducing such questionable improvements for
existing code which was well tested and is working fine:
1. May introduce bugs. Code is already working, error path maybe was or
was not tested. At least it was reviewed. Your change may break this.

2. Is unnecessary code churn. If there are no clear benefits (and for me
in case of max77686 there are no benefits) you just used mine and
maintainer's time.

This is even different kind of change than improvements for defensive
coding (like adding 'consts'). Here, the code does not necessarily
improve. Error paths and order of cleanup are hidden.

Best regards,
Krzysztof

2016-04-28 09:08:37

[permalink] [raw]

Subject: Re: [PATCH 1/7] mfd: as3722: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Thu, 21 Apr 2016, Laxman Dewangan wrote:

> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_regmap_add_irq_chip() for IRQ chip registration.
>
> This reduces the error code path and .remove callback for removing
> MFD child devices and deleting IRQ chip data.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> ---
> drivers/mfd/as3722.c | 31 +++++++++----------------------
> 1 file changed, 9 insertions(+), 22 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
> index e1f597f..f87342c 100644
> --- a/drivers/mfd/as3722.c
> +++ b/drivers/mfd/as3722.c
> @@ -385,9 +385,10 @@ static int as3722_i2c_probe(struct i2c_client *i2c,
> return ret;
>
> irq_flags = as3722->irq_flags | IRQF_ONESHOT;
> - ret = regmap_add_irq_chip(as3722->regmap, as3722->chip_irq,
> - irq_flags, -1, &as3722_irq_chip,
> - &as3722->irq_data);
> + ret = devm_regmap_add_irq_chip(as3722->dev, as3722->regmap,
> + as3722->chip_irq,
> + irq_flags, -1, &as3722_irq_chip,
> + &as3722->irq_data);
> if (ret < 0) {
> dev_err(as3722->dev, "Failed to add regmap irq: %d\n", ret);
> return ret;
> @@ -395,33 +396,20 @@ static int as3722_i2c_probe(struct i2c_client *i2c,
>
> ret = as3722_configure_pullups(as3722);
> if (ret < 0)
> - goto scrub;
> + return ret;
>
> - ret = mfd_add_devices(&i2c->dev, -1, as3722_devs,
> - ARRAY_SIZE(as3722_devs), NULL, 0,
> - regmap_irq_get_domain(as3722->irq_data));
> + ret = devm_mfd_add_devices(&i2c->dev, -1, as3722_devs,
> + ARRAY_SIZE(as3722_devs), NULL, 0,
> + regmap_irq_get_domain(as3722->irq_data));
> if (ret) {
> dev_err(as3722->dev, "Failed to add MFD devices: %d\n", ret);
> - goto scrub;
> + return ret;
> }
>
> device_init_wakeup(as3722->dev, true);
>
> dev_dbg(as3722->dev, "AS3722 core driver initialized successfully\n");
> return 0;
> -
> -scrub:
> - regmap_del_irq_chip(as3722->chip_irq, as3722->irq_data);
> - return ret;
> -}
> -
> -static int as3722_i2c_remove(struct i2c_client *i2c)
> -{
> - struct as3722 *as3722 = i2c_get_clientdata(i2c);
> -
> - mfd_remove_devices(as3722->dev);
> - regmap_del_irq_chip(as3722->chip_irq, as3722->irq_data);
> - return 0;
> }
>
> static int __maybe_unused as3722_i2c_suspend(struct device *dev)
> @@ -470,7 +458,6 @@ static struct i2c_driver as3722_i2c_driver = {
> .pm = &as3722_pm_ops,
> },
> .probe = as3722_i2c_probe,
> - .remove = as3722_i2c_remove,
> .id_table = as3722_i2c_id,
> };
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2016-04-28 09:08:59

[permalink] [raw]

Subject: Re: [PATCH 4/7] mfd: rc5t583: Use devm_mfd_add_devices and devm_request_threaded_irq

On Thu, 21 Apr 2016, Laxman Dewangan wrote:

> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_request_threaded_irq() for IRQ registration.
>
> This reduces the need of remove callback for removing MFD child
> devices and unregistering IRQ.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> ---
> drivers/mfd/rc5t583-irq.c | 11 ++---------
> drivers/mfd/rc5t583.c | 24 +++---------------------
> 2 files changed, 5 insertions(+), 30 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/rc5t583-irq.c b/drivers/mfd/rc5t583-irq.c
> index 3f8812d..f8dde59 100644
> --- a/drivers/mfd/rc5t583-irq.c
> +++ b/drivers/mfd/rc5t583-irq.c
> @@ -389,17 +389,10 @@ int rc5t583_irq_init(struct rc5t583 *rc5t583, int irq, int irq_base)
> irq_clear_status_flags(__irq, IRQ_NOREQUEST);
> }
>
> - ret = request_threaded_irq(irq, NULL, rc5t583_irq, IRQF_ONESHOT,
> - "rc5t583", rc5t583);
> + ret = devm_request_threaded_irq(rc5t583->dev, irq, NULL, rc5t583_irq,
> + IRQF_ONESHOT, "rc5t583", rc5t583);
> if (ret < 0)
> dev_err(rc5t583->dev,
> "Error in registering interrupt error: %d\n", ret);
> return ret;
> }
> -
> -int rc5t583_irq_exit(struct rc5t583 *rc5t583)
> -{
> - if (rc5t583->chip_irq)
> - free_irq(rc5t583->chip_irq, rc5t583);
> - return 0;
> -}
> diff --git a/drivers/mfd/rc5t583.c b/drivers/mfd/rc5t583.c
> index fc2b2d9..d12243d 100644
> --- a/drivers/mfd/rc5t583.c
> +++ b/drivers/mfd/rc5t583.c
> @@ -252,7 +252,6 @@ static int rc5t583_i2c_probe(struct i2c_client *i2c,
> struct rc5t583 *rc5t583;
> struct rc5t583_platform_data *pdata = dev_get_platdata(&i2c->dev);
> int ret;
> - bool irq_init_success = false;
>
> if (!pdata) {
> dev_err(&i2c->dev, "Err: Platform data not found\n");
> @@ -284,32 +283,16 @@ static int rc5t583_i2c_probe(struct i2c_client *i2c,
> /* Still continue with warning, if irq init fails */
> if (ret)
> dev_warn(&i2c->dev, "IRQ init failed: %d\n", ret);
> - else
> - irq_init_success = true;
> }
>
> - ret = mfd_add_devices(rc5t583->dev, -1, rc5t583_subdevs,
> - ARRAY_SIZE(rc5t583_subdevs), NULL, 0, NULL);
> + ret = devm_mfd_add_devices(rc5t583->dev, -1, rc5t583_subdevs,
> + ARRAY_SIZE(rc5t583_subdevs), NULL, 0, NULL);
> if (ret) {
> dev_err(&i2c->dev, "add mfd devices failed: %d\n", ret);
> - goto err_add_devs;
> + return ret;
> }
>
> return 0;
> -
> -err_add_devs:
> - if (irq_init_success)
> - rc5t583_irq_exit(rc5t583);
> - return ret;
> -}
> -
> -static int rc5t583_i2c_remove(struct i2c_client *i2c)
> -{
> - struct rc5t583 *rc5t583 = i2c_get_clientdata(i2c);
> -
> - mfd_remove_devices(rc5t583->dev);
> - rc5t583_irq_exit(rc5t583);
> - return 0;
> }
>
> static const struct i2c_device_id rc5t583_i2c_id[] = {
> @@ -324,7 +307,6 @@ static struct i2c_driver rc5t583_i2c_driver = {
> .name = "rc5t583",
> },
> .probe = rc5t583_i2c_probe,
> - .remove = rc5t583_i2c_remove,
> .id_table = rc5t583_i2c_id,
> };
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2016-04-28 09:08:07

[permalink] [raw]

Subject: Re: [PATCH 3/7] mfd: max77686: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Thu, 21 Apr 2016, Laxman Dewangan wrote:

> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_regmap_add_irq_chip() for IRQ chip registration.
>
> This reduces the error code path and .remove callback for removing
> MFD child devices and deleting IRQ chip data.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> CC: Chanwoo Choi <[email protected]>
> CC: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/mfd/max77686.c | 31 ++++++++-----------------------
> 1 file changed, 8 insertions(+), 23 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 7a0457e..7b68ed7 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -230,38 +230,24 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
> return -ENODEV;
> }
>
> - ret = regmap_add_irq_chip(max77686->regmap, max77686->irq,
> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
> - IRQF_SHARED, 0, irq_chip,
> - &max77686->irq_data);
> + ret = devm_regmap_add_irq_chip(&i2c->dev, max77686->regmap,
> + max77686->irq,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
> + IRQF_SHARED, 0, irq_chip,
> + &max77686->irq_data);
> if (ret < 0) {
> dev_err(&i2c->dev, "failed to add PMIC irq chip: %d\n", ret);
> return ret;
> }
>
> - ret = mfd_add_devices(max77686->dev, -1, cells, n_devs, NULL, 0, NULL);
> + ret = devm_mfd_add_devices(max77686->dev, -1, cells, n_devs, NULL,
> + 0, NULL);
> if (ret < 0) {
> dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
> - goto err_del_irqc;
> + return ret;
> }
>
> return 0;
> -
> -err_del_irqc:
> - regmap_del_irq_chip(max77686->irq, max77686->irq_data);
> -
> - return ret;
> -}
> -
> -static int max77686_i2c_remove(struct i2c_client *i2c)
> -{
> - struct max77686_dev *max77686 = i2c_get_clientdata(i2c);
> -
> - mfd_remove_devices(max77686->dev);
> -
> - regmap_del_irq_chip(max77686->irq, max77686->irq_data);
> -
> - return 0;
> }
>
> static const struct i2c_device_id max77686_i2c_id[] = {
> @@ -317,7 +303,6 @@ static struct i2c_driver max77686_i2c_driver = {
> .of_match_table = of_match_ptr(max77686_pmic_dt_match),
> },
> .probe = max77686_i2c_probe,
> - .remove = max77686_i2c_remove,
> .id_table = max77686_i2c_id,
> };
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2016-04-28 09:10:57

[permalink] [raw]

Subject: Re: [PATCH 6/7] mfd: tps65910: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Thu, 21 Apr 2016, Laxman Dewangan wrote:

> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_regmap_add_irq_chip() for IRQ chip registration.
>
> This reduces the error code path and .remove callback for removing
> MFD child devices and deleting IRQ chip data.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> CC: Tony Lindgren <[email protected]>
> CC: [email protected]
> ---
> drivers/mfd/tps65910.c | 25 ++++---------------------
> 1 file changed, 4 insertions(+), 21 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
> index 8086e5d..11cab15 100644
> --- a/drivers/mfd/tps65910.c
> +++ b/drivers/mfd/tps65910.c
> @@ -252,9 +252,10 @@ static int tps65910_irq_init(struct tps65910 *tps65910, int irq,
> }
>
> tps65910->chip_irq = irq;
> - ret = regmap_add_irq_chip(tps65910->regmap, tps65910->chip_irq,
> - IRQF_ONESHOT, pdata->irq_base,
> - tps6591x_irqs_chip, &tps65910->irq_data);
> + ret = devm_regmap_add_irq_chip(tps65910->dev, tps65910->regmap,
> + tps65910->chip_irq,
> + IRQF_ONESHOT, pdata->irq_base,
> + tps6591x_irqs_chip, &tps65910->irq_data);
> if (ret < 0) {
> dev_warn(tps65910->dev, "Failed to add irq_chip %d\n", ret);
> tps65910->chip_irq = 0;
> @@ -262,13 +263,6 @@ static int tps65910_irq_init(struct tps65910 *tps65910, int irq,
> return ret;
> }
>
> -static int tps65910_irq_exit(struct tps65910 *tps65910)
> -{
> - if (tps65910->chip_irq > 0)
> - regmap_del_irq_chip(tps65910->chip_irq, tps65910->irq_data);
> - return 0;
> -}
> -
> static bool is_volatile_reg(struct device *dev, unsigned int reg)
> {
> struct tps65910 *tps65910 = dev_get_drvdata(dev);
> @@ -516,22 +510,12 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
> regmap_irq_get_domain(tps65910->irq_data));
> if (ret < 0) {
> dev_err(&i2c->dev, "mfd_add_devices failed: %d\n", ret);
> - tps65910_irq_exit(tps65910);
> return ret;
> }
>
> return ret;
> }
>
> -static int tps65910_i2c_remove(struct i2c_client *i2c)
> -{
> - struct tps65910 *tps65910 = i2c_get_clientdata(i2c);
> -
> - tps65910_irq_exit(tps65910);
> -
> - return 0;
> -}
> -
> static const struct i2c_device_id tps65910_i2c_id[] = {
> { "tps65910", TPS65910 },
> { "tps65911", TPS65911 },
> @@ -546,7 +530,6 @@ static struct i2c_driver tps65910_i2c_driver = {
> .of_match_table = of_match_ptr(tps65910_of_match),
> },
> .probe = tps65910_i2c_probe,
> - .remove = tps65910_i2c_remove,
> .id_table = tps65910_i2c_id,
> };
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2016-04-28 09:10:04

[permalink] [raw]

Subject: Re: [PATCH 5/7] mfd: sec: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Thu, 21 Apr 2016, Laxman Dewangan wrote:

> Use devm_mfd_add_devices() for adding MFD child devices and
> devm_regmap_add_irq_chip() for IRQ chip registration.
>
> This reduces the error code path and .remove callback for removing
> MFD child devices and deleting IRQ chip data.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> CC: Sangbeom Kim <[email protected]>
> CC: Krzysztof Kozlowski <[email protected]>
> CC: [email protected]
> ---
> drivers/mfd/sec-core.c | 20 +++-----------------
> drivers/mfd/sec-irq.c | 14 +++++---------
> 2 files changed, 8 insertions(+), 26 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index 400e1d7..ca6b80d 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -481,29 +481,16 @@ static int sec_pmic_probe(struct i2c_client *i2c,
> /* If this happens the probe function is problem */
> BUG();
> }
> - ret = mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs, NULL,
> - 0, NULL);
> + ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
> + NULL, 0, NULL);
> if (ret)
> - goto err_mfd;
> + return ret;
>
> device_init_wakeup(sec_pmic->dev, sec_pmic->wakeup);
> sec_pmic_configure(sec_pmic);
> sec_pmic_dump_rev(sec_pmic);
>
> return ret;
> -
> -err_mfd:
> - sec_irq_exit(sec_pmic);
> - return ret;
> -}
> -
> -static int sec_pmic_remove(struct i2c_client *i2c)
> -{
> - struct sec_pmic_dev *sec_pmic = i2c_get_clientdata(i2c);
> -
> - mfd_remove_devices(sec_pmic->dev);
> - sec_irq_exit(sec_pmic);
> - return 0;
> }
>
> static void sec_pmic_shutdown(struct i2c_client *i2c)
> @@ -583,7 +570,6 @@ static struct i2c_driver sec_pmic_driver = {
> .of_match_table = of_match_ptr(sec_dt_match),
> },
> .probe = sec_pmic_probe,
> - .remove = sec_pmic_remove,
> .shutdown = sec_pmic_shutdown,
> .id_table = sec_pmic_id,
> };
> diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> index d77de43..5eb59c233d5 100644
> --- a/drivers/mfd/sec-irq.c
> +++ b/drivers/mfd/sec-irq.c
> @@ -483,10 +483,11 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> return -EINVAL;
> }
>
> - ret = regmap_add_irq_chip(sec_pmic->regmap_pmic, sec_pmic->irq,
> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> - sec_pmic->irq_base, sec_irq_chip,
> - &sec_pmic->irq_data);
> + ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic,
> + sec_pmic->irq,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> + sec_pmic->irq_base, sec_irq_chip,
> + &sec_pmic->irq_data);
> if (ret != 0) {
> dev_err(sec_pmic->dev, "Failed to register IRQ chip: %d\n", ret);
> return ret;
> @@ -500,8 +501,3 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
>
> return 0;
> }
> -
> -void sec_irq_exit(struct sec_pmic_dev *sec_pmic)
> -{
> - regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
> -}

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2016-04-28 09:01:07

[permalink] [raw]

Subject: Re: [PATCH 3/7] mfd: max77686: Use devm_mfd_add_devices and devm_regmap_add_irq_chip

On Mon, 25 Apr 2016, Krzysztof Kozlowski wrote:

> On 04/21/2016 02:25 PM, Laxman Dewangan wrote:
> > Use devm_mfd_add_devices() for adding MFD child devices and
> > devm_regmap_add_irq_chip() for IRQ chip registration.
> >
> > This reduces the error code path and .remove callback for removing
> > MFD child devices and deleting IRQ chip data.
> >
> > Signed-off-by: Laxman Dewangan <[email protected]>
> > CC: Chanwoo Choi <[email protected]>
> > CC: Krzysztof Kozlowski <[email protected]>
> > ---
> > drivers/mfd/max77686.c | 31 ++++++++-----------------------
> > 1 file changed, 8 insertions(+), 23 deletions(-)
>
> Switching existing code to devm-like interface doesn't bring huge
> benefits but looks okay and I'm fine with it:

This is pretty much my view, but it get's Laxman's patch count up. ;)

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2016-04-28 09:11:19