rtc: Fix rtc breakage found by commit f44f7f96a20

Since commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") landed in 2.6.39-rc1, we've seen a few reports of boot time hangs cropping up. It ends up that the problematic rtc drivers were calling rtc_device_register before it finished initializing critical data for the rtc driver to function.

Wolfram Sang noticed that this issue was actually more common then just the few reports seen, and motivated me to do a complete audit of all the RTC drivers to ensure they were not being registered before any critical drvdata was initialized.

This patchset is the result of that audit, along with earlier noted fixes from Wolfram's and Uwe.

Thanks again to Wolfram and Uwe for the patches and helping make these issues a priority.

John Stultz (7):
rtc: ds1286: Initialize drvdata before registering device
rtc: m41t80: Initialize clientdata before registering device
rtc: max8925: Initialize drvdata before registering device
rtc: max8998: Initialize drvdata before registering device
rtc: msm6242: Initialize drvdata before registering device
rtc: pcap: Initialize drvdata before registering device
rtc: rp5c01: Initialize drvdata before registering device

Uwe Kleine-König (1):
rtc: mc13xxx: Don't call rtc_device_register while holding lock

Wolfram Sang (3):
rtc: mxc: Initialize drvdata before registering device
rtc: davinci: Initialize drvdata before registering device
rtc: ep93xx: Initialize drvdata before registering device

 drivers/rtc/rtc-davinci.c |    5 +++--
 drivers/rtc/rtc-ds1286.c  |    2 +-
 drivers/rtc/rtc-ep93xx.c  |    5 ++---
 drivers/rtc/rtc-m41t80.c  |    5 +++--
 drivers/rtc/rtc-max8925.c |    5 +++--
 drivers/rtc/rtc-max8998.c |    5 +++--
 drivers/rtc/rtc-mc13xxx.c |    8 ++++++--
 drivers/rtc/rtc-msm6242.c |    3 ++-
 drivers/rtc/rtc-mxc.c     |   19 +++++++++++--------
 drivers/rtc/rtc-pcap.c    |    4 +++-
 drivers/rtc/rtc-rp5c01.c  |    5 +++--
 11 files changed, 40 insertions(+), 26 deletions(-)

1.7.3.2.146.gca209

From: Wolfram Sang <[email protected]>

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-mxc.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

index 826ab64..d814417 100644
@@ -418,14 +418,6 @@ static int __init mxc_rtc_probe(struct platform_device *pdev)
 		goto exit_put_clk;
 	}
 
-	rtc = rtc_device_register(pdev->name, &pdev->dev, &mxc_rtc_ops,
-				  THIS_MODULE);
-	if (IS_ERR(rtc)) {
-		ret = PTR_ERR(rtc);
-		goto exit_put_clk;
-	}
-
-	pdata->rtc = rtc;
 	platform_set_drvdata(pdev, pdata);
 
 	/* Configure and enable the RTC */

@@ -438,8 +430,19 @@ static int __init mxc_rtc_probe(struct platform_device *pdev)
 		pdata->irq = -1;
 	}
 
+	rtc = rtc_device_register(pdev->name, &pdev->dev, &mxc_rtc_ops,
+				  THIS_MODULE);
+	if (IS_ERR(rtc)) {
+		ret = PTR_ERR(rtc);
+		goto exit_clr_drvdata;
+	}
+
+	pdata->rtc = rtc;
+
 	return 0;
 
+exit_clr_drvdata:
+	platform_set_drvdata(pdev, NULL);
 exit_put_clk:
 	clk_disable(pdata->clk);
 	clk_put(pdata->clk);

1.7.3.2.146.gca209

From: Wolfram Sang <[email protected]>

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-davinci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

index 8d46838..755e1fe 100644
@@ -524,6 +524,8 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
 		goto fail2;
 	}
 
