Skip to content

Apm spf status fix #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
man-rav wants to merge 2 commits into
base: master
Choose a base branch
from
+20 −9
Open

Apm spf status fix #47

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e4aba1
audioreach-kernel: fix SPF state response handling
man-rav Jan 29, 2026
f2c1b6e
audioreach-kernel: use resource managed version of of_platform_populate
man-rav Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions audioreach-driver/q6apm_audio_pkt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ struct gpr_port_map {
u32 dst_port;
};

struct apm_cmd_rsp_get_spf_status_t {
/* Spf status
* @values
* 0 -> Not ready
* 1 -> Ready
*/
uint32_t status;
};

#define dev_to_audpkt_dev(_dev) container_of(_dev, struct q6apm_audio_pkt, dev)
#define cdev_to_audpkt_dev(_cdev) container_of(_cdev, struct q6apm_audio_pkt, cdev)

Expand Down Expand Up @@ -576,7 +585,7 @@ static int q6apm_audio_pkt_probe(gpr_device_t *adev)

q6apm_audio_is_adsp_ready();
AUDIO_PKT_INFO("Audio Packet Port Driver Initialized\n");
return of_platform_populate(dev->of_node, NULL, NULL, dev);
return devm_of_platform_populate(dev);

free_dev:
put_device(dev);
Expand All @@ -596,6 +605,7 @@ static int q6apm_audio_pkt_callback(struct gpr_resp_pkt *data, void *priv, int o
struct q6apm_audio_pkt *apm = dev_get_drvdata(&gdev->dev);
struct gpr_ibasic_rsp_result_t *result;
struct gpr_hdr *hdr = &data->hdr;
struct apm_cmd_rsp_get_spf_status_t *spf_status;
uint8_t *pkt = NULL;
uint16_t hdr_size, pkt_size;
unsigned long flags;
Expand Down Expand Up @@ -638,23 +648,24 @@ static int q6apm_audio_pkt_callback(struct gpr_resp_pkt *data, void *priv, int o
skb_queue_tail(&apm->queue, skb);
spin_unlock_irqrestore(&apm->queue_lock, flags);

if (hdr->opcode == APM_CMD_RSP_GET_SPF_STATE) {
result = data->payload;
spf_status = (struct apm_cmd_rsp_get_spf_status_t *)(pkt+hdr_size);
Copy link
Contributor

@nandamajay nandamajay Jan 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spf_status is a struct, but you’re assigning a pointer to it and later dereferencing as spf_status->status. This is undefined and will likely crash.

Use‑before‑init for pkt / hdr_size : pkt and hdr_size are used without initializing.

Does this patch is validated , before submitting ?

Copy link
Author

@man-rav man-rav Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with respect to spf_status structure pointer, fixed this on a follow-up commit had issue syncing changes in fork with PR.

For the query on Use-Before-init for "hdr_size" & "pkt" pointer , line 607 & line 623-628 have these initialized and setup for usage on line 653.

Playback and Capture use case has been validated on Rb3Gen2, with this change.

Copy link
Contributor

@nandamajay nandamajay Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update @man-rav

apm->result.opcode = result->opcode;
apm->result.status = result->status;
/* First word of result it state */
apm->state = spf_status->status;
}

/* wake up any blocking processes, waiting for new data */
wake_up_interruptible(&apm->readq);
if(hdr->opcode == APM_CMD_RSP_GET_SPF_STATE) {
result = data->payload;
apm->result.opcode = hdr->opcode;
apm->result.status = 0;
/* First word of result it state */
apm->state = hdr->opcode;
}

return 0;
}

static void q6apm_audio_pkt_remove(gpr_device_t *adev)
{
of_platform_depopulate(&adev->dev);
return;
}

#ifdef CONFIG_OF
Expand Down