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 <[email protected]> > Signed-off-by: Milo Kim <[email protected]> > --- > 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
>
signature.asc
Description: PGP signature
