Re: [PATCH 06/16] ASoC: Intel: Skylake: Add support for Load/Unload IPCs

From: Mark Brown <broonie@kernel.org>
To: Vinod Koul <vinod.koul@intel.com>
Cc: liam.r.girdwood@linux.intel.com, patches.audio@intel.com,
	alsa-devel@alsa-project.org,
	Dharageswari R <dharageswari.r@intel.com>,
	Jeeja KP <jeeja.kp@intel.com>
Subject: Re: [PATCH 06/16] ASoC: Intel: Skylake: Add support for Load/Unload IPCs
Date: Tue, 1 Dec 2015 22:58:54 +0000	[thread overview]
Message-ID: <20151201225854.GZ1929@sirena.org.uk> (raw)
In-Reply-To: <1448703121-5831-7-git-send-email-vinod.koul@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1731 bytes --]

On Sat, Nov 28, 2015 at 03:01:51PM +0530, Vinod Koul wrote:

> Some firmware modules can be loaded and unloaded to/from DSP.
> This can be done by invoking IPCs Load module and unload module
> respectively. So this patch starts this by adding support for
> these IPCs

In what way does it do this?

> +int skl_ipc_load_modules(struct sst_generic_ipc *ipc,
> +				u8 module_cnt, void *data)
> +{
> +	struct skl_ipc_header header = {0};
> +	u64 *ipc_header = (u64 *)(&header);
> +	int ret;
> +
> +	header.primary = IPC_MSG_TARGET(IPC_FW_GEN_MSG);
> +	header.primary |= IPC_MSG_DIR(IPC_MSG_REQUEST);
> +	header.primary |= IPC_GLB_TYPE(IPC_GLB_LOAD_MULTIPLE_MODS);
> +	header.primary |= IPC_LOAD_MODULE_CNT(module_cnt);
> +
> +	ret = sst_ipc_tx_message_wait(ipc, *ipc_header, data,
> +				(sizeof(u16) * module_cnt), NULL, 0);

So the multiple modules are a block of at most 255 16 bit words?  That's
a bit surprising - is it really a count of the number of modules or
rather the size of the block of data that's being squirted at the DSP?

> +int skl_ipc_unload_modules(struct sst_generic_ipc *ipc, u8 module_cnt,
> +							void *data)
> +{
> +	struct skl_ipc_header header = {0};
> +	u64 *ipc_header = (u64 *)(&header);
> +	int ret;
> +
> +	header.primary = IPC_MSG_TARGET(IPC_FW_GEN_MSG);
> +	header.primary |= IPC_MSG_DIR(IPC_MSG_REQUEST);
> +	header.primary |= IPC_GLB_TYPE(IPC_GLB_UNLOAD_MULTIPLE_MODS);
> +	header.primary |= IPC_LOAD_MODULE_CNT(module_cnt);
> +
> +	ret = sst_ipc_tx_message_wait(ipc, *ipc_header, data,
> +				(sizeof(u16) * module_cnt), NULL, 0);

This seems weird...  what's the data that's getting passed in?  This
doesn't seem to be actually managing anything, it's just some very basic
message formatting.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]