LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/2 v8] powerpc: introduce macro spin_event_timeout()
From: Jon Smirl @ 2009-05-26 16:27 UTC (permalink / raw)
  To: Geoff Thorpe; +Cc: linuxppc-dev, Timur Tabi, smaclennan
In-Reply-To: <4A1C16DF.9090802@freescale.com>

On Tue, May 26, 2009 at 12:20 PM, Geoff Thorpe
<Geoff.Thorpe@freescale.com> wrote:
> Timur Tabi wrote:
>> On Mon, May 25, 2009 at 12:46 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
>>
>>> I just tried using this. The !rc has the effect of making the error
>>> return be zero instead the normal not zero.
>>
>> You're confused. =A0It's not a "return code", it's a return value. =A0I
>> guess I should have called the parameter "ret" instead of "rc", but I
>> didn't expect people to get confused.
>>
>> 'rc' is the value of the expression when the loop terminates. =A0That's
>> what makes the most sense, because the developer will want to know
>> what that value is. =A0If you're expression happens to rely on negative
>> logic (e.g. wait until a bit is cleared), then of course it's going to
>> appear "backwards" when you test it.
>
> I've just been going through some hassles associated with wait_event
> variants and their return codes.
>
> Eg. wait_event_interruptible()'s return value is documented as; "The
> function will return -ERESTARTSYS if it was interrupted by a signal and
> 0 if @condition evaluated to true." And wait_event_timeout()'s return
> value is; "The function returns 0 if the @timeout elapsed, and the
> remaining jiffies if the condition evaluated to true before the timeout
> elapsed."
>
> In all cases it seems, they don't return the what the expression
> evaluates to, but instead return an indication about the wait outcome.
> This is why I've taken to doing things like;
> =A0 wait_event_***(queue, !(ret =3D try_something()));

That's a good trick, if you want the result of the condition pass the
assignment in.
That frees up the return value to indicate the error of the time out expiri=
ng.

I'd really like to keep a consistent if(rc!=3D0)error; model.

> So from this user's perspective (FWIW), it would come as a surprise if
> the return value reflected the evaluated expression rather than what
> happened w.r.t. the spin/timeout.




>
> Cheers,
> Geoff
>
>
>
>



--=20
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH 1/2 v8] powerpc: introduce macro spin_event_timeout()
From: Geoff Thorpe @ 2009-05-26 16:20 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, smaclennan
In-Reply-To: <ed82fe3e0905252027y198e78ct821753acacb1bdb1@mail.gmail.com>

Timur Tabi wrote:
> On Mon, May 25, 2009 at 12:46 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> 
>> I just tried using this. The !rc has the effect of making the error
>> return be zero instead the normal not zero.
> 
> You're confused.  It's not a "return code", it's a return value.  I
> guess I should have called the parameter "ret" instead of "rc", but I
> didn't expect people to get confused.
> 
> 'rc' is the value of the expression when the loop terminates.  That's
> what makes the most sense, because the developer will want to know
> what that value is.  If you're expression happens to rely on negative
> logic (e.g. wait until a bit is cleared), then of course it's going to
> appear "backwards" when you test it.

I've just been going through some hassles associated with wait_event
variants and their return codes.

Eg. wait_event_interruptible()'s return value is documented as; "The
function will return -ERESTARTSYS if it was interrupted by a signal and
0 if @condition evaluated to true." And wait_event_timeout()'s return
value is; "The function returns 0 if the @timeout elapsed, and the
remaining jiffies if the condition evaluated to true before the timeout
elapsed."

In all cases it seems, they don't return the what the expression
evaluates to, but instead return an indication about the wait outcome.
This is why I've taken to doing things like;
   wait_event_***(queue, !(ret = try_something()));

So from this user's perspective (FWIW), it would come as a surprise if
the return value reflected the evaluated expression rather than what
happened w.r.t. the spin/timeout.

Cheers,
Geoff

^ permalink raw reply

* [PATCH 1/2] [PATCH 1/2 v9] powerpc: introduce macro spin_event_timeout()
From: Timur Tabi @ 2009-05-26 15:21 UTC (permalink / raw)
  To: linuxppc-dev, galak, benh, jonsmirl, arnd, geert.uytterhoeven
In-Reply-To: <1243351302-32062-1-git-send-email-timur@freescale.com>

The macro spin_event_timeout() takes a condition and timeout value
(in microseconds) as parameters.  It spins until either the condition is true
or the timeout expires.  It returns the result of the condition when the loop
was terminated.

This primary purpose of this macro is to poll on a hardware register until a
status bit changes.  The timeout ensures that the loop still terminates if the
bit doesn't change as expected.  This macro makes it easier for driver
developers to perform this kind of operation properly.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

v9: changed the macro to return the value, instead of having it passed in

 arch/powerpc/include/asm/delay.h |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index f9200a6..1e2eb41 100644
--- a/arch/powerpc/include/asm/delay.h
+++ b/arch/powerpc/include/asm/delay.h
@@ -2,8 +2,11 @@
 #define _ASM_POWERPC_DELAY_H
 #ifdef __KERNEL__
 
+#include <asm/time.h>
+
 /*
  * Copyright 1996, Paul Mackerras.
+ * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -30,5 +33,38 @@ extern void udelay(unsigned long usecs);
 #define mdelay(n)	udelay((n) * 1000)
 #endif
 
+/**
+ * spin_event_timeout - spin until a condition gets true or a timeout elapses
+ * @condition: a C expression to evalate
+ * @timeout: timeout, in microseconds
+ * @delay: the number of microseconds to delay between each evaluation of
+ *         @condition
+ *
+ * The process spins until the condition evaluates to true (non-zero) or the
+ * timeout elapses.  The return value of this macro is the value of
+ * @condition when the loop terminates. This allows you to determine the cause
+ * of the loop terminates.  If the return value is zero, then you know a
+ * timeout has occurred.
+ *
+ * This primary purpose of this macro is to poll on a hardware register
+ * until a status bit changes.  The timeout ensures that the loop still
+ * terminates even if the bit never changes.  The delay is for devices that
+ * need a delay in between successive reads.
+ *
+ * gcc will optimize out the if-statement if @delay is a constant.
+ */
+#define spin_event_timeout(condition, timeout, delay)                          \
+({                                                                             \
+	typeof(condition) __ret;                                               \
+	unsigned long __loops = tb_ticks_per_usec * timeout;                   \
+	unsigned long __start = get_tbl();                                     \
+	while (!(__ret = (condition)) && (tb_ticks_since(__start) <= __loops)) \
+		if (delay)                                                     \
+			udelay(delay);                                         \
+		else                                                           \
+			cpu_relax();                                           \
+	__ret;		                                                       \
+})
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DELAY_H */
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH 2/2] [PATCH 2/2 v2] qe: add polling timeout to qe_issue_cmd()
From: Timur Tabi @ 2009-05-26 15:21 UTC (permalink / raw)
  To: linuxppc-dev, galak, benh, jonsmirl, arnd, geert.uytterhoeven
In-Reply-To: <1243351302-32062-2-git-send-email-timur@freescale.com>

The qe_issue_cmd() function (Freescale PowerPC QUICC Engine library) polls on
a register until a status bit changes, but does not include a timeout to
handle the situation if the bit never changes.  Change the code to use the new
spin_event_timeout() macro, which simplifies polling on a register without
a timeout.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch depends on my previous patch, "powerpc: introduce macro
spin_event_timeout()".

 arch/powerpc/sysdev/qe_lib/qe.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index 01bce37..05bdd43 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -111,6 +111,7 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
 {
 	unsigned long flags;
 	u8 mcn_shift = 0, dev_shift = 0;
+	u32 ret;
 
 	spin_lock_irqsave(&qe_lock, flags);
 	if (cmd == QE_RESET) {
@@ -138,11 +139,13 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
 	}
 
 	/* wait for the QE_CR_FLG to clear */
-	while(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG)
-		cpu_relax();
+	ret = spin_event_timeout((in_be32(&qe_immr->cp.cecr) & QE_CR_FLG) == 0,
+			   100, 0);
+	/* On timeout (e.g. failure), the expression will be false (ret == 0),
+	   otherwise it will be true (ret == 1). */
 	spin_unlock_irqrestore(&qe_lock, flags);
 
-	return 0;
+	return ret == 1;
 }
 EXPORT_SYMBOL(qe_issue_cmd);
 
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH 0/2] [V2] introduce macro spin_event_timeout()
From: Timur Tabi @ 2009-05-26 15:21 UTC (permalink / raw)
  To: linuxppc-dev, galak, benh, jonsmirl, arnd, geert.uytterhoeven


Introduce the spin_event_timeout() macro, and update the QE library to use it.

This version changes 'ret' to a return value of the macro instead of a
variable that's passed as a parameter.

^ permalink raw reply

* RE: Initialize DBCR0 for PPC440 targets
From: John Linn @ 2009-05-26 15:11 UTC (permalink / raw)
  To: srikanth krishnakar; +Cc: Linuxppc-dev
In-Reply-To: <6213bc560905242351r7682dcb2m898beef8b334e010@mail.gmail.com>

> -----Original Message-----
> From: srikanth krishnakar [mailto:skrishnakar@gmail.com]
> Sent: Monday, May 25, 2009 12:51 AM
> To: John Linn
> Cc: Linuxppc-dev@ozlabs.org
> Subject: Re: Initialize DBCR0 for PPC440 targets
> =

> Hi John,
> =

> I am not finding any conclusion of the plan to add DBCR0
> initialization code to head_44x.S after this discussion :

There was no conclusion and I haven't had time yet to chase it further.

> =

> http://www.nabble.com/Question-about-DBCR0-initialization-for-440-td23049=
044.html#a23049044
> =

> Can you please comment ?
> =

> Thanks,
> -Srikanth Krishnakar
> =

> On Mon, May 25, 2009 at 12:09 PM, Grant Likely
> <grant.likely@secretlab.ca> wrote:
> > On Mon, May 25, 2009 at 12:30 AM, srikanth krishnakar
> > <skrishnakar@gmail.com> wrote:
> >> Hello Grant,
> >>
> >> Is there any conclusion of the below discussion:
> >>
> >> http://www.nabble.com/Question-about-DBCR0-initialization-for-440-td23=
049044.html
> >>
> >> Xilinx target (virtex5) hangs (while running GDBServer & KGDB) without=

> >> the DBCR0 initialization.
> >>
> >> Can you please comment on this ?
> >
> > IIRC, John Linn was hacking on a patch. =A0Search the mailing list
> > archives for DBCR0.
> >
> > g.
> >
> > --
> > Grant Likely, B.Sc., P.Eng.
> > Secret Lab Technologies Ltd.
> >
> =

> =

> =

> --
> "The Good You Do, The Best You GET"
> =

> Regards
> Srikanth Krishnakar
> **********************


This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* Re: [alsa-devel] [PATCH V5 1/5] powerpc: introduce macro spin_event_timeout()
From: Mark Brown @ 2009-05-26 14:53 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, timur
In-Reply-To: <9e4733910905260612v40a70ad8haeb4c916a1a758b5@mail.gmail.com>

On Tue, May 26, 2009 at 09:12:10AM -0400, Jon Smirl wrote:
> On Tue, May 26, 2009 at 9:03 AM, Mark Brown

> > As previously mentioned you need to submit any changes you want to make
> > here as incremental patches against the PowerPC tree rather than as an
> > entire new patch. ?If you are forwarding on copies of the patch you
> > really ought to preserve Timur's authorship too with a From line in the
> > mail.

> I have reverted back to TImur's original patch  so you can just drop
> this patch if that is easier for you.
> You'll just need to coordinate the landing so that things land in the
> right order.

Like I say, since it's a new driver I'm just going to ignore the
ordering since it'll sort itself out in the merge window and we're
rather close to that.  Worst case something that doesn't build isn't
that much different from something that isn't there in terms of
usability.

^ permalink raw reply

* Re: [net-next-2.6 PATCH v2] can: SJA1000: generic OF platform bus driver
From: Wolfgang Grandegger @ 2009-05-26 14:23 UTC (permalink / raw)
  To: David Miller; +Cc: linuxppc-dev, devicetree-discuss, arnd, netdev
In-Reply-To: <20090526.022508.238274281.davem@davemloft.net>

David Miller wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 26 May 2009 10:10:30 +0100
> 
>> On Monday 25 May 2009, Wolfgang Grandegger wrote:
>>>> Right, that makes sense. However, most drivers use the field to store the
>>>> physical address, not the iomap token. Maybe there should be a new field
>>>> in struct sja1000_priv for the virtual address, but that would be a change
>>>> to the base driver, not just to the OF portion.
>>> Is that common practice? If yes, I will add a member to store the
>>> virtual address to struct sja1000_priv.
>> I grepped through the network driver for usage of ->base_addr, and
>> it's somewhat inconsistent. The majority of the users use it for
>> a physical address, but there are also a few that use it for the
>> __iomem token.
>>
>> Casts between unsigned long and qualified (__iomem, __user, const, ...)
>> pointers do not cause a warning, but can easily lead to bugs when
>> another user casts to an unqualified pointer.
> 
> It's such a baroque thing, there is no reason to set it at all if you
> ask me.  It's only use is to allow ISA and similar primitive bus
> devices to have their I/O ports changed via ifconfig.

