Re: [alsa-devel] [PATCH 1/2] ASoC: pcm5102a: Add support for PCM5102A codec

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

On Mon, May 23, 2016 at 10:37:18PM +0200, Emmanuel Fusté wrote:

> All the parameters are only rate and format.

Off the top of my head clocking constraints are another thing we could
know...

> And even rate is implementation dependent. Knowing the name of the hardware
> give nothing. You still have to look at the schematic or look at the board
> to know valid rates.

Fortunately we have mechanisms for constraining things between multiple
devices, it's not like this is a property unique to registerless CODECs.

> Yes this push more effort on the DT side, but I think this is better to
> describe in DT the actual "I2S only" hardware capabilities of the
> implementation of the device guided by wiring or design choice.
> Doing code cut&paste and two static assignment adjustment to create a "new"
> driver is completely overkill and nonflexible.

If someone was so motivated they could also make one big driver with a
table of per device constraints in it, it doesn't *have* to be separate
files.

> /* pcm5102a in HW mode with external SCK (4wire mode)
>  * at 128fs, 192fs or 256fs
>  */
> 	pcm5102a-4w: pcm5102a {
> 		compatible = "simple-i2s-codec";
> 		rate = <SNDRV_PCM_RATE_8000_192000>;
> 		format = < SNDRV_PCM_FMTBIT_S16_LE
> 			|SNDRV_PCM_FMTBIT_S24_LE
> 			|SNDRV_PCM_FMTBIT_S32_LE>;
> 	};

Those defines look awfully Linux specific...

> /* pcm5102a in HW mode with PLL on BCK (3wire mode)
>  * 16to384khz but only with bck rate of 64fs
>  */
> 	pcm5102a-4w: pcm5102a {
> 		compatible = "simple-i2s-codec";
> 		rate = <SNDRV_PCM_RATE_8000_192000
> 			^ SNDRV_PCM_RATE_8000>;
> 		format = <SNDRV_PCM_FMTBIT_S32_LE>;
> 	};

...and another way of doing the multiple options would be to have the
binding be per device and use the clock binding to describe where the
master clock comes from which then allows us to start and stop that
clock on demand and possibly (as our clock API integration improves)
constrain based on that.  It may be that in future or perhaps even just
for other users of the same board the set of constraints is different
(perhaps they choose some other clock rate for something in the system
due to other constraints which lets the master clock vary more for
example).  

It's also just more work for the second person to come across the
device, at best they have to go and type in the same data over again.

A brief look at the datasheet suggests that there's also a few GPIO
controls we might have wired up where we can expose them and of course
regulators we may want to control in some systems.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]