On Tue, Jan 25, 2011 at 12:19:35PM +0530, Varun Wadekar wrote: > drivers/rtc/rtc-tps6586x.c | 325 > ++++++++++++++++++++++++++++++++++++++++++ It looks like you still have the line wrapping issues. > +config RTC_DRV_TPS6586X > + tristate "TI TPS6586X RTC" > + depends on I2C && MFD_TPS6586X No need to depend on I2C, the MFD for the device deals with that for you. > +struct tps6586x_rtc { > + unsigned long epoch_start; > + int irq; All the indentation in the driver is broken - you should be using tabs for indentation. This may well be the same issue with your MUA that is causing the line wrapping. > +static int tps6586x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm > *alrm) > +{ > + if (rtc->irq == -1) > + return -EIO; > + if (rtc->irq_en && rtc->irq_en && (rtc->irq != -1)) { > + disable_irq(rtc->irq); > + rtc->irq_en = false; You're already checking rtc->irq further up, the check for rtc->irq_en is duplicated. Probably just need to replace the if with a single check for rtc->irq_en? > + if (alrm->enabled && (rtc->irq != -1)) { No need to check the IRQ here either. > +static int tps6586x_rtc_update_irq_enable(struct device *dev, > + unsigned int enabled) > +{ > + struct tps6586x_rtc *rtc = dev_get_drvdata(dev); > + > + if (rtc->irq == -1) > + return -EIO; > + > + enabled = !!enabled; > + if (enabled == rtc->irq_en) > + return 0; > + > + if (enabled) > + enable_irq(rtc->irq); > + else > + disable_irq(rtc->irq); This looks to be the same IRQ as is used for alarms above - how do these two bits of code play together? The IRQ hander... > +static irqreturn_t tps6586x_rtc_irq(int irq, void *data) > +{ > + struct device *dev = data; > + struct tps6586x_rtc *rtc = dev_get_drvdata(dev); > + > + rtc_update_irq(rtc->rtc, 1, RTC_IRQF | RTC_AF); > + return IRQ_HANDLED; ...only appears to handle alarm interrupts. > + rtc->epoch_start = mktime(TPS_EPOCH, 1, 1, 0, 0, 0); Should this be configurable in platform data? > + if (pdata && (pdata->irq >= 0)) { > + rtc->irq = pdata->irq; > + err = request_threaded_irq(pdata->irq, NULL, tps6586x_rtc_irq, > + IRQF_ONESHOT, "tps6586x-rtc", > + &pdev->dev); > + if (err) { > + dev_warn(&pdev->dev, "unable to request IRQ\n"); > + rtc->irq = -1; It'd be nice to print out the interrupt number here in case the issue is someone misspecified it. > +MODULE_DESCRIPTION("TI TPS6586x RTC driver"); > +MODULE_AUTHOR("NVIDIA Corporation"); > +MODULE_LICENSE("GPL"); Should really have a MODULE_ALIAS() too. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html