OK, I see, there are good reasons not to (mis-)use dev->base_addr. I
will prepare a patch for the SJA1000 CAN drivers.

Wolfgang.

^ permalink raw reply

* Re: [PATCH V4 1/5] The macro spin_event_timeout() takes a condition and timeout value
From: Timur Tabi @ 2009-05-26 14:20 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: alsa-devel, linuxppc-dev, Geert Uytterhoeven, broonie
In-Reply-To: <200905261349.49832.arnd@arndb.de>

Arnd Bergmann wrote:
> On Tuesday 26 May 2009, Geert Uytterhoeven wrote:
>> However, you can still improve useability by making the macro return the rc,
>> instead of letting the caller pass it, cfr. wait_event_timeout() and friends.

I had that originally, but somewhere during the seven revisions of my macro, it got lost.

I'll post a v9 soon.  This version will take 3 parameters and return __ret, so Jon will need to change his code (sorry).

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH V5 3/5] AC97 driver for mpc5200
From: Timur Tabi @ 2009-05-26 14:09 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, broonie
In-Reply-To: <20090526123410.17472.15251.stgit@terra>

Jon Smirl wrote:

> +static void psc_ac97_warm_reset(struct snd_ac97 *ac97)
> +{
> +	int rc;
> +	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
> +
> +	out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_AWR);
> +	spin_event_timeout(0, 3, 0, rc);

I still think you should use udelay() here.  spin_event_timeout(<constant>) will just reduce to a udelay() call anyway.  In addition, there's no "event" that you're waiting for.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH V5 1/5] powerpc: introduce macro spin_event_timeout()
From: Jon Smirl @ 2009-05-26 13:12 UTC (permalink / raw)
  To: Mark Brown; +Cc: linuxppc-dev, alsa-devel, timur
In-Reply-To: <20090526130301.GE8969@sirena.org.uk>

On Tue, May 26, 2009 at 9:03 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, May 26, 2009 at 08:34:06AM -0400, Jon Smirl wrote:
>> The macro spin_event_timeout() takes a condition and timeout value
>> (in microseconds) as parameters. =A0It spins until either the condition =
is true
>> or the timeout expires. =A0It returns the result of the condition when t=
he loop
>> was terminated.
>
> As previously mentioned you need to submit any changes you want to make
> here as incremental patches against the PowerPC tree rather than as an
> entire new patch. =A0If you are forwarding on copies of the patch you
> really ought to preserve Timur's authorship too with a From line in the
> mail.

I have reverted back to TImur's original patch  so you can just drop
this patch if that is easier for you.
You'll just need to coordinate the landing so that things land in the
right order.

--=20
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH V5 1/5] powerpc: introduce macro spin_event_timeout()
From: Mark Brown @ 2009-05-26 13:03 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, timur
In-Reply-To: <20090526123406.17472.79948.stgit@terra>

On Tue, May 26, 2009 at 08:34:06AM -0400, Jon Smirl wrote:
> The macro spin_event_timeout() takes a condition and timeout value
> (in microseconds) as parameters.  It spins until either the condition is true
> or the timeout expires.  It returns the result of the condition when the loop
> was terminated.

As previously mentioned you need to submit any changes you want to make
here as incremental patches against the PowerPC tree rather than as an
entire new patch.  If you are forwarding on copies of the patch you
really ought to preserve Timur's authorship too with a From line in the
mail.

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc: Add support for swiotlb on 32-bit
From: Ian Campbell @ 2009-05-26 12:51 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: FUJITA Tomonori, linuxppc-dev@ozlabs.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <4A173B72.5000201@goop.org>

On Fri, 2009-05-22 at 19:55 -0400, Jeremy Fitzhardinge wrote:
> Ian Campbell wrote:
> > On Thu, 2009-05-21 at 14:27 -0400, Becky Bruce wrote:
> >   
> >> I can work with that, but it's going to be a bit inefficient, as I  
> >> actually need the dma_addr_t, not the phys_addr_t, so I'll have to  
> >> convert.  In every case, this is a conversion I've already done and  
> >> that I need in the calling code as well. 
> >>     
> >
> > Does
> >
> >     dma_addr_t dma_map_range(struct device *hwdev, phys_addr_t addr,
> >     size_t size);
> >
> > work for you?
> >
> > If the range does not need mapping then it returns the dma address, if
> > you needed to calculate the dma address anyway to figure out if mapping
> > is required then this is fine. If the range does need mapping then it
> > returns NULL.
> >   
> 
> My only concern is whether dma_addr_t == 0 is actually equivalent to 
> NULL.  That is, can we be sure that address 0 will never be used?

It seems not, ~0UL might have been an option, but...

> Taking dma_alloc_coherent as a model, we could have something like:
> 
>     int dma_map_range(struct device *hwdev, phys_addr_t addr, size_t size, dma_addr_t *dma_addrp);
>       
> 
> where *dma_addrp is set if the function returns success (bool return 
> type might be clearer).

... this sounds like a good idea to me.

Ian.

^ permalink raw reply

* Re: [PATCH v5 0/4] Series short description
From: Roderick Colenbrander @ 2009-05-26 12:50 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <c8485d470905260541t65e35d86l8c676a2b016c0ff8@mail.gmail.com>

On Tue, May 26, 2009 at 2:41 PM, Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
> On Mon, May 25, 2009 at 10:47 PM, Roderick Colenbrander
> <thunderbird2k@gmail.com> wrote:
>> On Mon, May 25, 2009 at 4:41 PM, Grant Likely <grant.likely@secretlab.ca=
> wrote:
>>> Repost of ml510 series. =A0This time against 2.6.30-rc7 and with the .d=
ts
>>> file included.
>>>
>>> g.
>>>
>>> --
>>> Grant Likely, B.Sc. P.Eng.
>>> Secret Lab Technologies Ltd.
>>>
>>
>> Thanks, I will check it out tomorrow.
>>
>> Roderick
>>
>
> Hi,
>
> I have tested the ml510 patches. The code works properly but I had to
> make a few slight changes to get it functioning. The most important
> change was in xilinx_intc driver. For level interrupts the ack command
> was removed during the xilinx_intc rewrite which caused a null pointer
> derefence in the i8259 cascade code. I have now changed it to:
> /* Let xilinx_intc end the interrupt */
> desc->chip->mask(irq);
> desc->chip->unmask(irq);
>
> This seems to work properly. Further the driver now also has proper
> level support, so i think my dts file has some errors. The pci
> interrupts (a/b/c/d) are all active low, so they need to be changed to
> '3' for the sense level. Further the i8259 is active high which needs
> to be set to 2 instead of 3.
>
> Further I have tested the need for the outb to 0x4d0/0x4d1. Removing
> those lines causes interrupt timeouts and ide won't function.
> Freescale does the same in arch/powerpc/platforms/fsl_uli1575.c (which
> is a much newer version of this chipset) in quirk_final_uli1575 but
> they are doing it for some more interrupts. I think this code should
> be done using a similar quirk in virtex_ml510.c
>
> Roderick
>

I have just tested the following and it seemed to work fine.
static void __devinit ml510_ali_quirk_final(struct pci_dev *dev)
{
	/* Program irq 7 (usb/audio), 14/15 (ide) to level sensitive */
	/* This looks like a dirty hack to me --gcl */
	outb(0xc0, 0x4d0);
	outb(0xc0, 0x4d1);
}
DECLARE_PCI_FIXUP_FINAL(0x10b9, 0x1533, ml510_ali_quirk_final);

I'm not sure what is preferred level or edge interrupts as the ide
controller can be programmed to both (I would just have to write to
config register 0x44 and 0x75).

Roderick

^ permalink raw reply

* Re: [PATCH V4 1/5] The macro spin_event_timeout() takes a condition and timeout value
From: Arnd Bergmann @ 2009-05-26 12:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Geert Uytterhoeven, alsa-devel, Timur Tabi, broonie
In-Reply-To: <alpine.LRH.2.00.0905261348160.4897@vixen.sonytel.be>

On Tuesday 26 May 2009, Geert Uytterhoeven wrote:
> However, you can still improve useability by making the macro return the rc,
> instead of letting the caller pass it, cfr. wait_event_timeout() and friends.

Either that, or it should at least use the do { ... } while (0) construct
to make the macro a statement. All multi-line macros need to either
use ({ ... }) or do { ... } while (0) to make sure they behave well.

	Arnd <><

^ permalink raw reply

* Re: [PATCH v5 0/4] Series short description
From: Roderick Colenbrander @ 2009-05-26 12:41 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <c8485d470905251347m4e3b7022u4f0324dfc154cfb2@mail.gmail.com>

On Mon, May 25, 2009 at 10:47 PM, Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
> On Mon, May 25, 2009 at 4:41 PM, Grant Likely <grant.likely@secretlab.ca>=
 wrote:
>> Repost of ml510 series. =A0This time against 2.6.30-rc7 and with the .dt=
s
>> file included.
>>
>> g.
>>
>> --
>> Grant Likely, B.Sc. P.Eng.
>> Secret Lab Technologies Ltd.
>>
>
> Thanks, I will check it out tomorrow.
>
> Roderick
>

Hi,

I have tested the ml510 patches. The code works properly but I had to
make a few slight changes to get it functioning. The most important
change was in xilinx_intc driver. For level interrupts the ack command
was removed during the xilinx_intc rewrite which caused a null pointer
derefence in the i8259 cascade code. I have now changed it to:
/* Let xilinx_intc end the interrupt */
desc->chip->mask(irq);
desc->chip->unmask(irq);

This seems to work properly. Further the driver now also has proper
level support, so i think my dts file has some errors. The pci
interrupts (a/b/c/d) are all active low, so they need to be changed to
'3' for the sense level. Further the i8259 is active high which needs
to be set to 2 instead of 3.

Further I have tested the need for the outb to 0x4d0/0x4d1. Removing
those lines causes interrupt timeouts and ide won't function.
Freescale does the same in arch/powerpc/platforms/fsl_uli1575.c (which
is a much newer version of this chipset) in quirk_final_uli1575 but
they are doing it for some more interrupts. I think this code should
be done using a similar quirk in virtex_ml510.c

Roderick

^ permalink raw reply

* [PATCH V5 5/5] Fabric bindings for STAC9766 on the Efika
From: Jon Smirl @ 2009-05-26 12:34 UTC (permalink / raw)
  To: grant.likely, linuxppc-dev, alsa-devel, broonie, timur
In-Reply-To: <20090526123235.17472.73912.stgit@terra>

Fabric bindings for STAC9766 AC97 codec on the Efika.

Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
 sound/soc/fsl/Kconfig              |    8 +++
 sound/soc/fsl/Makefile             |    1 
 sound/soc/fsl/efika-audio-fabric.c |   90 ++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/fsl/efika-audio-fabric.c

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 79579ae..f571c6e 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -47,3 +47,11 @@ config SND_MPC52xx_SOC_PCM030
 	help
 	  Say Y if you want to add support for sound on the Phytec pcm030 baseboard.
 
+config SND_MPC52xx_SOC_EFIKA
+	tristate "SoC AC97 Audio support for bbplan Efika and STAC9766"
+	depends on PPC_EFIKA
+	select SND_SOC_MPC5200_AC97
+	select SND_SOC_STAC9766
+	help
+	  Say Y if you want to add support for sound on the Efika.
+
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 66d88c8..a83a739 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -17,4 +17,5 @@ obj-$(CONFIG_SND_SOC_MPC5200_AC97) += mpc5200_psc_ac97.o
 
 # MPC5200 Machine Support
 obj-$(CONFIG_SND_MPC52xx_SOC_PCM030) += pcm030-audio-fabric.o
+obj-$(CONFIG_SND_MPC52xx_SOC_EFIKA) += efika-audio-fabric.o
 
