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

[PATCH v3] 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 needs to be removed before this driver can be moved out of drivers/staging.

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

Suggested-by : Hans de Goede <hansg@kernel.org>
Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
v3:
- 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 | 92 ---------------------
 1 file changed, 92 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 84102c3aaf97..469de184b9b8 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,12 +55,10 @@ 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);
 	dummy_ptr = 0;
-
 	hmm_bo_device_exit(&bo_device);
 	hmm_initialized = false;
 }
--
2.25.1

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

[Andy Shevchenko]

You really need to slow down with the new versions.

On Tue, Jun 24, 2025 at 8:07 PM Abdelrahman Fekry
<abdelrahmanfekry375@gmail.com> 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 needs to be removed before this driver can be moved out of drivers/staging.

needs --> need
can --> may

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

registration

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

This is not a tag. Check Submitting Patches documentation and find the
issue and fix it accordingly.

-- 
With Best Regards,
Andy Shevchenko

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

[Abdelrahman Fekry]

Hi, Thanks for your review and most importantly your patience :)

On Tue, Jun 24, 2025 at 9:09 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> You really need to slow down with the new versions.
>
> On Tue, Jun 24, 2025 at 8:07 PM Abdelrahman Fekry
> <abdelrahmanfekry375@gmail.com> 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 needs to be removed before this driver can be moved out of drivers/staging.
>
> needs --> need
> can --> may
>
noted , will fix this
> > - Remove active_bo and free_bo attributes
> > - Remove group registeration calls form hmm_init() and hmm_cleanup()
>
> registration
>
noted , will fix this
> > Suggested-by : Hans de Goede <hansg@kernel.org>
>
> This is not a tag. Check Submitting Patches documentation and find the
> issue and fix it accordingly.
>
noted , i now know the issue and will fix it
> --
> With Best Regards,
> Andy Shevchenko

Best Regards,
Abdelrahman Fekry

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

[Dan Carpenter]

Resend this patch with the changes Andy asked for and I'll Ack it.

The other things we discussed can be done separately.  It's also fine
to never do them.  I'm not your boss.  ;)

regards,
dan carpenter

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

[Abdelrahman Fekry]

Hello Dan,

Thank you so much for your good words

On Fri, Jun 27, 2025 at 12:32 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Resend this patch with the changes Andy asked for and I'll Ack it.
On it , thanks.
>
> The other things we discussed can be done separately.  It's also fine
> to never do them.  I'm not your boss.  ;)
>
I think I will continue working on it. I'm very excited to contribute
to something as big as this .

> regards,
> dan carpenter
>
regards,
Abdelrahman Fekry

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

[Andy Shevchenko]

On Fri, Jun 27, 2025 at 9:39 AM Abdelrahman Fekry
<abdelrahmanfekry375@gmail.com> wrote:
> On Fri, Jun 27, 2025 at 12:32 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:

...

> > The other things we discussed can be done separately.  It's also fine
> > to never do them.  I'm not your boss.  ;)
> >
> I think I will continue working on it. I'm very excited to contribute
> to something as big as this .

This driver is a rabbit hole. You've been warned! :-)

-- 
With Best Regards,
Andy Shevchenko

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

[Abdelrahman Fekry]

On Fri, Jun 27, 2025 at 9:59 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, Jun 27, 2025 at 9:39 AM Abdelrahman Fekry
> <abdelrahmanfekry375@gmail.com> wrote:
> > On Fri, Jun 27, 2025 at 12:32 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> ...
>
> > > The other things we discussed can be done separately.  It's also fine
> > > to never do them.  I'm not your boss.  ;)
> > >
> > I think I will continue working on it. I'm very excited to contribute
> > to something as big as this .
>
> This driver is a rabbit hole. You've been warned! :-)
haha, well, i guess there is only one way to find out .

> With Best Regards,
> Andy Shevchenko
Best Regards,
Abdelrahman Fekry