+	platform_set_drvdata(pdev, davinci_rtc);
+
 	davinci_rtc->rtc = rtc_device_register(pdev->name, &pdev->dev,
 				    &davinci_rtc_ops, THIS_MODULE);
 	if (IS_ERR(davinci_rtc->rtc)) {

@@ -553,8 +555,6 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
 
 	rtcss_write(davinci_rtc, PRTCSS_RTC_CCTRL_CAEN, PRTCSS_RTC_CCTRL);
 
-	platform_set_drvdata(pdev, davinci_rtc);
-
 	device_init_wakeup(&pdev->dev, 0);
 
 	return 0;

@@ -562,6 +562,7 @@ static int __init davinci_rtc_probe(struct platform_device *pdev)
 fail4:
 	rtc_device_unregister(davinci_rtc->rtc);
 fail3:
+	platform_set_drvdata(pdev, NULL);
 	iounmap(davinci_rtc->base);
 fail2:
 	release_mem_region(davinci_rtc->pbase, davinci_rtc->base_size);

1.7.3.2.146.gca209

From: Wolfram Sang <[email protected]>

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-ep93xx.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

index 11ae64d..335551d 100644
@@ -151,6 +151,7 @@ static int __init ep93xx_rtc_probe(struct platform_device *pdev)
 		return -ENXIO;
 
 	pdev->dev.platform_data = ep93xx_rtc;
+	platform_set_drvdata(pdev, rtc);
 
 	rtc = rtc_device_register(pdev->name,
 				&pdev->dev, &ep93xx_rtc_ops, THIS_MODULE);

@@ -159,8 +160,6 @@ static int __init ep93xx_rtc_probe(struct platform_device *pdev)
 		goto exit;
 	}
 
-	platform_set_drvdata(pdev, rtc);
-
 	err = sysfs_create_group(&pdev->dev.kobj, &ep93xx_rtc_sysfs_files);
 	if (err)
 		goto fail;

@@ -168,9 +167,9 @@ static int __init ep93xx_rtc_probe(struct platform_device *pdev)
 	return 0;
 
 fail:
-	platform_set_drvdata(pdev, NULL);
 	rtc_device_unregister(rtc);
 exit:
+	platform_set_drvdata(pdev, NULL);
 	pdev->dev.platform_data = NULL;
 	return err;
 }

1.7.3.2.146.gca209

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-ds1286.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 60ce696..47e681d 100644
@@ -355,6 +355,7 @@ static int __devinit ds1286_probe(struct platform_device *pdev)
 		goto out;
 	}
 	spin_lock_init(&priv->lock);
+	platform_set_drvdata(pdev, priv);
 	rtc = rtc_device_register("ds1286", &pdev->dev,
 				  &ds1286_ops, THIS_MODULE);
 	if (IS_ERR(rtc)) {

@@ -362,7 +363,6 @@ static int __devinit ds1286_probe(struct platform_device *pdev)
 		goto out;
 	}
 	priv->rtc = rtc;
-	platform_set_drvdata(pdev, priv);
 	return 0;
 
 out:

1.7.3.2.146.gca209

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the clientdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the clientdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-m41t80.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

index 69fe664..eda128f 100644
@@ -783,6 +783,9 @@ static int m41t80_probe(struct i2c_client *client,
 		goto exit;
 	}
 
+	clientdata->features = id->driver_data;
+	i2c_set_clientdata(client, clientdata);
+
 	rtc = rtc_device_register(client->name, &client->dev,
 				  &m41t80_rtc_ops, THIS_MODULE);
 	if (IS_ERR(rtc)) {

@@ -792,8 +795,6 @@ static int m41t80_probe(struct i2c_client *client,
 	}
 
 	clientdata->rtc = rtc;
-	clientdata->features = id->driver_data;
-	i2c_set_clientdata(client, clientdata);
 
 	/* Make sure HT (Halt Update) bit is cleared */
 	rc = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_HOUR);

1.7.3.2.146.gca209

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-max8925.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

index 20494b5..3bc046f 100644
@@ -258,6 +258,8 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 	}
 
 	dev_set_drvdata(&pdev->dev, info);
+	/* XXX - isn't this redundant? */
+	platform_set_drvdata(pdev, info);
 
 	info->rtc_dev = rtc_device_register("max8925-rtc", &pdev->dev,
 					&max8925_rtc_ops, THIS_MODULE);

@@ -267,10 +269,9 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 		goto out_rtc;
 	}
 
-	platform_set_drvdata(pdev, info);
-
 	return 0;
 out_rtc:
+	platform_set_drvdata(pdev, NULL);
 	free_irq(chip->irq_base + MAX8925_IRQ_RTC_ALARM0, info);
 out_irq:
 	kfree(info);

1.7.3.2.146.gca209

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-max8998.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

