Messages in this thread | | | Subject | Re: [PATCH v2 05/16] ASoC: qcom: audioreach: add basic pkt alloc support | From | Srinivas Kandagatla <> | Date | Thu, 15 Jul 2021 11:31:49 +0100 |
| |
Thanks for review Pierre,
On 14/07/2021 17:30, Pierre-Louis Bossart wrote: > > > >> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig >> index cc7c1de2f1d9..721ea56b2cb5 100644 >> --- a/sound/soc/qcom/Kconfig >> +++ b/sound/soc/qcom/Kconfig >> @@ -103,6 +103,12 @@ config SND_SOC_QDSP6 >> audio drivers. This includes q6asm, q6adm, >> q6afe interfaces to DSP using apr. >> >> +config SND_SOC_QCOM_AUDIOREACH >> + tristate "SoC ALSA audio drives for Qualcomm AUDIOREACH" > > typo: drivers? > Will fix all the typos in next spin.
>> +static void *__audioreach_alloc_pkt(int payload_size, uint32_t opcode, >> + uint32_t token, uint32_t src_port, >> + uint32_t dest_port, bool has_cmd_hdr) >> +{ >> + struct apm_cmd_header *cmd_header; >> + struct gpr_pkt *pkt; >> + void *p; >> + int pkt_size = GPR_HDR_SIZE + payload_size; >> + >> + if (has_cmd_hdr) >> + pkt_size += APM_CMD_HDR_SIZE; >> + >> + p = kzalloc(pkt_size, GFP_ATOMIC); > > is GFP_ATOMIC required? it's the same question really than my earlier one on spinlock_irqsave, it's rather hard to figure out in what context this code will run.
I had some spinlocks in this patch for compress offload cases, so had to make it ATOMIC. having said that we could avoid ATOMIC here.
> >> + if (!p) >> + return ERR_PTR(-ENOMEM); >> + >> + pkt = p; >> + pkt->hdr.version = GPR_PKT_VER; >> + pkt->hdr.hdr_size = 6; > > magic number. looks like a missing macro...
There is already a macro for this, GPR_PKT_HEADER_WORD_SIZE I should have used it. > >> + pkt->hdr.pkt_size = pkt_size; >> + pkt->hdr.dest_port = dest_port; >> + pkt->hdr.src_port = src_port; >> + >> + pkt->hdr.dest_domain = GPR_DOMAIN_ID_ADSP; >> + pkt->hdr.src_domain = GPR_DOMAIN_ID_APPS; >> + pkt->hdr.token = token; >> + pkt->hdr.opcode = opcode; >> + >> + if (has_cmd_hdr) { >> + p = p + GPR_HDR_SIZE; >> + cmd_header = p; >> + cmd_header->payload_size = payload_size; >> + } >> + >> + return pkt; >> +} >
| |