Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752330AbbKQHkt (ORCPT ); Tue, 17 Nov 2015 02:40:49 -0500 Received: from devils.ext.ti.com ([198.47.26.153]:34721 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751600AbbKQHks (ORCPT ); Tue, 17 Nov 2015 02:40:48 -0500 Subject: Re: [PATCH] sound/soc/davinci: fix error case in mcasp_set_ctl_reg To: Pavel Machek , , , , , , , , References: <20151116174026.GA1735@amd> From: Peter Ujfalusi Message-ID: <564AD9EF.5060907@ti.com> Date: Tue, 17 Nov 2015 09:40:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151116174026.GA1735@amd> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1755 Lines: 54 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 > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/