linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: avictor.za@gmail.com (Andrew Victor)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] AT91 slow-clock resume: don't restore the PLL settings when the PLL was off
Date: Tue, 6 Apr 2010 23:45:42 +0200	[thread overview]
Message-ID: <x2qcd73a99e1004061445j8698c478z9f021d5f3dd8f81b@mail.gmail.com> (raw)
In-Reply-To: <20100303213341.GA654@alarsen.net>

hi Anders,

> From: Julien Langer <julien.langer@gmail.com>
>
> AT91: Don't try to restore the PLL settings on resume when the PLLs were turned
> off before suspending.
>
> We run into this problem with the PLLB on the at91: ohci-at91 disables the PLLB
> when going to suspend. The slowclock code however tries to do the same: It
> saves the PLLB register value and when restoring the value during resume it
> waits for the PLLB to lock again. However the PLL will never lock and the loop
> would run into its timeout because the slowclock code just stored and restored
> an empty register.
> Fix the problem by only restoring PLLA/PLLB when the registers were != 0.
>
> Signed-off-by: Julien Langer <julien.langer@gmail.com>
> Signed-off-by: Anders Larsen <al@alarsen.net>
> Cc: Andrew Victor <avictor.za@gmail.com>
> Cc: Russell King <linux@arm.linux.org.uk>

> @@ -199,16 +207,22 @@ ENTRY(at91_slow_clock)
>
>        /* Restore PLLB setting */
>        ldr     r3, .saved_pllbr
> +       cmp r3, #0
> +       beq 5f
>        str     r3, [r1, #(AT91_CKGR_PLLBR - AT91_PMC)]
>
>        wait_pllblock
>
> +5:
>        /* Restore PLLA setting */
>        ldr     r3, .saved_pllar
> +       cmp r3, #0
> +       beq 6f
>        str     r3, [r1, #(AT91_CKGR_PLLAR - AT91_PMC)]
>
>        wait_pllalock
>
> +6:

I don't think it's sufficient skip the "wait for lock" if the
PLLA/PLLB value is 0.
For example, since bit 29 of PLLA is always 1, the wait_pllalock will
always run - even if MULA is 0 (which means the PLLA is disabled) and
will therefore never lock.
Similarly, for other bits in the register which might happen to be set.

The code should rather be something like:
  Save PLLA
  Save PLLB
  ... wait for interrupt ....
  Restore PLLB
  if (PLLB & AT91_PMC_MUL != 0)
      Wait for PLLB to lock
  Restore PLLA
  if (PLLA & AT91_PMC_MUL != 0)
      Wait for PLLA to lock


Regards,
  Andrew Victor

  reply	other threads:[~2010-04-06 21:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-03 21:33 [PATCH 2/2] AT91 slow-clock resume: don't restore the PLL settings when the PLL was off Anders Larsen
2010-04-06 21:45 ` Andrew Victor [this message]
2010-04-08 10:56   ` Anders Larsen
2010-04-13  8:14     ` Andrew Victor
2010-04-13  8:46       ` Anders Larsen

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=x2qcd73a99e1004061445j8698c478z9f021d5f3dd8f81b@mail.gmail.com \
    --to=avictor.za@gmail.com \
    --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).