On Mon, Jun 14, 2010 at 3:55 PM, Eric Millbrandt
[off-list ref] wrote:

Call the gpio reset platform function instead of using the flawed
ac97 functionality of the MPC5200(b)

From MPC5200B User's Manual:
"Some AC97 devices goes to a test mode, if the Sync line is high
during the Res line is low (reset phase). To avoid this behavior the
Sync line must be also forced to zero during the reset phase. To do
that, the pin muxing should switch to GPIO mode and the GPIO control
register should be used to control the output lines."

Signed-off-by: Eric Millbrandt <redacted>
---

changes since v1
- Amended with comments from Mark Brown
- Fall back to the original reset implementation if no gpio pins are defi=

ned

=A0in the device tree

changes since v2
- Refactored to move the port_config manipulation to platform code.
- Remove the gpio pins from the device-tree

=A0sound/soc/fsl/mpc5200_psc_ac97.c | =A0 33 ++++++++++++++++++++++++++++=

+----

_ac97.c

97)

=A0{
=A0 =A0 =A0 =A0struct mpc52xx_psc __iomem *regs =3D psc_dma->psc_regs;

+ =A0 =A0 =A0 mutex_lock(&psc_dma->mutex);
+
=A0 =A0 =A0 =A0out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_AWR=

);

=A0 =A0 =A0 =A0udelay(3);
=A0 =A0 =A0 =A0out_be32(&regs->sicr, psc_dma->sicr);
+
+ =A0 =A0 =A0 mutex_unlock(&psc_dma->mutex);
=A0}

+#define =A0MPC52xx_PSC_SICR_ACRB (0x8 << 24)

Put this #define with the rest of the MPC52xx_PSC_SICR_* #defines.

=A0static void psc_ac97_cold_reset(struct snd_ac97 *ac97)
=A0{
=A0 =A0 =A0 =A0struct mpc52xx_psc __iomem *regs =3D psc_dma->psc_regs;

- =A0 =A0 =A0 /* Do a cold reset */
- =A0 =A0 =A0 out_8(&regs->op1, MPC52xx_PSC_OP_RES);
- =A0 =A0 =A0 udelay(10);
- =A0 =A0 =A0 out_8(&regs->op0, MPC52xx_PSC_OP_RES);
+ =A0 =A0 =A0 mutex_lock(&psc_dma->mutex);
+
+ =A0 =A0 =A0 switch (psc_dma->id) {
+ =A0 =A0 =A0 case 0:
+ =A0 =A0 =A0 case 1:
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc5200_psc_ac97_gpio_reset(psc_dma->id);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(psc_dma->dev, "cold reset\n");
+
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Notify the PSC that a reset has occurred=

 */

+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&regs->sicr, psc_dma->sicr | MPC52=

xx_PSC_SICR_ACRB);

+
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Re-enable RX and TX */
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_8(&regs->command, MPC52xx_PSC_TX_ENABLE=

 | MPC52xx_PSC_RX_ENABLE);

+
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
+ =A0 =A0 =A0 default:
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(psc_dma->dev,
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "Unable to determine PSC, n=

o cold-reset will be "

+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "performed\n");
+ =A0 =A0 =A0 }

You can drop the switch block and the error message.  You do exactly
the same thing in the common code.  Just call
mpc5200_psc_ac97_gpio_reset() unconditionally.

Otherwise, this version looks pretty good.  After you've respun both
patches (I've got comments to make on the other too), and if it's okay
by Mark, then I can merge both patches through my tree.

Cheers,
g.