Re: [PATCH v2 2/2] dt: binding: sound cs42l52 driver

[Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Wednesday 13 of November 2013 13:10:51 Brian Austin wrote:
> On Wed, 13 Nov 2013, Tomasz Figa wrote:
> 
> >>>> +		 0x00 through 0x0F.
> >>>> +		 Frequency = (64xFs)/(N+2)
> >>>
> >>> I suppose N means the value of chgfreq property here, but this should be
> >>> clearly stated. Same for Fs - I suppose it is sampling frequency, but
> >>> is it default, minimum, maximum or maybe something else?
> >>>
> >> Frequency = (64xFs)/(N+2)
> >> N = chgfreq value
> >> Fs = sample rate
> >
> > As I mentioned in second part of my comment, is it default, minimum,
> > maximum or what kind of sample rate? Or is the sample rate fixed?
> >
> ah! Sorry, the rate is fixed for the stream. So it varies for what rate is 
> being used.
> 

OK, so the charge pump frequency varies with sampling rate. Well, I guess
that's fine to provide a raw register value then. The name is still a bit
misleading, though.

Maybe calling it cirrus,chgfreq-divisor would be better instead? Also the
description in binding documentation should explicitly state that it's the
raw value that needs to be written to the CHGFREQ register.

> >>>> +  - mica-sel   : MIC A single ended input select.  For Single-Ended
> >>>> +		 configuration, select which MIC to use.
> >>>> +		 0x00 = MIC1
> >>>> +		 0x01 = MIC2
> >>>> +  - micb-sel   : MIC B single ended input select.  For Single-Ended
> >>>> +		 configuration, select which MIC to use.
> >>>> +		 0x00 = MIC1
> >>>> +		 0x01 = MIC2
> >>>
> >>> Could you explain what are MIC A, MIC B, MIC1 and MIC2?
> >>>
> >> I can add a little more but it is best explained in the datasheet. I can
> >> add a little more explaination and reference the datasheet section. I feel
> >> this file might be the wrong place to go into too much depth of the pieces
> >> of the device when the datasheet could be referenced. Would a reference be
> >> OK?
> >>
> >
> > I meant just explaining to me, for the purpose of this review. However it
> > seems like the datasheet is publicly available, so let me just check this
> > there.
> >
> Sorry about that.
> 
> The CS42L52 has multiple input selections for microphones. Single-ended 
> and differential inputs are allowed for both Left Channel (MICA) and Right 
> Channel (MICB). The +- pins can be used for stereo or mono mics.

OK. After looking at the datasheet for a bit, I'm pretty much convinced
that these two properties should be rather represented as mixer controls
instead, for the purpose of channel mixing.

Whether the "Mic X Sel" control would be present in the mixer or not would
be implied by mic-X-differential property in DT.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html