Messages in this thread | | | Date | Thu, 7 May 2015 20:08:43 +0200 | From | Alexandre Belloni <> | Subject | Re: [PATCH] RTC:Add RTC driver for Gemini SoC |
| |
Hi,
Please also send to the linux-rtc mailing list: rtc-linux@googlegroups.com
On 07/05/2015 at 19:32:54 +0200, Hans Ulli Kroll wrote :
Please always include a commit log.
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com> > ---
[...]
> +static int gemini_rtc_read_time(struct device *dev, struct rtc_time *tm) > +{ > + struct gemini_rtc *rtc = dev_get_drvdata(dev); > + > + unsigned int days, hour, min, sec; > + unsigned long offset, time; > + > + sec = ioread32(rtc->rtc_base + GEMINI_RTC_SECOND); > + min = ioread32(rtc->rtc_base + GEMINI_RTC_MINUTE); > + hour = ioread32(rtc->rtc_base + GEMINI_RTC_HOUR); > + days = ioread32(rtc->rtc_base + GEMINI_RTC_DAYS); > + offset = ioread32(rtc->rtc_base + GEMINI_RTC_RECORD);
Can you use readl() here?
> + > + time = offset + days * 86400 + hour * 3600 + min * 60 + sec; > + > + rtc_time_to_tm(time, tm); > + > + return 0; > +} > + > +static int gemini_rtc_set_time(struct device *dev, struct rtc_time *tm) > +{ > + struct gemini_rtc *rtc = dev_get_drvdata(dev); > + unsigned int sec, min, hour, day; > + unsigned long offset, time; > + > + if (tm->tm_year >= 2148) /* EPOCH Year + 179 */ > + return -EINVAL; > + > + rtc_tm_to_time(tm, &time); > + > + sec = ioread32(rtc->rtc_base + GEMINI_RTC_SECOND); > + min = ioread32(rtc->rtc_base + GEMINI_RTC_MINUTE); > + hour = ioread32(rtc->rtc_base + GEMINI_RTC_HOUR); > + day = ioread32(rtc->rtc_base + GEMINI_RTC_DAYS); > +
ditto.
> + offset = time - (day*86400 + hour*3600 + min*60 + sec); > +
If you use spaces around the * in read_time, please do the same here.
> + iowrite32(offset, rtc->rtc_base + GEMINI_RTC_RECORD); > + iowrite32(0x01, rtc->rtc_base + GEMINI_RTC_CR);
writel() ?
> + > + return 0; > +} > + > +static struct rtc_class_ops gemini_rtc_ops = { > + .read_time = gemini_rtc_read_time, > + .set_time = gemini_rtc_set_time, > +}; > + > +static int gemini_rtc_probe(struct platform_device *pdev) > +{ > + struct gemini_rtc *rtc; > + struct device *dev = &pdev->dev; > + struct resource *res; > + int ret; > + > + rtc = kzalloc(sizeof(*rtc), GFP_KERNEL); > + if (unlikely(!rtc)) > + return -ENOMEM; > + platform_set_drvdata(pdev, rtc); > + > + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > + if (!res) { > + ret = -ENODEV; > + goto err_mem; > + } > + rtc->rtc_irq = res->start; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + ret = -ENODEV; > + goto err_mem; > + } > + rtc->rtc_base = devm_ioremap(&pdev->dev, res->start, > + res->end - res->start + 1); > + > + ret = request_irq(rtc->rtc_irq, gemini_rtc_interrupt, > + IRQF_SHARED, pdev->name, dev); > + if (unlikely(ret)) > + goto err_mem; > + > + rtc->rtc_dev = rtc_device_register(pdev->name, dev, > + &gemini_rtc_ops, THIS_MODULE);
For those 3 functions, the alignment of the wrapped function parameters should match the open parenthesis.
[...]
> +static int __init gemini_rtc_init(void) > +{ > + return platform_driver_register(&gemini_rtc_driver); > +} > + > +static void __exit gemini_rtc_exit(void) > +{ > + platform_driver_unregister(&gemini_rtc_driver); > +} > + > +module_init(gemini_rtc_init); > +module_exit(gemini_rtc_exit);
I'd remove those and use module_platform_driver()
Thanks!
-- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
| |