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