Re: [PATCH v1 1/4] ASoC: dt-bindings: qcom: add LPASS LPR vote clock ID

[Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>]

On 7/8/2026 5:30 PM, sashiko-bot@kernel.org wrote:
>> 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
> [Severity: High]
> Is the corresponding driver implementation missing for this new clock ID?
>
> Looking at the clock provider in sound/soc/qcom/qdsp6/q6prm-clocks.c, the
> q6prm_clks array doesn't appear to have an entry for LPASS_HW_LPR_VOTE,
> which means the clock would never be registered when a client device attempts
> to request it.
>
> [Severity: High]
> Will the hardcoded limit in the driver reject lookups for this new ID?
>
> In sound/soc/qcom/qdsp6/q6dsp-lpass-clocks.c, Q6DSP_MAX_CLK_ID is hardcoded
> to 104. When a client device requests the new clock ID 104, it looks like it
> would be rejected here:
>
> 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) {
>          ...
>          return ERR_PTR(-EINVAL);
>      }
>      ...
> }
>
> Could this result in -EINVAL being returned instead of the expected clock?
>
> Also, if the clock is later added to the initialization array without updating
> the Q6DSP_MAX_CLK_ID macro, is there a risk of an out-of-bounds write during
> driver initialization since the array size remains 104?

Thanks for the review.

This is addressed in patch 2 of the series.

Patch 1 only adds the new binding ID (LPASS_HW_LPR_VOTE = 104) and 
updates Q6AFE_MAX_CLK_ID to 105.

Patch 2 adds the corresponding driver support by:

   - registering LPASS_HW_LPR_VOTE in q6prm_clks[]
   - adding Q6PRM_HW_LPR_VOTE
   - handling it through PARAM_ID_RSC_CPU_LPR
   - replacing the hardcoded Q6DSP_MAX_CLK_ID with Q6AFE_MAX_CLK_ID

So clock ID 104 will not be rejected, and there is no out-of-bounds 
access because the array size is updated to cover IDs 0..104.

Thanks,
Prasad

>>   
>> -#define Q6AFE_MAX_CLK_ID			104
>> +#define Q6AFE_MAX_CLK_ID			105
>>   
>>   #define LPASS_CLK_ATTRIBUTE_INVALID		0x0
>>   #define LPASS_CLK_ATTRIBUTE_COUPLE_NO		0x1