From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932120AbbFIRIB (ORCPT ); Tue, 9 Jun 2015 13:08:01 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:50532 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750857AbbFIRHx (ORCPT ); Tue, 9 Jun 2015 13:07:53 -0400 Date: Tue, 9 Jun 2015 18:07:38 +0100 From: Mark Brown To: Srinivas Kandagatla Cc: alsa-devel@alsa-project.org, Rob Herring , Patrick Lai , Banajit Goswami , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, kwestfie@codeaurora.org, linux-arm-msm@vger.kernel.org Message-ID: <20150609170738.GI14071@sirena.org.uk> References: <1433854702-23654-1-git-send-email-srinivas.kandagatla@linaro.org> <1433854776-23852-1-git-send-email-srinivas.kandagatla@linaro.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gtHcOevdugseuBBz" Content-Disposition: inline In-Reply-To: <1433854776-23852-1-git-send-email-srinivas.kandagatla@linaro.org> X-Cookie: The end of labor is to gain leisure. User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 94.175.94.161 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [PATCH v5 2/2] ASoC: qcom: add apq8016 sound card support X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on mezzanine.sirena.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --gtHcOevdugseuBBz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jun 09, 2015 at 01:59:36PM +0100, Srinivas Kandagatla wrote: > + if (cpu_dai->id == MI2S_QUATERNARY) { > + /* Configure the Quat MI2S to TLMM */ > + writel(readl(pdata->mic_iomux) | > + MIC_CTRL_QUA_WS_SLAVE_SEL_10 | > + MIC_CTRL_TLMM_SCLK_EN, > + pdata->mic_iomux); > + > + return 0; > + } else if (cpu_dai->id == MI2S_PRIMARY) { > + writel(readl(pdata->spkr_iomux) | > + SPKR_CTL_PRI_WS_SLAVE_SEL_11, > + pdata->spkr_iomux); > + > + return 0; > + } Why not just do these one time at probe, we don't undo them when we shut the DAI down? > + > + dev_err(card->dev, "unsupported cpu dai configuration\n"); > + > + return -EINVAL; It'd be clearer if this were part of the above code (which should still be written as a switch statement) - I was just asking myself what happens if we fall off the end of the if/else chain. > + /** > + * External codec is ADV7533 > + * and internal codec digital part is inside apq8016 > + * and analog part is in PMIC PM8916 > + **/ > + if (of_property_read_bool(np, "external")) > + name = "ADV7533"; > + else > + name = "WCD"; If this is all the property is doing why not just put a link name property in the DT rather than this? That makes the driver a bit more general and is more idiomatic. > + dai_link->name = dai_link->stream_name = name; Write two assignment statements, it's clearer and more idiomatic. --gtHcOevdugseuBBz Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJVdx1ZAAoJECTWi3JdVIfQxFgH/jzNcFHvXyGnxqHPImTkc0q2 /o2hYvRa049axJM0b/OYPL8lGfGM+Hlaf1J6lynKb012CZWyraAsj9rZkK+K0vSs ruPFIdiiYHRhxcCSPbxh+oPIZA+BUrezGS6HnKnClIo5S0j37T7nxT0aSPhvet+k /4ja1MpjppB+5xYtTr58eJKFi9R2AuvlR7QoKrxDVSvXrGOqqUT5DkyJilorSJFa xRYJd4Rzq8zX8U08a16SsdOvrcXFCTQcUEmSIfApS/YqJd9yCsWbxA2G0OERxucR M9hDaxRg3dBwnYO80NDvrCDLusDCx7eoAvdJkbtmpR/x1snn0TSHopgQpES0Ngo= =WHZu -----END PGP SIGNATURE----- --gtHcOevdugseuBBz--