[PATCH 11/15] media: atomisp: Replace atomisp_drvfs attr with using driver.dev_groups attr

[Hans de Goede <hdegoede@redhat.com>]

sysfs attributes preferably should not be manually be registered but
instead the driver.groups / driver.dev_groups driver struct members
should be used to have the driver core handle this in a race free
manner.

Using driver.groups would be the most direct replacement for
driver_[add|remove]_file, but some of the attributes actually need access
to the struct atomisp_device (*), so as part of modernizing this part of
the atomisp driver this change also makes the sysfs attribute device
attributes instead of driver attributes.

*) Before this change accessing these attributes without the driver having
bound would result in a NULL pointer deref, this commit fixes this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_drvfs.c | 148 +++++++-----------
 .../staging/media/atomisp/pci/atomisp_drvfs.h |   5 +-
 .../staging/media/atomisp/pci/atomisp_v4l2.c  |   9 +-
 3 files changed, 62 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
index 1df534bf54d3..293171da1266 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c
@@ -27,31 +27,17 @@
 #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)
+
 /*
- * _iunit_debug:
- * dbglvl: iunit css driver trace level
  * dbgopt: iunit debug option:
  *        bit 0: binary list
  *        bit 1: running binary
  *        bit 2: memory statistic
-*/
-struct _iunit_debug {
-	struct device_driver	*drv;
-	struct atomisp_device	*isp;
-	unsigned int		dbglvl;
-	unsigned int		dbgfun;
-	unsigned int		dbgopt;
-};
-
-#define OPTION_BIN_LIST			BIT(0)
-#define OPTION_BIN_RUN			BIT(1)
-#define OPTION_VALID			(OPTION_BIN_LIST \
-					| OPTION_BIN_RUN)
-
-static struct _iunit_debug iunit_debug = {
-	.dbglvl = 0,
-	.dbgopt = OPTION_BIN_LIST,
-};
+ */
+unsigned int dbgopt = OPTION_BIN_LIST;
 
 static inline int iunit_dump_dbgopt(struct atomisp_device *isp,
 				    unsigned int opt)
@@ -88,34 +74,44 @@ static inline int iunit_dump_dbgopt(struct atomisp_device *isp,
 	return ret;
 }
 
-static ssize_t iunit_dbglvl_show(struct device_driver *drv, char *buf)
+static ssize_t dbglvl_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
 {
-	iunit_debug.dbglvl = dbg_level;
-	return sysfs_emit(buf, "dtrace level:%u\n", iunit_debug.dbglvl);
+	unsigned int dbglvl = ia_css_debug_get_dtrace_level();
+
+	return sysfs_emit(buf, "dtrace level:%u\n", dbglvl);
 }
 
