All of lore.kernel.org
 help / color / mirror / Atom feed
From: robert.jarzmik@free.fr (Robert Jarzmik)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] ARM: pxa27x: fix ac97 controller warm reset code
Date: Fri, 04 Jan 2013 21:34:08 +0100	[thread overview]
Message-ID: <87obh4it6n.fsf@free.fr> (raw)
In-Reply-To: <50E6AF88.8050800@compulab.co.il> (Igor Grinberg's message of "Fri, 04 Jan 2013 12:31:36 +0200")

Igor Grinberg <grinberg@compulab.co.il> writes:

> On 01/04/13 08:50, Igor Grinberg wrote:
>> On 01/03/13 16:39, Mike Dunn wrote:
>>> fb1bf8cd13bfa7ed0364ab0d82f717fc020d35f6 
>>> ([ARM] pxa: introduce processor specific pxa27x_assert_ac97reset())
>>> which changed the mfp to a GPIO input instead of a high output.
>
> Looking at this one more time...
> fb1bf8cd13 ([ARM] pxa: introduce processor specific pxa27x_assert_ac97reset())
> also removed the call to pxa_gpio_mode() function which effectively set
> AF to GPIO and configured the GPIO to output high.
> (Later b1d9bf1d ([ARM] pxa: remove pxa_gpio_mode() and files) removed the
> pxa_gpio_mode() function)
>
> See below...
> The DRIVE_HIGH does not really configures the GPIO to output high, but
> only sets the MFP_LPM_DRIVE_HIGH bit which in turn is only effective in
> low power modes.
> This means, that by doing the above, you just configure the MFP for GPIO output,
> but do not assign it a value, so it gets driven with some undefined value.
> This is not safe.
>
> Can you please, check if the attached patch below does the job?

This is not the original behaviour before commit
fb1bf8cd13bfa7ed0364ab0d82f717fc020d35f6.

The original behaviour was :
 - on = 1 => set GPIO as output GPIO, set to 1
 - on = 0 => set GPIO to the alternate function ac97reset, driven by PXA2xx AC97
 IP.

If you don't set the alternate function, the GCR register usage for reset is
useless, isn't it ? So why do you set the GPIO as "input" with on == 0 ?

And a question for Mike : when you made your tests, was it you intended
behaviour to never set ac97 alternate function and use direct output GPIO
setting with gpio_set_value(, X) ? Or am I missing something here ?

Cheers.

--
Robert

  parent reply	other threads:[~2013-01-04 20:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-03 14:39 [PATCH v2] ARM: pxa27x: fix ac97 controller warm reset code Mike Dunn
2013-01-04  6:50 ` Igor Grinberg
2013-01-04 10:31   ` Igor Grinberg
2013-01-04 20:29     ` Mike Dunn
2013-01-04 20:34     ` Robert Jarzmik [this message]
2013-01-05 13:06       ` Mike Dunn
2013-01-06  7:11       ` Igor Grinberg
2013-01-06 14:00         ` Robert Jarzmik
2013-01-06 16:10           ` Igor Grinberg
2013-01-06 19:04           ` Mike Dunn
2013-01-06 19:13         ` Mike Dunn

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=87obh4it6n.fsf@free.fr \
    --to=robert.jarzmik@free.fr \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.