[alsa-devel] [PATCH v2] ASoC: rt5640: Add minimal supportforRT5642

[Bard Liao]

> On 04/16/2014 08:24 PM, bardliao at realtek.com wrote:
> > From: Bard Liao <bardliao at realtek.com>
> >
> > We have been using rt5640.c codec driver with RT5642 codec chip before commit
> > 022d21f004c1 ("ASoC: rt5640: add rt5639 support"). That commits starts using
> > device ID reading in reset register for adding device specific controls and
> > routes runtime.
> >
> > Now since device ID appears to be different between RT5640 and RT5642 the
> > driver doesn't add those controls and routes that are valid also on RT5642.
> >
> > Fix this by adding a device ID found by debugging and minimal code for
> > supporting RT5642.
> >
> > Signed-off-by: Jarkko Nikula <jarkko.nikula at linux.intel.com>
> 
> Is this derived from Jarkko's patch? If so, shouldn't he be listed as
> the author, not you? If it wasn't, then presumably his S-o-b line
> shouldn't be in the patch description.

I am sorry. I don't know the meaning of S-o-b's order.

> 
> > Signed-off-by: Bard Liao <bardliao at realtek.com>
> 
> This patch causes problems for me. I see an enormous amount of spew
> during kernel boot along the lines of:
> 
> > [    2.285515] rt5640 0-001c: ASoC: no source widget found for OUT MIXL
> > [    2.291899] rt5640 0-001c: ASoC: Failed to add route OUT MIXL -> OUT MIXL Switch -> RECMIXL
> > [    2.300306] rt5640 0-001c: ASoC: no source widget found for OUT MIXR
> > [    2.306662] rt5640 0-001c: ASoC: Failed to add route OUT MIXR -> OUT MIXR Switch -> RECMIXR
> > [    2.315662] rt5640 0-001c: ASoC: no sink widget found for Stereo DAC MIXL
> 
> (but repeated for about 28 widget/route pairs)
> 
> Perhaps it's related to:
> 
> > diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
> 
> > -#define RT5639_RESET_ID                              0x0008
> > -#define RT5640_RESET_ID                              0x000c
> 
> These values cover at least bits 3:2.

Actually, only bits 2:1 is related to device ID. Others are for other informations.

> 
> > +/* SW Reset & Device ID (0x00) */
> > +#define RT5640_ID_MASK                               (0x3 << 1)
> > +#define RT5640_ID_5639                               (0x0 << 1)
> > +#define RT5640_ID_5640                               (0x1 << 1)
> > +#define RT5640_ID_5642                               (0x3 << 1)

It's my fault. 5640's ID should be (0x2 << 1) instead (0x1 << 1).

> 
> whereas these values cover bits 2:1. Should the shift be 2? Even with
> that fixed, the old 5639 value was 2 << 2 but the new value here is 0 <<
> 2. Similarly, the old 5640 value was 3 <<2 whereas the new value is 1 <<
> 2. There's obviously quite some confusion here.
> 
> The schematic of my board says I have an RT5640, everything worked
> before this commit, and register 0 (where these values are stored) reads
> as 0xc which matches the RT5640 value before this commit but not after.
> 
> I can see why this patch causes the driver to support the wrong chip.
> However, I can't imagine why that causes all the log spew at startup.
> Perhaps the driver is just broken on RT5639 at present (although I don't
> recall seeing any issues when booting on a board that actually had
> one...) Is part of the driver keying off this now incorrect ID register
> read, yet some other part of the driver registering widgets/routes based
> on which entry matched in struct i2c_device_id rt5640_i2c_id, hence
> they're falling out of sync due to this change? If so, that seems like
> another bug that needs fixing.

We will check and fix it.

Thanks.

> 
> ------Please consider the environment before printing this e-mail.