On Fri, Aug 31, 2018 at 04:19:02PM +0100, Liam Girdwood wrote:

> +/* SSP Configuration Request - SOF_IPC_DAI_SSP_CONFIG */
> +struct sof_ipc_dai_ssp_params {

> +	/* MCLK */
> +	uint32_t mclk_direction;
> +	uint32_t mclk_keep_active;
> +	uint32_t bclk_keep_active;
> +	uint32_t fs_keep_active;

Is this assuming a 1:1 relationship between the serial port and the
MCLK?  That's not going to be true for all systems.  This definition
does feel a bit specific to the SSP (or I guess just general serial
ports used for audio as opposed to fixed function I2S/PCM controllers).
I don't know if it's worth splitting up though.

> +	uint16_t frame_pulse_width;
> +	uint32_t quirks; // FIXME: is 32 bits enough ?

Better go for 640K to be sure :) ...

> +/* HDA Configuration Request - SOF_IPC_DAI_HDA_CONFIG */
> +struct sof_ipc_dai_hda_params {
> +	struct sof_ipc_hdr hdr;
> +	/* TODO */
> +} __attribute__((packed));

...more seriously I do notice a bunch of these FIXME and TODO comments
in here, given that it's an ABI it seems like those all need sorting
before things are merged.  In this case perhaps just delete the HDA
stuff until someone works out what to do?