Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Thu, Jun 10, 2010 at 5:14 PM, Mark Brown wrote: > On 10 Jun 2010, at 23:07, Grant Likely wrote: > >> To me, this seems firmly in the realm of silicon bug workaround. >> Considering that the pins aren't relocatable (ie, the GPIO numbers >> never change), I've got no problem having the GPIO reset logic added >> to arch/powerpc/platforms/52xx and hard coding the GPIO numbers. > > Since the pins are fixed I agree - I was assuming that the DT updates > were there because there were multiple pin mux options for the AC'97 > controller so we needed the DT to say which were in use on the board. :-) I originally also made the same assumption, until I actually went and looked at the data sheet. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Wed, Jun 9, 2010 at 4:30 AM, Mark Brown wrote: > On Wed, Jun 09, 2010 at 08:13:30AM +0200, Wolfgang Denk wrote: >> In message >> you wrote: > >> > Would making a change in uboot be a better solution? Eric, can you >> > verify if changing uboot also fixes the problem? > >> To me it seems better if the driver itself does what needs to be done >> instead of relying on specific settings that may or may not be done in >> U-Boot. Keep in mind that drivers may be loaded as modules, and that >> we even see cases where the same port serves multiple purposes by >> loading different driver modules (yes, this is not exactly a clever >> idea, but hardware designers come up with such solutions). > > I do tend to agree that having the driver be able to cope with things is > a bit more robust - it's not terribly discoverable for users and people > are often justifiably nervous about updating their bootloader. > > It might, however, be sensible to make the GPIO based reset be optional > based on having the OF data for the GPIOs. That way existing DTs will > work without changes and systems that can use the reset implementation > in the controller will be able to do so. To me, this seems firmly in the realm of silicon bug workaround. Considering that the pins aren't relocatable (ie, the GPIO numbers never change), I've got no problem having the GPIO reset logic added to arch/powerpc/platforms/52xx and hard coding the GPIO numbers. I completely agree that it should not require a firmware update to get the hardware to work correctly. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On 10 Jun 2010, at 23:07, Grant Likely wrote: > To me, this seems firmly in the realm of silicon bug workaround. > Considering that the pins aren't relocatable (ie, the GPIO numbers > never change), I've got no problem having the GPIO reset logic added > to arch/powerpc/platforms/52xx and hard coding the GPIO numbers. Since the pins are fixed I agree - I was assuming that the DT updates were there because there were multiple pin mux options for the AC'97 controller so we needed the DT to say which were in use on the board. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Wed, Jun 09, 2010 at 10:41:23AM -0400, Jon Smirl wrote: > On Wed, Jun 9, 2010 at 10:32 AM, Mark Brown > > Please include this quote in the changelog for the patch, if this a > > documented workaround from the vendor that's a very different thing to > > something that you've found happens to work on your systems (which is > > more what your changelog sounded like). > Mark, is there a way to ask the chip if it is in test mode? We need to > be sure that's whats happening and it isn't some other glitch. I'd need to work with the hardware teams to obtain this information, but I can confirm that having SYNC high while doing a reset is not supported and so fixing the driver to not do that is a good idea independantly of what it actually does to the chip. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Wed, Jun 9, 2010 at 10:32 AM, Mark Brown wrote: > On Wed, Jun 09, 2010 at 10:21:40AM -0400, Eric Millbrandt wrote: > > [Please fix your MUA to word wrap paragraphs to within 80 characters, > I've reflowed the text below.] > >> From the MPC5200B user 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." > > Please include this quote in the changelog for the patch, if this a > documented workaround from the vendor that's a very different thing to > something that you've found happens to work on your systems (which is > more what your changelog sounded like). > Mark, is there a way to ask the chip if it is in test mode? We need to be sure that's whats happening and it isn't some other glitch. -- Jon Smirl jonsm...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Wed, Jun 09, 2010 at 10:21:40AM -0400, Eric Millbrandt wrote: [Please fix your MUA to word wrap paragraphs to within 80 characters, I've reflowed the text below.] > From the MPC5200B user 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." Please include this quote in the changelog for the patch, if this a documented workaround from the vendor that's a very different thing to something that you've found happens to work on your systems (which is more what your changelog sounded like). ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 0/2] mpc5200 ac97 gpio reset
> -Original Message- > From: Mark Brown [mailto:broo...@opensource.wolfsonmicro.com] > Sent: Wednesday, June 09, 2010 06:31 > To: Wolfgang Denk > Cc: Jon Smirl; Eric Millbrandt; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 0/2] mpc5200 ac97 gpio reset --snip-- > It might, however, be sensible to make the GPIO based reset be optional > based on having the OF data for the GPIOs. That way existing DTs will > work without changes and systems that can use the reset implementation > in the controller will be able to do so. This is a reasonable request. I will respin the patch to do this. Eric Millbrandt This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message. Thank you. Please consider the environment before printing this email. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 0/2] mpc5200 ac97 gpio reset
> -Original Message- > From: Jon Smirl [mailto:jonsm...@gmail.com] > Sent: Wednesday, June 09, 2010 08:22 > To: Wolfgang Denk > Cc: Eric Millbrandt; Mark Brown; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 0/2] mpc5200 ac97 gpio reset > > On Wed, Jun 9, 2010 at 2:13 AM, Wolfgang Denk wrote: > > Dear Jon Smirl, > > > > In message @mail.gmail.com> you wrote: > >> > >> Would making a change in uboot be a better solution? Eric, can you > >> verify if changing uboot also fixes the problem? > > > > To me it seems better if the driver itself does what needs to be done > > instead of relying on specific settings that may or may not be done in > > U-Boot. Keep in mind that drivers may be loaded as modules, and that > > we even see cases where the same port serves multiple purposes by > > loading different driver modules (yes, this is not exactly a clever > > idea, but hardware designers come up with such solutions). > > > Someone with a scope can verify this, but my understanding of what > happens is that uboot by default puts the pins into GPIO mode. Linux > boots and reprograms the pins into AC97 mode. When the pins are > reprogrammed it generates glitches on the reset line. About half of > the time these glitches put the attached codec into test mode. Once > the chip is in test mode it won't respond to normal commands. > Most of the pins on the 5200 default to gpio when the chip comes out of reset. U-Boot then sets the pin muxing by writing to the port configuration register. This value is defined as CONFIG_SYS_GPS_PORT_CONFIG in the U-Boot board header file. The implementation of ac97 gives the codec control of the bus clock. This means that the codec is usually be clocking BITCLK when the 5200 pulls the reset line low, which is the cause of the problem. Freescale should have handled this on the silicon by having the ac97 cold reset function pull the SYNC and SDATAOUT lines low during cold reset, but instead they just documented the problem. >From the MPC5200B user 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." > Another strategy would be to leave reset as is. If the chip is > unresponsive send it the commands to get it out of test mode. That > could be made part of the reset logic in the Linux driver. This is what we used to do when we were using the out-of-tree ac97 driver from Sylvain Munaut. We would generate a cold reset/warm reset sequence in U-Boot. We modified the Linux ac97 driver so that the cold reset functions was a no-op. It did avoid the problem, but it caused other problems. Sometimes it is necessary to cold reset the codec in Linux to resync the ac97 bus. > > > > > > > > Best regards, > > > > Wolfgang Denk > > > > -- > > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de > > The typical page layout program is nothing more than an electronic > > light table for cutting and pasting documents. > > > > > > -- > Jon Smirl > jonsm...@gmail.com Eric Millbrandt -DISCLAIMER: an automatically appended disclaimer may follow. By posting- -to a public e-mail mailing list I hereby grant permission to distribute- -and copy this message.- This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message. Thank you. Please consider the environment before printing this email. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Wed, Jun 9, 2010 at 2:13 AM, Wolfgang Denk wrote: > Dear Jon Smirl, > > In message you > wrote: >> >> Would making a change in uboot be a better solution? Eric, can you >> verify if changing uboot also fixes the problem? > > To me it seems better if the driver itself does what needs to be done > instead of relying on specific settings that may or may not be done in > U-Boot. Keep in mind that drivers may be loaded as modules, and that > we even see cases where the same port serves multiple purposes by > loading different driver modules (yes, this is not exactly a clever > idea, but hardware designers come up with such solutions). Someone with a scope can verify this, but my understanding of what happens is that uboot by default puts the pins into GPIO mode. Linux boots and reprograms the pins into AC97 mode. When the pins are reprogrammed it generates glitches on the reset line. About half of the time these glitches put the attached codec into test mode. Once the chip is in test mode it won't respond to normal commands. Does the opposite happen? What if uboot has the pins in AC97 mode and Linux reprograms them into GPIO mode. Will it also put the chip into test mode? The piece of information I've been missing is an understanding of what is making the glitches that trigger test mode. Another strategy would be to leave reset as is. If the chip is unresponsive send it the commands to get it out of test mode. That could be made part of the reset logic in the Linux driver. > > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de > The typical page layout program is nothing more than an electronic > light table for cutting and pasting documents. > -- Jon Smirl jonsm...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Wed, Jun 09, 2010 at 08:13:30AM +0200, Wolfgang Denk wrote: > In message you > wrote: > > Would making a change in uboot be a better solution? Eric, can you > > verify if changing uboot also fixes the problem? > To me it seems better if the driver itself does what needs to be done > instead of relying on specific settings that may or may not be done in > U-Boot. Keep in mind that drivers may be loaded as modules, and that > we even see cases where the same port serves multiple purposes by > loading different driver modules (yes, this is not exactly a clever > idea, but hardware designers come up with such solutions). I do tend to agree that having the driver be able to cope with things is a bit more robust - it's not terribly discoverable for users and people are often justifiably nervous about updating their bootloader. It might, however, be sensible to make the GPIO based reset be optional based on having the OF data for the GPIOs. That way existing DTs will work without changes and systems that can use the reset implementation in the controller will be able to do so. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
Dear Jon Smirl, In message you wrote: > > Would making a change in uboot be a better solution? Eric, can you > verify if changing uboot also fixes the problem? To me it seems better if the driver itself does what needs to be done instead of relying on specific settings that may or may not be done in U-Boot. Keep in mind that drivers may be loaded as modules, and that we even see cases where the same port serves multiple purposes by loading different driver modules (yes, this is not exactly a clever idea, but hardware designers come up with such solutions). Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de The typical page layout program is nothing more than an electronic light table for cutting and pasting documents. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] mpc5200 ac97 gpio reset
On Tue, Jun 8, 2010 at 12:46 PM, Eric Millbrandt wrote: > These patches reimplement the reset fuction in the ac97 to use gpio pins > instead of using the mpc5200 ac97 reset functionality in the psc. This > avoids a problem in which attached ac97 devices go into "test" mode appear > unresponsive. I'm aware of this problem but I was unable to solve it when I first wrote the driver. I didn't have access to a scope so I wasn't able to figure out what was causing the lock up. I worked around it by looping in reset until the chip did actually reset, but Mark wouldn't let me submit that code. A while ago Maximilian Mueth sent me a note saying another solution was to configure the PSC into AC97 mode in uboot. His impression was that uboot was setting PSC into its default mode. Then Linux booted and set the PSC into AC97 mode. The transition from default mode into AC97 mode caused glitches on the AC97 reset pins and triggered test mode. Would making a change in uboot be a better solution? Eric, can you verify if changing uboot also fixes the problem? I'm glad were finally getting to the root cause of the problem. > > These patches were tested on a pcm030 baseboard and on custom hardware with > a wm9715 audio codec/touchscreen controller. > > Eric Millbrandt > > --- > > Eric Millbrandt (2): > powerpc/5200: Export port-config > sound/soc: mpc5200_psc_ac97: Use gpio pins for cold reset. > > arch/powerpc/boot/dts/lite5200.dts | 3 + > arch/powerpc/boot/dts/lite5200b.dts | 3 + > arch/powerpc/boot/dts/pcm030.dts | 3 + > arch/powerpc/boot/dts/pcm032.dts | 3 + > arch/powerpc/include/asm/mpc52xx.h | 2 + > arch/powerpc/platforms/52xx/mpc52xx_common.c | 61 +++ > sound/soc/fsl/mpc5200_dma.h | 5 ++ > sound/soc/fsl/mpc5200_psc_ac97.c | 83 - > 8 files changed, 159 insertions(+), 4 deletions(-) > > -DISCLAIMER: an automatically appended disclaimer may follow. By posting- > -to a public e-mail mailing list I hereby grant permission to distribute- > -and copy this message.- > > > This e-mail and the information, including any attachments, it contains are > intended to be a confidential communication only to the person or entity to > whom it is addressed and may contain information that is privileged. If the > reader of this message is not the intended recipient, you are hereby notified > that any dissemination, distribution or copying of this communication is > strictly prohibited. If you have received this communication in error, please > immediately notify the sender and destroy the original message. > > Thank you. > > Please consider the environment before printing this email. > -- Jon Smirl jonsm...@gmail.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
