Re: [PATCH] rtc: AB8500 RTC driver

[Rabin VINCENT]

On Tue, May 25, 2010 at 22:51:59 +0200, Andrew Morton wrote:
> On Fri, 21 May 2010 20:26:36 +0530
> Rabin Vincent <rabin.vincent@xxxxxxxxxxxxxx> wrote:
> > +static int __devexit ab8500_rtc_remove(struct platform_device *pdev)
> > +{
> > +	struct rtc_device *rtc = platform_get_drvdata(pdev);
> > +	int irq = platform_get_irq_byname(pdev, "ALARM");
> 
> Seems a bit fragile.  I guess that platform_get_irq_byname("ALARM")
> will always return the same value, but still.  Would it be better to
> store the irq number in private storage?  Unsure..

Several other drivers do a platform_get_irq() in their remove().  We'd
have probably stored it locally anyway, but this driver actually gets by
without allocating any private storage, so it didn't seem necessary to
add some just for this.

We've removed the unnecessary "if (irq >= 0)" condition before
free_irq() to make it clear to the reader that the
platform_get_irq_byname() call won't fail.

All your other comments should be addressed in the updated patch below.
Thanks.

Rabin