[PATCH v5] staging: media: atomisp: remove debug sysfs attributes active_bo and free_bo

[PATCH v5] staging: media: atomisp: remove debug sysfs attributes active_bo and free_bo

[Abdelrahman Fekry]

The sysfs attributes active_bo and free_bo expose internal buffer
state used only for debugging purposes. These are not part of
any standard kernel ABI, and need to be removed before this
driver may be moved out of drivers/staging.

- Remove active_bo and free_bo attributes
- Remove group registration calls form hmm_init() and hmm_cleanup()

Suggested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
v5:
- remove stray change

v4: https://lore.kernel.org/all/20250627072939.21447-1-abdelrahmanfekry375@gmail.com/
- fix typos

v3: https://lore.kernel.org/all/20250624170746.47188-1-abdelrahmanfekry375@gmail.com/
- remove blank line

v2: https://lore.kernel.org/all/20250624144943.39297-1-abdelrahmanfekry375@gmail.com/
- add Suggested-by line
- remove unnecessary comments

v1: https://lore.kernel.org/all/20250624130841.34693-1-abdelrahmanfekry375@gmail.com/

---
 drivers/staging/media/atomisp/pci/hmm/hmm.c | 91 ---------------------
 1 file changed, 91 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 84102c3aaf97..f998b57f90c4 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -28,88 +28,6 @@ struct hmm_bo_device bo_device;
 static ia_css_ptr dummy_ptr = mmgr_EXCEPTION;
 static bool hmm_initialized;

-/*
- * p: private
- * v: vmalloc
- */
-static const char hmm_bo_type_string[] = "pv";
-
-static ssize_t bo_show(struct device *dev, struct device_attribute *attr,
-		       char *buf, struct list_head *bo_list, bool active)
-{
-	ssize_t ret = 0;
-	struct hmm_buffer_object *bo;
-	unsigned long flags;
-	int i;
-	long total[HMM_BO_LAST] = { 0 };
-	long count[HMM_BO_LAST] = { 0 };
-	int index1 = 0;
-	int index2 = 0;
-
-	ret = scnprintf(buf, PAGE_SIZE, "type pgnr\n");
-	if (ret <= 0)
-		return 0;
-
-	index1 += ret;
-
-	spin_lock_irqsave(&bo_device.list_lock, flags);
-	list_for_each_entry(bo, bo_list, list) {
-		if ((active && (bo->status & HMM_BO_ALLOCED)) ||
-		    (!active && !(bo->status & HMM_BO_ALLOCED))) {
-			ret = scnprintf(buf + index1, PAGE_SIZE - index1,
-					"%c %d\n",
-					hmm_bo_type_string[bo->type], bo->pgnr);
-
-			total[bo->type] += bo->pgnr;
-			count[bo->type]++;
-			if (ret > 0)
-				index1 += ret;
-		}
-	}
-	spin_unlock_irqrestore(&bo_device.list_lock, flags);
-
-	for (i = 0; i < HMM_BO_LAST; i++) {
-		if (count[i]) {
-			ret = scnprintf(buf + index1 + index2,
-					PAGE_SIZE - index1 - index2,
-					"%ld %c buffer objects: %ld KB\n",
-					count[i], hmm_bo_type_string[i],
-					total[i] * 4);
-			if (ret > 0)
-				index2 += ret;
-		}
-	}
-
-	/* Add trailing zero, not included by scnprintf */
-	return index1 + index2 + 1;
-}
-
-static ssize_t active_bo_show(struct device *dev, struct device_attribute *attr,
-			      char *buf)
-{
-	return bo_show(dev, attr, buf, &bo_device.entire_bo_list, true);
-}
-
-static ssize_t free_bo_show(struct device *dev, struct device_attribute *attr,
-			    char *buf)
-{
-	return bo_show(dev, attr, buf, &bo_device.entire_bo_list, false);
-}
-
-
-static DEVICE_ATTR_RO(active_bo);
-static DEVICE_ATTR_RO(free_bo);
-
-static struct attribute *sysfs_attrs_ctrl[] = {
-	&dev_attr_active_bo.attr,
-	&dev_attr_free_bo.attr,
-	NULL
-};
-
-static struct attribute_group atomisp_attribute_group[] = {
-	{.attrs = sysfs_attrs_ctrl },
-};
-
 int hmm_init(void)
 {
 	int ret;
@@ -130,14 +48,6 @@ int hmm_init(void)
 	 */
 	dummy_ptr = hmm_alloc(1);

-	if (!ret) {
-		ret = sysfs_create_group(&atomisp_dev->kobj,
-					 atomisp_attribute_group);
-		if (ret)
-			dev_err(atomisp_dev,
-				"%s Failed to create sysfs\n", __func__);
-	}
-
 	return ret;
 }

