Re: [PATCH RFC v3 3/7] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders

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

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

On Fri, Aug 14, 2015 at 12:30:41PM +0300, Jyri Sarha wrote:

> +struct hdmi_codec_ops {
> +	/* For runtime clock configuration from ASoC machine driver.
> +	 * A direct forward from set_sysclk in struct snd_soc_dai_ops.
> +	 * Optional */
> +	int (*set_clk)(struct device *dev, int clk_id, int freq);

I'd be much happier if we were using the clock API as the external
interface here, it's where we want to be internally too and it's going
to be easier to not introduce any external dependencies on the ASoC
internal stuff.

> +	/* Called when ASoC starts an audio stream setup. The call
> +	 * provides an audio abort callback for stoping an ongoing
> +	 * stream if the HDMI audio becomes unavailable.
> +	 * Optional */
> +	int (*audio_startup)(struct device *dev,
> +			     void (*abort_cb)(struct device *dev));

I'm a bit confused about what is going to use abort_cb() and why they
wouldn't just call shutdown instead?

> +/* HDMI codec initalization data */
> +struct hdmi_codec_pdata {
> +	struct device *dev; /* The HDMI encoder registering the codec */

Shouldn't this just be dev->parent?

> +enum {
> +	DAI_ID_I2C = 0,
> +	DAI_ID_SPDIF,
> +};

I2C?  :P

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