| Message ID | 20151116174026.GA1735@amd (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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) > >
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++) { 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)
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 <pavel@denx.de>