diff --git a/sound/soc/fsl/efika-audio-fabric.c b/sound/soc/fsl/efika-audio-fabric.c
new file mode 100644
index 0000000..85b0e75
--- /dev/null
+++ b/sound/soc/fsl/efika-audio-fabric.c
@@ -0,0 +1,90 @@
+/*
+ * Efika driver for the PSC of the Freescale MPC52xx
+ * configured as AC97 interface
+ *
+ * Copyright 2008 Jon Smirl, Digispeaker
+ * Author: Jon Smirl <jonsmirl@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/dma-mapping.h>
+
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+#include <sound/soc-of-simple.h>
+
+#include "mpc5200_dma.h"
+#include "mpc5200_psc_ac97.h"
+#include "../codecs/stac9766.h"
+
+static struct snd_soc_device device;
+static struct snd_soc_card card;
+
+static struct snd_soc_dai_link efika_fabric_dai[] = {
+{
+	.name = "AC97",
+	.stream_name = "AC97 Analog",
+	.codec_dai = &stac9766_dai[STAC9766_DAI_AC97_ANALOG],
+	.cpu_dai = &psc_ac97_dai[MPC5200_AC97_NORMAL],
+},
+{
+	.name = "AC97",
+	.stream_name = "AC97 IEC958",
+	.codec_dai = &stac9766_dai[STAC9766_DAI_AC97_DIGITAL],
+	.cpu_dai = &psc_ac97_dai[MPC5200_AC97_SPDIF],
+},
+};
+
+static __init int efika_fabric_init(void)
+{
+	struct platform_device *pdev;
+	int rc;
+
+	if (!machine_is_compatible("bplan,efika"))
+		return -ENODEV;
+
+	card.platform = &mpc5200_audio_dma_platform;
+	card.name = "Efika";
+	card.dai_link = efika_fabric_dai;
+	card.num_links = ARRAY_SIZE(efika_fabric_dai);
+
+	device.card = &card;
+	device.codec_dev = &soc_codec_dev_stac9766;
+
+	pdev = platform_device_alloc("soc-audio", 1);
+	if (!pdev) {
+		pr_err("efika_fabric_init: platform_device_alloc() failed\n");
+		return -ENODEV;
+	}
+
+	platform_set_drvdata(pdev, &device);
+	device.dev = &pdev->dev;
+
+	rc = platform_device_add(pdev);
+	if (rc) {
+		pr_err("efika_fabric_init: platform_device_add() failed\n");
+		return -ENODEV;
+	}
+	return 0;
+}
+
+module_init(efika_fabric_init);
+
+
+MODULE_AUTHOR("Jon Smirl <jonsmirl@gmail.com>");
+MODULE_DESCRIPTION(DRV_NAME ": mpc5200 Efika fabric driver");
+MODULE_LICENSE("GPL");
+

^ permalink raw reply related

* [PATCH V5 4/5] Support for AC97 on Phytec pmc030 base board.
From: Jon Smirl @ 2009-05-26 12:34 UTC (permalink / raw)
  To: grant.likely, linuxppc-dev, alsa-devel, broonie, timur
In-Reply-To: <20090526123235.17472.73912.stgit@terra>

Support for AC97 on Phytec pmc030 base board. A wm9712 AC97 codec is used.

Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
 sound/soc/fsl/Kconfig               |    7 +++
 sound/soc/fsl/Makefile              |    3 +
 sound/soc/fsl/pcm030-audio-fabric.c |   90 +++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/fsl/pcm030-audio-fabric.c

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 3bce952..79579ae 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -39,4 +39,11 @@ config SND_SOC_MPC5200_AC97
 	help
 	  Say Y here to support the MPC5200 PSCs in AC97 mode.
 
+config SND_MPC52xx_SOC_PCM030
+	tristate "SoC AC97 Audio support for Phytec pcm030 and WM9712"
+	depends on PPC_MPC5200_SIMPLE
+	select SND_SOC_MPC5200_AC97
+	select SND_SOC_WM9712
+	help
+	  Say Y if you want to add support for sound on the Phytec pcm030 baseboard.
 
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 14631a1..66d88c8 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -15,3 +15,6 @@ obj-$(CONFIG_SND_MPC52xx_DMA) += mpc5200_dma.o
 obj-$(CONFIG_SND_SOC_MPC5200_I2S) += mpc5200_psc_i2s.o
 obj-$(CONFIG_SND_SOC_MPC5200_AC97) += mpc5200_psc_ac97.o
 
+# MPC5200 Machine Support
+obj-$(CONFIG_SND_MPC52xx_SOC_PCM030) += pcm030-audio-fabric.o
+
diff --git a/sound/soc/fsl/pcm030-audio-fabric.c b/sound/soc/fsl/pcm030-audio-fabric.c
new file mode 100644
index 0000000..8766f7a
--- /dev/null
+++ b/sound/soc/fsl/pcm030-audio-fabric.c
@@ -0,0 +1,90 @@
+/*
+ * Phytec pcm030 driver for the PSC of the Freescale MPC52xx
+ * configured as AC97 interface
+ *
+ * Copyright 2008 Jon Smirl, Digispeaker
+ * Author: Jon Smirl <jonsmirl@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/dma-mapping.h>
+
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+#include <sound/soc-of-simple.h>
+
+#include "mpc5200_dma.h"
+#include "mpc5200_psc_ac97.h"
+#include "../codecs/wm9712.h"
+
+static struct snd_soc_device device;
+static struct snd_soc_card card;
+
+static struct snd_soc_dai_link pcm030_fabric_dai[] = {
+{
+	.name = "AC97",
+	.stream_name = "AC97 Analog",
+	.codec_dai = &wm9712_dai[WM9712_DAI_AC97_HIFI],
+	.cpu_dai = &psc_ac97_dai[MPC5200_AC97_NORMAL],
+},
+{
+	.name = "AC97",
+	.stream_name = "AC97 IEC958",
+	.codec_dai = &wm9712_dai[WM9712_DAI_AC97_AUX],
+	.cpu_dai = &psc_ac97_dai[MPC5200_AC97_SPDIF],
+},
+};
+
+static __init int pcm030_fabric_init(void)
+{
+	struct platform_device *pdev;
+	int rc;
+
+	if (!machine_is_compatible("phytec,pcm030"))
+		return -ENODEV;
+
+	card.platform = &mpc5200_audio_dma_platform;
+	card.name = "pcm030";
+	card.dai_link = pcm030_fabric_dai;
+	card.num_links = ARRAY_SIZE(pcm030_fabric_dai);
+
+	device.card = &card;
+	device.codec_dev = &soc_codec_dev_wm9712;
+
+	pdev = platform_device_alloc("soc-audio", 1);
+	if (!pdev) {
+		pr_err("pcm030_fabric_init: platform_device_alloc() failed\n");
+		return -ENODEV;
+	}
+
+	platform_set_drvdata(pdev, &device);
+	device.dev = &pdev->dev;
+
+	rc = platform_device_add(pdev);
+	if (rc) {
+		pr_err("pcm030_fabric_init: platform_device_add() failed\n");
+		return -ENODEV;
+	}
+	return 0;
+}
+
+module_init(pcm030_fabric_init);
+
+
+MODULE_AUTHOR("Jon Smirl <jonsmirl@gmail.com>");
+MODULE_DESCRIPTION(DRV_NAME ": mpc5200 pcm030 fabric driver");
+MODULE_LICENSE("GPL");
+

^ permalink raw reply related

* [PATCH V5 3/5] AC97 driver for mpc5200
From: Jon Smirl @ 2009-05-26 12:34 UTC (permalink / raw)
  To: grant.likely, linuxppc-dev, alsa-devel, broonie, timur
In-Reply-To: <20090526123235.17472.73912.stgit@terra>

AC97 driver for mpc5200

I've implemented retries for when the AC97 hardware doesn't reset on
first try. About 10% of the time both the Efika and pcm030 AC97 codecs
don't reset on first try and need to be poked multiple times.  Failure
is indicated by not having the link clock start ticking. Every once in
a while even five pokes won't get the link started and I have to power
cycle.

Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
 sound/soc/fsl/Kconfig            |   11 +
 sound/soc/fsl/Makefile           |    1 
 sound/soc/fsl/mpc5200_psc_ac97.c |  331 ++++++++++++++++++++++++++++++++++++++
 sound/soc/fsl/mpc5200_psc_ac97.h |   15 ++
 4 files changed, 358 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/fsl/mpc5200_psc_ac97.c
 create mode 100644 sound/soc/fsl/mpc5200_psc_ac97.h

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 1918c78..3bce952 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -29,3 +29,14 @@ config SND_SOC_MPC5200_I2S
 	select PPC_BESTCOMM_GEN_BD
 	help
 	  Say Y here to support the MPC5200 PSCs in I2S mode.
+
+config SND_SOC_MPC5200_AC97
+	tristate "Freescale MPC5200 PSC in AC97 mode driver"
+	depends on PPC_MPC52xx && PPC_BESTCOMM
+	select AC97_BUS
+	select SND_MPC52xx_DMA
+	select PPC_BESTCOMM_GEN_BD
+	help
+	  Say Y here to support the MPC5200 PSCs in AC97 mode.
+
+
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 7731ef2..14631a1 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -13,4 +13,5 @@ obj-$(CONFIG_SND_SOC_MPC8610) += snd-soc-fsl-ssi.o snd-soc-fsl-dma.o
 # MPC5200 Platform Support
 obj-$(CONFIG_SND_MPC52xx_DMA) += mpc5200_dma.o
 obj-$(CONFIG_SND_SOC_MPC5200_I2S) += mpc5200_psc_i2s.o
+obj-$(CONFIG_SND_SOC_MPC5200_AC97) += mpc5200_psc_ac97.o
 
diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c
new file mode 100644
index 0000000..9f2df15
--- /dev/null
+++ b/sound/soc/fsl/mpc5200_psc_ac97.c
@@ -0,0 +1,331 @@
+/*
+ * linux/sound/mpc5200-ac97.c -- AC97 support for the Freescale MPC52xx chip.
+ *
+ * Copyright (C) 2009 Jon Smirl, Digispeaker
+ * Author: Jon Smirl <jonsmirl@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include <asm/time.h>
+#include <asm/delay.h>
+#include <asm/mpc52xx_psc.h>
+
+#include "mpc5200_dma.h"
+#include "mpc5200_psc_ac97.h"
+
+#define DRV_NAME "mpc5200-psc-ac97"
+
+/* ALSA only supports a single AC97 device so static is recommend here */
+static struct psc_dma *psc_dma;
+
+static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
+{
+	int rc;
+	unsigned int val;
+
+	/* Wait for command send status zero = ready */
+	spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
+				MPC52xx_PSC_SR_CMDSEND), 100, 0, rc);
+	if (rc == 0) {
+		pr_err("timeout on ac97 bus (rdy)\n");
+		return -ENODEV;
+	}
+	/* Send the read */
+	out_be32(&psc_dma->psc_regs->ac97_cmd, (1<<31) | ((reg & 0x7f) << 24));
+
+	/* Wait for the answer */
+	spin_event_timeout((in_be16(&psc_dma->psc_regs->sr_csr.status) &
+				MPC52xx_PSC_SR_DATA_VAL), 100, 0, rc);
+	if (rc == 0) {
+		pr_err("timeout on ac97 read (val) %x\n",
+				in_be16(&psc_dma->psc_regs->sr_csr.status));
+		return -ENODEV;
+	}
+	/* Get the data */
+	val = in_be32(&psc_dma->psc_regs->ac97_data);
+	if (((val >> 24) & 0x7f) != reg) {
+		pr_err("reg echo error on ac97 read\n");
+		return -ENODEV;
+	}
+	val = (val >> 8) & 0xffff;
+
+	return (unsigned short) val;
+}
+
+static void psc_ac97_write(struct snd_ac97 *ac97,
+				unsigned short reg, unsigned short val)
+{
+	int rc;
+
+	/* Wait for command status zero = ready */
+	spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
+				MPC52xx_PSC_SR_CMDSEND), 100, 0, rc);
+	if (rc == 0) {
+		pr_err("timeout on ac97 bus (write)\n");
+		return;
+	}
+	/* Write data */
+	out_be32(&psc_dma->psc_regs->ac97_cmd,
+			((reg & 0x7f) << 24) | (val << 8));
+}
+
+static void psc_ac97_warm_reset(struct snd_ac97 *ac97)
+{
+	int rc;
+	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
+
+	out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_AWR);
+	spin_event_timeout(0, 3, 0, rc);
+	out_be32(&regs->sicr, psc_dma->sicr);
+}
+
+static void psc_ac97_cold_reset(struct snd_ac97 *ac97)
+{
+	int rc;
+	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
+
+	/* Do a cold reset */
+	out_8(&regs->op1, MPC52xx_PSC_OP_RES);
+	spin_event_timeout(0, 10, 0, rc);
+	out_8(&regs->op0, MPC52xx_PSC_OP_RES);
+	spin_event_timeout(0, 50, 0, rc);
+	psc_ac97_warm_reset(ac97);
+}
+
+struct snd_ac97_bus_ops soc_ac97_ops = {
+	.read		= psc_ac97_read,
+	.write		= psc_ac97_write,
+	.reset		= psc_ac97_cold_reset,
+	.warm_reset	= psc_ac97_warm_reset,
+};
+EXPORT_SYMBOL_GPL(soc_ac97_ops);
+
+static int psc_ac97_hw_analog_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *params,
+				 struct snd_soc_dai *cpu_dai)
+{
+	struct psc_dma *psc_dma = cpu_dai->private_data;
+
+	dev_dbg(psc_dma->dev, "%s(substream=%p) p_size=%i p_bytes=%i"
+		" periods=%i buffer_size=%i  buffer_bytes=%i channels=%i"
+		" rate=%i format=%i\n",
+		__func__, substream, params_period_size(params),
+		params_period_bytes(params), params_periods(params),
+		params_buffer_size(params), params_buffer_bytes(params),
+		params_channels(params), params_rate(params),
+		params_format(params));
+
+
+	if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		if (params_channels(params) == 1)
+			psc_dma->slots |= 0x00000100;
+		else
+			psc_dma->slots |= 0x00000300;
+	} else {
+		if (params_channels(params) == 1)
+			psc_dma->slots |= 0x01000000;
+		else
+			psc_dma->slots |= 0x03000000;
+	}
+	out_be32(&psc_dma->psc_regs->ac97_slots, psc_dma->slots);
+
+	return 0;
+}
+
+static int psc_ac97_hw_digital_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *params,
+				 struct snd_soc_dai *cpu_dai)
+{
+	struct psc_dma *psc_dma = cpu_dai->private_data;
+
+	if (params_channels(params) == 1)
+		out_be32(&psc_dma->psc_regs->ac97_slots, 0x01000000);
+	else
+		out_be32(&psc_dma->psc_regs->ac97_slots, 0x03000000);
+
+	return 0;
+}
+
+static int psc_ac97_trigger(struct snd_pcm_substream *substream, int cmd,
+							struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_STOP:
+		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
+			psc_dma->slots &= 0xFFFF0000;
+		else
+			psc_dma->slots &= 0x0000FFFF;
+
+		out_be32(&psc_dma->psc_regs->ac97_slots, psc_dma->slots);
+		break;
+	}
+	return 0;
+}
+
+static int psc_ac97_probe(struct platform_device *pdev,
+					struct snd_soc_dai *cpu_dai)
+{
+	struct psc_dma *psc_dma = cpu_dai->private_data;
+	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
+
+	/* Go */
+	out_8(&regs->command, MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE);
+	return 0;
+}
+
+/* ---------------------------------------------------------------------
+ * ALSA SoC Bindings
+ *
+ * - Digital Audio Interface (DAI) template
+ * - create/destroy dai hooks
+ */
+
+/**
+ * psc_ac97_dai_template: template CPU Digital Audio Interface
+ */
+static struct snd_soc_dai_ops psc_ac97_analog_ops = {
+	.hw_params	= psc_ac97_hw_analog_params,
+	.trigger	= psc_ac97_trigger,
+};
+
+static struct snd_soc_dai_ops psc_ac97_digital_ops = {
+	.hw_params	= psc_ac97_hw_digital_params,
+};
+
+struct snd_soc_dai psc_ac97_dai[] = {
+{
+	.name   = "AC97",
+	.ac97_control = 1,
+	.probe	= psc_ac97_probe,
+	.playback = {
+		.channels_min   = 1,
+		.channels_max   = 6,
+		.rates          = SNDRV_PCM_RATE_8000_48000,
+		.formats = SNDRV_PCM_FMTBIT_S32_BE,
+	},
+	.capture = {
+		.channels_min   = 1,
+		.channels_max   = 2,
+		.rates          = SNDRV_PCM_RATE_8000_48000,
+		.formats = SNDRV_PCM_FMTBIT_S32_BE,
+	},
+	.ops = &psc_ac97_analog_ops,
+},
+{
+	.name   = "SPDIF",
+	.ac97_control = 1,
+	.playback = {
+		.channels_min   = 1,
+		.channels_max   = 2,
+		.rates          = SNDRV_PCM_RATE_32000 | \
+			SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_BE,
+	},
+	.ops = &psc_ac97_digital_ops,
+} };
+EXPORT_SYMBOL_GPL(psc_ac97_dai);
+
+
+
+/* ---------------------------------------------------------------------
+ * OF platform bus binding code:
+ * - Probe/remove operations
+ * - OF device match table
+ */
+static int __devinit psc_ac97_of_probe(struct of_device *op,
+				      const struct of_device_id *match)
+{
+	int rc, i;
+	struct snd_ac97 ac97;
+	struct mpc52xx_psc __iomem *regs;
+
+	rc = mpc5200_audio_dma_create(op);
+	if (rc != 0)
+		return rc;
+
+	for (i = 0; i < ARRAY_SIZE(psc_ac97_dai); i++)
+		psc_ac97_dai[i].dev = &op->dev;
+
+	rc = snd_soc_register_dais(psc_ac97_dai, ARRAY_SIZE(psc_ac97_dai));
+	if (rc != 0) {
+		dev_err(&op->dev, "Failed to register DAI\n");
+		return rc;
+	}
+
+	psc_dma = dev_get_drvdata(&op->dev);
+	regs = psc_dma->psc_regs;
+	ac97.private_data = psc_dma;
+
+	for (i = 0; i < ARRAY_SIZE(psc_ac97_dai); i++)
+		psc_ac97_dai[i].private_data = psc_dma;
+
+	psc_dma->imr = 0;
+	out_be16(&psc_dma->psc_regs->isr_imr.imr, psc_dma->imr);
+
+	/* Configure the serial interface mode to AC97 */
+	psc_dma->sicr = MPC52xx_PSC_SICR_SIM_AC97 | MPC52xx_PSC_SICR_ENAC97;
+	out_be32(&regs->sicr, psc_dma->sicr);
+
+	/* No slots active */
+	out_be32(&regs->ac97_slots, 0x00000000);
+
+	return 0;
+}
+
+static int __devexit psc_ac97_of_remove(struct of_device *op)
+{
+	return mpc5200_audio_dma_destroy(op);
+}
+
+/* Match table for of_platform binding */
+static struct of_device_id psc_ac97_match[] __devinitdata = {
+	{ .compatible = "fsl,mpc5200-psc-ac97", },
+	{ .compatible = "fsl,mpc5200b-psc-ac97", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, psc_ac97_match);
+
+static struct of_platform_driver psc_ac97_driver = {
+	.match_table = psc_ac97_match,
+	.probe = psc_ac97_of_probe,
+	.remove = __devexit_p(psc_ac97_of_remove),
+	.driver = {
+		.name = "mpc5200-psc-ac97",
+		.owner = THIS_MODULE,
+	},
+};
+
+/* ---------------------------------------------------------------------
+ * Module setup and teardown; simply register the of_platform driver
+ * for the PSC in AC97 mode.
+ */
+static int __init psc_ac97_init(void)
+{
+	return of_register_platform_driver(&psc_ac97_driver);
+}
+module_init(psc_ac97_init);
+
+static void __exit psc_ac97_exit(void)
+{
+	of_unregister_platform_driver(&psc_ac97_driver);
+}
+module_exit(psc_ac97_exit);
+
+MODULE_AUTHOR("Jon Smirl <jonsmirl@gmail.com>");
+MODULE_DESCRIPTION("mpc5200 AC97 module");
+MODULE_LICENSE("GPL");
+
diff --git a/sound/soc/fsl/mpc5200_psc_ac97.h b/sound/soc/fsl/mpc5200_psc_ac97.h
new file mode 100644
index 0000000..4bc18c3
--- /dev/null
+++ b/sound/soc/fsl/mpc5200_psc_ac97.h
@@ -0,0 +1,15 @@
+/*
+ * Freescale MPC5200 PSC in AC97 mode
+ * ALSA SoC Digital Audio Interface (DAI) driver
+ *
+ */
+
+#ifndef __SOUND_SOC_FSL_MPC52xx_PSC_AC97_H__
+#define __SOUND_SOC_FSL_MPC52xx_PSC_AC97_H__
+
+extern struct snd_soc_dai psc_ac97_dai[];
+
+#define MPC5200_AC97_NORMAL 0
+#define MPC5200_AC97_SPDIF 1
+
+#endif /* __SOUND_SOC_FSL_MPC52xx_PSC_AC97_H__ */

^ permalink raw reply related

* [PATCH V5 2/5] Main rewite of the mpc5200 audio DMA code
From: Jon Smirl @ 2009-05-26 12:34 UTC (permalink / raw)
  To: grant.likely, linuxppc-dev, alsa-devel, broonie, timur
In-Reply-To: <20090526123235.17472.73912.stgit@terra>

Rewrite the mpc5200 audio DMA code to support both I2S and AC97.

Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
 sound/soc/fsl/Kconfig           |    1 
 sound/soc/fsl/mpc5200_dma.c     |  442 ++++++++++++++++++++++++---------------
 sound/soc/fsl/mpc5200_dma.h     |   33 +--
 sound/soc/fsl/mpc5200_psc_i2s.c |  247 ++++------------------
 sound/soc/fsl/mpc5200_psc_i2s.h |   12 +
 5 files changed, 344 insertions(+), 391 deletions(-)
 create mode 100644 sound/soc/fsl/mpc5200_psc_i2s.h

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index dc79bdf..1918c78 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -25,7 +25,6 @@ config SND_SOC_MPC8610_HPCD
 config SND_SOC_MPC5200_I2S
 	tristate "Freescale MPC5200 PSC in I2S mode driver"
 	depends on PPC_MPC52xx && PPC_BESTCOMM
-	select SND_SOC_OF_SIMPLE
 	select SND_MPC52xx_DMA
 	select PPC_BESTCOMM_GEN_BD
 	help
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 6850392..efec33a 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -3,23 +3,13 @@
  * ALSA SoC Platform driver
  *
  * Copyright (C) 2008 Secret Lab Technologies Ltd.
+ * Copyright (C) 2009 Jon Smirl, Digispeaker
  */
 
-#include <linux/init.h>
 #include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/delay.h>
 #include <linux/of_device.h>
-#include <linux/of_platform.h>
-#include <linux/dma-mapping.h>
 
-#include <sound/core.h>
-#include <sound/pcm.h>
-#include <sound/pcm_params.h>
-#include <sound/initval.h>
 #include <sound/soc.h>
-#include <sound/soc-of-simple.h>
 
 #include <sysdev/bestcomm/bestcomm.h>
 #include <sysdev/bestcomm/gen_bd.h>
@@ -27,10 +17,6 @@
 
 #include "mpc5200_dma.h"
 
-MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
-MODULE_DESCRIPTION("Freescale MPC5200 PSC in DMA mode ASoC Driver");
-MODULE_LICENSE("GPL");
-
 /*
  * Interrupt handlers
  */
@@ -50,7 +36,7 @@ static irqreturn_t psc_dma_status_irq(int irq, void *_psc_dma)
 	if (psc_dma->capture.active && (isr & MPC52xx_PSC_IMR_ORERR))
 		psc_dma->stats.overrun_count++;
 
-	out_8(&regs->command, 4 << 4);	/* reset the error status */
+	out_8(&regs->command, MPC52xx_PSC_RST_ERR_STAT);
 
 	return IRQ_HANDLED;
 }
