linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/5200: tighten up ac97 reset timing
@ 2010-09-03 15:45 Eric Millbrandt
  2010-09-03 16:02 ` Wolfram Sang
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Millbrandt @ 2010-09-03 15:45 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, Eric Millbrandt

Tighten up time timing around the gpio reset functionality.  Add a 200ns
delay before remuxing the pins back to ac97 to comply with the ac97 spec.
This delay ensures that sync and sdata_out are held low while reset
transitions to high.

Signed-off-by: Eric Millbrandt <emillbrandt@dekaresearch.com>
---

Scope shots are availible upon request.

 arch/powerpc/platforms/52xx/mpc52xx_common.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/pl=
atforms/52xx/mpc52xx_common.c
index 6e90531..79adfe7 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -326,11 +326,14 @@ int mpc5200_psc_ac97_gpio_reset(int psc_number)
        clrbits8(&wkup_gpio->wkup_dvo, reset);

        /* wait at lease 1 us */
-       udelay(2);
+       udelay(1);

        /* Deassert reset */
        setbits8(&wkup_gpio->wkup_dvo, reset);

+       /* wait at least 200ns */
+       __delay(7);
+
        /* Restore pin-muxing */
        out_be32(&simple_gpio->port_config, mux);

--
1.6.3.1

-DISCLAIMER: an automatically appended disclaimer may follow. By posting-
-to a public e-mail mailing list I hereby grant permission to distribute-
-and copy this message.-

This e-mail and the information, including any attachments, it contains are=
 intended to be a confidential communication only to the person or entity t=
o whom it is addressed and may contain information that is privileged. If t=
he reader of this message is not the intended recipient, you are hereby not=
ified that any dissemination, distribution or copying of this communication=
 is strictly prohibited. If you have received this communication in error, =
please immediately notify the sender and destroy the original message.

Thank you.

Please consider the environment before printing this email.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/5200: tighten up ac97 reset timing
  2010-09-03 15:45 [PATCH] powerpc/5200: tighten up ac97 reset timing Eric Millbrandt
@ 2010-09-03 16:02 ` Wolfram Sang
  2010-09-03 16:17   ` Eric Millbrandt
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2010-09-03 16:02 UTC (permalink / raw)
  To: Eric Millbrandt; +Cc: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

On Fri, Sep 03, 2010 at 11:45:22AM -0400, Eric Millbrandt wrote:
> Tighten up time timing around the gpio reset functionality.  Add a 200ns
> delay before remuxing the pins back to ac97 to comply with the ac97 spec.
> This delay ensures that sync and sdata_out are held low while reset
> transitions to high.
> 
> Signed-off-by: Eric Millbrandt <emillbrandt@dekaresearch.com>
> ---
> 
> Scope shots are availible upon request.
> 
>  arch/powerpc/platforms/52xx/mpc52xx_common.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> index 6e90531..79adfe7 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> @@ -326,11 +326,14 @@ int mpc5200_psc_ac97_gpio_reset(int psc_number)
>         clrbits8(&wkup_gpio->wkup_dvo, reset);
> 
>         /* wait at lease 1 us */

Maybe you could fix this typo as well while you are here?

> -       udelay(2);
> +       udelay(1);
> 
>         /* Deassert reset */
>         setbits8(&wkup_gpio->wkup_dvo, reset);
> 
> +       /* wait at least 200ns */
> +       __delay(7);

ndelay(200)?

> +
>         /* Restore pin-muxing */
>         out_be32(&simple_gpio->port_config, mux);
> 
> --
> 1.6.3.1
> 
> -DISCLAIMER: an automatically appended disclaimer may follow. By posting-
> -to a public e-mail mailing list I hereby grant permission to distribute-
> -and copy this message.-
> 
> This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message.
> 
> Thank you.
> 
> Please consider the environment before printing this email.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] powerpc/5200: tighten up ac97 reset timing
  2010-09-03 16:02 ` Wolfram Sang
