23 Jan
2017
23 Jan
'17
6:59 p.m.
On Mon, Jan 23, 2017 at 11:41:47AM +0100, Romain Perier wrote:
- 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.
- /* 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.