[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20131105153210.e4c93cf589fbbe46fcd831c1@linux-foundation.org>
Date: Tue, 5 Nov 2013 15:32:10 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Laxman Dewangan <ldewangan@...dia.com>
Cc: <lee.jones@...aro.org>, <sameo@...ux.intel.com>,
<broonie@...nel.org>, <linus.walleij@...aro.org>,
<devicetree@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
<rtc-linux@...glegroups.com>, <rob.herring@...xeda.com>,
<mark.rutland@....com>, <pawel.moll@....com>,
<swarren@...dotorg.org>, <rob@...dley.net>,
<ijc+devicetree@...lion.org.uk>, <grant.likely@...aro.org>,
Florian Lobmaier <florian.lobmaier@....com>
Subject: Re: [PATCH V6 3/3] drivers/rtc/rtc-as3722: add RTC driver
On Mon, 21 Oct 2013 17:17:21 +0530 Laxman Dewangan <ldewangan@...dia.com> wrote:
> The ams AS3722 is a compact system PMU suitable for mobile phones,
> tablets etc.
>
> Add a driver to support accessing the RTC found on the ams AS3722
> PMIC using RTC framework.
>
> ...
>
> +static int as3722_rtc_probe(struct platform_device *pdev)
> +{
> + struct as3722 *as3722 = dev_get_drvdata(pdev->dev.parent);
> + struct as3722_rtc *as3722_rtc;
> + int ret;
> +
> + as3722_rtc = devm_kzalloc(&pdev->dev, sizeof(*as3722_rtc), GFP_KERNEL);
> + if (!as3722_rtc)
> + return -ENOMEM;
> +
> + as3722_rtc->as3722 = as3722;
> + as3722_rtc->dev = &pdev->dev;
> + platform_set_drvdata(pdev, as3722_rtc);
> +
> + /* Enable the RTC to make sure it is running. */
> + ret = as3722_update_bits(as3722, AS3722_RTC_CONTROL_REG,
> + AS3722_RTC_ON | AS3722_RTC_ALARM_WAKEUP_EN,
> + AS3722_RTC_ON | AS3722_RTC_ALARM_WAKEUP_EN);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "RTC_CONTROL reg write failed: %d\n", ret);
> + return ret;
> + }
> +
> + device_init_wakeup(&pdev->dev, 1);
> +
> + as3722_rtc->rtc = rtc_device_register("as3722", &pdev->dev,
> + &as3722_rtc_ops, THIS_MODULE);
Could use devm_rtc_device_register() here.
> + if (IS_ERR(as3722_rtc->rtc)) {
> + ret = PTR_ERR(as3722_rtc->rtc);
> + dev_err(&pdev->dev, "RTC register failed: %d\n", ret);
> + return ret;
> + }
> +
> + as3722_rtc->alarm_irq = platform_get_irq(pdev, 0);
> + dev_info(&pdev->dev, "RTC interrupt %d\n", as3722_rtc->alarm_irq);
> +
> + ret = request_threaded_irq(as3722_rtc->alarm_irq, NULL,
> + as3722_alarm_irq, IRQF_ONESHOT | IRQF_EARLY_RESUME,
> + "rtc-alarm", as3722_rtc);
devm_request_threaded_irq()?
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to request alarm IRQ %d: %d\n",
> + as3722_rtc->alarm_irq, ret);
> + goto scrub;
> + }
> + disable_irq(as3722_rtc->alarm_irq);
Why is this necessary?
Would it be safer to do this before the request_threaded_irq()?
> + return 0;
> +scrub:
> + rtc_device_unregister(as3722_rtc->rtc);
> + return ret;
> +}
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists