[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d5982c46-6353-4ffd-8a7a-126ac795f751@kernel.org>
Date: Sun, 6 Jul 2025 18:13:03 +0200
From: Hans de Goede <hansg@...nel.org>
To: Abdelrahman Fekry <abdelrahmanfekry375@...il.com>, andy@...nel.org,
mchehab@...nel.org, sakari.ailus@...ux.intel.com, gregkh@...uxfoundation.org
Cc: linux-kernel-mentees@...ts.linux.dev, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
skhan@...uxfoundation.com, dan.carpenter@...aro.org
Subject: Re: [PATCH v5] staging: media: atomisp: remove debug sysfs attributes
active_bo and free_bo
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@...nel.org>
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@...il.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hansg@...nel.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
>
Powered by blists - more mailing lists