Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754637AbbDTU5P (ORCPT ); Mon, 20 Apr 2015 16:57:15 -0400 Received: from mail-wg0-f52.google.com ([74.125.82.52]:36177 "EHLO mail-wg0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754330AbbDTU5M (ORCPT ); Mon, 20 Apr 2015 16:57:12 -0400 MIME-Version: 1.0 In-Reply-To: <552FB1CD.3040401@metafoo.de> References: <1429134141-17924-1-git-send-email-cernekee@chromium.org> <1429134141-17924-2-git-send-email-cernekee@chromium.org> <552FB1CD.3040401@metafoo.de> From: Kevin Cernekee Date: Mon, 20 Apr 2015 13:56:46 -0700 Message-ID: Subject: Re: [alsa-devel] [PATCH 2/3] ASoC: tas571x: New driver for TI TAS571x power amplifiers To: Lars-Peter Clausen Cc: lgirdwood@gmail.com, broonie@kernel.org, devicetree@vger.kernel.org, alsa-devel@alsa-project.org, Andrew Bresticker , "linux-kernel@vger.kernel.org" , dgreid@chromium.org, Olof Johansson Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1885 Lines: 53 On Thu, Apr 16, 2015 at 5:57 AM, Lars-Peter Clausen wrote: > On 04/15/2015 11:42 PM, Kevin Cernekee wrote: >> >> Introduce a new codec driver for the Texas Instruments >> TAS5711/TAS5717/TAS5719 power amplifier chips. These chips are typically >> used to take an I2S digital audio input and drive 10-20W into a pair of >> speakers. >> >> Signed-off-by: Kevin Cernekee > > > Looks pretty good. Comments inlune. Thanks for the review. I'm working on a V2 incorporating the feedback from you and Mark. >> +static int tas571x_i2c_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct tas571x_private *priv; >> + struct device *dev = &client->dev; >> + int i; >> + >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + i2c_set_clientdata(client, priv); >> + >> + priv->mclk = devm_clk_get(dev, "mclk"); >> + if (PTR_ERR(priv->mclk) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; > > > If you want to make the clock optional use > > if (PTR_ERR(priv->mclk) == -ENOENT) > return PTR_ERR(priv->mclk); > > This makes sure that the case where the clock is specified, but there is a > error with the specification (e.g. incorrect DT cells) is handled properly. Did you mean: > if (PTR_ERR(priv->mclk) != -ENOENT) > return PTR_ERR(priv->mclk); I don't see this in other codec drivers, but I do see the explicit EPROBE_DEFER check in max98090, max98095, pcm512x, and wm8960. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/