Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1527705
| From | Sebastian Reichel <sre@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 1/5] power: supply: tps65217: Move IRQ related operation into single function |
| Date | 2016-11-22 18:00 +0100 |
| Message-ID | <sGmAn-1lo-27@gated-at.bofh.it> (permalink) |
| References | <sDLOy-5pg-19@gated-at.bofh.it> <sDLYe-5sD-23@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
[Multipart message — attachments visible in raw view] - view raw
Hi,
On Tue, Nov 15, 2016 at 10:18:51PM +0900, Milo Kim wrote:
> TPS65217 charger driver handles the charger interrupt through the IRQ or
> polling. Both cases can be requested in single function.
>
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
> drivers/power/supply/tps65217_charger.c | 70 ++++++++++++++++++---------------
> 1 file changed, 38 insertions(+), 32 deletions(-)
I don't see the advantage of this. It's more lines of codes than
before and does not really increase readability.
-- Sebastian
>
> diff --git a/drivers/power/supply/tps65217_charger.c b/drivers/power/supply/tps65217_charger.c
> index 9fd019f..04f8322 100644
> --- a/drivers/power/supply/tps65217_charger.c
> +++ b/drivers/power/supply/tps65217_charger.c
> @@ -195,12 +195,48 @@ static const struct power_supply_desc tps65217_charger_desc = {
> .num_properties = ARRAY_SIZE(tps65217_ac_props),
> };
>
> +static int tps65217_charger_request_interrupt(struct platform_device *pdev)
> +{
> + struct tps65217_charger *charger = platform_get_drvdata(pdev);
> + int irq;
> + int ret;
> +
> + irq = platform_get_irq_byname(pdev, "AC");
> + if (irq < 0)
> + irq = -ENXIO;
> +
> + charger->irq = irq;
> +
> + if (irq != -ENXIO) {
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + tps65217_charger_irq, 0,
> + "tps65217-charger", charger);
> + if (ret) {
> + dev_err(charger->dev,
> + "Unable to register irq %d err %d\n", irq, ret);
> + return ret;
> + }
> +
> + /* Check current state */
> + tps65217_charger_irq(irq, charger);
> + return 0;
> + }
> +
> + charger->poll_task = kthread_run(tps65217_charger_poll_task, charger,
> + "ktps65217charger");
> + if (IS_ERR(charger->poll_task)) {
> + ret = PTR_ERR(charger->poll_task);
> + dev_err(charger->dev, "Unable to run kthread err %d\n", ret);
> + }
> +
> + return 0;
> +}
> +
> static int tps65217_charger_probe(struct platform_device *pdev)
> {
> struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
> struct tps65217_charger *charger;
> struct power_supply_config cfg = {};
> - int irq;
> int ret;
>
> dev_dbg(&pdev->dev, "%s\n", __func__);
> @@ -224,43 +260,13 @@ static int tps65217_charger_probe(struct platform_device *pdev)
> return PTR_ERR(charger->ac);
> }
>
> - irq = platform_get_irq_byname(pdev, "AC");
> - if (irq < 0)
> - irq = -ENXIO;
> - charger->irq = irq;
> -
> ret = tps65217_config_charger(charger);
> if (ret < 0) {
> dev_err(charger->dev, "charger config failed, err %d\n", ret);
> return ret;
> }
>
> - if (irq != -ENXIO) {
> - ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> - tps65217_charger_irq,
> - 0, "tps65217-charger",
> - charger);
> - if (ret) {
> - dev_err(charger->dev,
> - "Unable to register irq %d err %d\n", irq,
> - ret);
> - return ret;
> - }
> -
> - /* Check current state */
> - tps65217_charger_irq(irq, charger);
> - } else {
> - charger->poll_task = kthread_run(tps65217_charger_poll_task,
> - charger, "ktps65217charger");
> - if (IS_ERR(charger->poll_task)) {
> - ret = PTR_ERR(charger->poll_task);
> - dev_err(charger->dev,
> - "Unable to run kthread err %d\n", ret);
> - return ret;
> - }
> - }
> -
> - return 0;
> + return tps65217_charger_request_interrupt(pdev);
> }
>
> static int tps65217_charger_remove(struct platform_device *pdev)
> --
> 2.9.3
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/5] power: supply: tps65217: Support USB charger feature Milo Kim <woogyom.kim@gmail.com> - 2016-11-15 14:20 +0100
[PATCH 5/5] power: supply: tps65217: Use generic name for power supply structures Milo Kim <woogyom.kim@gmail.com> - 2016-11-15 14:20 +0100
[PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data Milo Kim <woogyom.kim@gmail.com> - 2016-11-15 14:20 +0100
Re: [PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data Sebastian Reichel <sre@kernel.org> - 2016-11-22 18:00 +0100
Re: [PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data Sebastian Reichel <sre@kernel.org> - 2016-11-22 18:00 +0100
Re: [PATCH 2/5] power: supply: tps65217: Remove IRQ data from driver data Milo Kim <woogyom.kim@gmail.com> - 2016-11-23 12:50 +0100
[PATCH 4/5] power: supply: tps65217: Use generic name for charger online Milo Kim <woogyom.kim@gmail.com> - 2016-11-15 14:20 +0100
[PATCH 1/5] power: supply: tps65217: Move IRQ related operation into single function Milo Kim <woogyom.kim@gmail.com> - 2016-11-15 14:30 +0100
Re: [PATCH 1/5] power: supply: tps65217: Move IRQ related operation into single function Sebastian Reichel <sre@kernel.org> - 2016-11-22 18:00 +0100
Re: [PATCH 1/5] power: supply: tps65217: Move IRQ related operation into single function Sebastian Reichel <sre@kernel.org> - 2016-11-22 18:00 +0100
[PATCH 3/5] power: supply: tps65217: Support USB charger interrupt Milo Kim <woogyom.kim@gmail.com> - 2016-11-15 14:30 +0100
csiph-web