Re: [PATCH v2 12/16] ASoC: qcom: audioreach: add q6prm support

On 14/07/2021 18:09, Pierre-Louis Bossart wrote:

...

+static int q6prm_send_cmd_sync(struct q6prm *prm, struct gpr_pkt *pkt,

• 	uint32_t rsp_opcode)
  

+{

• gpr_device_t *gdev = prm->gdev;
• struct gpr_hdr *hdr = &pkt->hdr;
• int rc;
• mutex_lock(&prm->lock);
• prm->result.opcode = 0;
• prm->result.status = 0;
• rc = gpr_send_pkt(prm->gdev, pkt);
• if (rc < 0)

• goto err;
  

• if (rsp_opcode)

• rc = wait_event_timeout(prm->wait,
  

• 			(prm->result.opcode == hdr->opcode) ||
  

• 			(prm->result.opcode == rsp_opcode),
  

• 			5 * HZ);
  

• else

• rc = wait_event_timeout(prm->wait,
  

• 			(prm->result.opcode == hdr->opcode),
  

• 			5 * HZ);
  

• if (!rc) {

• dev_err(&gdev->dev, "CMD timeout for [%x] opcode\n",
  

• 	hdr->opcode);
  

• rc = -ETIMEDOUT;
  

• } else if (prm->result.status > 0) {

• dev_err(&gdev->dev, "DSP returned error[%x] %x\n", hdr->opcode,
  

• 	prm->result.status);
  

• rc = -EINVAL;
  

• } else {

• dev_err(&gdev->dev, "DSP returned [%x]\n",
  

• 	prm->result.status);
  

• rc = 0;
  

• }

+err:

• mutex_unlock(&prm->lock);
• return rc;

+}

In patch7 you had more or less the same code. can a helper be used?

Its possible, will abstract this out in audioreach.c.

+int audioreach_graph_send_cmd_sync(struct q6apm_graph *graph,

• 		   struct gpr_pkt *pkt, uint32_t rsp_opcode)
  

+{

• struct device *dev = graph->dev;
• struct gpr_hdr *hdr = &pkt->hdr;
• int rc;
• mutex_lock(&graph->cmd_lock);
• graph->result.opcode = 0;
• graph->result.status = 0;
• rc = gpr_send_port_pkt(graph->port, pkt);
• if (rc < 0)

• goto err;
  

• if (rsp_opcode)

• rc = wait_event_timeout(graph->cmd_wait,
  

• 			(graph->result.opcode == hdr->opcode) ||
  

• 			(graph->result.opcode == rsp_opcode),
  

• 			5 * HZ);
  

• else

• rc = wait_event_timeout(graph->cmd_wait,
  

• 			(graph->result.opcode == hdr->opcode),
  

• 			5 * HZ);
  

• if (!rc) {

• dev_err(dev, "CMD timeout for [%x] opcode\n", hdr->opcode);
  

• rc = -ETIMEDOUT;
  

• } else if (graph->result.status > 0) {

• dev_err(dev, "DSP returned error[%x] %x\n", hdr->opcode,
  

• 	graph->result.status);
  

• rc = -EINVAL;
  

• } else {

• dev_err(dev, "DSP returned [%x]\n", graph->result.status);
  

• rc = 0;
  

• }

+err:

• mutex_unlock(&graph->cmd_lock);
• return rc;

+} +EXPORT_SYMBOL_GPL(audioreach_graph_send_cmd_sync);

...

+static int q6prm_set_hw_core_req(struct device *dev, uint32_t hw_block_id, bool enable) +{

• struct prm_cmd_request_hw_core *req;
• struct apm_module_param_data *param_data;
• struct gpr_pkt *pkt;
• struct q6prm *prm = dev_get_drvdata(dev);
• gpr_device_t *gdev = prm->gdev;
• void *p;
• int rc = 0;
• uint32_t opcode, rsp_opcode;
• if (enable) {

• opcode = PRM_CMD_REQUEST_HW_RSC;
  

• rsp_opcode = PRM_CMD_RSP_REQUEST_HW_RSC;
  

• } else {

• opcode = PRM_CMD_RELEASE_HW_RSC;
  

• rsp_opcode = PRM_CMD_RSP_RELEASE_HW_RSC;
  

• }
• p = audioreach_alloc_cmd_pkt(sizeof(*req), opcode, 0, gdev->svc.id,

• 		     GPR_PRM_MODULE_IID);
  

• if (IS_ERR(p))

• return -ENOMEM;
  

• pkt = p;
• req = p + GPR_HDR_SIZE + APM_CMD_HDR_SIZE;
• param_data = &req->param_data;
• param_data->module_instance_id = GPR_PRM_MODULE_IID;
• param_data->error_code = 0;
• param_data->param_id = PARAM_ID_RSC_HW_CORE;
• param_data->param_size = sizeof(*req) - APM_MODULE_PARAM_DATA_SIZE;
• req->hw_clk_id = hw_block_id;
• q6prm_send_cmd_sync(prm, pkt, rsp_opcode);
• kfree(pkt);
• return rc;

rc is not assigned, should this not trap the result of sending a command?

My bad! will fix such instances.

--srini