@ 2010-09-03 16:17   ` Eric Millbrandt
  2010-09-03 16:22     ` Wolfram Sang
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Millbrandt @ 2010-09-03 16:17 UTC (permalink / raw)
  To: 'Wolfram Sang'; +Cc: linuxppc-dev@lists.ozlabs.org

On Fri, 3 Sep 2010 at 12:02:09 Wolfram Sang wrote:
> On Fri, Sep 03, 2010 at 11:45:22AM -0400, Eric Millbrandt wrote:
>> Tighten up time timing around the gpio reset functionality. Add a 200ns
>> delay before remuxing the pins back to ac97 to comply with the ac97
>> spec. This delay ensures that sync and sdata_out are held low while
>> reset transitions to high.
>>
>> Signed-off-by: Eric Millbrandt <emillbrandt@dekaresearch.com>
>> ---
>>
>> Scope shots are availible upon request.
>>
>>  arch/powerpc/platforms/52xx/mpc52xx_common.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>  diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c
>> b/arch/powerpc/platforms/52xx/mpc52xx_common.c index 6e90531..79adfe7
>> 100644 --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c +++
>> b/arch/powerpc/platforms/52xx/mpc52xx_common.c @@ -326,11 +326,14 @@
>> int mpc5200_psc_ac97_gpio_reset(int
> psc_number)
>>         clrbits8(&wkup_gpio->wkup_dvo, reset);
>>
>>         /* wait at lease 1 us */
>
> Maybe you could fix this typo as well while you are here?

Can do.
>
>> -       udelay(2);
>> +       udelay(1);
>>
>>         /* Deassert reset */
>>         setbits8(&wkup_gpio->wkup_dvo, reset);
>>
>> +       /* wait at least 200ns */
>> +       __delay(7);
>
> ndelay(200)?

Is ndelay defined for powerpc?  I was under the impression that it was
being redefined to udelay in linux/delay.h.

#ifndef ndelay
static inline void ndelay(unsigned long x)
{
        udelay(DIV_ROUND_UP(x, 1000));
}
#define ndelay(x) ndelay(x)
#endif

-DISCLAIMER: an automatically appended disclaimer may follow. By posting-
-to a public e-mail mailing list I hereby grant permission to distribute-
-and copy this message.-




This e-mail and the information, including any attachments, it contains are=
 intended to be a confidential communication only to the person or entity t=
o whom it is addressed and may contain information that is privileged. If t=
he reader of this message is not the intended recipient, you are hereby not=
ified that any dissemination, distribution or copying of this communication=
 is strictly prohibited. If you have received this communication in error, =
please immediately notify the sender and destroy the original message.

Thank you.

Please consider the environment before printing this email.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/5200: tighten up ac97 reset timing
  2010-09-03 16:17   ` Eric Millbrandt
@ 2010-09-03 16:22     ` Wolfram Sang
  2010-09-03 16:40       ` Eric Millbrandt
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2010-09-03 16:22 UTC (permalink / raw)
  To: Eric Millbrandt; +Cc: linuxppc-dev@lists.ozlabs.org

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

> >> -       udelay(2);
> >> +       udelay(1);
> >>
> >>         /* Deassert reset */
> >>         setbits8(&wkup_gpio->wkup_dvo, reset);
> >>
> >> +       /* wait at least 200ns */
> >> +       __delay(7);
> >
> > ndelay(200)?
> 
> Is ndelay defined for powerpc?  I was under the impression that it was
> being redefined to udelay in linux/delay.h.
> 
> #ifndef ndelay
> static inline void ndelay(unsigned long x)
> {
>         udelay(DIV_ROUND_UP(x, 1000));
> }
> #define ndelay(x) ndelay(x)
> #endif

Yes, but it is way more readable. Does the extra delay hurt? The value
of 7 looks a bit magic to me. Are you sure it will do for various clock
frequencies?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] powerpc/5200: tighten up ac97 reset timing
  2010-09-03 16:22     ` Wolfram Sang
