lkml.org 
[lkml]   [2021]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 05/16] ASoC: qcom: audioreach: add basic pkt alloc support
From
Date
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;
>> +}
>

\
 
 \ /
  Last update: 2021-07-15 12:32    [W:0.075 / U:0.880 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site