From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751109AbdHaKtV (ORCPT ); Thu, 31 Aug 2017 06:49:21 -0400 Received: from mx2.suse.de ([195.135.220.15]:45243 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750757AbdHaKtU (ORCPT ); Thu, 31 Aug 2017 06:49:20 -0400 Date: Thu, 31 Aug 2017 12:49:17 +0200 Message-ID: From: Takashi Iwai To: Mark Brown Cc: Alexandre Belloni , Julia Lawall , alsa-devel@alsa-project.org, garsilva@embeddedor.com, arvind.yadav.cs@gmail.com, bhumirks@gmail.com, andriy.shevchenko@linux.intel.com, nicolas.ferre@microchip.com, perex@perex.cz, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, Christophe JAILLET Subject: Re: [alsa-devel] [PATCH] ALSA: ac97c: Fix an error handling path in 'atmel_ac97c_probe()' In-Reply-To: <20170831103716.n5f7mpzr2gatulmw@sirena.org.uk> References: <20170831044042.23306-1-christophe.jaillet@wanadoo.fr> <20170831081021.g4luo557ggtnyfyg@piout.net> <20170831095615.lpon4a36vil6avma@piout.net> <20170831103716.n5f7mpzr2gatulmw@sirena.org.uk> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 31 Aug 2017 12:37:16 +0200, Mark Brown wrote: > > On Thu, Aug 31, 2017 at 12:23:14PM +0200, Takashi Iwai wrote: > > > Ah, wait, now I see your point. It was introduced by the very recent > > patch through Mark's asoc tree (since it was wrongly labeled as "ASoC" > > while it isn't). That patch looks indeed fishy. The change in > > atmel_ac97c_resume() is also bad. > > The resume check looks fine? The function appears to do nothing other > than the clk_prepare_enable(). Well, the patch behaves correctly but the code is ugly: int ret = clk_prepare_enable(chip->pclk); return ret; > > So, I'd prefer reverting the wrong commit instead, and leave some > > comment about the uselessness of clk_prepare_enable() return value > > check. > > I'd rather keep the error checking there, it means that people don't > need to open the code and verify it when they go scanning for potential > problems. OK. Takashi From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Date: Thu, 31 Aug 2017 10:49:17 +0000 Subject: Re: [alsa-devel] [PATCH] ALSA: ac97c: Fix an error handling path in 'atmel_ac97c_probe()' Message-Id: List-Id: References: <20170831044042.23306-1-christophe.jaillet@wanadoo.fr> <20170831081021.g4luo557ggtnyfyg@piout.net> <20170831095615.lpon4a36vil6avma@piout.net> <20170831103716.n5f7mpzr2gatulmw@sirena.org.uk> In-Reply-To: <20170831103716.n5f7mpzr2gatulmw@sirena.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Mark Brown Cc: Alexandre Belloni , Julia Lawall , alsa-devel@alsa-project.org, garsilva@embeddedor.com, arvind.yadav.cs@gmail.com, bhumirks@gmail.com, andriy.shevchenko@linux.intel.com, nicolas.ferre@microchip.com, perex@perex.cz, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, Christophe JAILLET On Thu, 31 Aug 2017 12:37:16 +0200, Mark Brown wrote: > > On Thu, Aug 31, 2017 at 12:23:14PM +0200, Takashi Iwai wrote: > > > Ah, wait, now I see your point. It was introduced by the very recent > > patch through Mark's asoc tree (since it was wrongly labeled as "ASoC" > > while it isn't). That patch looks indeed fishy. The change in > > atmel_ac97c_resume() is also bad. > > The resume check looks fine? The function appears to do nothing other > than the clk_prepare_enable(). Well, the patch behaves correctly but the code is ugly: int ret = clk_prepare_enable(chip->pclk); return ret; > > So, I'd prefer reverting the wrong commit instead, and leave some > > comment about the uselessness of clk_prepare_enable() return value > > check. > > I'd rather keep the error checking there, it means that people don't > need to open the code and verify it when they go scanning for potential > problems. OK. Takashi