From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 1/2] ASoC: Add support for Conexant CX2072X CODEC Date: Fri, 03 May 2019 10:05:25 +0200 Message-ID: References: <20190423141336.12568-1-tiwai@suse.de> <20190423141336.12568-2-tiwai@suse.de> <20190427175938.GJ14916@sirena.org.uk> <20190503064729.GF14916@sirena.org.uk> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 4B2DCF8075A for ; Fri, 3 May 2019 10:05:25 +0200 (CEST) In-Reply-To: <20190503064729.GF14916@sirena.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" To: Mark Brown Cc: alsa-devel@alsa-project.org, Pierre-Louis Bossart List-Id: alsa-devel@alsa-project.org On Fri, 03 May 2019 08:47:29 +0200, Mark Brown wrote: > > > > > +#define cx2072x_plbk_eq_en_info snd_ctl_boolean_mono_info > > > > Why not just use the function directly rather than hiding it? > > > Just a standard idiom. Can be replaced if preferred. > > Please. BTW, a good reason for the style above is that it makes code more undrestandable. For defining a ctl element, typically we define three callback functions, info, get and put, in that order: static int foo_info() { .... } static int foo_get() { .... } static int foo_put() { ... }; and often the actual snd_kcontrol_new table containing these callbacks appears much later, where you'd need to scroll down a few screens to read that point. In the above, especially defining the info callback at the beginning is important. By reading foo_init() at first, readers can know which type (int, boolean, enum, etc) and the number of elements to be used in get/put callbacks beforehand. It's a commonly seen mistake, for example, that a wrong type (e.g. integer) is passed to info callback while enum type is used in the get/put. The #define above keeps this foo_info() definition while optimizing with the standard helper. If we drop this and set snd_ctl_boolean_mono_info directly in the snd_kcontrol_new entry, you'll have to go and back the screen just to take a look at the info callback. That's why I called it a standard idiom. It's not strictly necessary, but often help reading / reviewing the code. Though, it's just another bikeshed theme, so I'm not sticking with this style. thanks, Takashi