Re: [PATCH 2/3] ASoC: Ux500: Add machine-driver

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1575 bytes --]

On Mon, Jun 11, 2012 at 02:22:33PM +0200, Ola Lilja wrote:

> >> +static inline const char *get_mclk_str(enum mclk mclk_sel)
> >> +{
> >> +	switch (mclk_sel) {
> >> +	case MCLK_SYSCLK:
> >> +		return "MCLK_SYSCLK";
> >> +	case MCLK_ULPCLK:
> >> +		return "MCLK_ULPCLK";

> > Why not just drop the MCLK_ from the strings?  I'd expect that'll be
> > implied by the context...

> I like having some kind of prefix before the alternatives in my enums,
> even if you could figure out what its for without it.

Is the control name not enough for that?  I'm only suggesting this for
the strings...

> >> +	/* Digital interface - Clocks */
> >> +	SOC_SINGLE("Digital Interface Master Generator Switch",
> >> +		AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENMASTGEN,
> >> +		1, 0),

> > This enables and disables a 256fs (or similar ratio) clock output from
> > the device?

> This is the clocking of the digital interface which is used together with the
> bit-clock switches when we are in gated mode (see discussions regarding these
> parameters in earlier review of the codec-driver).

> >> +	SOC_SINGLE("Digital Interface 0 Bit-clock Switch",
> >> +		AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK0,
> >> +		1, 0),
> >> +	SOC_SINGLE("Digital Interface 1 Bit-clock Switch",
> >> +		AB8500_DIGIFCONF1, AB8500_DIGIFCONF1_ENFSBITCLK1,
> >> +		1, 0),

> > I suspect that these are doing what DAIFMT_CONT is for...

> See comment above.

Can you refresh my memory on this discussion please?  There's been
several occasions where I've been waiting for new code to make unclear
things clearer.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]