On 10/29/2025 4:05 PM, Jingyi Wang wrote:
> From: Gokul krishna Krishnakumar <[email protected]>
>
...
> +
> + /* Clear ping bit master kernel */
> + ret = qcom_smem_state_update_bits(q6v5->ping_state,
> BIT(q6v5->ping_bit), 0);
> + if (ret) {
> + pr_err("Failed to clear master kernel bits\n");
> + return ret;
> + }
> +
> + if (ping_failed)
> + return ping_failed;
> +
> + return 0;
Prefer to just have:
return ping_failed;> +}
> +EXPORT_SYMBOL_GPL(qcom_q6v5_ping_subsystem);
> +
> +int qcom_q6v5_ping_subsystem_init(struct qcom_q6v5 *q6v5, struct
> platform_device *pdev)
> +{
> + int ret = -ENODEV;
> +
> + q6v5->ping_state = devm_qcom_smem_state_get(&pdev->dev, "ping",
> &q6v5->ping_bit);
> + if (IS_ERR(q6v5->ping_state)) {
> + dev_err(&pdev->dev, "failed to acquire smem state %ld\n",
Change from "failed" to "Failed" to align with log format in this file.>
+ PTR_ERR(q6v5->ping_state));
> + return ret;
> + }
> +
> + q6v5->pong_irq = platform_get_irq_byname(pdev, "pong");
> + if (q6v5->pong_irq < 0)
> + return q6v5->pong_irq;
> +
Maybe place here for before any chance of q6v5_pong_interrupt:
init_completion(&q6v5->ping_done);> + ret =
devm_request_threaded_irq(&pdev->dev, q6v5->pong_irq, NULL,
> + q6v5_pong_interrupt,
> IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + "q6v5 pong", q6v5);
> + if (ret)
> + dev_err(&pdev->dev, "failed to acquire pong IRQ\n");
Change from "failed" to "Failed" to align with log format in this file.> +
> + init_completion(&q6v5->ping_done);
Better to have the init_completion before the pong_irq's
devm_request_threaded_irq.> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_q6v5_ping_subsystem_init);
> +
> /**
> * qcom_q6v5_init() - initializer of the q6v5 common struct
...
> +static int qcom_pas_attach(struct rproc *rproc)
> +{
> + int ret;
> + struct qcom_pas *adsp = rproc->priv;
> + bool ready_state;
> + bool crash_state;
> +
> + if (!adsp->q6v5.early_boot)
> + return -EINVAL;
> +
> + ret = irq_get_irqchip_state(adsp->q6v5.fatal_irq,
> + IRQCHIP_STATE_LINE_LEVEL, &crash_state);
> +
> + if (crash_state) {
> + dev_err(adsp->dev, "Sub system has crashed before driver
> probe\n");
> + adsp->rproc->state = RPROC_CRASHED;
> + return -EINVAL;
> + }
> +
> + ret = irq_get_irqchip_state(adsp->q6v5.ready_irq,
> + IRQCHIP_STATE_LINE_LEVEL, &ready_state);
> +
> + if (ready_state) {
> + dev_info(adsp->dev, "Sub system has boot-up before driver
> probe\n");
> + adsp->rproc->state = RPROC_DETACHED;
> + } else {
> + ret = wait_for_completion_timeout(&adsp->q6v5.subsys_booted,
> +
> msecs_to_jiffies(EARLY_BOOT_RETRY_INTERVAL_MS));
> + if (!ret) {
> + dev_err(adsp->dev, "Timeout on waiting for subsystem
> interrupt\n");
> + return -ETIMEDOUT;
> + }
> + }
How about:
if (unlikely(!ready_state)) {
ret = wait_for_completion_timeout(&adsp->q6v5.subsys_booted,
msecs_to_jiffies(EARLY_BOOT_RETRY_INTERVAL_MS));
if (!ret) {
dev_err(adsp->dev, "Timeout on waiting for subsystem
interrupt\n");
return -ETIMEDOUT;
}
}
dev_info(adsp->dev, "Sub system has boot-up before driver probe\n");
adsp->rproc->state = RPROC_DETACHED;
--
Thx and BRs,
Aiqun(Maria) Yu
