On 11/16/2015 07:40 PM, Pavel Machek wrote:

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...

Signed-off-by: Pavel Machek pavel@denx.de

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index b960e62..a739ca8 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -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?

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, ...);

}

static bool mcasp_is_synchronous(struct davinci_mcasp *mcasp)