index 3f7bc6b..2e48aa6 100644
@@ -265,6 +265,8 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 	info->rtc = max8998->rtc;
 	info->irq = max8998->irq_base + MAX8998_IRQ_ALARM0;
 
+	platform_set_drvdata(pdev, info);
+
 	info->rtc_dev = rtc_device_register("max8998-rtc", &pdev->dev,
 			&max8998_rtc_ops, THIS_MODULE);
 

@@ -274,8 +276,6 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 		goto out_rtc;
 	}
 
-	platform_set_drvdata(pdev, info);
-
 	ret = request_threaded_irq(info->irq, NULL, max8998_rtc_alarm_irq, 0,
 			"rtc-alarm0", info);
 

@@ -293,6 +293,7 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 	return 0;
 
 out_rtc:
+	platform_set_drvdata(pdev, NULL);
 	kfree(info);
 	return ret;
 }

1.7.3.2.146.gca209

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-msm6242.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

index 6782062..fcb113c 100644
@@ -214,6 +214,7 @@ static int __init msm6242_rtc_probe(struct platform_device *dev)
 		error = -ENOMEM;
 		goto out_free_priv;
 	}
+	platform_set_drvdata(dev, priv);
 
 	rtc = rtc_device_register("rtc-msm6242", &dev->dev, &msm6242_rtc_ops,
 				  THIS_MODULE);

@@ -223,10 +224,10 @@ static int __init msm6242_rtc_probe(struct platform_device *dev)
 	}
 
 	priv->rtc = rtc;
-	platform_set_drvdata(dev, priv);
 	return 0;
 
 out_unmap:
+	platform_set_drvdata(dev, NULL);
 	iounmap(priv->regs);
 out_free_priv:
 	kfree(priv);

1.7.3.2.146.gca209

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-pcap.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

index a633abc..cd4f198 100644
@@ -151,6 +151,8 @@ static int __devinit pcap_rtc_probe(struct platform_device *pdev)
 
 	pcap_rtc->pcap = dev_get_drvdata(pdev->dev.parent);
 
+	platform_set_drvdata(pdev, pcap_rtc);
+
 	pcap_rtc->rtc = rtc_device_register("pcap", &pdev->dev,
 				  &pcap_rtc_ops, THIS_MODULE);
 	if (IS_ERR(pcap_rtc->rtc)) {

@@ -158,7 +160,6 @@ static int __devinit pcap_rtc_probe(struct platform_device *pdev)
 		goto fail_rtc;
 	}
 
-	platform_set_drvdata(pdev, pcap_rtc);
 
 	timer_irq = pcap_to_irq(pcap_rtc->pcap, PCAP_IRQ_1HZ);
 	alarm_irq = pcap_to_irq(pcap_rtc->pcap, PCAP_IRQ_TODA);

@@ -177,6 +178,7 @@ fail_alarm:
 fail_timer:
 	rtc_device_unregister(pcap_rtc->rtc);
 fail_rtc:
+	platform_set_drvdata(pdev, NULL);
 	kfree(pcap_rtc);
 	return err;
 }

1.7.3.2.146.gca209

Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") uncovered an issue in a number of RTC drivers, where the drivers call rtc_device_register before initializing the device or platform drvdata.

This frequently results in null pointer dereferences when the rtc_device_register immediately makes use of the rtc device, calling rtc_read_alarm.

The solution is to ensure the drvdata is initialized prior to registering the rtc device.

---
 drivers/rtc/rtc-rp5c01.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

index 694da39..359da6d 100644
@@ -249,15 +249,15 @@ static int __init rp5c01_rtc_probe(struct platform_device *dev)
 
 	spin_lock_init(&priv->lock);
 
+	platform_set_drvdata(dev, priv);
+
 	rtc = rtc_device_register("rtc-rp5c01", &dev->dev, &rp5c01_rtc_ops,
 				  THIS_MODULE);
 	if (IS_ERR(rtc)) {
 		error = PTR_ERR(rtc);
 		goto out_unmap;
 	}
-
 	priv->rtc = rtc;
-	platform_set_drvdata(dev, priv);
 
 	error = sysfs_create_bin_file(&dev->dev.kobj, &priv->nvram_attr);
 	if (error)

@@ -268,6 +268,7 @@ static int __init rp5c01_rtc_probe(struct platform_device *dev)
 out_unregister:
 	rtc_device_unregister(rtc);
 out_unmap:
