lkml.org 
[lkml]   [2011]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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]
\
 
 \ /
  Last update: 2011-11-27 10:21    [W:0.698 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site