Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751530AbdGZL3D (ORCPT ); Wed, 26 Jul 2017 07:29:03 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:37696 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751328AbdGZL3C (ORCPT ); Wed, 26 Jul 2017 07:29:02 -0400 Date: Wed, 26 Jul 2017 12:28:17 +0100 From: Mark Brown To: Arvind Yadav Cc: perex@perex.cz, tiwai@suse.com, krzk@kernel.org, sbkim73@samsung.com, lgirdwood@gmail.com, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Message-ID: <20170726112817.sdcfz25nhiitkhd2@sirena.org.uk> References: <84c0014d4dbd466f52c45d8efbd2c1080d8174fb.1501047400.git.arvind.yadav.cs@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ixcskedgoiumihx5" Content-Disposition: inline In-Reply-To: <84c0014d4dbd466f52c45d8efbd2c1080d8174fb.1501047400.git.arvind.yadav.cs@gmail.com> X-Cookie: Do not remove tag under penalty of law. User-Agent: NeoMutt/20170609 (1.8.3) X-SA-Exim-Connect-IP: 2001:470:1f1d:6b5::3 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [PATCH v2 01/11] ASoC: samsung: s3c2412: Handle return value of clk_prepare_enable. X-SA-Exim-Version: 4.2.1 (built Tue, 02 Aug 2016 21:08:31 +0000) X-SA-Exim-Scanned: No (on mezzanine.sirena.org.uk); Unknown failure Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2003 Lines: 61 --ixcskedgoiumihx5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 26, 2017 at 11:15:25AM +0530, Arvind Yadav wrote: > --- a/sound/soc/samsung/s3c2412-i2s.c > +++ b/sound/soc/samsung/s3c2412-i2s.c > @@ -65,13 +65,16 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai) > s3c2412_i2s.iis_cclk =3D devm_clk_get(dai->dev, "i2sclk"); > if (IS_ERR(s3c2412_i2s.iis_cclk)) { > pr_err("failed to get i2sclk clock\n"); > - return PTR_ERR(s3c2412_i2s.iis_cclk); > + ret =3D PTR_ERR(s3c2412_i2s.iis_cclk); > + goto err; > } > =20 Why are we making this unrelated change? None of the error handling we jump to is relevant if this fails... > /* Set MPLL as the source for IIS CLK */ > =20 > clk_set_parent(s3c2412_i2s.iis_cclk, clk_get(NULL, "mpll")); > - clk_prepare_enable(s3c2412_i2s.iis_cclk); > + ret =3D clk_prepare_enable(s3c2412_i2s.iis_cclk); > + if (ret) > + goto err; > =20 > s3c2412_i2s.iis_cclk =3D s3c2412_i2s.iis_pclk; > =20 > @@ -80,6 +83,11 @@ static int s3c2412_i2s_probe(struct snd_soc_dai *dai) > S3C_GPIO_PULL_NONE); > =20 > return 0; > + > +err: > + clk_disable(s3c2412_i2s.iis_pclk); This will disable the clock if we failed to enable it which is clearly not correct. It's also matching a clk_prepare_enable() with a clk_disable() which is going to leave an unbalanced prepare. --ixcskedgoiumihx5 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAll4fNAACgkQJNaLcl1U h9DkhQf/bachrzLQ5WsOE6lusQ2jBuikcm4zuO6h5bQps7WwKWqUYDy9WgUEalJS JnGMmf50ejPQN4DWvem4AgubLD1e6ZsiXvp9vPn6zRiLpNqNdFYzZHg5VPBtq4/G lkMZTF87M+tE0pQkLkqddnzAyOti3pZJsaSpE9SCjc/GfJqSfZl/c/6G3E6fT/2N JkrLYA0vh6fjpeib5m6WiHW/n9GsDJViL61i7kRXCTwThIbNcYrLACrBYgCAp6KB nNLAf9H9Bo8J2oMcTKrAG0LY8NZPyPiKNFxu7PXOAG9l+aI+Acc3onvHGsrDMd9G NWmKvm7KlVlubG7o6UQ4GnpmFg+mkw== =belb -----END PGP SIGNATURE----- --ixcskedgoiumihx5--