2023-03-09 22:51:02

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCHv1 03/11] power: supply: generic-adc-battery: convert to managed resources

Convert driver to use managed resources to simplify driver code.

Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/power/supply/generic-adc-battery.c | 81 ++++++----------------
1 file changed, 23 insertions(+), 58 deletions(-)

diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
index 66039c665dd1..917bd2a6cc52 100644
--- a/drivers/power/supply/generic-adc-battery.c
+++ b/drivers/power/supply/generic-adc-battery.c
@@ -23,6 +23,7 @@
#include <linux/iio/consumer.h>
#include <linux/iio/types.h>
#include <linux/power/generic-adc-battery.h>
+#include <linux/devm-helpers.h>

#define JITTER_DEFAULT 10 /* hope 10ms is enough */

@@ -266,14 +267,13 @@ static int gab_probe(struct platform_device *pdev)
* copying the static properties and allocating extra memory for holding
* the extra configurable properties received from platform data.
*/
- properties = kcalloc(ARRAY_SIZE(gab_props) +
- ARRAY_SIZE(gab_chan_name),
- sizeof(*properties),
- GFP_KERNEL);
- if (!properties) {
- ret = -ENOMEM;
- goto first_mem_fail;
- }
+ properties = devm_kcalloc(&pdev->dev,
+ ARRAY_SIZE(gab_props) +
+ ARRAY_SIZE(gab_chan_name),
+ sizeof(*properties),
+ GFP_KERNEL);
+ if (!properties)
+ return -ENOMEM;

memcpy(properties, gab_props, sizeof(gab_props));

@@ -282,12 +282,13 @@ static int gab_probe(struct platform_device *pdev)
* based on the channel supported by consumer device.
*/
for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++) {
- adc_bat->channel[chan] = iio_channel_get(&pdev->dev,
- gab_chan_name[chan]);
+ adc_bat->channel[chan] = devm_iio_channel_get(&pdev->dev, gab_chan_name[chan]);
if (IS_ERR(adc_bat->channel[chan])) {
ret = PTR_ERR(adc_bat->channel[chan]);
+ if (ret != -ENODEV)
+ return dev_err_probe(&pdev->dev, ret, "Failed to get ADC channel %s\n", gab_chan_name[chan]);
adc_bat->channel[chan] = NULL;
- } else {
+ } else if (adc_bat->channel[chan]) {
/* copying properties for supported channels only */
int index2;

@@ -302,10 +303,8 @@ static int gab_probe(struct platform_device *pdev)
}

/* none of the channels are supported so let's bail out */
- if (!any) {
- ret = -ENODEV;
- goto second_mem_fail;
- }
+ if (!any)
+ return dev_err_probe(&pdev->dev, -ENODEV, "Failed to get any ADC channel\n");

/*
* Total number of properties is equal to static properties
@@ -316,25 +315,24 @@ static int gab_probe(struct platform_device *pdev)
psy_desc->properties = properties;
psy_desc->num_properties = index;

- adc_bat->psy = power_supply_register(&pdev->dev, psy_desc, &psy_cfg);
- if (IS_ERR(adc_bat->psy)) {
- ret = PTR_ERR(adc_bat->psy);
- goto err_reg_fail;
- }
+ adc_bat->psy = devm_power_supply_register(&pdev->dev, psy_desc, &psy_cfg);
+ if (IS_ERR(adc_bat->psy))
+ return dev_err_probe(&pdev->dev, PTR_ERR(adc_bat->psy), "Failed to register power-supply device\n");

- INIT_DELAYED_WORK(&adc_bat->bat_work, gab_work);
+ ret = devm_delayed_work_autocancel(&pdev->dev, &adc_bat->bat_work, gab_work);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Failed to register delayed work\n");

- adc_bat->charge_finished = devm_gpiod_get_optional(&pdev->dev,
- "charged", GPIOD_IN);
+ adc_bat->charge_finished = devm_gpiod_get_optional(&pdev->dev, "charged", GPIOD_IN);
if (adc_bat->charge_finished) {
int irq;

irq = gpiod_to_irq(adc_bat->charge_finished);
- ret = request_any_context_irq(irq, gab_charged,
+ ret = devm_request_any_context_irq(&pdev->dev, irq, gab_charged,
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
"battery charged", adc_bat);
if (ret < 0)
- goto gpio_req_fail;
+ return dev_err_probe(&pdev->dev, ret, "Failed to register irq\n");
}

platform_set_drvdata(pdev, adc_bat);
@@ -343,38 +341,6 @@ static int gab_probe(struct platform_device *pdev)
schedule_delayed_work(&adc_bat->bat_work,
msecs_to_jiffies(0));
return 0;
-
-gpio_req_fail:
- power_supply_unregister(adc_bat->psy);
-err_reg_fail:
- for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++) {
- if (adc_bat->channel[chan])
- iio_channel_release(adc_bat->channel[chan]);
- }
-second_mem_fail:
- kfree(properties);
-first_mem_fail:
- return ret;
-}
-
-static int gab_remove(struct platform_device *pdev)
-{
- int chan;
- struct gab *adc_bat = platform_get_drvdata(pdev);
-
- power_supply_unregister(adc_bat->psy);
-
- if (adc_bat->charge_finished)
- free_irq(gpiod_to_irq(adc_bat->charge_finished), adc_bat);
-
- for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++) {
- if (adc_bat->channel[chan])
- iio_channel_release(adc_bat->channel[chan]);
- }
-
- kfree(adc_bat->psy_desc.properties);
- cancel_delayed_work_sync(&adc_bat->bat_work);
- return 0;
}

static int __maybe_unused gab_suspend(struct device *dev)
@@ -408,7 +374,6 @@ static struct platform_driver gab_driver = {
.pm = &gab_pm_ops,
},
.probe = gab_probe,
- .remove = gab_remove,
};
module_platform_driver(gab_driver);

--
2.39.2



2023-03-10 08:21:49

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCHv1 03/11] power: supply: generic-adc-battery: convert to managed resources

On Thu, Mar 9, 2023 at 11:50 PM Sebastian Reichel <[email protected]> wrote:

> Convert driver to use managed resources to simplify driver code.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

Excellent
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2023-03-13 07:14:18

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCHv1 03/11] power: supply: generic-adc-battery: convert to managed resources

Hi Sebastian, All

To me this does look like a great simplification :)

On 3/10/23 00:50, Sebastian Reichel wrote:
> Convert driver to use managed resources to simplify driver code.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

Reviewed-by: Matti Vaittinen <[email protected]>



--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~