On Thu, Oct 30, 2008 at 07:15:44PM +0800, Tiejun Chen wrote: Looks reasonable but I have a few comments: > diff --git a/arch/mips/mti-malta/Makefile b/arch/mips/mti-malta/Makefile > index cef2db8..26284fc 100644 > --- a/arch/mips/mti-malta/Makefile > +++ b/arch/mips/mti-malta/Makefile > @@ -9,7 +9,7 @@ obj-y := malta-amon.o malta-cmdline.o \ > malta-display.o malta-init.o malta-int.o \ > malta-memory.o malta-mtd.o \ > malta-platform.o malta-reset.o \ > - malta-setup.o malta-time.o > + malta-setup.o malta-time.o malta-rtc.o Could you please fold malta-rtc.c into the existing malta-platform.c instead of creating a new file? > +++ b/arch/mips/mti-malta/malta-rtc.c > @@ -0,0 +1,73 @@ > + struct platform_device *pdev; > + int ret; > + > + pdev = platform_device_alloc("rtc_cmos", -1); > + if (!pdev) > + return -ENOMEM; > + > + ret = platform_device_add_resources(pdev, malta_platform_rtc_resource, > + ARRAY_SIZE(malta_platform_rtc_resource)); > + if (ret) > + goto err; > + > + ret = platform_device_add(pdev); > + if (ret) > + goto err; You can simplify this a bit by using a static struct platform_device like: static struct platform_device rtc_device = { .name = "rtc_cmos", .id = -1, .resource = &malta_platform_rtc_resource, .num_resources = ARRAY_SIZE(malta_platform_rtc_resource), }; > + > + /* Try setting rtc as BCD mode to support > + * current alarm code if possible. > + */ > + if (!RTC_ALWAYS_BCD) > + CMOS_WRITE(CMOS_READ(RTC_CONTROL) & ~RTC_DM_BINARY, RTC_CONTROL); RTC_ALWAYS_BCD is 0, so the if condition is always true and the if can be eleminated. Can you fix that? Ralf