+	platform_set_drvdata(dev, NULL);
 	iounmap(priv->regs);
 out_free_priv:
 	kfree(priv);

1.7.3.2.146.gca209

From: Uwe Kleine-König <[email protected]>

Since commit f44f7f9 (RTC: Initialize kernel state from RTC) rtc_device_register reads the programmed alarm. As reading the alarm needs to take the mc13xxx lock, release it before calling rtc_device_register.

This fixes a deadlock during boot:

	INFO: task swapper:1 blocked for more than 120 seconds.
	"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
	swapper         D c02b175c     0     1      0 0x00000000
	[<c02b175c>] (schedule+0x304/0x4f4) from [<c02b25a8>] (__mutex_lock_slowpath+0x7c/0x110)
	[<c02b25a8>] (__mutex_lock_slowpath+0x7c/0x110) from [<c020b4cc>] (mc13xxx_rtc_read_time+0x1c/0x118)
	[<c020b4cc>] (mc13xxx_rtc_read_time+0x1c/0x118) from [<c0208f04>] (__rtc_read_time+0x58/0x5c)
	[<c0208f04>] (__rtc_read_time+0x58/0x5c) from [<c0209508>] (rtc_read_time+0x30/0x48)
	[<c0209508>] (rtc_read_time+0x30/0x48) from [<c0209dd4>] (__rtc_read_alarm+0x1c/0x290)
	[<c0209dd4>] (__rtc_read_alarm+0x1c/0x290) from [<c0208d58>] (rtc_device_register+0x150/0x27c)
	[<c0208d58>] (rtc_device_register+0x150/0x27c) from [<c02b0b74>] (mc13xxx_rtc_probe+0x128/0x17c)
	[<c02b0b74>] (mc13xxx_rtc_probe+0x128/0x17c) from [<c01d5280>] (platform_drv_probe+0x1c/0x24)
	[<c01d5280>] (platform_drv_probe+0x1c/0x24) from [<c01d3e58>] (driver_probe_device+0x80/0x1a8)
	[<c01d3e58>] (driver_probe_device+0x80/0x1a8) from [<c01d400c>] (__driver_attach+0x8c/0x90)
	[<c01d400c>] (__driver_attach+0x8c/0x90) from [<c01d3654>] (bus_for_each_dev+0x60/0x8c)
	[<c01d3654>] (bus_for_each_dev+0x60/0x8c) from [<c01d2f6c>] (bus_add_driver+0x180/0x248)
	[<c01d2f6c>] (bus_add_driver+0x180/0x248) from [<c01d4664>] (driver_register+0x70/0x15c)
	[<c01d4664>] (driver_register+0x70/0x15c) from [<c01d5700>] (platform_driver_probe+0x18/0x98)
	[<c01d5700>] (platform_driver_probe+0x18/0x98) from [<c00273a8>] (do_one_initcall+0x2c/0x168)
	[<c00273a8>] (do_one_initcall+0x2c/0x168) from [<c00083ac>] (kernel_init+0xa0/0x150)
	[<c00083ac>] (kernel_init+0xa0/0x150) from [<c0033ff8>] (kernel_thread_exit+0x0/0x8)

---
 drivers/rtc/rtc-mc13xxx.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

index c5ac037..a1a278b 100644
@@ -349,11 +349,15 @@ static int __devinit mc13xxx_rtc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_alarm_irq_request;
 
+	mc13xxx_unlock(mc13xxx);
+
 	priv->rtc = rtc_device_register(pdev->name,
 			&pdev->dev, &mc13xxx_rtc_ops, THIS_MODULE);
 	if (IS_ERR(priv->rtc)) {
 		ret = PTR_ERR(priv->rtc);
 
+		mc13xxx_lock(mc13xxx);
+
 		mc13xxx_irq_free(mc13xxx, MC13XXX_IRQ_TODA, priv);
 err_alarm_irq_request:
 

@@ -365,12 +369,12 @@ err_reset_irq_status:
 		mc13xxx_irq_free(mc13xxx, MC13XXX_IRQ_RTCRST, priv);
 err_reset_irq_request:
 
+		mc13xxx_unlock(mc13xxx);
+
 		platform_set_drvdata(pdev, NULL);
 		kfree(priv);
 	}
 
-	mc13xxx_unlock(mc13xxx);
-
 	return ret;
 }
 

1.7.3.2.146.gca209