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
