[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <878c2301-7c3e-4325-917f-cc1d7ce4191a@kernel.org>
Date: Sun, 6 Jul 2025 18:32:58 +0200
From: Hans de Goede <hansg@...nel.org>
To: Abdelrahman Fekry <abdelrahmanfekry375@...il.com>, mchehab@...nel.org,
sakari.ailus@...ux.intel.com, andy@...nel.org, gregkh@...uxfoundation.org
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-staging@...ts.linux.dev, linux-kernel-mentees@...ts.linux.dev,
skhan@...uxfoundation.org, dan.carpenter@...aro.org
Subject: Re: [PATCH v3] staging: media: atomisp: Remove custom sysfs
attributes from atomisp_drvfs.c
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@...nel.org>
> Reviewed-by: Andy Shevchenko <andy@...nel.org>
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@...il.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
>
Powered by blists - more mailing lists