Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754017Ab2HFIiw (ORCPT ); Mon, 6 Aug 2012 04:38:52 -0400 Received: from hqemgate04.nvidia.com ([216.228.121.35]:11177 "EHLO hqemgate04.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753851Ab2HFIiu convert rfc822-to-8bit (ORCPT ); Mon, 6 Aug 2012 04:38:50 -0400 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Mon, 06 Aug 2012 01:33:14 -0700 From: Venu Byravarasu To: Anthony Olech , Andrew Morton CC: Mark Brown , Paul Gortmaker , Samuel Ortiz , Alessandro Zummo , "rtc-linux@googlegroups.com" , LKML , David Dajun Chen Date: Mon, 6 Aug 2012 14:08:12 +0530 Subject: RE: [NEW DRIVER V2 4/7] DA9058 RTC driver Thread-Topic: [NEW DRIVER V2 4/7] DA9058 RTC driver Thread-Index: Ac1zpu97Arhoj29oS8CBj1aswGMeXgAA1ReA Message-ID: References: <201208060739.q767djnO005723@ubuntu> In-Reply-To: <201208060739.q767djnO005723@ubuntu> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: en-US MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5255 Lines: 189 > -----Original Message----- > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- > owner@vger.kernel.org] On Behalf Of Anthony Olech > Sent: Monday, August 06, 2012 2:14 AM > To: Andrew Morton > Cc: Mark Brown; Paul Gortmaker; Samuel Ortiz; Alessandro Zummo; rtc- > linux@googlegroups.com; LKML; David Dajun Chen > Subject: [NEW DRIVER V2 4/7] DA9058 RTC driver > > This is the RTC component driver of the Dialog DA9058 PMIC. > This driver is just one component of the whole DA9058 PMIC driver. > It depends on the core DA9058 MFD driver. > > Signed-off-by: Anthony Olech > Signed-off-by: David Dajun Chen > --- > + > +static int da9058_rtc_check_param(struct rtc_time *rtc_tm) > +{ > + if ((rtc_tm->tm_sec > DA9058_RTC_SECONDS_LIMIT) || (rtc_tm- > >tm_sec < 0)) > + return -EIO; > + > + if ((rtc_tm->tm_min > DA9058_RTC_MINUTES_LIMIT) || (rtc_tm- > >tm_min < 0)) > + return -EIO; > + > + if ((rtc_tm->tm_hour > DA9058_RTC_HOURS_LIMIT) || (rtc_tm- > >tm_hour < 0)) > + return -EIO; > + > + if (rtc_tm->tm_mday == 0) > + return -EIO; Why limit check is not done for mday, like it is done for other params? > + > + if ((rtc_tm->tm_mon > DA9058_RTC_MONTHS_LIMIT) || (rtc_tm- > >tm_mon <= 0)) > + return -EIO; > + > + if ((rtc_tm->tm_year > DA9058_RTC_YEARS_LIMIT) || (rtc_tm- > >tm_year < 0)) > + return -EIO; > + > + return 0; > +} > + > +static int da9058_rtc_readtime(struct device *dev, struct rtc_time *tm) > +{ > + struct da9058_rtc *rtc = dev_get_drvdata(dev); > + struct da9058 *da9058 = rtc->da9058; > + unsigned int rtc_time[6]; > + int ret; > + > + ret = da9058_bulk_read(da9058, DA9058_COUNTS_REG, rtc_time, 6); > + if (ret) > + return ret; > + > + tm->tm_sec = rtc_time[0] & DA9058_RTC_SECS_MASK; Usually most of the PMIC RTCs provide RTC time registers in BCD Format. Plz make sure that is not the case with your device, otherwise these masks may not be valid. > + tm->tm_min = rtc_time[1] & DA9058_RTC_MINS_MASK; > + > + tm->tm_hour = rtc_time[2] & DA9058_RTC_HRS_MASK; > + > + tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon, > + tm->tm_year); > + tm->tm_year += 100; > + tm->tm_mon -= 1; > + > + return 0; > +} > + > +static int da9058_rtc_settime(struct device *dev, struct rtc_time *tm) > +{ > + struct da9058_rtc *rtc = dev_get_drvdata(dev); > + struct da9058 *da9058 = rtc->da9058; > + unsigned int rtc_ctrl, val, rtc_time[6]; > + int ret; > + > + tm->tm_year -= 111; Why 111 is subtracted here and only 100 is added in read_time() above? If this is what you really wanted to have, it would be better to add few comments justifying these magic numbers. > + tm->tm_mon += 1; > +static irqreturn_t da9058_rtc_timer_alarm_handler(int irq, void *data) > +{ > + struct da9058_rtc *rtc = data; > + > + da9058_rtc_stop_alarm(rtc); > + rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF); > + Whether start RTC is not needed here explicitly? > + return IRQ_HANDLED; > +} > + > + rtc->tick_irq = DA9058_IRQ_ETICK; > + ret = request_threaded_irq(da9058_to_virt_irq_num(da9058, > + rtc->tick_irq), > + NULL, da9058_rtc_tick_alarm_handler, > + IRQF_TRIGGER_RISING | IRQF_ONESHOT, > + "DA9058 RTC Tick Alarm", rtc); > + if (ret) { > + dev_err(&pdev->dev, > + "Failed to get rtc timer alarm IRQ %d: %d\n", > + DA9058_IRQ_ETICK, ret); > + goto err4; > + } > + > + ret = 0; > + goto exit; You can have return 0 here, instead of above two statements. > + > +err4: > + free_irq(da9058_to_virt_irq_num(da9058, rtc->alarm_irq), rtc); > +err3: > +err2: > + rtc_device_unregister(rtc->rtc_dev); > +err1: > +err0: > + platform_set_drvdata(pdev, NULL); > +exit: > + return ret; > +} > + > +static int __devexit da9058_rtc_remove(struct platform_device *pdev) > +{ > + struct da9058_rtc *rtc = platform_get_drvdata(pdev); > + struct da9058 *da9058 = rtc->da9058; > + > + free_irq(da9058_to_virt_irq_num(da9058, rtc->alarm_irq), rtc); > + free_irq(da9058_to_virt_irq_num(da9058, rtc->tick_irq), rtc); > + > + rtc_device_unregister(rtc->rtc_dev); > + > + return 0; > +} > + > +static struct platform_driver da9058_rtc_driver = { > + .probe = da9058_rtc_probe, > + .remove = __devexit_p(da9058_rtc_remove), > + .driver = { > + .name = "da9058-rtc", Some of the rtc drivers had name as rtc-deviceName and others have deviceName-rtc. Can some experts comment which one is correct practice? > + .owner = THIS_MODULE, > + }, > +}; > + > +module_platform_driver(da9058_rtc_driver); > + > +MODULE_DESCRIPTION("Dialog DA9058 PMIC Real Time Clock Driver"); > +MODULE_AUTHOR("Anthony Olech "); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:da9058-rtc"); > -- > end-of-patch for NEW DRIVER V2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/