From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751930AbbEGSIq (ORCPT ); Thu, 7 May 2015 14:08:46 -0400 Received: from down.free-electrons.com ([37.187.137.238]:37545 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751407AbbEGSIp (ORCPT ); Thu, 7 May 2015 14:08:45 -0400 Date: Thu, 7 May 2015 20:08:43 +0200 From: Alexandre Belloni To: Hans Ulli Kroll Cc: linux-kernel@vger.kernel.org, Arnd Bergmann Subject: Re: [PATCH] RTC:Add RTC driver for Gemini SoC Message-ID: <20150507180843.GA3244@piout.net> References: <1431019974-19750-1-git-send-email-ulli.kroll@googlemail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1431019974-19750-1-git-send-email-ulli.kroll@googlemail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- [...] > +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