linux-mips
[Top] [All Lists]

Re: [PATCH] Support RTC on Malta

To: Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH] Support RTC on Malta
From: "tiejun.chen" <tiejun.chen@windriver.com>
Date: Wed, 26 Nov 2008 09:19:12 +0800
Cc: linux-mips@linux-mips.org
In-reply-to: <20081030120510.GH26256@linux-mips.org>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1225365345-15635-1-git-send-email-tiejun.chen@windriver.com> <20081030120510.GH26256@linux-mips.org>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Thunderbird 2.0.0.17 (X11/20080925)
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
> 


<Prev in Thread] Current Thread [Next in Thread>