@@ -145,7 +55,6 @@ void hmm_cleanup(void)
 {
 	if (dummy_ptr == mmgr_EXCEPTION)
 		return;
-	sysfs_remove_group(&atomisp_dev->kobj, atomisp_attribute_group);

 	/* free dummy memory first */
 	hmm_free(dummy_ptr);
--
2.25.1

Re: [PATCH v5] staging: media: atomisp: remove debug sysfs attributes active_bo and free_bo

[Andy Shevchenko]

On Fri, Jun 27, 2025 at 01:06:04PM +0300, Abdelrahman Fekry wrote:
> The sysfs attributes active_bo and free_bo expose internal buffer
> state used only for debugging purposes. These are not part of
> any standard kernel ABI, and need to be removed before this
> driver may be moved out of drivers/staging.
> 
> - Remove active_bo and free_bo attributes
> - Remove group registration calls form hmm_init() and hmm_cleanup()

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5] staging: media: atomisp: remove debug sysfs attributes active_bo and free_bo

[Dan Carpenter]

On Fri, Jun 27, 2025 at 01:06:04PM +0300, Abdelrahman Fekry wrote:
> The sysfs attributes active_bo and free_bo expose internal buffer
> state used only for debugging purposes. These are not part of
> any standard kernel ABI, and need to be removed before this
> driver may be moved out of drivers/staging.
> 
> - Remove active_bo and free_bo attributes
> - Remove group registration calls form hmm_init() and hmm_cleanup()
> 
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
> ---

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

Re: [PATCH v5] staging: media: atomisp: remove debug sysfs attributes active_bo and free_bo

[Hans de Goede]

Hi,

On 27-Jun-25 12:06 PM, Abdelrahman Fekry wrote:
> The sysfs attributes active_bo and free_bo expose internal buffer
> state used only for debugging purposes. These are not part of
> any standard kernel ABI, and need to be removed before this
> driver may be moved out of drivers/staging.
> 
> - Remove active_bo and free_bo attributes
> - Remove group registration calls form hmm_init() and hmm_cleanup()
> 
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

I have merged this in my media-atomisp branch:
https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/log/?h=media-atomisp

And this patch will be included in my next
pull-request to Mauro (to media subsystem maintainer)

Regards,

Hans




> ---
> v5:
> - remove stray change
> 
> v4: https://lore.kernel.org/all/20250627072939.21447-1-abdelrahmanfekry375@gmail.com/
> - fix typos
> 
> v3: https://lore.kernel.org/all/20250624170746.47188-1-abdelrahmanfekry375@gmail.com/
> - remove blank line
> 
> v2: https://lore.kernel.org/all/20250624144943.39297-1-abdelrahmanfekry375@gmail.com/
> - add Suggested-by line
> - remove unnecessary comments
> 
> v1: https://lore.kernel.org/all/20250624130841.34693-1-abdelrahmanfekry375@gmail.com/
> 
> ---
>  drivers/staging/media/atomisp/pci/hmm/hmm.c | 91 ---------------------
>  1 file changed, 91 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
> index 84102c3aaf97..f998b57f90c4 100644
> --- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
> +++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
> @@ -28,88 +28,6 @@ struct hmm_bo_device bo_device;
>  static ia_css_ptr dummy_ptr = mmgr_EXCEPTION;
>  static bool hmm_initialized;
> 
> -/*
> - * p: private
> - * v: vmalloc
> - */
> -static const char hmm_bo_type_string[] = "pv";
> -
> -static ssize_t bo_show(struct device *dev, struct device_attribute *attr,
> -		       char *buf, struct list_head *bo_list, bool active)
> -{
> -	ssize_t ret = 0;
> -	struct hmm_buffer_object *bo;
> -	unsigned long flags;
> -	int i;
> -	long total[HMM_BO_LAST] = { 0 };
> -	long count[HMM_BO_LAST] = { 0 };
> -	int index1 = 0;
> -	int index2 = 0;
> -
> -	ret = scnprintf(buf, PAGE_SIZE, "type pgnr\n");
> -	if (ret <= 0)
> -		return 0;
> -
> -	index1 += ret;
> -
> -	spin_lock_irqsave(&bo_device.list_lock, flags);
> -	list_for_each_entry(bo, bo_list, list) {
> -		if ((active && (bo->status & HMM_BO_ALLOCED)) ||
> -		    (!active && !(bo->status & HMM_BO_ALLOCED))) {
> -			ret = scnprintf(buf + index1, PAGE_SIZE - index1,
> -					"%c %d\n",
> -					hmm_bo_type_string[bo->type], bo->pgnr);
> -
> -			total[bo->type] += bo->pgnr;
> -			count[bo->type]++;
> -			if (ret > 0)
> -				index1 += ret;
> -		}
> -	}
> -	spin_unlock_irqrestore(&bo_device.list_lock, flags);
> -
> -	for (i = 0; i < HMM_BO_LAST; i++) {
> -		if (count[i]) {
> -			ret = scnprintf(buf + index1 + index2,
> -					PAGE_SIZE - index1 - index2,
> -					"%ld %c buffer objects: %ld KB\n",
> -					count[i], hmm_bo_type_string[i],
> -					total[i] * 4);
> -			if (ret > 0)
> -				index2 += ret;
> -		}
> -	}
> -
> -	/* Add trailing zero, not included by scnprintf */
> -	return index1 + index2 + 1;
> -}
> -
> -static ssize_t active_bo_show(struct device *dev, struct device_attribute *attr,
> -			      char *buf)
> -{
> -	return bo_show(dev, attr, buf, &bo_device.entire_bo_list, true);
> -}
> -
> -static ssize_t free_bo_show(struct device *dev, struct device_attribute *attr,
> -			    char *buf)
> -{
> -	return bo_show(dev, attr, buf, &bo_device.entire_bo_list, false);
> -}
> -
> -
> -static DEVICE_ATTR_RO(active_bo);
> -static DEVICE_ATTR_RO(free_bo);
> -
> -static struct attribute *sysfs_attrs_ctrl[] = {
> -	&dev_attr_active_bo.attr,
> -	&dev_attr_free_bo.attr,
> -	NULL
> -};
> -
> -static struct attribute_group atomisp_attribute_group[] = {
> -	{.attrs = sysfs_attrs_ctrl },
> -};
> -
>  int hmm_init(void)
>  {
>  	int ret;
> @@ -130,14 +48,6 @@ int hmm_init(void)
>  	 */
>  	dummy_ptr = hmm_alloc(1);
> 
> -	if (!ret) {
> -		ret = sysfs_create_group(&atomisp_dev->kobj,
> -					 atomisp_attribute_group);
> -		if (ret)
> -			dev_err(atomisp_dev,
> -				"%s Failed to create sysfs\n", __func__);
> -	}
> -
>  	return ret;
>  }
> 
> @@ -145,7 +55,6 @@ void hmm_cleanup(void)
>  {
>  	if (dummy_ptr == mmgr_EXCEPTION)
>  		return;
> -	sysfs_remove_group(&atomisp_dev->kobj, atomisp_attribute_group);
> 
>  	/* free dummy memory first */
>  	hmm_free(dummy_ptr);
> --
> 2.25.1
>