On Fri, Nov 25, 2011 at 10:52:07AM +0800, zhzhl555@xxxxxxxxx wrote:
> + writel(t, SYS_TOYWRITE1);
> + while (readl(SYS_COUNTER_CNTRL) & SYS_CNTRL_TS)
> + usleep_range(1000, 3000);
> + __asm__ volatile ("sync");
Timeout?
> +
> +static int __devinit ls1x_rtc_probe(struct platform_device *pdev)
> +{
> + struct rtc_device *rtcdev;
> + unsigned long v;
> + int ret;
> +
> + v = readl(SYS_COUNTER_CNTRL);
> + if (!(v & RTC_CNTR_OK)) {
> + dev_err(&pdev->dev, "rtc counters not working\n");
> + ret = -ENODEV;
> + goto err;
> + }
> + ret = -ETIMEDOUT;
Why not putting this line to the corresponding dev_err-block?
> + /*set to 1 HZ if needed*/
Minor: Spaces around comment-markers, here and in other places
/* Comment */
> + if (readl(SYS_TOYTRIM) != 32767) {
> + v = 0x100000;
> + while ((readl(SYS_COUNTER_CNTRL) & SYS_CNTRL_TTS) && --v)
> + usleep_range(1000, 3000);
Timeout?
> +
> + if (!v) {
> + dev_err(&pdev->dev, "time out\n");
> + goto err;
> + }
> + writel(32767, SYS_TOYTRIM);
> + __asm__ volatile("sync");
> + }
> + /*this loop coundn't be endless*/
> + while (readl(SYS_COUNTER_CNTRL) & SYS_CNTRL_TTS)
> + usleep_range(1000, 3000);
Timeout again. First, the comment does not help. Why are you sure the loop
could not be endless? And: Does it really need to be a usleep_range() instead
of a simple msleep()? And to make sure: There is no interrupt signalling the
status changed?
> +
> + rtcdev = rtc_device_register("ls1x-rtc", &pdev->dev,
> + &ls1x_rtc_ops , THIS_MODULE);
> + if (IS_ERR(rtcdev)) {
> + ret = PTR_ERR(rtcdev);
> + goto err;
> + }
> +
> + platform_set_drvdata(pdev, rtcdev);
> + return 0;
> +err:
> + return ret;
> +}
> +
...
> +static int __init ls1x_rtc_init(void)
> +{
> + return platform_driver_probe(&ls1x_rtc_driver, ls1x_rtc_probe);
> +}
> +
> +static void __exit ls1x_rtc_exit(void)
> +{
> + platform_driver_unregister(&ls1x_rtc_driver);
> +}
> +
> +
> +module_init(ls1x_rtc_init);
> +module_exit(ls1x_rtc_exit);
Please use the new module_platform_driver()-macro.
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
