Re: [PATCH V6 3/3] drivers/rtc/rtc-as3722: add RTC driver

On Mon, 21 Oct 2013 17:17:21 +0530 Laxman Dewangan [off-list ref] 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;
+}
+