Re: [PATCH v3] staging: media: atomisp: Remove custom sysfs attributes from atomisp_drvfs.c

[Hans de Goede <hansg@kernel.org>]

Hi,

Thank you for your patch.

On 4-Jul-25 6:10 PM, Abdelrahman Fekry wrote:
> Continue the cleanup of the AtomISP driver as discussed with Hans and Andy
> in [1].
> 
> Tackle TODO item: "Remove custom sysfs files created by atomisp_drvfs.c":
> - Remove the sysfs attributes `dbglvl`, `dbgfun`, and `dbgopt`.
> - Delete their associated show/store handler functions.
> - Remove the corresponding attribute group definitions.
> - Keep `dbg_attr_groups[]` as an empty array to preserve compatibility.

That is the last bit left over in the atomisp_drvfs.c file.

Instead of pointing atomisp_pci_driver.driver.dev_groups to the empty
`dbg_attr_groups[]` the initialization of atomisp_pci_driver.driver.dev_groups
can simply be removed (making it NULL, which is allowed).

And then the entire atomisp_drvfs.[c|h] files can both be removed.

Since I'm about the send out a pull-request with atomisp changes
for kernel 6.17 I've made this change myself and squashed it
into your patch (keeping you as the author of course), see:

https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/commit/?h=media-atomisp&id=532fd10da0f58e7f313ff576bb1e6cab1f758bbe

And I've also dropped the TODO list item for this :)

Regards,

Hans



> 
> Link: https://lore.kernel.org/all/836dc6b6-2821-47fc-8f24-0838f979af76@kernel.org/ [1]
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
> ---
> v3:
> - fix style warning
> v2:
> - modify the reference link line.
> 
>  .../staging/media/atomisp/pci/atomisp_drvfs.c | 138 ------------------
>  1 file changed, 138 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> index 31c82c3c0d33..c25fd3ff003d 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
> @@ -1,9 +1,4 @@
>  // SPDX-License-Identifier: GPL-2.0
> -/*
> - * Support for atomisp driver sysfs interface
> - *
> - * Copyright (c) 2014 Intel Corporation. All Rights Reserved.
> - */
> 
>  #include <linux/device.h>
>  #include <linux/err.h>
> @@ -16,140 +11,7 @@
>  #include "hmm/hmm.h"
>  #include "ia_css_debug.h"
> 
> -#define OPTION_BIN_LIST			BIT(0)
> -#define OPTION_BIN_RUN			BIT(1)
> -#define OPTION_VALID			(OPTION_BIN_LIST | OPTION_BIN_RUN)
> -
> -/*
> - * dbgopt: iunit debug option:
> - *        bit 0: binary list
> - *        bit 1: running binary
> - *        bit 2: memory statistic
> - */
> -static unsigned int dbgopt = OPTION_BIN_LIST;
> -
> -static inline int iunit_dump_dbgopt(struct atomisp_device *isp,
> -				    unsigned int opt)
> -{
> -	int ret = 0;
> -
> -	if (opt & OPTION_VALID) {
> -		if (opt & OPTION_BIN_LIST) {
> -			ret = atomisp_css_dump_blob_infor(isp);
> -			if (ret) {
> -				dev_err(isp->dev, "%s dump blob infor err[ret:%d]\n",
> -					__func__, ret);
> -				goto opt_err;
> -			}
> -		}
> -
> -		if (opt & OPTION_BIN_RUN) {
> -			if (isp->asd.streaming) {
> -				atomisp_css_dump_sp_raw_copy_linecount(true);
> -				atomisp_css_debug_dump_isp_binary();
> -			} else {
> -				ret = -EPERM;
> -				dev_err(isp->dev, "%s dump running bin err[ret:%d]\n",
> -					__func__, ret);
> -				goto opt_err;
> -			}
> -		}
> -	} else {
> -		ret = -EINVAL;
> -		dev_err(isp->dev, "%s dump nothing[ret=%d]\n", __func__, ret);
> -	}
> -
> -opt_err:
> -	return ret;
> -}
> -
> -static ssize_t dbglvl_show(struct device *dev, struct device_attribute *attr,
> -			   char *buf)
> -{
> -	unsigned int dbglvl = ia_css_debug_get_dtrace_level();
> -
> -	return sysfs_emit(buf, "dtrace level:%u\n", dbglvl);
> -}
> -
> -static ssize_t dbglvl_store(struct device *dev, struct device_attribute *attr,
> -			    const char *buf, size_t size)
> -{
> -	unsigned int dbglvl;
> -	int ret;
> -
> -	ret = kstrtouint(buf, 10, &dbglvl);
> -	if (ret)
> -		return ret;
> -
> -	if (dbglvl < 1 || dbglvl > 9)
> -		return -ERANGE;
> -
> -	ia_css_debug_set_dtrace_level(dbglvl);
> -	return size;
> -}
> -static DEVICE_ATTR_RW(dbglvl);
> -
> -static ssize_t dbgfun_show(struct device *dev, struct device_attribute *attr,
> -			   char *buf)
> -{
> -	unsigned int dbgfun = atomisp_get_css_dbgfunc();
> -
> -	return sysfs_emit(buf, "dbgfun opt:%u\n", dbgfun);
> -}
> -
> -static ssize_t dbgfun_store(struct device *dev, struct device_attribute *attr,
> -			    const char *buf, size_t size)
> -{
> -	struct atomisp_device *isp = dev_get_drvdata(dev);
> -	unsigned int opt;
> -	int ret;
> -
> -	ret = kstrtouint(buf, 10, &opt);
> -	if (ret)
> -		return ret;
> -
> -	return atomisp_set_css_dbgfunc(isp, opt);
> -}
> -static DEVICE_ATTR_RW(dbgfun);
> -
> -static ssize_t dbgopt_show(struct device *dev, struct device_attribute *attr,
> -			   char *buf)
> -{
> -	return sysfs_emit(buf, "option:0x%x\n", dbgopt);
> -}
> -
> -static ssize_t dbgopt_store(struct device *dev, struct device_attribute *attr,
> -			    const char *buf, size_t size)
> -{
> -	struct atomisp_device *isp = dev_get_drvdata(dev);
> -	unsigned int opt;
> -	int ret;
> -
> -	ret = kstrtouint(buf, 10, &opt);
> -	if (ret)
> -		return ret;
> -
> -	dbgopt = opt;
> -	ret = iunit_dump_dbgopt(isp, dbgopt);
> -	if (ret)
> -		return ret;
> -
> -	return size;
> -}
> -static DEVICE_ATTR_RW(dbgopt);
> -
> -static struct attribute *dbg_attrs[] = {
> -	&dev_attr_dbglvl.attr,
> -	&dev_attr_dbgfun.attr,
> -	&dev_attr_dbgopt.attr,
> -	NULL
> -};
> -
> -static const struct attribute_group dbg_attr_group = {
> -	.attrs = dbg_attrs,
> -};
> 
>  const struct attribute_group *dbg_attr_groups[] = {
> -	&dbg_attr_group,
>  	NULL
>  };
> --
> 2.25.1
>