Re: [PATCH v3 2/4] soc: qcom: llcc-qcom: Add support for LLCC V6

[Melody Olvera]

On 3/26/2025 6:39 AM, Konrad Dybcio wrote:
> On 3/24/25 9:29 PM, Melody Olvera wrote:
> > Add support for LLCC V6. V6 adds several additional usecase IDs,
> > rearrages several registers and offsets, and supports slice IDs
> > over 31, so add a new function for programming LLCC V6.
> >
> > Signed-off-by: Melody Olvera <quic_molvera@xxxxxxxxxxx>
> > ---
> [...]
>
> > +
> > +	if (config->parent_slice_id && config->fixed_size) {
> > +		attr2_val |= FIELD_PREP(ATTR2_PARENT_SCID_MASK, config->parent_slice_id);
> > +		attr2_val |= ATTR2_IN_A_GROUP_MASK;
> > +	}
> This is fragile if parent_slice_id == 0, but let's say this is not an issue
> for now..

Agreed, but I don't anticipate that being an issue. I don't think any 
slice ID is/will be 0.

> > +
> > +	attr3_val = MAX_CAP_TO_BYTES(config->max_cap);
> > +	attr3_val /= drv_data->num_banks;
> > +	attr3_val >>= CACHE_LINE_SIZE_SHIFT;
> > +
> > +	ret = regmap_write(drv_data->bcast_regmap, attr0_cfg, attr0_val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(drv_data->bcast_regmap, attr1_cfg, attr1_val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(drv_data->bcast_regmap, attr2_cfg, attr2_val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(drv_data->bcast_regmap, attr3_cfg, attr3_val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	slice_offset = config->slice_id % 32;
> > +	reg_offset = (config->slice_id / 32) * 4;
> > +
> > +	wren = config->write_scid_en << slice_offset;If I'm reading the wrappers right, you should be able to drop both the
> shifting and intermediate variables with regmap_assign_bits()

I'm not so sure. I tried with regmap_assign_bits and it seems the 
correct way to use it would be roughly:

regmap_assign_bits(drv_data->bcast_regmap,
            cfg->reg_offset[LLCC_TRP_WRS_EN], BIT(config->slice_id),
            (bool)config->write_scid_en);

but the third argument is an unsigned int (the BIT(config->slice_id)). I 
tried just putting the slice_id there,
but got some bizarre results leading me to believe that's not the 
correct way to use this api. If I'm missing
something, let me know, but AFAICT, this is six one way, a half-dozen 
another.

Thanks,
Melody

> Looks good otherwise
>
> Konrad