@ 2010-09-03 16:40       ` Eric Millbrandt
  2010-09-03 16:49         ` Wolfram Sang
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Millbrandt @ 2010-09-03 16:40 UTC (permalink / raw)
  To: 'Wolfram Sang'; +Cc: linuxppc-dev@lists.ozlabs.org

On Fri, 3 Sep 2010 at 12:22:24 Wolfram Sang wrote:
>>>> -       udelay(2);
>>>> +       udelay(1);
>>>>
>>>>         /* Deassert reset */
>>>>         setbits8(&wkup_gpio->wkup_dvo, reset);
>>>>
>>>> +       /* wait at least 200ns */
>>>> +       __delay(7);
>>>
>>> ndelay(200)?
>>  Is ndelay defined for powerpc?  I was under the impression that it was
>> being redefined to udelay in linux/delay.h.
>>
>> #ifndef ndelay
>> static inline void ndelay(unsigned long x)
>> {
>>         udelay(DIV_ROUND_UP(x, 1000));
>> }
>> #define ndelay(x) ndelay(x)
>> #endif
>
> Yes, but it is way more readable. Does the extra delay hurt? The value
> of 7 looks a bit magic to me. Are you sure it will do for various
> clock frequencies?
>

The reset is happening in the gpio spinlock, so I am trying to keep busy
waiting to a minimum.  The magic value of 7 calculates to *roughly* 200ns
with a system timebase of 33mhz.  This timebase drives the mpc5200(b) at
its max clock speed of 400mhz, so slower frequencies should just extend the
delay, which is acceptable.  If you strongly object to the use of __delay
I can change it to ndelay.


-DISCLAIMER: an automatically appended disclaimer may follow. By posting-
-to a public e-mail mailing list I hereby grant permission to distribute-
-and copy this message.-

This e-mail and the information, including any attachments, it contains are=
 intended to be a confidential communication only to the person or entity t=
o whom it is addressed and may contain information that is privileged. If t=
he reader of this message is not the intended recipient, you are hereby not=
ified that any dissemination, distribution or copying of this communication=
 is strictly prohibited. If you have received this communication in error, =
please immediately notify the sender and destroy the original message.

Thank you.

Please consider the environment before printing this email.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/5200: tighten up ac97 reset timing
  2010-09-03 16:40       ` Eric Millbrandt
@ 2010-09-03 16:49         ` Wolfram Sang
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2010-09-03 16:49 UTC (permalink / raw)
  To: Eric Millbrandt; +Cc: linuxppc-dev@lists.ozlabs.org

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]


> > Yes, but it is way more readable. Does the extra delay hurt? The value
> > of 7 looks a bit magic to me. Are you sure it will do for various
> > clock frequencies?
> >
> 
> The reset is happening in the gpio spinlock, so I am trying to keep busy
> waiting to a minimum.  The magic value of 7 calculates to *roughly* 200ns
> with a system timebase of 33mhz.  This timebase drives the mpc5200(b) at
> its max clock speed of 400mhz, so slower frequencies should just extend the
> delay, which is acceptable.  If you strongly object to the use of __delay
> I can change it to ndelay.

That might be a valid reason, dunno the details here. I'll leave the
final decision for Grant. If the use of __delay is acceptable, the
calculation that led to 7 should be explained, I think.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-09-03 16:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-03 15:45 [PATCH] powerpc/5200: tighten up ac97 reset timing Eric Millbrandt
2010-09-03 16:02 ` Wolfram Sang
2010-09-03 16:17   ` Eric Millbrandt
2010-09-03 16:22     ` Wolfram Sang
2010-09-03 16:40       ` Eric Millbrandt
2010-09-03 16:49         ` Wolfram Sang

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).