Re: [PATCH 2/6] soc: qcom: llcc: Refactor llcc driver to support multiple configuration

[Komal Bajaj]

On 8/10/2023 5:52 PM, Bryan O'Donoghue wrote:
> On 10/08/2023 07:11, Komal Bajaj wrote:
> > +    if (!cfgs || cfgs->num_config != DEF_NUM_CFG) {
> > +        ret = -EINVAL;
> > +        goto err;
> > +    }
> > +    cfg = &cfgs->llcc_config[DEF_NUM_CFG - 1];
>
> This is a bit of a redundant check.
>
> You add in the check for num_config != 1, then deref llc_config[0] but 
> in patch #4 you get an index and check that index against num_config

Hi Bryan, Thanks for reviewing the patch.
Correct, in patch#4, index is checked against num_config, but the 
condition also checks for equality case.
For ex. in patch#6, num_config is 4, so index can vary from 0-3.

> I'm not seeing how at this point in your series, how num_config could 
> be anything other than 1.
>
> I'd do away with the DEF_NUM_CFG define in this code/series completely.
>
> num_config should encode all the necessary detail we need, DEF_NUM_CFG 
> just adds noise.

Got your point, will remove the macro DEF_NUM_CFG from the series.

Thanks
Komal

> ---
> bod