From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Brown Subject: Re: [PATCH] ASoC: Add max98088 CODEC driver Date: Wed, 13 Oct 2010 11:32:24 +0100 Message-ID: <20101013103224.GK6424@rakim.wolfsonmicro.main> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from opensource2.wolfsonmicro.com (opensource.wolfsonmicro.com [80.75.67.52]) by alsa0.perex.cz (Postfix) with ESMTP id CC35610381C for ; Wed, 13 Oct 2010 12:32:29 +0200 (CEST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: Peter Hsiang Cc: "alsa-devel@alsa-project.org" , Takashi Iwai , Peter Ujfalusi , "linux-kernel@vger.kernel.org" , Jesse Marroquin , Liam Girdwood List-Id: alsa-devel@alsa-project.org On Tue, Oct 12, 2010 at 06:20:10PM -0700, Peter Hsiang wrote: This looks pretty good, waiting for Liam's review but I think we can do further stuff incrementally. Minor comments: > + > + /* Setup an array of texts for the equalizer enum. > + * This is based on Mark Brown's equalizer driver code. > + * It has been extended to support multiple equalizers. > + */ The code that you're basing this on supports multiple equalizers too - things like the WM8994 have several. Given that all those supported by the original code are identical there didn't seem to be any point in restricting which EQ a given setup can be deployed on - it's just one more thing that can go wrong. > +static int max98088_remove(struct snd_soc_codec *codec) > +{ > + if (codec->control_data) > + max98088_set_bias_level(codec, SND_SOC_BIAS_OFF); The control_data check should be redundant here. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752364Ab0JMKcb (ORCPT ); Wed, 13 Oct 2010 06:32:31 -0400 Received: from opensource.wolfsonmicro.com ([80.75.67.52]:55532 "EHLO opensource2.wolfsonmicro.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751736Ab0JMKca (ORCPT ); Wed, 13 Oct 2010 06:32:30 -0400 Date: Wed, 13 Oct 2010 11:32:24 +0100 From: Mark Brown To: Peter Hsiang Cc: Jaroslav Kysela , Takashi Iwai , Liam Girdwood , Peter Ujfalusi , "alsa-devel@alsa-project.org" , "linux-kernel@vger.kernel.org" , Jesse Marroquin Subject: Re: [PATCH] ASoC: Add max98088 CODEC driver Message-ID: <20101013103224.GK6424@rakim.wolfsonmicro.main> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Cookie: 667: User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 12, 2010 at 06:20:10PM -0700, Peter Hsiang wrote: This looks pretty good, waiting for Liam's review but I think we can do further stuff incrementally. Minor comments: > + > + /* Setup an array of texts for the equalizer enum. > + * This is based on Mark Brown's equalizer driver code. > + * It has been extended to support multiple equalizers. > + */ The code that you're basing this on supports multiple equalizers too - things like the WM8994 have several. Given that all those supported by the original code are identical there didn't seem to be any point in restricting which EQ a given setup can be deployed on - it's just one more thing that can go wrong. > +static int max98088_remove(struct snd_soc_codec *codec) > +{ > + if (codec->control_data) > + max98088_set_bias_level(codec, SND_SOC_BIAS_OFF); The control_data check should be redundant here.