[alsa-devel] [PATCH 2/3] ASoC: wm8741: Set OSR mode in hw_params()

[Charles Keepax]

On Tue, Sep 05, 2017 at 08:36:24PM +0200, Sergej Sawazki wrote:
> Am 05.09.2017 um 18:12 schrieb Charles Keepax:
> > On Mon, Sep 04, 2017 at 09:34:12PM +0200, Sergej Sawazki wrote:
> > > +	/* oversampling rate */
> > > +	if (params_rate(params) > 96000)
> > > +		mode |= 0x0040;
> > > +	else if (params_rate(params) > 48000)
> > > +		mode |= 0x0020;
> > 
> > Should this not have a case for <= 48k as well? To reset the mode
> > if it was set on a previous playback.
> > 
> 
> It is reset in here:
> 
>     u16 mode = snd_soc_read(codec, WM8741_MODE_CONTROL_1) & 0x183;
> 
> For <= 48k we simply leave it 0. I though about an empty else statement, but
> it is kind of strange, too.
> 

Oops.. sorry yes that is fine.

> > > +
> > >   	/* bit size */
> > >   	switch (params_width(params)) {
> > >   	case 16:
> > > @@ -243,6 +250,7 @@ static int wm8741_hw_params(struct snd_pcm_substream *substream,
> > >   		params_width(params), params_rate(params));
> > >   	snd_soc_write(codec, WM8741_FORMAT_CONTROL, iface);
> > > +	snd_soc_write(codec, WM8741_MODE_CONTROL_1, mode);
> > 
> > Better to use an update_bits rather than basically open-code it
> > with read and then this write.
> > 
> 
> I have used write() and not update_bits() here to be consistent with the way
> the WM8741_FORMAT_CONTROL register is written. If we change it to
> update_bits(), we should refactor the WM8741_FORMAT_CONTROL write to use
> update_bits() too, right?
> 

Yeah that should probably be an update bits too by the looks of
it.

Thanks,
Charles