@@ -81,21 +67,36 @@ static void psc_dma_bcom_enqueue_next_buffer(struct psc_dma_stream *s)
 		s->period_next_pt = s->period_start;
 }
 
+static void psc_dma_bcom_enqueue_tx(struct psc_dma_stream *s)
+{
+	while (s->appl_ptr < s->runtime->control->appl_ptr) {
+
+		if (bcom_queue_full(s->bcom_task))
+			return;
+
+		s->appl_ptr += s->period_size;
+
+		psc_dma_bcom_enqueue_next_buffer(s);
+	}
+}
+
 /* Bestcomm DMA irq handler */
-static irqreturn_t psc_dma_bcom_irq(int irq, void *_psc_dma_stream)
+static irqreturn_t psc_dma_bcom_irq_tx(int irq, void *_psc_dma_stream)
 {
 	struct psc_dma_stream *s = _psc_dma_stream;
 
+	spin_lock(&s->psc_dma->lock);
 	/* For each finished period, dequeue the completed period buffer
 	 * and enqueue a new one in it's place. */
 	while (bcom_buffer_done(s->bcom_task)) {
 		bcom_retrieve_buffer(s->bcom_task, NULL, NULL);
+
 		s->period_current_pt += s->period_bytes;
 		if (s->period_current_pt >= s->period_end)
 			s->period_current_pt = s->period_start;
-		psc_dma_bcom_enqueue_next_buffer(s);
-		bcom_enable(s->bcom_task);
 	}
+	psc_dma_bcom_enqueue_tx(s);
+	spin_unlock(&s->psc_dma->lock);
 
 	/* If the stream is active, then also inform the PCM middle layer
 	 * of the period finished event. */
@@ -105,49 +106,33 @@ static irqreturn_t psc_dma_bcom_irq(int irq, void *_psc_dma_stream)
 	return IRQ_HANDLED;
 }
 
-/**
- * psc_dma_startup: create a new substream
- *
- * This is the first function called when a stream is opened.
- *
- * If this is the first stream open, then grab the IRQ and program most of
- * the PSC registers.
- */
-int psc_dma_startup(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai)
+static irqreturn_t psc_dma_bcom_irq_rx(int irq, void *_psc_dma_stream)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
-	int rc;
+	struct psc_dma_stream *s = _psc_dma_stream;
 
