Messages in this thread | | | Subject | Re: [PATCH] sound/soc/davinci: fix error case in mcasp_set_ctl_reg | From | Peter Ujfalusi <> | Date | Tue, 17 Nov 2015 09:40:31 +0200 |
| |
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) > >
-- Péter
| |