[RFC PATCH] theory about CS8247 failed reads

[RFC PATCH] theory about CS8247 failed reads

[Brian Bloniarz]

On my M-Audio Delta 66 (rev. E), I see this line:
[   23.248721] invalid CS8427 signature 0x0: let me try again...

This is caused by the first request to the CS8427 failing, and
this workaround:

> commit 4eb4550ab37d351ab0973ccec921a5a2d8560ec7,
> Author: Takashi Iwai <tiwai@suse.de>
> Date:   Thu Sep 20 17:34:57 2007 +0200
>
>     [ALSA] Workaround for invalid signature read of CS8427
>     
>     Reading the signature of CS8427 over SPI/I2C fails on some devices
>     by mysterious reason.  In most cases, however, it succeeds at the
>     sequential read.  So, let's give a second chance to check the signature
>     again.
>     
>     Signed-off-by: Takashi Iwai <tiwai@suse.de>
>     Signed-off-by: Jaroslav Kysela <perex@suse.cz>

I have a theory, please read the patch and patch description below.
With the patch applied, the printk goes away.

Does this sound like a reasonable theory? Does anyone recall, did this
failed read occur on any cards besides M-Audio Delta? I'm wondering if
this is the whole explanation, in which case the previous workaround
could be removed.

------------------------

[PATCH] [ALSA] ice1712 delta - initialize SPI clock.

The driver was using an initial value for the clock on the SPI bus
which was read from ICE1712 EEPROM,
ice->eeprom.data[ICE_EEP1_GPIO_STATE] & ICE1712_DELTA_AP_CCLK (0x02)

It appears some cards have it default high, some cards
have it default low. On my Delta 66 rev. E:
$ cat /proc/asound/M66/ice1712 | grep 'GPIO state'
  GPIO state       : 0x70 /* ICE1712_DELTA_AP_CCLK bit is zero */
On my Audiophile 2496:
$ cat /proc/asound/M2496/ice1712 | grep 'GPIO state'
  GPIO state       : 0xfe /* ICE1712_DELTA_AP_CCLK bit is one */

It must be raised before the first SPI write happens, or the write will
fail, leading to:

[   23.248721] invalid CS8427 signature 0x0: let me try again...

I theorize that 4eb4550ab37d351ab0973ccec921a5a2d8560ec7
is no longer needed, it was a different way to workaround
the problem.

Signed-off-by: Brian Bloniarz <brian.bloniarz@gmail.com>
---
 sound/pci/ice1712/delta.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/sound/pci/ice1712/delta.c b/sound/pci/ice1712/delta.c
index 7b62de0..84a6031 100644
--- a/sound/pci/ice1712/delta.c
+++ b/sound/pci/ice1712/delta.c
@@ -622,6 +622,13 @@ static int __devinit snd_ice1712_delta_init(struct snd_ice1712 *ice)
                break;
        }
 
