From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751386AbdHaKbi (ORCPT <rfc822;w@1wt.eu>);
        Thu, 31 Aug 2017 06:31:38 -0400
Received: from mx2.suse.de ([195.135.220.15]:43235 "EHLO mx1.suse.de"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1750922AbdHaKbh (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 31 Aug 2017 06:31:37 -0400
Date: Thu, 31 Aug 2017 12:31:33 +0200
Message-ID: <s5h1snsjaey.wl-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: "Mark Brown" <broonie@kernel.org>
Cc: "Alexandre Belloni" <alexandre.belloni@free-electrons.com>,
        <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" <christophe.jaillet@wanadoo.fr>
Subject: Re: [alsa-devel] [PATCH] ALSA: ac97c: Fix an error handling path in 'atmel_ac97c_probe()'
In-Reply-To: <20170831101903.avhj5tqywpvfsj4u@sirena.org.uk>
References: <20170831044042.23306-1-christophe.jaillet@wanadoo.fr>
        <20170831081021.g4luo557ggtnyfyg@piout.net>
        <20170831101903.avhj5tqywpvfsj4u@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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, 31 Aug 2017 12:19:03 +0200,
Mark Brown wrote:
> 
> On Thu, Aug 31, 2017 at 10:10:21AM +0200, Alexandre Belloni wrote:
> 
> > And here is the fallout of the stupid, brainless "fixing" of issues
> > reported by static analysis tools.
> 
> > This clk_prepare_enable will never fail. If it was going to fail, the
> > platform would never boot to a point were it is able to execute that
> > code. It is really annoying to have so much churn for absolutely 0
> > benefit.
> 
> It may currently be the case that the SoCs you're looking at happen to
> make this clock essential but that doesn't mean that it's not going to
> be different in some future SoC, nor that we can't have a software bug
> that this will detect.  Being consistent with our error checking also
> means that we can spot places where it might practically be a problem
> more easily, it's even easier if the error checking is there first time
> but it's still worth it to go back later.

... yes, but only when it's done correctly.

This is again a typical problem by such a trivial fix patch: the code
looks as if it were trivial and correct, buried in a patch series that
easily leads to the oversight by the maintainer's review.


thanks,

Takashi