Messages in this thread | | | Date | Sun, 27 Nov 2011 10:18:03 +0100 | From | Wolfram Sang <> | Subject | Re: [rtc-linux] [PATCH V1] MIPS: Add RTC support for loongson1B |
| |
On Fri, Nov 25, 2011 at 10:52:07AM +0800, zhzhl555@gmail.com 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/ | [unhandled content-type:application/pgp-signature] | |