+       /* initialize the SPI clock to high */
+       unsigned char tmp;
+       tmp = snd_ice1712_read(ice, ICE1712_IREG_GPIO_DATA);
+       tmp |= ICE1712_DELTA_AP_CCLK;
+       snd_ice1712_write(ice, ICE1712_IREG_GPIO_DATA, tmp);
+       udelay(5);
+
        /* initialize spdif */
        switch (ice->eeprom.subvendor) {
        case ICE1712_SUBDEVICE_AUDIOPHILE:
-- 
1.7.1

Re: [RFC PATCH] theory about CS8247 failed reads

[Takashi Iwai]

At Mon, 17 Jan 2011 23:20:03 -0800,
Brian Bloniarz wrote:
> 
> On my M-Audio Delta 66 (rev. E), I see this line:
> [   23.248721] invalid CS8427 signature 0x0: let me try again...
> 
> This is caused by the first request to the CS8427 failing, and
> this workaround:
> 
> > commit 4eb4550ab37d351ab0973ccec921a5a2d8560ec7,
> > Author: Takashi Iwai <tiwai@suse.de>
> > Date:   Thu Sep 20 17:34:57 2007 +0200
> >
> >     [ALSA] Workaround for invalid signature read of CS8427
> >     
> >     Reading the signature of CS8427 over SPI/I2C fails on some devices
> >     by mysterious reason.  In most cases, however, it succeeds at the
> >     sequential read.  So, let's give a second chance to check the signature
> >     again.
> >     
> >     Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >     Signed-off-by: Jaroslav Kysela <perex@suse.cz>
> 
> I have a theory, please read the patch and patch description below.
> With the patch applied, the printk goes away.
> 
> Does this sound like a reasonable theory? Does anyone recall, did this
> failed read occur on any cards besides M-Audio Delta? I'm wondering if
> this is the whole explanation, in which case the previous workaround
> could be removed.
> 
> ------------------------
> 
> [PATCH] [ALSA] ice1712 delta - initialize SPI clock.
> 
> The driver was using an initial value for the clock on the SPI bus
> which was read from ICE1712 EEPROM,
> ice->eeprom.data[ICE_EEP1_GPIO_STATE] & ICE1712_DELTA_AP_CCLK (0x02)
> 
> It appears some cards have it default high, some cards
> have it default low. On my Delta 66 rev. E:
> $ cat /proc/asound/M66/ice1712 | grep 'GPIO state'
>   GPIO state       : 0x70 /* ICE1712_DELTA_AP_CCLK bit is zero */
> On my Audiophile 2496:
> $ cat /proc/asound/M2496/ice1712 | grep 'GPIO state'
>   GPIO state       : 0xfe /* ICE1712_DELTA_AP_CCLK bit is one */
> 
> It must be raised before the first SPI write happens, or the write will
> fail, leading to:
> 
> [   23.248721] invalid CS8427 signature 0x0: let me try again...
> 
> I theorize that 4eb4550ab37d351ab0973ccec921a5a2d8560ec7
> is no longer needed, it was a different way to workaround
> the problem.

Yes, your fix looks sane (and safe).
I'll queue it up later for 2.6.38 merge.

The retry-code can be kept for a while as it's basically harmless for
the succeeded case.  We can remove it eventually later, but I doubt we
can be 100% sure about such a thing ;)


thanks,

Takashi


> 
> Signed-off-by: Brian Bloniarz <brian.bloniarz@gmail.com>
> ---
>  sound/pci/ice1712/delta.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/pci/ice1712/delta.c b/sound/pci/ice1712/delta.c
> index 7b62de0..84a6031 100644
> --- a/sound/pci/ice1712/delta.c
> +++ b/sound/pci/ice1712/delta.c
> @@ -622,6 +622,13 @@ static int __devinit snd_ice1712_delta_init(struct snd_ice1712 *ice)
>                 break;
>         }
>  
> +       /* initialize the SPI clock to high */
> +       unsigned char tmp;
> +       tmp = snd_ice1712_read(ice, ICE1712_IREG_GPIO_DATA);
> +       tmp |= ICE1712_DELTA_AP_CCLK;
> +       snd_ice1712_write(ice, ICE1712_IREG_GPIO_DATA, tmp);
> +       udelay(5);
> +
>         /* initialize spdif */
>         switch (ice->eeprom.subvendor) {
>         case ICE1712_SUBDEVICE_AUDIOPHILE:
> -- 
> 1.7.1
> 

Re: [RFC PATCH] theory about CS8247 failed reads

[Brian Bloniarz]

On 01/18/2011 06:21 AM, Takashi Iwai wrote:
> Yes, your fix looks sane (and safe).
> I'll queue it up later for 2.6.38 merge.
Sounds great, thanks!

> The retry-code can be kept for a while as it's basically harmless for
> the succeeded case.  We can remove it eventually later, but I doubt we
> can be 100% sure about such a thing ;)
Sure, that makes sense to me.

Thanks,
-Brian