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

[Jyri Sarha]

On 08/14/15 19:18, Mark Brown wrote:
> 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.
>

Sounds better. I'll change that.

>> +	/* 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?
>

audio_shutdown() is for ASoC side to tell video side that audio playback 
has stopped.

The abort_cb() is for video side to inform ASoC that current audio 
stream can not continue anymore and it should be aborted. The similar 
mechanism is currently in use in sound/soc/omap/omap-hdmi-audio.c.

>> +/* 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
>

Right, should be I2S. Thanks!

Best regards,
Jyri