-	dev_dbg(psc_dma->dev, "psc_dma_startup(substream=%p)\n", substream);
+	spin_lock(&s->psc_dma->lock);
+	/* For each finished period, dequeue the completed period buffer
+	 * and enqueue a new one in it's place. */
+	while (bcom_buffer_done(s->bcom_task)) {
+		bcom_retrieve_buffer(s->bcom_task, NULL, NULL);
 
-	if (!psc_dma->playback.active &&
-	    !psc_dma->capture.active) {
-		/* Setup the IRQs */
-		rc = request_irq(psc_dma->irq, &psc_dma_status_irq, IRQF_SHARED,
-				 "psc-dma-status", psc_dma);
-		rc |= request_irq(psc_dma->capture.irq,
-				  &psc_dma_bcom_irq, IRQF_SHARED,
-				  "psc-dma-capture", &psc_dma->capture);
-		rc |= request_irq(psc_dma->playback.irq,
-				  &psc_dma_bcom_irq, IRQF_SHARED,
-				  "psc-dma-playback", &psc_dma->playback);
-		if (rc) {
-			free_irq(psc_dma->irq, psc_dma);
-			free_irq(psc_dma->capture.irq,
-				 &psc_dma->capture);
-			free_irq(psc_dma->playback.irq,
-				 &psc_dma->playback);
-			return -ENODEV;
-		}
+		s->period_current_pt += s->period_bytes;
+		if (s->period_current_pt >= s->period_end)
+			s->period_current_pt = s->period_start;
+
+		psc_dma_bcom_enqueue_next_buffer(s);
 	}
+	spin_unlock(&s->psc_dma->lock);
 
-	return 0;
+	/* If the stream is active, then also inform the PCM middle layer
+	 * of the period finished event. */
+	if (s->active)
+		snd_pcm_period_elapsed(s->stream);
+
+	return IRQ_HANDLED;
 }
 