-static ssize_t iunit_dbglvl_store(struct device_driver *drv, const char *buf,
-				  size_t size)
+static ssize_t dbglvl_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t size)
 {
-	if (kstrtouint(buf, 10, &iunit_debug.dbglvl)
-	    || iunit_debug.dbglvl < 1
-	    || iunit_debug.dbglvl > 9) {
+	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(iunit_debug.dbglvl);
 
+	ia_css_debug_set_dtrace_level(dbglvl);
 	return size;
 }
+static DEVICE_ATTR_RW(dbglvl);
 
-static ssize_t iunit_dbgfun_show(struct device_driver *drv, char *buf)
+static ssize_t dbgfun_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
 {
-	iunit_debug.dbgfun = atomisp_get_css_dbgfunc();
-	return sysfs_emit(buf, "dbgfun opt:%u\n", iunit_debug.dbgfun);
+	unsigned int dbgfun = atomisp_get_css_dbgfunc();
+
+	return sysfs_emit(buf, "dbgfun opt:%u\n", dbgfun);
 }
 
-static ssize_t iunit_dbgfun_store(struct device_driver *drv, const char *buf,
-				  size_t size)
+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;
 
@@ -123,23 +119,20 @@ static ssize_t iunit_dbgfun_store(struct device_driver *drv, const char *buf,
 	if (ret)
 		return ret;
 
-	ret = atomisp_set_css_dbgfunc(iunit_debug.isp, opt);
-	if (ret)
-		return ret;
+	return atomisp_set_css_dbgfunc(isp, opt);
+}
+static DEVICE_ATTR_RW(dbgfun);
 
-	iunit_debug.dbgfun = opt;
-
-	return size;
+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 iunit_dbgopt_show(struct device_driver *drv, char *buf)
-{
-	return sysfs_emit(buf, "option:0x%x\n", iunit_debug.dbgopt);
-}
-
-static ssize_t iunit_dbgopt_store(struct device_driver *drv, const char *buf,
-				  size_t size)
+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;
 
@@ -147,56 +140,27 @@ static ssize_t iunit_dbgopt_store(struct device_driver *drv, const char *buf,
 	if (ret)
 		return ret;
 
-	iunit_debug.dbgopt = opt;
-	ret = iunit_dump_dbgopt(iunit_debug.isp, iunit_debug.dbgopt);
+	dbgopt = opt;
+	ret = iunit_dump_dbgopt(isp, dbgopt);
 	if (ret)
 		return ret;
 
 	return size;
 }
+static DEVICE_ATTR_RW(dbgopt);
 
-static const struct driver_attribute iunit_drvfs_attrs[] = {
-	__ATTR(dbglvl, 0644, iunit_dbglvl_show, iunit_dbglvl_store),
-	__ATTR(dbgfun, 0644, iunit_dbgfun_show, iunit_dbgfun_store),
-	__ATTR(dbgopt, 0644, iunit_dbgopt_show, iunit_dbgopt_store),
+static struct attribute *dbg_attrs[] = {
+	&dev_attr_dbglvl.attr,
+	&dev_attr_dbgfun.attr,
+	&dev_attr_dbgopt.attr,
+	NULL
 };
 
-static int iunit_drvfs_create_files(struct device_driver *drv)
-{
-	int i, ret = 0;
+static const struct attribute_group dbg_attr_group = {
+	.attrs = dbg_attrs,
+};
 
-	for (i = 0; i < ARRAY_SIZE(iunit_drvfs_attrs); i++)
-		ret |= driver_create_file(drv, &iunit_drvfs_attrs[i]);
-
-	return ret;
-}
-
-static void iunit_drvfs_remove_files(struct device_driver *drv)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(iunit_drvfs_attrs); i++)
-		driver_remove_file(drv, &iunit_drvfs_attrs[i]);
-}
-
-int atomisp_drvfs_init(struct atomisp_device *isp)
-{
-	struct device_driver *drv = isp->dev->driver;
-	int ret;
-
-	iunit_debug.isp = isp;
-	iunit_debug.drv = drv;
-
-	ret = iunit_drvfs_create_files(iunit_debug.drv);
-	if (ret) {
-		dev_err(isp->dev, "drvfs_create_files error: %d\n", ret);
-		iunit_drvfs_remove_files(iunit_debug.drv);
-	}
-
-	return ret;
-}
-
-void atomisp_drvfs_exit(void)
-{
-	iunit_drvfs_remove_files(iunit_debug.drv);
-}
+const struct attribute_group *dbg_attr_groups[] = {
+	&dbg_attr_group,
+	NULL
+};
diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.h b/drivers/staging/media/atomisp/pci/atomisp_drvfs.h
index 8f4cc722b881..8495cc133c06 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.h
@@ -19,7 +19,8 @@
 #ifndef	__ATOMISP_DRVFS_H__
 #define	__ATOMISP_DRVFS_H__
 
-int atomisp_drvfs_init(struct atomisp_device *isp);
-void atomisp_drvfs_exit(void);
+#include <linux/sysfs.h>
+
+extern const struct attribute_group *dbg_attr_groups[];
 
 #endif /* __ATOMISP_DRVFS_H__ */
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 6e8c9add35f9..0c78c1d82659 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1468,8 +1468,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 		goto error_unload_firmware;
 	}
 
-	atomisp_drvfs_init(isp);
-
 	return 0;
 
 error_unload_firmware:
@@ -1497,8 +1495,6 @@ static void atomisp_pci_remove(struct pci_dev *pdev)
 {
 	struct atomisp_device *isp = pci_get_drvdata(pdev);
 
-	atomisp_drvfs_exit();
-
 	pm_runtime_get_sync(&pdev->dev);
 	pm_runtime_forbid(&pdev->dev);
 	dev_pm_domain_set(&pdev->dev, NULL);
@@ -1529,11 +1525,12 @@ static const struct pci_device_id atomisp_pci_tbl[] = {
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, ATOMISP_PCI_DEVICE_SOC_CHT)},
 	{0,}
 };
-
 MODULE_DEVICE_TABLE(pci, atomisp_pci_tbl);
 
-
 static struct pci_driver atomisp_pci_driver = {
+	.driver = {
+		.dev_groups = dbg_attr_groups,
+	},
 	.name = "atomisp-isp2",
 	.id_table = atomisp_pci_tbl,
 	.probe = atomisp_pci_probe,
-- 
2.43.0