ASoC: rt5640: correct 5640's device ID
diff mbox

Message ID 1398838125-13546-1-git-send-email-bardliao@realtek.com
State Accepted
Commit 4eefa0d850a834cf641892e10583f8081d3f8c60
Headers show

Commit Message

Bard Liao April 30, 2014, 6:08 a.m. UTC
From: Bard Liao <bardliao@realtek.com>

This patch correct rt5640's device ID

Signed-off-by: Bard Liao <bardliao@realtek.com>
---
We will send another patch to fix the issue about the "Failed to add route"
error.
---
 sound/soc/codecs/rt5640.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Warren April 30, 2014, 4:09 p.m. UTC | #1
On 04/30/2014 12:08 AM, bardliao@realtek.com wrote:
> From: Bard Liao <bardliao@realtek.com>
> 
> This patch correct rt5640's device ID
> 
> Signed-off-by: Bard Liao <bardliao@realtek.com>
> ---
> We will send another patch to fix the issue about the "Failed to add route"
> error.

Tested-by: Stephen Warren <swarren@nvidia.com>

(On NVIDIA Tegra Beaver/RT5640, Dalmore/RT5640, Jetson TK1/RT5639)

Note that this patch alone (on top of next-20140429) completely
eliminates any of the "failed to add route" errors that I mentioned
earlier, although perhaps there's still a need to investigate why they
happened.
Mark Brown April 30, 2014, 6:25 p.m. UTC | #2
On Wed, Apr 30, 2014 at 02:08:45PM +0800, bardliao@realtek.com wrote:
> From: Bard Liao <bardliao@realtek.com>
> 
> This patch correct rt5640's device ID

Applied, thanks.
Bard Liao May 5, 2014, 3:36 a.m. UTC | #3
> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Thursday, May 01, 2014 12:10 AM
> To: Bard Liao; broonie@kernel.org; lgirdwood@gmail.com
> Cc: alsa-devel@alsa-project.org; lars@metafoo.de; Flove; Oder Chiou
> Subject: Re: [PATCH] ASoC: rt5640: correct 5640's device ID
> 
> On 04/30/2014 12:08 AM, bardliao@realtek.com wrote:
> > From: Bard Liao <bardliao@realtek.com>
> >
> > This patch correct rt5640's device ID
> >
> > Signed-off-by: Bard Liao <bardliao@realtek.com>
> > ---
> > We will send another patch to fix the issue about the "Failed to add route"
> > error.
> 
> Tested-by: Stephen Warren <swarren@nvidia.com>
> 
> (On NVIDIA Tegra Beaver/RT5640, Dalmore/RT5640, Jetson TK1/RT5639)
> 
> Note that this patch alone (on top of next-20140429) completely eliminates any of
> the "failed to add route" errors that I mentioned earlier, although perhaps there's
> still a need to investigate why they happened.
> 

The "failed to add route" errors is coming from we didn't handle a default case when
determining which codec is attached. As a result, no _dapm_new_controls is called.
That's why once we can determine codecs properly, the errors will be gone.
Actually, there are only three possible ID values, and all of them are in the switch cases now.
So, I am thinking if we need to add a default case to handle unexpected cases.


> ------Please consider the environment before printing this e-mail.
Stephen Warren May 5, 2014, 4:14 p.m. UTC | #4
On 05/04/2014 09:36 PM, Bard Liao wrote:
>> -----Original Message-----
>> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
>> Sent: Thursday, May 01, 2014 12:10 AM
>> To: Bard Liao; broonie@kernel.org; lgirdwood@gmail.com
>> Cc: alsa-devel@alsa-project.org; lars@metafoo.de; Flove; Oder Chiou
>> Subject: Re: [PATCH] ASoC: rt5640: correct 5640's device ID
>>
>> On 04/30/2014 12:08 AM, bardliao@realtek.com wrote:
>>> From: Bard Liao <bardliao@realtek.com>
>>>
>>> This patch correct rt5640's device ID
>>>
>>> Signed-off-by: Bard Liao <bardliao@realtek.com>
>>> ---
>>> We will send another patch to fix the issue about the "Failed to add route"
>>> error.
>>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>>
>> (On NVIDIA Tegra Beaver/RT5640, Dalmore/RT5640, Jetson TK1/RT5639)
>>
>> Note that this patch alone (on top of next-20140429) completely eliminates any of
>> the "failed to add route" errors that I mentioned earlier, although perhaps there's
>> still a need to investigate why they happened.
>>
> 
> The "failed to add route" errors is coming from we didn't handle a default case when
> determining which codec is attached. As a result, no _dapm_new_controls is called.
> That's why once we can determine codecs properly, the errors will be gone.
> Actually, there are only three possible ID values, and all of them are in the switch cases now.
> So, I am thinking if we need to add a default case to handle unexpected cases.

Ah, that makes sense.

It's probably worth adding a default case, so that a meaningful error
message can be printed e.g. if the I2C read of the ID register gets
corrupted, or someone tries to use the driver on a chip that isn't (yet)
supported by it.

Patch
diff mbox

diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index ded2059..895ca14 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -195,7 +195,7 @@ 
 /* 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_5640				(0x2 << 1)
 #define RT5640_ID_5642				(0x3 << 1)