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 <varkabhadram@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html