Re: [PATCH v3 10/17] ASoC: Intel: avs: Add basefw runtime-parameter requests

[Ranjani Sridharan <ranjani.sridharan@linux.intel.com>]

On Fri, 2022-03-04 at 15:57 +0100, Cezary Rojewski wrote:
> Each module may expose a range of runtime parameters. For basefw,
> implement handlers for: FIRMWARE_CONFIG, HARDWARE_CONFIG and
> MODULES_INFO. These are used by driver to dynamically allocate
> resources
> in respect to platform details, reducing number of hardcodes and code
> duplications that would otherwise be needed to be defined within the
> driver code.
> 
> Signed-off-by: Amadeusz Sławiński <
> amadeuszx.slawinski@linux.intel.com>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
>  sound/soc/intel/avs/messages.c | 215
> +++++++++++++++++++++++++++++++++
>  sound/soc/intel/avs/messages.h | 179 +++++++++++++++++++++++++++
>  2 files changed, 394 insertions(+)
> 
> diff --git a/sound/soc/intel/avs/messages.c
> b/sound/soc/intel/avs/messages.c
> index f7d00e541323..7a2a7206df4b 100644
> --- a/sound/soc/intel/avs/messages.c
> +++ b/sound/soc/intel/avs/messages.c
> @@ -463,3 +463,218 @@ int avs_ipc_set_d0ix(struct avs_dev *adev, bool
> enable_pg, bool streaming)
>  
>  	return ret;
>  }
> +
> +int avs_ipc_get_fw_config(struct avs_dev *adev, struct avs_fw_cfg
> *cfg)
> +{
> +	struct avs_tlv *tlv;
> +	size_t payload_size;
> +	size_t offset = 0;
> +	u8 *payload;
> +	int ret;
> +
> +	ret = avs_ipc_get_large_config(adev, AVS_BASEFW_MOD_ID,
> AVS_BASEFW_INST_ID,
> +				       AVS_BASEFW_FIRMWARE_CONFIG,
> NULL, 0,
> +				       &payload, &payload_size);
> +	if (ret)
> +		return ret;
> +
> +	while (offset < payload_size) {
> +		tlv = (struct avs_tlv *)(payload + offset);
> +
> +		switch (tlv->type) {
> +		case AVS_FW_CFG_FW_VERSION:
> +			memcpy(&cfg->fw_version, tlv->value,
> +				sizeof(cfg->fw_version));
> +			break;
> +
> +		case AVS_FW_CFG_MEMORY_RECLAIMED:
> +			cfg->memory_reclaimed = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_SLOW_CLOCK_FREQ_HZ:
> +			cfg->slow_clock_freq_hz = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_FAST_CLOCK_FREQ_HZ:
> +			cfg->fast_clock_freq_hz = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_ALH_SUPPORT_LEVEL:
> +			cfg->alh_support = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_IPC_DL_MAILBOX_BYTES:
> +			cfg->ipc_dl_mailbox_bytes = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_IPC_UL_MAILBOX_BYTES:
> +			cfg->ipc_ul_mailbox_bytes = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_TRACE_LOG_BYTES:
> +			cfg->trace_log_bytes = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MAX_PPL_COUNT:
> +			cfg->max_ppl_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MAX_ASTATE_COUNT:
> +			cfg->max_astate_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MAX_MODULE_PIN_COUNT:
> +			cfg->max_module_pin_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MODULES_COUNT:
> +			cfg->modules_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MAX_MOD_INST_COUNT:
> +			cfg->max_mod_inst_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MAX_LL_TASKS_PER_PRI_COUNT:
> +			cfg->max_ll_tasks_per_pri_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_LL_PRI_COUNT:
> +			cfg->ll_pri_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MAX_DP_TASKS_COUNT:
> +			cfg->max_dp_tasks_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_MAX_LIBS_COUNT:
> +			cfg->max_libs_count = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_XTAL_FREQ_HZ:
> +			cfg->xtal_freq_hz = *tlv->value;
> +			break;
> +
> +		case AVS_FW_CFG_POWER_GATING_POLICY:
> +			cfg->power_gating_policy = *tlv->value;
> +			break;
> +
> +		/* Known but not useful to us. */
> +		case AVS_FW_CFG_DMA_BUFFER_CONFIG:
> +		case AVS_FW_CFG_SCHEDULER_CONFIG:
> +		case AVS_FW_CFG_CLOCKS_CONFIG:
> +			break;
> +
> +		default:
> +			dev_info(adev->dev, "Unrecognized fw param:
> %d\n",
> +				 tlv->type);
> +			break;
> +		}
> +
> +		offset += sizeof(*tlv) + tlv->length;
> +	}
> +
> +	kfree(payload);
I think it would be easier to understand this kfree if payload was also
allocated in this function in stead of inside the get_large_config().
Thanks,
Ranjani