On Sun, Feb 09, 2020 at 10:47:42AM -0500, Adam Serbinski wrote: > > +#define AFE_API_VERSION_PCM_CONFIG 0x1 > +/* Enumeration for the auxiliary PCM synchronization signal > + * provided by an external source. > + */ > + > +#define AFE_PORT_PCM_SYNC_SRC_EXTERNAL 0x0 > +/* Enumeration for the auxiliary PCM synchronization signal > + * provided by an internal source. > + */ This is a *weird* commenting style for these #defines and it's not consistent within the block, I'm seeing at least 3 different styles. > +/* Payload of the #AFE_PARAM_ID_PCM_CONFIG command's > + * (PCM configuration parameter). > + */ > + > +struct afe_param_id_pcm_cfg { Similar weird commenting here, please follow coding-style.rst. > + switch (cfg->fmt & SND_SOC_DAIFMT_MASTER_MASK) { > + case SND_SOC_DAIFMT_CBS_CFS: > + pcfg->pcm_cfg.sync_src = AFE_PORT_PCM_SYNC_SRC_INTERNAL; > + break; > + case SND_SOC_DAIFMT_CBM_CFM: > + /* CPU is slave */ > + pcfg->pcm_cfg.sync_src = AFE_PORT_PCM_SYNC_SRC_EXTERNAL; > + break; > + default: > + break; > + } Why is this not returning an error on unsupported values? > + > + switch (cfg->sample_rate) { > + case 8000: > + pcfg->pcm_cfg.frame_setting = AFE_PORT_PCM_BITS_PER_FRAME_128; > + break; > + case 16000: > + pcfg->pcm_cfg.frame_setting = AFE_PORT_PCM_BITS_PER_FRAME_64; > + break; > + } Same here.