linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: akpm@linux-foundation.org (Andrew Morton)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V4]ARM: NUC900: add RTC driver support for nuc910 and nuc920
Date: Mon, 30 Nov 2009 14:23:20 -0800	[thread overview]
Message-ID: <20091130142320.01f0c212.akpm@linux-foundation.org> (raw)
In-Reply-To: <4B126B03.1060901@gmail.com>

On Sun, 29 Nov 2009 20:37:23 +0800
Wan ZongShun <mcuos.com@gmail.com> wrote:

> Dear Alessandro,
> 
> I fixed this patch and submitted it again.
> 
> thanks!
> 
> signed-off-by: Wan ZongShun <mcuos.com@gmail.com>

That's not a terribly useful changelog.

>
> ...
>
> +static void check_rtc_power(struct nuc900_rtc *nuc900_rtc)
> +{
> +	unsigned int i;
> +	__raw_writel(INIRRESET, nuc900_rtc->rtc_reg + REG_RTC_INIR);
> +
> +	mdelay(10);
> +
> +	__raw_writel(AERPOWERON, nuc900_rtc->rtc_reg + REG_RTC_AER);
> +
> +	for (i = 0; i < 1000000; i++) {
> +		if (__raw_readl(nuc900_rtc->rtc_reg + REG_RTC_AER) & AERRWENB)
> +			break;
> +	}
> +}

I don't like that function much.

- It's not obvious what it actually does (I don't know), so it should
  have some comment explaining this.

- It's called "check_rtc_power", but it doesn't actually "check"
  anything.

- If that enormously expensive loop times out, the function will not
  inform the caller of this, so it will be called again and again and
  will continue to be enormously expensive.

>
> ...
>

      reply	other threads:[~2009-11-30 22:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-29 12:37 [PATCH V4]ARM: NUC900: add RTC driver support for nuc910 and nuc920 Wan ZongShun
2009-11-30 22:23 ` Andrew Morton [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091130142320.01f0c212.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).