Re: [PATCH v4 2/4] ASoC: es8328: Add support for slave mode
From: Mark Brown <hidden>
Date: 2017-01-23 17:59:17
Also in:
alsa-devel, linux-rockchip
On Mon, Jan 23, 2017 at 11:41:47AM +0100, Romain Perier wrote:
quoted hunk
+ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBM_CFM: + master = true; + break; + case SND_SOC_DAIFMT_CBS_CFS: + master = false; + break; + default: + return -EINVAL; + }
Please use the normal kernel coding style. People read in part by pattern matching so this is important to ease review and coding style problems are a big warning sign that the code also has problems with other, more substantial, understanding of how the kernel is expected to work.
quoted hunk
- /* Master serial port mode, with BCLK generated automatically */ - snd_soc_update_bits(codec, ES8328_MASTERMODE, - ES8328_MASTERMODE_MSC, ES8328_MASTERMODE_MSC); + if (master) { + /* Master serial port mode, with BCLK generated automatically */ + snd_soc_update_bits(codec, ES8328_MASTERMODE, + ES8328_MASTERMODE_MSC, + ES8328_MASTERMODE_MSC); + } else { + /* Slave serial port mode */ + snd_soc_update_bits(codec, ES8328_MASTERMODE, + ES8328_MASTERMODE_MSC, + 0); + }
Why not just directly do this in the switch statement? This appears to be the only place where we change behaviour so setting a variable at the top of the function then using it at the bottom doesn't seem to add anything.
Attachments
- signature.asc [application/pgp-signature] 488 bytes