[PATCH v2] soc: qcom: llcc: Add per slice counter and common llcc slice descriptor

[PATCH v2] soc: qcom: llcc: Add per slice counter and common llcc slice descriptor

[Unnathi Chalicheemala]

When a client driver calls llcc_slice_getd() multiple times or when
multiple client drivers vote for the same slice, there can be mismatch
in activate/deactivate count.

Add an atomic per-slice counter to track the number of
llcc_slice_activate() and llcc_slice_deactivate() calls per slice.
Also introduce a common LLCC slice descriptor to ensure consistent
tracking of slice usage.

Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
---
Changes in v2:
- Dropped bitmap as refcount is replacing it.
- Modified commit message to explain problem first.
- Moved NULL pointer checks to the beginning in llcc_slice_getd().
- Replace incorrect usage of atomic_inc_return() with atomic_inc().
- Use devm_kcalloc() to allocate memory for common slice descriptor.
- Link to v1: https://lore.kernel.org/all/20241008194636.3075093-1-quic_uchalich@quicinc.com/
---
 drivers/soc/qcom/llcc-qcom.c       | 60 +++++++++++++++++++++-----------------
 include/linux/soc/qcom/llcc-qcom.h |  6 ++--
 2 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 192edc3f64dc3eee12ab5ebdb9034cd0e2010891..b0cfdd30967a1a3390a9c0ef6bc01b333244884c 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -3853,30 +3853,25 @@ static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
 struct llcc_slice_desc *llcc_slice_getd(u32 uid)
 {
 	const struct llcc_slice_config *cfg;
-	struct llcc_slice_desc *desc;
 	u32 sz, count;
 
-	if (IS_ERR(drv_data))
+	if (IS_ERR(drv_data) || !drv_data)
 		return ERR_CAST(drv_data);
 
+	if (IS_ERR_OR_NULL(drv_data->desc) || !drv_data->cfg)
+		return ERR_PTR(-ENODEV);
+
 	cfg = drv_data->cfg;
 	sz = drv_data->cfg_size;
 
-	for (count = 0; cfg && count < sz; count++, cfg++)
+	for (count = 0; count < sz; count++, cfg++)
 		if (cfg->usecase_id == uid)
 			break;
 
-	if (count == sz || !cfg)
+	if (count == sz)
 		return ERR_PTR(-ENODEV);
 
-	desc = kzalloc(sizeof(*desc), GFP_KERNEL);
-	if (!desc)
-		return ERR_PTR(-ENOMEM);
-
-	desc->slice_id = cfg->slice_id;
-	desc->slice_size = cfg->max_cap;
-
-	return desc;
+	return &drv_data->desc[count];
 }
 EXPORT_SYMBOL_GPL(llcc_slice_getd);
 
@@ -3887,7 +3882,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_getd);
 void llcc_slice_putd(struct llcc_slice_desc *desc)
 {
 	if (!IS_ERR_OR_NULL(desc))
-		kfree(desc);
+		WARN(atomic_read(&desc->refcount), " Slice %d is still active\n", desc->slice_id);
 }
 EXPORT_SYMBOL_GPL(llcc_slice_putd);
 
@@ -3963,7 +3958,8 @@ int llcc_slice_activate(struct llcc_slice_desc *desc)
 		return -EINVAL;
 
 	mutex_lock(&drv_data->lock);
-	if (test_bit(desc->slice_id, drv_data->bitmap)) {
+	if ((atomic_read(&desc->refcount)) >= 1) {
+		atomic_inc(&desc->refcount);
 		mutex_unlock(&drv_data->lock);
 		return 0;
 	}
@@ -3977,7 +3973,7 @@ int llcc_slice_activate(struct llcc_slice_desc *desc)
 		return ret;
 	}
 
-	__set_bit(desc->slice_id, drv_data->bitmap);
+	atomic_inc(&desc->refcount);
 	mutex_unlock(&drv_data->lock);
 
 	return ret;
@@ -4003,10 +3999,12 @@ int llcc_slice_deactivate(struct llcc_slice_desc *desc)
 		return -EINVAL;
 
 	mutex_lock(&drv_data->lock);
-	if (!test_bit(desc->slice_id, drv_data->bitmap)) {
+	if ((atomic_read(&desc->refcount)) > 1) {
+		atomic_dec(&desc->refcount);
 		mutex_unlock(&drv_data->lock);
 		return 0;
 	}
+
 	act_ctrl_val = ACT_CTRL_OPCODE_DEACTIVATE << ACT_CTRL_OPCODE_SHIFT;
 
 	ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
@@ -4016,7 +4014,7 @@ int llcc_slice_deactivate(struct llcc_slice_desc *desc)
 		return ret;
 	}
 
-	__clear_bit(desc->slice_id, drv_data->bitmap);
+	atomic_set(&desc->refcount, 0);
 	mutex_unlock(&drv_data->lock);
 
 	return ret;
@@ -4060,7 +4058,7 @@ static int _qcom_llcc_cfg_program(const struct llcc_slice_config *config,
 	u32 attr1_val;
 	u32 attr0_val;
 	u32 max_cap_cacheline;
-	struct llcc_slice_desc desc;
+	struct llcc_slice_desc *desc;
 
 	attr1_val = config->cache_mode;
 	attr1_val |= config->probe_target_ways << ATTR1_PROBE_TARGET_WAYS_SHIFT;
@@ -4209,8 +4207,11 @@ static int _qcom_llcc_cfg_program(const struct llcc_slice_config *config,
 	}
 
 	if (config->activate_on_init) {
-		desc.slice_id = config->slice_id;
-		ret = llcc_slice_activate(&desc);
+		desc = llcc_slice_getd(config->usecase_id);
+		if (PTR_ERR_OR_ZERO(desc))
+			return -EINVAL;
+
+		ret = llcc_slice_activate(desc);
 	}
 
 	return ret;
@@ -4360,6 +4361,12 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
 	sz = drv_data->cfg_size;
 	llcc_table = drv_data->cfg;
 
+	for (i = 0; i < sz; i++) {
+		drv_data->desc[i].slice_id = llcc_table[i].slice_id;
+		drv_data->desc[i].slice_size = llcc_table[i].max_cap;
+		atomic_set(&drv_data->desc[i].refcount, 0);
+	}
+
 	if (drv_data->version >= LLCC_VERSION_6_0_0_0) {
 		for (i = 0; i < sz; i++) {
 			ret = _qcom_llcc_cfg_program_v6(&llcc_table[i], cfg);
@@ -4525,17 +4532,16 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 	llcc_cfg = cfg->sct_data;
 	sz = cfg->size;
 
-	for (i = 0; i < sz; i++)
-		if (llcc_cfg[i].slice_id > drv_data->max_slices)
-			drv_data->max_slices = llcc_cfg[i].slice_id;
-
-	drv_data->bitmap = devm_bitmap_zalloc(dev, drv_data->max_slices,
-					      GFP_KERNEL);
-	if (!drv_data->bitmap) {
+	drv_data->desc = devm_kcalloc(dev, sz, sizeof(struct llcc_slice_desc), GFP_KERNEL);
+	if (IS_ERR_OR_NULL(drv_data->desc)) {
 		ret = -ENOMEM;
 		goto err;
 	}
 
+	for (i = 0; i < sz; i++)
+		if (llcc_cfg[i].slice_id > drv_data->max_slices)
+			drv_data->max_slices = llcc_cfg[i].slice_id;
+
 	drv_data->cfg = llcc_cfg;
 	drv_data->cfg_size = sz;
 	drv_data->edac_reg_offset = cfg->edac_reg_offset;
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index 7a69210a250c4646b7fd6cf400995e35d3f00493..6badd8343619dd65183beb57a0195e3258829a2e 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -80,10 +80,12 @@
  * struct llcc_slice_desc - Cache slice descriptor
  * @slice_id: llcc slice id
  * @slice_size: Size allocated for the llcc slice
+ * @refcount: Counter to track activate/deactivate slice count
  */
 struct llcc_slice_desc {
 	u32 slice_id;
 	size_t slice_size;
+	atomic_t refcount;
 };
 
 /**
@@ -143,10 +145,10 @@ struct llcc_edac_reg_offset {
  * @cfg_size: size of the config data table
  * @max_slices: max slices as read from device tree
  * @num_banks: Number of llcc banks
- * @bitmap: Bit map to track the active slice ids
  * @ecc_irq: interrupt for llcc cache error detection and reporting
  * @ecc_irq_configured: 'True' if firmware has already configured the irq propagation
  * @version: Indicates the LLCC version
+ * @desc: Array pointer of llcc_slice_desc
  */
 struct llcc_drv_data {
 	struct regmap **regmaps;
@@ -158,10 +160,10 @@ struct llcc_drv_data {
 	u32 cfg_size;
 	u32 max_slices;
 	u32 num_banks;
-	unsigned long *bitmap;
 	int ecc_irq;
 	bool ecc_irq_configured;
 	u32 version;
+	struct llcc_slice_desc *desc;
 };
 
 #if IS_ENABLED(CONFIG_QCOM_LLCC)

---
base-commit: f02769e7f272d6f42b9767f066c5a99afd2338f3
change-id: 20250603-llcc_refcount-a17df8f94220

Best regards,
-- 
Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>

Re: [PATCH v2] soc: qcom: llcc: Add per slice counter and common llcc slice descriptor

[Konrad Dybcio]

On 6/27/25 12:03 AM, Unnathi Chalicheemala wrote:
> When a client driver calls llcc_slice_getd() multiple times or when
> multiple client drivers vote for the same slice, there can be mismatch
> in activate/deactivate count.
> 
> Add an atomic per-slice counter to track the number of
> llcc_slice_activate() and llcc_slice_deactivate() calls per slice.
> Also introduce a common LLCC slice descriptor to ensure consistent
> tracking of slice usage.
> 
> Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
> ---
> Changes in v2:
> - Dropped bitmap as refcount is replacing it.
> - Modified commit message to explain problem first.
> - Moved NULL pointer checks to the beginning in llcc_slice_getd().
> - Replace incorrect usage of atomic_inc_return() with atomic_inc().
> - Use devm_kcalloc() to allocate memory for common slice descriptor.
> - Link to v1: https://lore.kernel.org/all/20241008194636.3075093-1-quic_uchalich@quicinc.com/
> ---
>  drivers/soc/qcom/llcc-qcom.c       | 60 +++++++++++++++++++++-----------------
>  include/linux/soc/qcom/llcc-qcom.h |  6 ++--
>  2 files changed, 37 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 192edc3f64dc3eee12ab5ebdb9034cd0e2010891..b0cfdd30967a1a3390a9c0ef6bc01b333244884c 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -3853,30 +3853,25 @@ static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
>  struct llcc_slice_desc *llcc_slice_getd(u32 uid)
>  {
>  	const struct llcc_slice_config *cfg;
> -	struct llcc_slice_desc *desc;
>  	u32 sz, count;
>  
> -	if (IS_ERR(drv_data))
> +	if (IS_ERR(drv_data) || !drv_data)

IS_ERR_OR_NULL()

>  		return ERR_CAST(drv_data);
>  
> +	if (IS_ERR_OR_NULL(drv_data->desc) || !drv_data->cfg)
> +		return ERR_PTR(-ENODEV);
> +
>  	cfg = drv_data->cfg;
>  	sz = drv_data->cfg_size;
>  
> -	for (count = 0; cfg && count < sz; count++, cfg++)
> +	for (count = 0; count < sz; count++, cfg++)
>  		if (cfg->usecase_id == uid)
>  			break;
>  
> -	if (count == sz || !cfg)
> +	if (count == sz)
>  		return ERR_PTR(-ENODEV);
>  
> -	desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> -	if (!desc)
> -		return ERR_PTR(-ENOMEM);
> -
> -	desc->slice_id = cfg->slice_id;
> -	desc->slice_size = cfg->max_cap;
> -
> -	return desc;

This hunk looks unrelated to the problem you described and could go
into a separate commit (can these ever reasonably be NULL, bar
programmer error?)

> +	return &drv_data->desc[count];

count is a poor name (especially since 'sz' holds the number of slices
we have), we can change it to just 'i' or something

>  }
>  EXPORT_SYMBOL_GPL(llcc_slice_getd);
>  
> @@ -3887,7 +3882,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_getd);
>  void llcc_slice_putd(struct llcc_slice_desc *desc)
>  {
>  	if (!IS_ERR_OR_NULL(desc))
> -		kfree(desc);
> +		WARN(atomic_read(&desc->refcount), " Slice %d is still active\n", desc->slice_id);
>  }
>  EXPORT_SYMBOL_GPL(llcc_slice_putd);
>  
> @@ -3963,7 +3958,8 @@ int llcc_slice_activate(struct llcc_slice_desc *desc)
>  		return -EINVAL;
>  
>  	mutex_lock(&drv_data->lock);
> -	if (test_bit(desc->slice_id, drv_data->bitmap)) {
> +	if ((atomic_read(&desc->refcount)) >= 1) {
> +		atomic_inc(&desc->refcount);

if (atomic_inc_return(&desc->refcount) > 1)

should do the same thing, in a single call.. but then you'd need
to roll back below on failure.. but then regmap should never fail
because it's just sugar syntax for MMIO accesses.. but then we've
had issues before when regmap core broke.. so many dillemas..

>  		mutex_unlock(&drv_data->lock);

You can also switch to scoped guards in a separate commit, which would
be a very welcome change

[...]

>  	if (config->activate_on_init) {
> -		desc.slice_id = config->slice_id;
> -		ret = llcc_slice_activate(&desc);
> +		desc = llcc_slice_getd(config->usecase_id);
> +		if (PTR_ERR_OR_ZERO(desc))
> +			return -EINVAL;

I don't think it can return a nullptr

Konrad

Re: [PATCH v2] soc: qcom: llcc: Add per slice counter and common llcc slice descriptor

[Dan Carpenter]

Hi Unnathi,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Unnathi-Chalicheemala/soc-qcom-llcc-Add-per-slice-counter-and-common-llcc-slice-descriptor/20250627-060459
base:   f02769e7f272d6f42b9767f066c5a99afd2338f3
patch link:    https://lore.kernel.org/r/20250626-llcc_refcount-v2-1-d05ec8169734%40oss.qualcomm.com
patch subject: [PATCH v2] soc: qcom: llcc: Add per slice counter and common llcc slice descriptor
config: um-randconfig-r071-20250630 (https://download.01.org/0day-ci/archive/20250630/202506301401.m1NkXwJf-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202506301401.m1NkXwJf-lkp@intel.com/

smatch warnings:
drivers/soc/qcom/llcc-qcom.c:3859 llcc_slice_getd() warn: passing zero to 'ERR_CAST'

vim +/ERR_CAST +3859 drivers/soc/qcom/llcc-qcom.c

a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3853  struct llcc_slice_desc *llcc_slice_getd(u32 uid)
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3854  {
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3855  	const struct llcc_slice_config *cfg;
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3856  	u32 sz, count;
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3857  
bc13c2450f770e drivers/soc/qcom/llcc-qcom.c  Unnathi Chalicheemala 2025-06-26  3858  	if (IS_ERR(drv_data) || !drv_data)

Why are we adding a NULL check here?  So far as I can see, this
can only be NULL in probe() between the failed allocation and
the error handling.  Can actually reach this function during that
stage of the probe()?

Also it feels like it's not done consistenly at all.  Why does
llcc_update_act_ctrl() not check for NULL?

Right now we're returning:
1) -EPROBE_DEFER if probe hasn't been called
2) NULL if it hasn't succeeded and we hit a (very narrow) race window
3) or -EPROBE_DEFER if probe() has failed.

If we really hit a NULL here then we should probably return
-EPROBE_DEFER and not ERR_CAST(NULL).  It needs to be documented.
Btw, I've written a blog about how NULL and error pointers are
supposed to work when you use a mix or error pointers and NULL.

https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/

regards,
dan carpenter

72d1cd033154f5 drivers/soc/qcom/llcc-slice.c Jordan Crouse         2018-12-11 @3859  		return ERR_CAST(drv_data);
72d1cd033154f5 drivers/soc/qcom/llcc-slice.c Jordan Crouse         2018-12-11  3860  
bc13c2450f770e drivers/soc/qcom/llcc-qcom.c  Unnathi Chalicheemala 2025-06-26  3861  	if (IS_ERR_OR_NULL(drv_data->desc) || !drv_data->cfg)
bc13c2450f770e drivers/soc/qcom/llcc-qcom.c  Unnathi Chalicheemala 2025-06-26  3862  		return ERR_PTR(-ENODEV);
bc13c2450f770e drivers/soc/qcom/llcc-qcom.c  Unnathi Chalicheemala 2025-06-26  3863  
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3864  	cfg = drv_data->cfg;
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3865  	sz = drv_data->cfg_size;
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3866  
bc13c2450f770e drivers/soc/qcom/llcc-qcom.c  Unnathi Chalicheemala 2025-06-26  3867  	for (count = 0; count < sz; count++, cfg++)
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3868  		if (cfg->usecase_id == uid)
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3869  			break;
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3870  
bc13c2450f770e drivers/soc/qcom/llcc-qcom.c  Unnathi Chalicheemala 2025-06-26  3871  	if (count == sz)
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3872  		return ERR_PTR(-ENODEV);
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3873  
bc13c2450f770e drivers/soc/qcom/llcc-qcom.c  Unnathi Chalicheemala 2025-06-26  3874  	return &drv_data->desc[count];
a3134fb09e0bc5 drivers/soc/qcom/llcc-slice.c Rishabh Bhatnagar     2018-05-23  3875  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki