Ralf Baechle wrote: > 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, I already re-sended another patch, [PATCH v2] Support RTC on Malta. Best Regards Tiejun > Ralf >