-int psc_dma_hw_free(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai)
+static int psc_dma_hw_free(struct snd_pcm_substream *substream)
 {
 	snd_pcm_set_runtime_buffer(substream, NULL);
 	return 0;
@@ -159,8 +144,7 @@ int psc_dma_hw_free(struct snd_pcm_substream *substream,
  * This function is called by ALSA to start, stop, pause, and resume the DMA
  * transfer of data.
  */
-int psc_dma_trigger(struct snd_pcm_substream *substream, int cmd,
-			   struct snd_soc_dai *dai)
+static int psc_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
@@ -168,8 +152,8 @@ int psc_dma_trigger(struct snd_pcm_substream *substream, int cmd,
 	struct psc_dma_stream *s;
 	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
 	u16 imr;
-	u8 psc_cmd;
 	unsigned long flags;
+	int i;
 
 	if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
 		s = &psc_dma->capture;
@@ -189,68 +173,48 @@ int psc_dma_trigger(struct snd_pcm_substream *substream, int cmd,
 				(s->period_bytes * runtime->periods);
 		s->period_next_pt = s->period_start;
 		s->period_current_pt = s->period_start;
+		s->period_size = runtime->period_size;
 		s->active = 1;
 
-		/* First; reset everything */
+		/* track appl_ptr so that we have a better chance of detecting
+		 * end of stream and not over running it.
+		 */
+		s->runtime = runtime;
+		s->appl_ptr = s->runtime->control->appl_ptr -
+				(runtime->period_size * runtime->periods);
+
+		/* Fill up the bestcomm bd queue and enable DMA.
+		 * This will begin filling the PSC's fifo.
+		 */
+		spin_lock_irqsave(&psc_dma->lock, flags);
+
 		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE) {
-			out_8(&regs->command, MPC52xx_PSC_RST_RX);
-			out_8(&regs->command, MPC52xx_PSC_RST_ERR_STAT);
+			bcom_gen_bd_rx_reset(s->bcom_task);
+			for (i = 0; i < runtime->periods; i++)
+				if (!bcom_queue_full(s->bcom_task))
+					psc_dma_bcom_enqueue_next_buffer(s);
 		} else {
-			out_8(&regs->command, MPC52xx_PSC_RST_TX);
-			out_8(&regs->command, MPC52xx_PSC_RST_ERR_STAT);
+			bcom_gen_bd_tx_reset(s->bcom_task);
+			psc_dma_bcom_enqueue_tx(s);
 		}
 
-		/* Next, fill up the bestcomm bd queue and enable DMA.
-		 * This will begin filling the PSC's fifo. */
-		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
-			bcom_gen_bd_rx_reset(s->bcom_task);
-		else
-			bcom_gen_bd_tx_reset(s->bcom_task);
-		while (!bcom_queue_full(s->bcom_task))
-			psc_dma_bcom_enqueue_next_buffer(s);
 		bcom_enable(s->bcom_task);
-
-		/* Due to errata in the dma mode; need to line up enabling
-		 * the transmitter with a transition on the frame sync
-		 * line */
-
-		spin_lock_irqsave(&psc_dma->lock, flags);
-		/* first make sure it is low */
-		while ((in_8(&regs->ipcr_acr.ipcr) & 0x80) != 0)
-			;
-		/* then wait for the transition to high */
-		while ((in_8(&regs->ipcr_acr.ipcr) & 0x80) == 0)
-			;
-		/* Finally, enable the PSC.
-		 * Receiver must always be enabled; even when we only want
-		 * transmit.  (see 15.3.2.3 of MPC5200B User's Guide) */
-		psc_cmd = MPC52xx_PSC_RX_ENABLE;
-		if (substream->pstr->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			psc_cmd |= MPC52xx_PSC_TX_ENABLE;
-		out_8(&regs->command, psc_cmd);
 		spin_unlock_irqrestore(&psc_dma->lock, flags);
 
+		out_8(&regs->command, MPC52xx_PSC_RST_ERR_STAT);
+
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
-		/* Turn off the PSC */
 		s->active = 0;
-		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE) {
-			if (!psc_dma->playback.active) {
-				out_8(&regs->command, 2 << 4);	/* reset rx */
-				out_8(&regs->command, 3 << 4);	/* reset tx */
-				out_8(&regs->command, 4 << 4);	/* reset err */
-			}
-		} else {
-			out_8(&regs->command, 3 << 4);	/* reset tx */
-			out_8(&regs->command, 4 << 4);	/* reset err */
-			if (!psc_dma->capture.active)
-				out_8(&regs->command, 2 << 4);	/* reset rx */
-		}
 
+		spin_lock_irqsave(&psc_dma->lock, flags);
 		bcom_disable(s->bcom_task);
-		while (!bcom_queue_empty(s->bcom_task))
-			bcom_retrieve_buffer(s->bcom_task, NULL, NULL);
+		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
+			bcom_gen_bd_rx_reset(s->bcom_task);
+		else
+			bcom_gen_bd_tx_reset(s->bcom_task);
+		spin_unlock_irqrestore(&psc_dma->lock, flags);
 
 		break;
 
@@ -265,44 +229,11 @@ int psc_dma_trigger(struct snd_pcm_substream *substream, int cmd,
 		imr |= MPC52xx_PSC_IMR_TXEMP;
 	if (psc_dma->capture.active)
 		imr |= MPC52xx_PSC_IMR_ORERR;
-	out_be16(&regs->isr_imr.imr, imr);
+	out_be16(&regs->isr_imr.imr, psc_dma->imr | imr);
 
 	return 0;
 }
 
-/**
- * psc_dma_shutdown: shutdown the data transfer on a stream
- *
- * Shutdown the PSC if there are no other substreams open.
- */
-void psc_dma_shutdown(struct snd_pcm_substream *substream,
-			     struct snd_soc_dai *dai)
-{
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
-
-	dev_dbg(psc_dma->dev, "psc_dma_shutdown(substream=%p)\n", substream);
-
-	/*
-	 * If this is the last active substream, disable the PSC and release
-	 * the IRQ.
-	 */
-	if (!psc_dma->playback.active &&
-	    !psc_dma->capture.active) {
-
-		/* Disable all interrupts and reset the PSC */
-		out_be16(&psc_dma->psc_regs->isr_imr.imr, 0);
-		out_8(&psc_dma->psc_regs->command, 3 << 4); /* reset tx */
-		out_8(&psc_dma->psc_regs->command, 2 << 4); /* reset rx */
-		out_8(&psc_dma->psc_regs->command, 1 << 4); /* reset mode */
-		out_8(&psc_dma->psc_regs->command, 4 << 4); /* reset error */
-
-		/* Release irqs */
-		free_irq(psc_dma->irq, psc_dma);
-		free_irq(psc_dma->capture.irq, &psc_dma->capture);
-		free_irq(psc_dma->playback.irq, &psc_dma->playback);
-	}
-}
 
 /* ---------------------------------------------------------------------
  * The PSC DMA 'ASoC platform' driver
@@ -312,62 +243,78 @@ void psc_dma_shutdown(struct snd_pcm_substream *substream,
  * interaction with the attached codec
  */
 
-static const struct snd_pcm_hardware psc_dma_pcm_hardware = {
+static const struct snd_pcm_hardware psc_dma_hardware = {
 	.info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		SNDRV_PCM_INFO_BATCH,
 	.formats = SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE |
-		   SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE,
+		SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE,
 	.rate_min = 8000,
 	.rate_max = 48000,
-	.channels_min = 2,
+	.channels_min = 1,
 	.channels_max = 2,
 	.period_bytes_max	= 1024 * 1024,
 	.period_bytes_min	= 32,
 	.periods_min		= 2,
 	.periods_max		= 256,
 	.buffer_bytes_max	= 2 * 1024 * 1024,
-	.fifo_size		= 0,
+	.fifo_size		= 512,
 };
 
-static int psc_dma_pcm_open(struct snd_pcm_substream *substream)
+static int psc_dma_open(struct snd_pcm_substream *substream)
 {
+	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
 	struct psc_dma_stream *s;
+	int rc;
 
-	dev_dbg(psc_dma->dev, "psc_dma_pcm_open(substream=%p)\n", substream);
+	dev_dbg(psc_dma->dev, "psc_dma_open(substream=%p)\n", substream);
 
 	if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
 		s = &psc_dma->capture;
 	else
 		s = &psc_dma->playback;
 
-	snd_soc_set_runtime_hwparams(substream, &psc_dma_pcm_hardware);
+	snd_soc_set_runtime_hwparams(substream, &psc_dma_hardware);
+
+	rc = snd_pcm_hw_constraint_integer(runtime,
+		SNDRV_PCM_HW_PARAM_PERIODS);
+	if (rc < 0) {
+		dev_err(substream->pcm->card->dev, "invalid buffer size\n");
+		return rc;
+	}
 
 	s->stream = substream;
 	return 0;
 }
 
-static int psc_dma_pcm_close(struct snd_pcm_substream *substream)
+static int psc_dma_close(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
 	struct psc_dma_stream *s;
 
-	dev_dbg(psc_dma->dev, "psc_dma_pcm_close(substream=%p)\n", substream);
+	dev_dbg(psc_dma->dev, "psc_dma_close(substream=%p)\n", substream);
 
 	if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
 		s = &psc_dma->capture;
 	else
 		s = &psc_dma->playback;
 
+	if (!psc_dma->playback.active &&
+	    !psc_dma->capture.active) {
+
+		/* Disable all interrupts and reset the PSC */
+		out_be16(&psc_dma->psc_regs->isr_imr.imr, psc_dma->imr);
+		out_8(&psc_dma->psc_regs->command, 4 << 4); /* reset error */
+	}
 	s->stream = NULL;
 	return 0;
 }
 
 static snd_pcm_uframes_t
-psc_dma_pcm_pointer(struct snd_pcm_substream *substream)
+psc_dma_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
@@ -384,60 +331,78 @@ psc_dma_pcm_pointer(struct snd_pcm_substream *substream)
 	return bytes_to_frames(substream->runtime, count);
 }
 
-static struct snd_pcm_ops psc_dma_pcm_ops = {
-	.open		= psc_dma_pcm_open,
-	.close		= psc_dma_pcm_close,
+static int
+psc_dma_hw_params(struct snd_pcm_substream *substream,
+			 struct snd_pcm_hw_params *params)
+{
+	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
+
+	return 0;
+}
+
+static struct snd_pcm_ops psc_dma_ops = {
+	.open		= psc_dma_open,
+	.close		= psc_dma_close,
+	.hw_free	= psc_dma_hw_free,
 	.ioctl		= snd_pcm_lib_ioctl,
-	.pointer	= psc_dma_pcm_pointer,
+	.pointer	= psc_dma_pointer,
+	.trigger	= psc_dma_trigger,
+	.hw_params	= psc_dma_hw_params,
 };
 
-static u64 psc_dma_pcm_dmamask = 0xffffffff;
-static int psc_dma_pcm_new(struct snd_card *card, struct snd_soc_dai *dai,
+static u64 psc_dma_dmamask = 0xffffffff;
+static int psc_dma_new(struct snd_card *card, struct snd_soc_dai *dai,
 			   struct snd_pcm *pcm)
 {
 	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
-	size_t size = psc_dma_pcm_hardware.buffer_bytes_max;
+	struct psc_dma *psc_dma = rtd->dai->cpu_dai->private_data;
+	size_t size = psc_dma_hardware.buffer_bytes_max;
 	int rc = 0;
 
-	dev_dbg(rtd->socdev->dev, "psc_dma_pcm_new(card=%p, dai=%p, pcm=%p)\n",
+	dev_dbg(rtd->socdev->dev, "psc_dma_new(card=%p, dai=%p, pcm=%p)\n",
 		card, dai, pcm);
 
 	if (!card->dev->dma_mask)
-		card->dev->dma_mask = &psc_dma_pcm_dmamask;
+		card->dev->dma_mask = &psc_dma_dmamask;
 	if (!card->dev->coherent_dma_mask)
 		card->dev->coherent_dma_mask = 0xffffffff;
 
 	if (pcm->streams[0].substream) {
-		rc = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, size,
-					&pcm->streams[0].substream->dma_buffer);
+		rc = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->card->dev,
+				size, &pcm->streams[0].substream->dma_buffer);
 		if (rc)
 			goto playback_alloc_err;
 	}
 
 	if (pcm->streams[1].substream) {
-		rc = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, size,
-					&pcm->streams[1].substream->dma_buffer);
+		rc = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->card->dev,
+				size, &pcm->streams[1].substream->dma_buffer);
 		if (rc)
 			goto capture_alloc_err;
 	}
 
+	if (rtd->socdev->card->codec->ac97)
+		rtd->socdev->card->codec->ac97->private_data = psc_dma;
+
 	return 0;
 
  capture_alloc_err:
 	if (pcm->streams[0].substream)
 		snd_dma_free_pages(&pcm->streams[0].substream->dma_buffer);
+
  playback_alloc_err:
 	dev_err(card->dev, "Cannot allocate buffer(s)\n");
+
 	return -ENOMEM;
 }
 
-static void psc_dma_pcm_free(struct snd_pcm *pcm)
+static void psc_dma_free(struct snd_pcm *pcm)
 {
 	struct snd_soc_pcm_runtime *rtd = pcm->private_data;
 	struct snd_pcm_substream *substream;
 	int stream;
 
-	dev_dbg(rtd->socdev->dev, "psc_dma_pcm_free(pcm=%p)\n", pcm);
+	dev_dbg(rtd->socdev->dev, "psc_dma_free(pcm=%p)\n", pcm);
 
 	for (stream = 0; stream < 2; stream++) {
 		substream = pcm->streams[stream].substream;
@@ -449,10 +414,151 @@ static void psc_dma_pcm_free(struct snd_pcm *pcm)
 	}
 }
 
-struct snd_soc_platform psc_dma_pcm_soc_platform = {
+struct snd_soc_platform mpc5200_audio_dma_platform = {
 	.name		= "mpc5200-psc-audio",
-	.pcm_ops	= &psc_dma_pcm_ops,
-	.pcm_new	= &psc_dma_pcm_new,
-	.pcm_free	= &psc_dma_pcm_free,
+	.pcm_ops	= &psc_dma_ops,
+	.pcm_new	= &psc_dma_new,
+	.pcm_free	= &psc_dma_free,
 };
+EXPORT_SYMBOL_GPL(mpc5200_audio_dma_platform);
+
+int mpc5200_audio_dma_create(struct of_device *op)
+{
+	phys_addr_t fifo;
+	struct psc_dma *psc_dma;
+	struct resource res;
+	int size, irq, rc;
+	const __be32 *prop;
+	void __iomem *regs;
+
+	/* Fetch the registers and IRQ of the PSC */
+	irq = irq_of_parse_and_map(op->node, 0);
+	if (of_address_to_resource(op->node, 0, &res)) {
+		dev_err(&op->dev, "Missing reg property\n");
+		return -ENODEV;
+	}
+	regs = ioremap(res.start, 1 + res.end - res.start);
+	if (!regs) {
+		dev_err(&op->dev, "Could not map registers\n");
+		return -ENODEV;
+	}
+
+	/* Allocate and initialize the driver private data */
+	psc_dma = kzalloc(sizeof *psc_dma, GFP_KERNEL);
+	if (!psc_dma) {
+		iounmap(regs);
+		return -ENOMEM;
+	}
+
+	/* Get the PSC ID */
+	prop = of_get_property(op->node, "cell-index", &size);
+	if (!prop || size < sizeof *prop)
+		return -ENODEV;
+
+	spin_lock_init(&psc_dma->lock);
+	psc_dma->id = be32_to_cpu(*prop);
+	psc_dma->irq = irq;
+	psc_dma->psc_regs = regs;
+	psc_dma->fifo_regs = regs + sizeof *psc_dma->psc_regs;
+	psc_dma->dev = &op->dev;
+	psc_dma->playback.psc_dma = psc_dma;
+	psc_dma->capture.psc_dma = psc_dma;
+	snprintf(psc_dma->name, sizeof psc_dma->name, "PSC%u", psc_dma->id);
+
+	/* Find the address of the fifo data registers and setup the
+	 * DMA tasks */
+	fifo = res.start + offsetof(struct mpc52xx_psc, buffer.buffer_32);
+	psc_dma->capture.bcom_task =
+		bcom_psc_gen_bd_rx_init(psc_dma->id, 10, fifo, 512);
+	psc_dma->playback.bcom_task =
+		bcom_psc_gen_bd_tx_init(psc_dma->id, 10, fifo);
+	if (!psc_dma->capture.bcom_task ||
+	    !psc_dma->playback.bcom_task) {
+		dev_err(&op->dev, "Could not allocate bestcomm tasks\n");
+		iounmap(regs);
+		kfree(psc_dma);
+		return -ENODEV;
+	}
+
+	/* Disable all interrupts and reset the PSC */
+	out_be16(&psc_dma->psc_regs->isr_imr.imr, psc_dma->imr);
+	 /* reset receiver */
+	out_8(&psc_dma->psc_regs->command, MPC52xx_PSC_RST_RX);
+	 /* reset transmitter */
+	out_8(&psc_dma->psc_regs->command, MPC52xx_PSC_RST_TX);
+	 /* reset error */
+	out_8(&psc_dma->psc_regs->command, MPC52xx_PSC_RST_ERR_STAT);
+	 /* reset mode */
+	out_8(&psc_dma->psc_regs->command, MPC52xx_PSC_SEL_MODE_REG_1);
+
+	/* Set up mode register;
+	 * First write: RxRdy (FIFO Alarm) generates rx FIFO irq
+	 * Second write: register Normal mode for non loopback
+	 */
+	out_8(&psc_dma->psc_regs->mode, 0);
+	out_8(&psc_dma->psc_regs->mode, 0);
+
+	/* Set the TX and RX fifo alarm thresholds */
+	out_be16(&psc_dma->fifo_regs->rfalarm, 0x100);
+	out_8(&psc_dma->fifo_regs->rfcntl, 0x4);
+	out_be16(&psc_dma->fifo_regs->tfalarm, 0x100);
+	out_8(&psc_dma->fifo_regs->tfcntl, 0x7);
+
+	/* Lookup the IRQ numbers */
+	psc_dma->playback.irq =
+		bcom_get_task_irq(psc_dma->playback.bcom_task);
+	psc_dma->capture.irq =
+		bcom_get_task_irq(psc_dma->capture.bcom_task);
+
+	rc = request_irq(psc_dma->irq, &psc_dma_status_irq, IRQF_SHARED,
+			 "psc-dma-status", psc_dma);
+	rc |= request_irq(psc_dma->capture.irq,
+			  &psc_dma_bcom_irq_rx, IRQF_SHARED,
+			  "psc-dma-capture", &psc_dma->capture);
+	rc |= request_irq(psc_dma->playback.irq,
+			  &psc_dma_bcom_irq_tx, IRQF_SHARED,
+			  "psc-dma-playback", &psc_dma->playback);
+	if (rc) {
+		free_irq(psc_dma->irq, psc_dma);
+		free_irq(psc_dma->capture.irq,
+			 &psc_dma->capture);
+		free_irq(psc_dma->playback.irq,
+			 &psc_dma->playback);
+		return -ENODEV;
+	}
 
+	/* Save what we've done so it can be found again later */
+	dev_set_drvdata(&op->dev, psc_dma);
+
+	/* Tell the ASoC OF helpers about it */
+	return snd_soc_register_platform(&mpc5200_audio_dma_platform);
+}
+EXPORT_SYMBOL_GPL(mpc5200_audio_dma_create);
+
+int mpc5200_audio_dma_destroy(struct of_device *op)
+{
+	struct psc_dma *psc_dma = dev_get_drvdata(&op->dev);
+
+	dev_dbg(&op->dev, "mpc5200_audio_dma_destroy()\n");
+
+	snd_soc_unregister_platform(&mpc5200_audio_dma_platform);
+
+	bcom_gen_bd_rx_release(psc_dma->capture.bcom_task);
+	bcom_gen_bd_tx_release(psc_dma->playback.bcom_task);
+
+	/* Release irqs */
+	free_irq(psc_dma->irq, psc_dma);
+	free_irq(psc_dma->capture.irq, &psc_dma->capture);
+	free_irq(psc_dma->playback.irq, &psc_dma->playback);
+
+	iounmap(psc_dma->psc_regs);
+	kfree(psc_dma);
+	dev_set_drvdata(&op->dev, NULL);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mpc5200_audio_dma_destroy);
+
+MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
+MODULE_DESCRIPTION("Freescale MPC5200 PSC in DMA mode ASoC Driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/fsl/mpc5200_dma.h b/sound/soc/fsl/mpc5200_dma.h
index a33232c..2000803 100644
--- a/sound/soc/fsl/mpc5200_dma.h
+++ b/sound/soc/fsl/mpc5200_dma.h
@@ -5,8 +5,10 @@
 #ifndef __SOUND_SOC_FSL_MPC5200_DMA_H__
 #define __SOUND_SOC_FSL_MPC5200_DMA_H__
 
+#define PSC_STREAM_NAME_LEN 32
+
 /**
- * psc_dma_stream - Data specific to a single stream (playback or capture)
+ * psc_ac97_stream - Data specific to a single stream (playback or capture)
  * @active:		flag indicating if the stream is active
  * @psc_dma:		pointer back to parent psc_dma data structure
  * @bcom_task:		bestcomm task structure
@@ -17,6 +19,9 @@
  * @period_bytes:	size of DMA period in bytes
  */
 struct psc_dma_stream {
+	struct snd_pcm_runtime *runtime;
+	snd_pcm_uframes_t appl_ptr;
+
 	int active;
 	struct psc_dma *psc_dma;
 	struct bcom_task *bcom_task;
@@ -27,6 +32,7 @@ struct psc_dma_stream {
 	dma_addr_t period_next_pt;
 	dma_addr_t period_current_pt;
 	int period_bytes;
+	int period_size;
 };
 
 /**
@@ -48,9 +54,12 @@ struct psc_dma {
 	struct mpc52xx_psc_fifo __iomem *fifo_regs;
 	unsigned int irq;
 	struct device *dev;
-	struct snd_soc_dai dai;
 	spinlock_t lock;
 	u32 sicr;
+	uint sysclk;
+	int imr;
+	int id;
+	unsigned int slots;
 
 	/* per-stream data */
 	struct psc_dma_stream playback;
@@ -58,24 +67,14 @@ struct psc_dma {
 
 	/* Statistics */
 	struct {
-		int overrun_count;
-		int underrun_count;
+		unsigned long overrun_count;
+		unsigned long underrun_count;
 	} stats;
 };
 
+int mpc5200_audio_dma_create(struct of_device *op);
+int mpc5200_audio_dma_destroy(struct of_device *op);
 
-int psc_dma_startup(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai);
-
-int psc_dma_hw_free(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai);
-
-void psc_dma_shutdown(struct snd_pcm_substream *substream,
-			     struct snd_soc_dai *dai);
-
-int psc_dma_trigger(struct snd_pcm_substream *substream, int cmd,
-			   struct snd_soc_dai *dai);
-
-extern struct snd_soc_platform psc_dma_pcm_soc_platform;
+extern struct snd_soc_platform mpc5200_audio_dma_platform;
 
 #endif /* __SOUND_SOC_FSL_MPC5200_DMA_H__ */
diff --git a/sound/soc/fsl/mpc5200_psc_i2s.c b/sound/soc/fsl/mpc5200_psc_i2s.c
index 12a7917..ce8de90 100644
--- a/sound/soc/fsl/mpc5200_psc_i2s.c
+++ b/sound/soc/fsl/mpc5200_psc_i2s.c
@@ -3,34 +3,22 @@
  * ALSA SoC Digital Audio Interface (DAI) driver
  *
  * Copyright (C) 2008 Secret Lab Technologies Ltd.
+ * Copyright (C) 2009 Jon Smirl, Digispeaker
  */
 
-#include <linux/init.h>
 #include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/delay.h>
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
-#include <linux/dma-mapping.h>
 
-#include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
-#include <sound/initval.h>
 #include <sound/soc.h>
-#include <sound/soc-of-simple.h>
 
-#include <sysdev/bestcomm/bestcomm.h>
-#include <sysdev/bestcomm/gen_bd.h>
 #include <asm/mpc52xx_psc.h>
 
+#include "mpc5200_psc_i2s.h"
 #include "mpc5200_dma.h"
 
-MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
-MODULE_DESCRIPTION("Freescale MPC5200 PSC in I2S mode ASoC Driver");
-MODULE_LICENSE("GPL");
-
 /**
  * PSC_I2S_RATES: sample rates supported by the I2S
  *
@@ -46,8 +34,7 @@ MODULE_LICENSE("GPL");
  * PSC_I2S_FORMATS: audio formats supported by the PSC I2S mode
  */
 #define PSC_I2S_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE | \
-			 SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S24_BE | \
-			 SNDRV_PCM_FMTBIT_S32_BE)
+			 SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE)
 
 static int psc_i2s_hw_params(struct snd_pcm_substream *substream,
 				 struct snd_pcm_hw_params *params,
@@ -82,8 +69,6 @@ static int psc_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 	out_be32(&psc_dma->psc_regs->sicr, psc_dma->sicr | mode);
 
-	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
-
 	return 0;
 }
 
@@ -140,16 +125,13 @@ static int psc_i2s_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int format)
  * psc_i2s_dai_template: template CPU Digital Audio Interface
  */
 static struct snd_soc_dai_ops psc_i2s_dai_ops = {
-	.startup	= psc_dma_startup,
 	.hw_params	= psc_i2s_hw_params,
-	.hw_free	= psc_dma_hw_free,
-	.shutdown	= psc_dma_shutdown,
-	.trigger	= psc_dma_trigger,
 	.set_sysclk	= psc_i2s_set_sysclk,
 	.set_fmt	= psc_i2s_set_fmt,
 };
 
-static struct snd_soc_dai psc_i2s_dai_template = {
+struct snd_soc_dai psc_i2s_dai[] = {{
+	.name   = "I2S",
 	.playback = {
 		.channels_min = 2,
 		.channels_max = 2,
@@ -163,71 +145,8 @@ static struct snd_soc_dai psc_i2s_dai_template = {
 		.formats = PSC_I2S_FORMATS,
 	},
 	.ops = &psc_i2s_dai_ops,
-};
-
-/* ---------------------------------------------------------------------
- * Sysfs attributes for debugging
- */
-
-static ssize_t psc_i2s_status_show(struct device *dev,
-			   struct device_attribute *attr, char *buf)
-{
-	struct psc_dma *psc_dma = dev_get_drvdata(dev);
-
-	return sprintf(buf, "status=%.4x sicr=%.8x rfnum=%i rfstat=0x%.4x "
-			"tfnum=%i tfstat=0x%.4x\n",
-			in_be16(&psc_dma->psc_regs->sr_csr.status),
-			in_be32(&psc_dma->psc_regs->sicr),
-			in_be16(&psc_dma->fifo_regs->rfnum) & 0x1ff,
-			in_be16(&psc_dma->fifo_regs->rfstat),
-			in_be16(&psc_dma->fifo_regs->tfnum) & 0x1ff,
-			in_be16(&psc_dma->fifo_regs->tfstat));
-}
-
-static int *psc_i2s_get_stat_attr(struct psc_dma *psc_dma, const char *name)
-{
-	if (strcmp(name, "playback_underrun") == 0)
-		return &psc_dma->stats.underrun_count;
-	if (strcmp(name, "capture_overrun") == 0)
-		return &psc_dma->stats.overrun_count;
-
-	return NULL;
-}
-
-static ssize_t psc_i2s_stat_show(struct device *dev,
-				 struct device_attribute *attr, char *buf)
-{
-	struct psc_dma *psc_dma = dev_get_drvdata(dev);
-	int *attrib;
-
-	attrib = psc_i2s_get_stat_attr(psc_dma, attr->attr.name);
-	if (!attrib)
-		return 0;
-
-	return sprintf(buf, "%i\n", *attrib);
-}
-
-static ssize_t psc_i2s_stat_store(struct device *dev,
-				  struct device_attribute *attr,
-				  const char *buf,
-				  size_t count)
-{
-	struct psc_dma *psc_dma = dev_get_drvdata(dev);
-	int *attrib;
-
-	attrib = psc_i2s_get_stat_attr(psc_dma, attr->attr.name);
-	if (!attrib)
-		return 0;
-
-	*attrib = simple_strtoul(buf, NULL, 0);
-	return count;
-}
-
-static DEVICE_ATTR(status, 0644, psc_i2s_status_show, NULL);
-static DEVICE_ATTR(playback_underrun, 0644, psc_i2s_stat_show,
-			psc_i2s_stat_store);
-static DEVICE_ATTR(capture_overrun, 0644, psc_i2s_stat_show,
-			psc_i2s_stat_store);
+} };
+EXPORT_SYMBOL_GPL(psc_i2s_dai);
 
 /* ---------------------------------------------------------------------
  * OF platform bus binding code:
@@ -237,82 +156,26 @@ static DEVICE_ATTR(capture_overrun, 0644, psc_i2s_stat_show,
 static int __devinit psc_i2s_of_probe(struct of_device *op,
 				      const struct of_device_id *match)
 {
-	phys_addr_t fifo;
+	int rc;
 	struct psc_dma *psc_dma;
-	struct resource res;
-	int size, psc_id, irq, rc;
-	const __be32 *prop;
-	void __iomem *regs;
-
-	dev_dbg(&op->dev, "probing psc i2s device\n");
-
-	/* Get the PSC ID */
-	prop = of_get_property(op->node, "cell-index", &size);
-	if (!prop || size < sizeof *prop)
-		return -ENODEV;
-	psc_id = be32_to_cpu(*prop);
-
-	/* Fetch the registers and IRQ of the PSC */
-	irq = irq_of_parse_and_map(op->node, 0);
-	if (of_address_to_resource(op->node, 0, &res)) {
-		dev_err(&op->dev, "Missing reg property\n");
-		return -ENODEV;
-	}
-	regs = ioremap(res.start, 1 + res.end - res.start);
-	if (!regs) {
-		dev_err(&op->dev, "Could not map registers\n");
-		return -ENODEV;
-	}
+	struct mpc52xx_psc __iomem *regs;
 
-	/* Allocate and initialize the driver private data */
-	psc_dma = kzalloc(sizeof *psc_dma, GFP_KERNEL);
-	if (!psc_dma) {
-		iounmap(regs);
-		return -ENOMEM;
-	}
-	spin_lock_init(&psc_dma->lock);
-	psc_dma->irq = irq;
-	psc_dma->psc_regs = regs;
-	psc_dma->fifo_regs = regs + sizeof *psc_dma->psc_regs;
-	psc_dma->dev = &op->dev;
-	psc_dma->playback.psc_dma = psc_dma;
-	psc_dma->capture.psc_dma = psc_dma;
-	snprintf(psc_dma->name, sizeof psc_dma->name, "PSC%u", psc_id+1);
-
-	/* Fill out the CPU DAI structure */
-	memcpy(&psc_dma->dai, &psc_i2s_dai_template, sizeof psc_dma->dai);
-	psc_dma->dai.private_data = psc_dma;
-	psc_dma->dai.name = psc_dma->name;
-	psc_dma->dai.id = psc_id;
-
-	/* Find the address of the fifo data registers and setup the
-	 * DMA tasks */
-	fifo = res.start + offsetof(struct mpc52xx_psc, buffer.buffer_32);
-	psc_dma->capture.bcom_task =
-		bcom_psc_gen_bd_rx_init(psc_id, 10, fifo, 512);
-	psc_dma->playback.bcom_task =
-		bcom_psc_gen_bd_tx_init(psc_id, 10, fifo);
-	if (!psc_dma->capture.bcom_task ||
-	    !psc_dma->playback.bcom_task) {
-		dev_err(&op->dev, "Could not allocate bestcomm tasks\n");
-		iounmap(regs);
-		kfree(psc_dma);
-		return -ENODEV;
+	rc = mpc5200_audio_dma_create(op);
+	if (rc != 0)
+		return rc;
+
+	rc = snd_soc_register_dais(psc_i2s_dai, ARRAY_SIZE(psc_i2s_dai));
+	if (rc != 0) {
+		pr_err("Failed to register DAI\n");
+		return 0;
 	}
 
-	/* Disable all interrupts and reset the PSC */
-	out_be16(&psc_dma->psc_regs->isr_imr.imr, 0);
-	out_8(&psc_dma->psc_regs->command, 3 << 4); /* reset transmitter */
-	out_8(&psc_dma->psc_regs->command, 2 << 4); /* reset receiver */
-	out_8(&psc_dma->psc_regs->command, 1 << 4); /* reset mode */
-	out_8(&psc_dma->psc_regs->command, 4 << 4); /* reset error */
+	psc_dma = dev_get_drvdata(&op->dev);
+	regs = psc_dma->psc_regs;
 
 	/* Configure the serial interface mode; defaulting to CODEC8 mode */
 	psc_dma->sicr = MPC52xx_PSC_SICR_DTS1 | MPC52xx_PSC_SICR_I2S |
 			MPC52xx_PSC_SICR_CLKPOL;
-	if (of_get_property(op->node, "fsl,cellslave", NULL))
-		psc_dma->sicr |= MPC52xx_PSC_SICR_CELLSLAVE |
-				 MPC52xx_PSC_SICR_GENCLK;
 	out_be32(&psc_dma->psc_regs->sicr,
 		 psc_dma->sicr | MPC52xx_PSC_SICR_SIM_CODEC_8);
 
@@ -321,66 +184,37 @@ static int __devinit psc_i2s_of_probe(struct of_device *op,
 	if (!of_get_property(op->node, "codec-handle", NULL))
 		return 0;
 
-	/* Set up mode register;
-	 * First write: RxRdy (FIFO Alarm) generates rx FIFO irq
-	 * Second write: register Normal mode for non loopback
-	 */
-	out_8(&psc_dma->psc_regs->mode, 0);
-	out_8(&psc_dma->psc_regs->mode, 0);
-
-	/* Set the TX and RX fifo alarm thresholds */
-	out_be16(&psc_dma->fifo_regs->rfalarm, 0x100);
-	out_8(&psc_dma->fifo_regs->rfcntl, 0x4);
-	out_be16(&psc_dma->fifo_regs->tfalarm, 0x100);
-	out_8(&psc_dma->fifo_regs->tfcntl, 0x7);
-
-	/* Lookup the IRQ numbers */
-	psc_dma->playback.irq =
-		bcom_get_task_irq(psc_dma->playback.bcom_task);
-	psc_dma->capture.irq =
-		bcom_get_task_irq(psc_dma->capture.bcom_task);
-
-	/* Save what we've done so it can be found again later */
-	dev_set_drvdata(&op->dev, psc_dma);
-
-	/* Register the SYSFS files */
-	rc = device_create_file(psc_dma->dev, &dev_attr_status);
-	rc |= device_create_file(psc_dma->dev, &dev_attr_capture_overrun);
-	rc |= device_create_file(psc_dma->dev, &dev_attr_playback_underrun);
-	if (rc)
-		dev_info(psc_dma->dev, "error creating sysfs files\n");
-
-	snd_soc_register_platform(&psc_dma_pcm_soc_platform);
-
-	/* Tell the ASoC OF helpers about it */
-	of_snd_soc_register_platform(&psc_dma_pcm_soc_platform, op->node,
-				     &psc_dma->dai);
+	/* Due to errata in the dma mode; need to line up enabling
+	 * the transmitter with a transition on the frame sync
+	 * line */
+
+	/* first make sure it is low */
+	while ((in_8(&regs->ipcr_acr.ipcr) & 0x80) != 0)
+		;
+	/* then wait for the transition to high */
+	while ((in_8(&regs->ipcr_acr.ipcr) & 0x80) == 0)
+		;
+	/* Finally, enable the PSC.
+	 * Receiver must always be enabled; even when we only want
+	 * transmit.  (see 15.3.2.3 of MPC5200B User's Guide) */
+
+	/* Go */
+	out_8(&psc_dma->psc_regs->command,
+			MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE);
 
 	return 0;
+
 }
 
 static int __devexit psc_i2s_of_remove(struct of_device *op)
 {
-	struct psc_dma *psc_dma = dev_get_drvdata(&op->dev);
-
-	dev_dbg(&op->dev, "psc_i2s_remove()\n");
-
-	snd_soc_unregister_platform(&psc_dma_pcm_soc_platform);
-
-	bcom_gen_bd_rx_release(psc_dma->capture.bcom_task);
-	bcom_gen_bd_tx_release(psc_dma->playback.bcom_task);
-
-	iounmap(psc_dma->psc_regs);
-	iounmap(psc_dma->fifo_regs);
-	kfree(psc_dma);
-	dev_set_drvdata(&op->dev, NULL);
-
-	return 0;
+	return mpc5200_audio_dma_destroy(op);
 }
 
 /* Match table for of_platform binding */
 static struct of_device_id psc_i2s_match[] __devinitdata = {
 	{ .compatible = "fsl,mpc5200-psc-i2s", },
+	{ .compatible = "fsl,mpc5200b-psc-i2s", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, psc_i2s_match);
@@ -411,4 +245,7 @@ static void __exit psc_i2s_exit(void)
 }
 module_exit(psc_i2s_exit);
 
+MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
+MODULE_DESCRIPTION("Freescale MPC5200 PSC in I2S mode ASoC Driver");
+MODULE_LICENSE("GPL");
 
diff --git a/sound/soc/fsl/mpc5200_psc_i2s.h b/sound/soc/fsl/mpc5200_psc_i2s.h
new file mode 100644
index 0000000..ce55e07
--- /dev/null
+++ b/sound/soc/fsl/mpc5200_psc_i2s.h
@@ -0,0 +1,12 @@
+/*
+ * Freescale MPC5200 PSC in I2S mode
+ * ALSA SoC Digital Audio Interface (DAI) driver
+ *
+ */
+
+#ifndef __SOUND_SOC_FSL_MPC52xx_PSC_I2S_H__
+#define __SOUND_SOC_FSL_MPC52xx_PSC_I2S_H__
+
+extern struct snd_soc_dai psc_i2s_dai[];
+
+#endif /* __SOUND_SOC_FSL_MPC52xx_PSC_I2S_H__ */

^ permalink raw reply related

* [PATCH V5 1/5] powerpc: introduce macro spin_event_timeout()
From: Jon Smirl @ 2009-05-26 12:34 UTC (permalink / raw)
  To: grant.likely, linuxppc-dev, alsa-devel, broonie, timur
In-Reply-To: <20090526123235.17472.73912.stgit@terra>

The macro spin_event_timeout() takes a condition and timeout value
(in microseconds) as parameters.  It spins until either the condition is true
or the timeout expires.  It returns the result of the condition when the loop
was terminated.

This primary purpose of this macro is to poll on a hardware register until a
status bit changes.  The timeout ensures that the loop still terminates if the
bit doesn't change as expected.  This macro makes it easier for driver
developers to perform this kind of operation properly.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 arch/powerpc/include/asm/delay.h |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index f9200a6..af4a270 100644
--- a/arch/powerpc/include/asm/delay.h
+++ b/arch/powerpc/include/asm/delay.h
@@ -2,8 +2,11 @@
 #define _ASM_POWERPC_DELAY_H
 #ifdef __KERNEL__
 
+#include <asm/time.h>
+
 /*
  * Copyright 1996, Paul Mackerras.
+ * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -30,5 +33,35 @@ extern void udelay(unsigned long usecs);
 #define mdelay(n)	udelay((n) * 1000)
 #endif
 
+/**
+ * spin_event_timeout - spin until a condition gets true or a timeout elapses
+ * @condition: a C expression to evalate
+ * @timeout: timeout, in microseconds
+ * @delay: the number of microseconds to delay between eache evaluation of
+ *         @condition
+ * @rc: the last value of the condition
+ *
+ * The process spins until the condition evaluates to true (non-zero) or the
+ * timeout elapses.  Upon exit, @rc contains the value of the condition. This
+ * allows you to test the condition without incurring any side effects.
+ *
+ * This primary purpose of this macro is to poll on a hardware register
+ * until a status bit changes.  The timeout ensures that the loop still
+ * terminates even if the bit never changes.  The delay is for devices that
+ * need a delay in between successive reads.
+ *
+ * gcc will optimize out the if-statement if @delay is a constant.
+ */
+#define spin_event_timeout(condition, timeout, delay, rc)                   \
+{                                                                           \
+	unsigned long __loops = tb_ticks_per_usec * timeout;                \
+	unsigned long __start = get_tbl();                                  \
+	while (!(rc = (condition)) && (tb_ticks_since(__start) <= __loops)) \
+		if (delay)                                                  \
+			udelay(delay);                                      \
+		else	                                                    \
+			cpu_relax();                                        \
+}
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DELAY_H */

^ permalink raw reply related

* [PATCH V5 0/5] AC97 driver for mpc5200
From: Jon Smirl @ 2009-05-26 12:34 UTC (permalink / raw)
  To: grant.likely, linuxppc-dev, alsa-devel, broonie, timur

Version 5. Modified to use spin_event_timeout() as originally written.

---

Jon Smirl (5):
      Fabric bindings for STAC9766 on the Efika
      Support for AC97 on Phytec pmc030 base board.
      AC97 driver for mpc5200
      Main rewite of the mpc5200 audio DMA code
      powerpc: introduce macro spin_event_timeout()


 arch/powerpc/include/asm/delay.h    |   33 +++
 sound/soc/fsl/Kconfig               |   27 ++
 sound/soc/fsl/Makefile              |    5 
 sound/soc/fsl/efika-audio-fabric.c  |   90 +++++++
 sound/soc/fsl/mpc5200_dma.c         |  442 ++++++++++++++++++++++-------------
 sound/soc/fsl/mpc5200_dma.h         |   33 +--
 sound/soc/fsl/mpc5200_psc_ac97.c    |  331 ++++++++++++++++++++++++++
 sound/soc/fsl/mpc5200_psc_ac97.h    |   15 +
 sound/soc/fsl/mpc5200_psc_i2s.c     |  247 +++-----------------
 sound/soc/fsl/mpc5200_psc_i2s.h     |   12 +
 sound/soc/fsl/pcm030-audio-fabric.c |   90 +++++++
 11 files changed, 934 insertions(+), 391 deletions(-)
 create mode 100644 sound/soc/fsl/efika-audio-fabric.c
 create mode 100644 sound/soc/fsl/mpc5200_psc_ac97.c
 create mode 100644 sound/soc/fsl/mpc5200_psc_ac97.h
 create mode 100644 sound/soc/fsl/mpc5200_psc_i2s.h
 create mode 100644 sound/soc/fsl/pcm030-audio-fabric.c

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH resend4 2/3] itimers: fix periodic tics precision
From: Thomas Gleixner @ 2009-05-26 12:04 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Peter Zijlstra, linux-kernel@vger.kernel.org, Oleg Nesterov,
	linuxppc-dev, Ingo Molnar, Andrew Morton
In-Reply-To: <20090526084455.672c652f@dhcp-lab-109.englab.brq.redhat.com>

On Tue, 26 May 2009, Stanislaw Gruszka wrote:
> On Mon, 25 May 2009 14:51:32 +0200
> Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> 
> > On Mon, 25 May 2009 14:32:14 +0200 (CEST)
> > Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > > On Mon, 25 May 2009, Stanislaw Gruszka wrote:
> > > > @@ -904,6 +905,7 @@ void __init time_init(void)
> > > >  	tb_ticks_per_usec = ppc_tb_freq / 1000000;
> > > >  	tb_to_us = mulhwu_scale_factor(ppc_tb_freq, 1000000);
> > > >  	calc_cputime_factors();
> > > > +	cputime_one = jiffies_to_cputime(1);
> > > 
> > >   1) The variable name is misleading.
> > 
> > What about cputime_one_jiffy ?
> > 
> > >   2) The patch breaks all powerpc platforms which have
> > >   CONFIG_VIRT_CPU_ACCOUNTING=n and ia64 with
> > >   CONFIG_VIRT_CPU_ACCOUNTING=y
> > 
> > Stupid me, in asm-generic/cputime.h should be 
> > #define cputime_one jiffies_to_cputime(1)
> 
> Hmmm, I'm confused. Perhaps I missed something, but I think patch was ok.
> For powerpc and ia64 and CONFIG_VIRT_CPU_ACCOUNTING=n definitions from
> asm-generic/cputime.h where used. In this file was:
> 
> #define cputime_one (1UL) 

  In time_init you add unconditionally:

> > > > @@ -904,6 +905,7 @@ void __init time_init(void)
> > > >         tb_ticks_per_usec = ppc_tb_freq / 1000000;
> > > >         tb_to_us = mulhwu_scale_factor(ppc_tb_freq, 1000000);
> > > >         calc_cputime_factors();
> > > > +       cputime_one = jiffies_to_cputime(1);

  I doubt that the compiler will happy about that as it expands to

      (1UL) = jiffies_to_cputime(1)
  or 
      jiffies_to_cputime(1) = jiffies_to_cputime(1)

  in the CONFIG_VIRT_CPU_ACCOUTING=n case.
 
> and that correct as jiffies_to_cputime(x) is just (x)
> 
> For CONFIG_VIRT_CPU_ACCOUTING=y:
> - For powerpc additional variable was  declared and computed in
>   initialization  time. Declaration of was in __KERENEL__ scope.
> - For ia64: cputime_one was defined as jiffies_to_cputime(1)

  My bad, I missed the ia64 hunk.
 
> Does we really need such optimization (because before usage of
> jiffies_to_cputime(1) was just fine) ?

Well, there is a subtle difference between working and nobody noticing
the overhead.

If we notice such heavy instructions in a code path we change then
ignoring the optimization with the argument that it worked before is
just plain wrong.

Thanks,

	tglx

^ permalink raw reply

* Re: drivers/video/logo/logo_linux_mono.c build error
From: Geert Uytterhoeven @ 2009-05-26 11:52 UTC (permalink / raw)
  To: Andrew Morton; +Cc: sfr, linuxppc-dev
In-Reply-To: <alpine.LRH.2.00.0905140919230.22602@vixen.sonytel.be>

	Hi Andrew,

On Thu, 14 May 2009, Geert Uytterhoeven wrote:
> On Wed, 13 May 2009, Andrew Morton wrote:
> > On Tue, 28 Apr 2009 09:24:53 +0200 (CEST)
> > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> > > On Tue, 28 Apr 2009, Stephen Rothwell wrote:
> > > > On Mon, 27 Apr 2009 14:50:31 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > > powerpc allmodconfig, current mainline:
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > > 
> > > > > drivers/video/logo/logo_linux_mono.c:11: error: logo_linux_mono_data causes a section type conflict
> > > > > 
> > > > > switching it from __initconst to __initdata "fixes" it.
> > > > 
> > > > Interesting.  The program that generates the .c file above produces
> > > > __initdata for me ...
> > > 
> > > I assume Andrew once built in that tree using the recent logo patches that
> > >   1. Makes the logos const and switch them from __initdata to __initconst,
> > >   2. Fix the missing dependency of the generated files on the generator program.
> > > Hence if you revert the logo patches, you have to manually remove the generated
> > > files.
> > > 
> > > Is my assumption correct?
> > 
> > Nope - mrproper removes every .c file from drivers/video/logo/ apart
> > from logo.c
> > 
> > 
> > 
> > The failure is caused by your fbdev-move-logo-externs-to-header-file.patch.
> 
> Which is not in current mainline, so the above reference was incorrectly
> fueling my assumption?
> 
> > Without that patch, drivers/video/logo/logo_linux_mono.c is generated as
> > 
> > 	static unsigned char logo_linux_mono_data[] __initdata = {
> > 	
> > and with that patch, drivers/video/logo/logo_linux_mono.c is generated as
> > 
> > 	static const unsigned char logo_linux_mono_data[] __initconst = {
> > 
> > and it's the switch from __initdata to __initconst which causes this
> > section conflict in my setup.
> 
> But logo_linux_mono_data is referenced by 
> 
>     const struct linux_logo logo_linux_mono __initconst = { 
> 	    .type           = LINUX_LOGO_MONO,
> 	    .width          = 80,
> 	    .height         = 80,
> 	    .data           = logo_linux_mono_data
>     };      
> 
> only, which is also __initconst? Where does the section type conflict come
> from?

Do you have any follow-up info on this one?

Thx!

With kind regards,

Geert Uytterhoeven
Software Architect
Techsoft Centre

Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

^ permalink raw reply

* Re: [PATCH V4 1/5] The macro spin_event_timeout() takes a condition and timeout value
From: Geert Uytterhoeven @ 2009-05-26 11:51 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel, broonie
In-Reply-To: <ed82fe3e0905260432xdc9556cp74edc723eafe24f7@mail.gmail.com>

On Tue, 26 May 2009, Timur Tabi wrote:
> On Tue, May 26, 2009 at 2:29 AM, Geert Uytterhoeven
> <Geert.Uytterhoeven@sonycom.com> wrote:
> 
> > static inline function, returning rc, instead of a macro?
> 
> It won't work as an inline function ...
> 
> >> +     unsigned long __loops = tb_ticks_per_usec * timeout;                \
> >> +     unsigned long __start = get_tbl();                                  \
> >> +     while ((rc = (condition)) && (tb_ticks_since(__start) <= __loops)) \
> 
> There's no way to pass a expression to an inline function.  It has to
> be a macro.  Here, the loop evaluates "condition" at every pass.  If
> this were an inline function, "condition" would be evaluated once when
> the function were called, and never again.

You're right, I missed that part. Sorry about that.

However, you can still improve useability by making the macro return the rc,
instead of letting the caller pass it, cfr. wait_event_timeout() and friends.

With kind regards,

Geert Uytterhoeven
Software Architect
Techsoft Centre

Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox