Skip to content

Commit

Permalink
media: atomisp: Replace atomisp_drvfs attr with using driver.dev_grou…
Browse files Browse the repository at this point in the history 
…ps attr

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 <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
  • Loading branch information
@jwrdegoede @mchehab
jwrdegoede authored and mchehab committed Feb 1, 2024
1 parent 781bf4c commit b033018
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 98 deletions.
144 changes: 54 additions & 90 deletions drivers/staging/media/atomisp/pci/atomisp_drvfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -88,115 +74,93 @@ 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;

ret = kstrtouint(buf, 10, &opt);
if (ret)
return ret;

ret = atomisp_set_css_dbgfunc(iunit_debug.isp, opt);
if (ret)
return ret;

iunit_debug.dbgfun = opt;

return size;
return atomisp_set_css_dbgfunc(isp, opt);
}
static DEVICE_ATTR_RW(dbgfun);

static ssize_t iunit_dbgopt_show(struct device_driver *drv, char *buf)
static ssize_t dbgopt_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
return sysfs_emit(buf, "option:0x%x\n", iunit_debug.dbgopt);
return sysfs_emit(buf, "option:0x%x\n", 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;

ret = kstrtouint(buf, 10, &opt);
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;

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;
}
static const struct attribute_group dbg_attr_group = {
.attrs = dbg_attrs,
};

void atomisp_drvfs_exit(void)
{
iunit_drvfs_remove_files(iunit_debug.drv);
}
const struct attribute_group *dbg_attr_groups[] = {
&dbg_attr_group,
NULL
};
5 changes: 3 additions & 2 deletions drivers/staging/media/atomisp/pci/atomisp_drvfs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ */
9 changes: 3 additions & 6 deletions drivers/staging/media/atomisp/pci/atomisp_v4l2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit b033018

Please sign in to comment.