From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753738AbaIBLEQ (ORCPT ); Tue, 2 Sep 2014 07:04:16 -0400 Received: from cantor2.suse.de ([195.135.220.15]:37931 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753699AbaIBLEO (ORCPT ); Tue, 2 Sep 2014 07:04:14 -0400 Date: Tue, 02 Sep 2014 13:04:08 +0200 Message-ID: From: Takashi Iwai To: Varka Bhadram Cc: Jean-Francois Moine , Xiubo Li , broonie@kernel.org, perex@perex.cz, lgirdwood@gmail.com, andrew@lunn.ch, kuninori.morimoto.gx@renesas.com, jsarha@ti.com, devicetree@vger.kernel.org, alsa-devel@alsa-project.org, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code. In-Reply-To: <54059F20.4080509@gmail.com> References: <1409649969-15759-1-git-send-email-Li.Xiubo@freescale.com> <1409649969-15759-2-git-send-email-Li.Xiubo@freescale.com> <54059A35.3020303@gmail.com> <20140902123848.78c4c62a@armhf> <54059F20.4080509@gmail.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Tue, 02 Sep 2014 16:12:40 +0530, Varka Bhadram wrote: > > On 09/02/2014 04:08 PM, Jean-Francois Moine wrote: > > On Tue, 02 Sep 2014 15:51:41 +0530 > > Varka Bhadram wrote: > > > >>> + switch (((np == bitclkmaster) << 4) | (np == framemaster)) { > >>> + case 0x11: > >>> + return SND_SOC_DAIFMT_CBS_CFS; > >>> + case 0x10: > >>> + return SND_SOC_DAIFMT_CBS_CFM; > >>> + case 0x01: > >>> + return SND_SOC_DAIFMT_CBM_CFS; > >>> + default: > >>> + return SND_SOC_DAIFMT_CBM_CFM; > >>> + } > >>> + > >>> + /* Shouldn't be here */ > >>> + return -EINVAL; > >>> +} > >> It will be nice if we declare the switch case numbers as macros (specific name)... > > I don't see which macros: the values are just 2 booleans. > > > I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable > names to those...? The whole switch block is too hackish, makes unnecessarily complex. It can be more strightforwardly like: if (np == bitclkmaster) return np == framemater ? SND_SOC_DAIFMT_CBS_CFS : SND_SOC_DAIFMT_CBS_CFM; else return np == framemaster ? SND_SOC_DAIFMT_CBM_CFS : SND_SOC_DAIFMT_CBM_CFM; Or, if you love efficiency and complexity, something like: #define SND_SOC_DAIFMT(_np, _clk, _frame) \ ((((_np) != (_clk)) | (((_np) != (_frame)) << 1) << 12) + (1 << 12) Then return SND_SOC_DAIFMT(np, blkclkmaster, framemaster); Takashi