Re: [alsa-devel] [PATCH v5] ASoC: TSCS42xx: Add support for Tempo Semiconductor's TSCS42xx audio CODEC

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Thu, Dec 14, 2017 at 04:43:10PM -0600, Steven Eckhoff wrote:
> On Thu, Dec 14, 2017 at 04:36:17PM -0600, Steven Eckhoff wrote:
> > On Thu, Dec 14, 2017 at 09:32:44AM +0000, Charles Keepax wrote:
> > 
> > > Not sure what you mean here but setting up CODEC registers
> > > directly from the machine driver is probably not ideal. You
> > > should probably be looking into regmap_register_patch this lets
> > > you apply a bunch of register settings to the chip every time you
> > > do a regmap_cache_sync. This is useful for situations like the
> > > chip has terrible register defaults you want to correct.
> > 
> > Thanks I will keep that in mind. For now our defaults are okay, but
> > this may come in handy in other drivers. I have removed this bit from
> > the next version since this can be set by the machine driver and it
> > makes more sense there since the settings were configuring how the I2S
> > interfaces were connected.
> 
> I apologize I had to reread that.
> 
> The settings were setting whether the bclk and lrclk would be shared
> between the DAC and ADC interfaces. I thought this would make sense in
> the machine driver since it knows how the devices are connected. Is
> there a better way to do this?

Hmm... might need to think about the case of whether to share a
clock depends a little on what the hardware actually looks like.

But basically my point is that writing random CODEC registers
from the machine driver is not very nice. You want to be making
use of ASoC calls etc. to have the CODEC driver apply the setting
you want.

For example you might call snd_soc_dai_set_fmt to configure
who is master/slave, but you wouldn't want to write the bit
that selects master mode on the CODEC directly from the machine
driver.

Thanks,
Charles