Re: [PATCH v2 2/5] ASoC: dt-bindings: qcom: add LPASS LPR vote clock ID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Adding the clock ID `LPASS_HW_LPR_VOTE` (104) without updating the hardcoded driver limit `Q6DSP_MAX_CLK_ID` causes valid device tree requests to be rejected and creates an out-of-bounds write hazard.
- [High] The driver implementation for the newly added `LPASS_HW_LPR_VOTE` clock ID and `PARAM_ID_RSC_CPU_LPR` parameter is completely missing.
--

--- Patch [2]: [PATCH v2 2/5] ASoC: dt-bindings: qcom: add LPASS LPR vote clock ID ---
commit 806d87026efc23f9d5588f86e5e4c815dd4852cd
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>

ASoC: dt-bindings: qcom: add LPASS LPR vote clock ID

Add a new clock ID, LPASS_HW_LPR_VOTE, to represent the LPASS low-power
resource (LPR) vote through the PRM interface. Also update
Q6AFE_MAX_CLK_ID to account for the newly added clock ID.

> The LPASS PRM supports a resource voting mechanism to control low-power
> states via PARAM_ID_RSC_CPU_LPR. Exposing this as a q6prm clock ID allows
> clients to request the LPR vote using the existing qcom,q6prm clock
> provider interface.

[Severity: High]
Is the driver implementation for this functionality missing from this patch?

Looking at sound/soc/qcom/qdsp6/q6prm-clocks.c, the q6prm_clks[] array does
not appear to map the new LPASS_HW_LPR_VOTE clock ID, which means it will
not be registered during probe.

Additionally, q6prm_set_hw_core_req() hardcodes PARAM_ID_RSC_HW_CORE and
does not seem to have the logic to handle the newly documented
PARAM_ID_RSC_CPU_LPR parameter ID:

sound/soc/qcom/qdsp6/q6prm.c:q6prm_set_hw_core_req() {
    ...
    param_data->param_id = PARAM_ID_RSC_HW_CORE;
    ...
}

Are there corresponding driver changes expected to accompany this binding?

> diff --git a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
> index 45850f2d43425..06ca2c287ab2f 100644
> --- a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
> +++ b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
> @@ -236,8 +236,9 @@
>  #define LPASS_HW_AVTIMER_VOTE		101
>  #define LPASS_HW_MACRO_VOTE		102
>  #define LPASS_HW_DCODEC_VOTE		103
> +#define LPASS_HW_LPR_VOTE		104
>  
> -#define Q6AFE_MAX_CLK_ID			104
> +#define Q6AFE_MAX_CLK_ID			105

[Severity: High]
Does the hardcoded limit in the driver also need to be updated to support
this new clock ID?

In sound/soc/qcom/qdsp6/q6dsp-lpass-clocks.c, Q6DSP_MAX_CLK_ID is hardcoded
to 104. Since LPASS_HW_LPR_VOTE is exactly 104, device tree requests for
it will fail the bounds check and be rejected:

sound/soc/qcom/qdsp6/q6dsp-lpass-clocks.c:q6dsp_of_clk_hw_get() {
    ...
    if (idx >= Q6DSP_MAX_CLK_ID || attr > LPASS_CLK_ATTRIBUTE_COUPLE_DIVISOR) {
        dev_err(cc->dev, "Invalid clk specifier (%d, %d)\n", idx, attr);
        return ERR_PTR(-EINVAL);
    }
    ...
}

Also, if the provider driver is updated to register this clock, will it
cause an out-of-bounds array write during probe? The target array seems to
be sized based on the old maximum limit (104 elements):

sound/soc/qcom/qdsp6/q6dsp-lpass-clocks.c:q6dsp_clock_dev_probe() {
    ...
    cc->clks[id] = clk;
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713184549.929569-1-prasad.kumpatla@oss.qualcomm.com?part=2