Skip to contentSkip to conversation

sound/soc/davinci: fix error case in mcasp_set_ctl_reg

PATCH In review Pavel Machek 2 participants 2 messages Nov 16, 2015 – Nov 17, 2015

Subsystems (2)
Sound ASoC
lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

PATCH 1/1 sound/soc/davinci: fix error case in mcasp_set_ctl_reg

This fixes typo in comment and fixes mcasp_set_ctl_reg to actually printk on error as author wanted, and cleans it up. Yes, i will end up being 1001 in the old code.

Signed-off-by Pavel Machek <[email protected]>
sound/soc/davinci/davinci-mcasp.c +4 -5
index b960e62..a739ca8 100644
@@ -148,15 +150,14 @@ static void mcasp_set_ctl_reg(struct davinci_mcasp *mcasp, u32 ctl_reg, u32 val)
 
 	mcasp_set_bits(mcasp, ctl_reg, val);
 
-	/* programming GBLCTL needs to read back from GBLCTL and verfiy */
+	/* programming GBLCTL needs to read back from GBLCTL and verify */
 	/* loop count is to avoid the lock-up */
-	for (i = 0; i < 1000; i++) {
+	for (i = 0; i <= 1000; i++) {
 		if ((mcasp_get_reg(mcasp, ctl_reg) & val) == val)
-			break;
+			return;
 	}
 
-	if (i == 1000 && ((mcasp_get_reg(mcasp, ctl_reg) & val) != val))
-		printk(KERN_ERR "GBLCTL write error\n");
+	printk(KERN_ERR "GBLCTL write error\n");
 }
 
 static bool mcasp_is_synchronous(struct davinci_mcasp *mcasp)
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#1 lore raw
Quoted 4 lines · Pavel Machek

This fixes typo in comment and fixes mcasp_set_ctl_reg to actually printk on error as author wanted, and cleans it up. Yes, i will end up being 1001 in the old code.

Yeah, the original code had the additional GBLCTL register check after the timeout. Which was pointless IMHO.
I'm fine with the change, but...

Quoted 8 lines
Signed-off-by Pavel Machek <[email protected]>
sound/soc/davinci/davinci-mcasp.c +0 -0
index b960e62..a739ca8 100644

sound/soc/davinci/davinci-mcasp.c static void mcasp_set_ctl_reg(struct davinci_mcasp *mcasp, u32 ctl_reg, u32 val) · line 155

Quoted 8 lines
sound/soc/davinci/davinci-mcasp.c +2 -2
@@ -148,15 +150,14 @@ static void mcasp_set_ctl_reg(struct davinci_mcasp *mcasp, u32 ctl_reg, u32 val)
 
 	mcasp_set_bits(mcasp, ctl_reg, val);
 
-	/* programming GBLCTL needs to read back from GBLCTL and verfiy */
+	/* programming GBLCTL needs to read back from GBLCTL and verify */
 	/* loop count is to avoid the lock-up */
-	for (i = 0; i < 1000; i++) {
+	for (i = 0; i <= 1000; i++) {

Does it really make any difference to change this from looping 1000 times to 1001 times?

sound/soc/davinci/davinci-mcasp.c static void mcasp_set_ctl_reg(struct davinci_mcasp *mcasp, u32 ctl_reg, u32 val) · line 160

Quoted 8 lines
sound/soc/davinci/davinci-mcasp.c +2 -3
 		if ((mcasp_get_reg(mcasp, ctl_reg) & val) == val)
-			break;
+			return;
 	}
 
-	if (i == 1000 && ((mcasp_get_reg(mcasp, ctl_reg) & val) != val))
-		printk(KERN_ERR "GBLCTL write error\n");
+	printk(KERN_ERR "GBLCTL write error\n");

Can you change this to dev_err(mcasp->dev, ...);

Quoted 5 lines
sound/soc/davinci/davinci-mcasp.c +0 -0
 }
 
 static bool mcasp_is_synchronous(struct davinci_mcasp *mcasp)
Signature · 1 line
Péter