ASoC: rt286: Fix the runtime error in the booting

PATCH Superseded Oder Chiou 2 participants 4 messages Oct 2, 2015 – Oct 5, 2015

Subsystems (3)

Sound ASoC Realtek audio codecs

lore

Share & tools

lore

Oder Chiou

author maintainer

2015-10-02 05:30 UTC #0 lore raw

PATCH 1/1 ASoC: rt286: Fix the runtime error in the booting

The struct rt286_index_def was used by the cache function, so it cannot be declared as const.

Signed-off-by Oder Chiou <[email protected]>

---
 sound/soc/codecs/rt286.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sound/soc/codecs/rt286.c +1 -1

index 1fbdb4f..79107dc 100644
@@ -49,7 +49,7 @@ struct rt286_priv {
 	int clk_id;
 };
 
-static const struct reg_default rt286_index_def[] = {
+static struct reg_default rt286_index_def[] = {
 	{ 0x01, 0xaaaa },
 	{ 0x02, 0x8aaa },
 	{ 0x03, 0x0002 },

Signature · 1 line

1.8.1.1.439.g50a6b54

Mark Brown

maintainer

2015-10-02 10:11 UTC #1 lore raw

sound/soc/codecs/rt286.c struct rt286_priv { · line 55

Quoted 8 lines · Oder Chiou

The struct rt286_index_def was used by the cache function, so it cannot be declared as const.
sound/soc/codecs/rt286.c +1 -1
-static const struct reg_default rt286_index_def[] = {
+static struct reg_default rt286_index_def[] = {
 	{ 0x01, 0xaaaa },
 	{ 0x02, 0x8aaa },
 	{ 0x03, 0x0002 },

This isn't obvious and seems likely to break - why is this ever being modified and how is that safe?

Oder Chiou

author maintainer

2015-10-05 04:10 UTC #2 lore raw

Quoted 12 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Friday, October 02, 2015 6:12 PM
To: Oder Chiou
Cc: [email protected]; [email protected]; Flove; Bard Liao; John Lin; [email protected]
Subject: Re: [PATCH] ASoC: rt286: Fix the runtime error in the booting
Quoted 2 lines · Oder Chiou
The struct rt286_index_def was used by the cache function, so it cannot be declared as const.

Quoted 8 lines

sound/soc/codecs/rt286.c struct rt286_priv { · line 55
Quoted 5 lines
sound/soc/codecs/rt286.c +1 -1
-static const struct reg_default rt286_index_def[] = {
+static struct reg_default rt286_index_def[] = {
 	{ 0x01, 0xaaaa },
 	{ 0x02, 0x8aaa },
 	{ 0x03, 0x0002 },
This isn't obvious and seems likely to break - why is this ever being modified and how is that safe?

The table was used for the cache function of the index table, so it would be changed the value in the index register writing. The breaking is in that the variable of type "const" is changing. And the wrong modification was committed by "c418a84a8c8f98b1a0f30cd68d0cdf40d77aed01". The modifications of the commit are correct in the conventional case, but it will be breaking in case of rt286 and rt298.

Mark Brown

maintainer

2015-10-05 09:58 UTC #3 lore raw

Quoted 9 lines · Oder Chiou

Quoted 2 lines
This isn't obvious and seems likely to break - why is this ever being modified and how is that safe?
The table was used for the cache function of the index table, so it would be changed the value in the index register writing. The breaking is in that the variable of type "const" is changing. And the wrong modification was committed by "c418a84a8c8f98b1a0f30cd68d0cdf40d77aed01". The modifications of the commit are correct in the conventional case, but it will be breaking in case of rt286 and rt298.

I'm sorry but this isn't clarifying anything at all for me. You are simply stating that this is safe but not explaning why - you need to explain why. If nothing else if a driver is modifying global static data that's bad practice, the driver should be copying the data into somewhere driver private and then modifying that.

Please also remember to include human readable descriptions of commits so that any humans reading your mail can do so directly.