Hi Alan, first of all - there are these kernel-doc warnings. Info(drivers/staging/fpga/fpga-mgr.c:37): Scanning doc for fpga_mgr_low_level_state Warning(drivers/staging/fpga/fpga-mgr.c:43): No description found for return value of 'fpga_mgr_low_level_state' Info(drivers/staging/fpga/fpga-mgr.c:51): Scanning doc for __fpga_mgr_reset Warning(drivers/staging/fpga/fpga-mgr.c:55): No description found for return value of '__fpga_mgr_reset' Info(drivers/staging/fpga/fpga-mgr.c:69): Scanning doc for fpga_mgr_reset Warning(drivers/staging/fpga/fpga-mgr.c:73): No description found for return value of 'fpga_mgr_reset' Info(drivers/staging/fpga/fpga-mgr.c:88): Scanning doc for __fpga_mgr_stage_init Warning(drivers/staging/fpga/fpga-mgr.c:92): No description found for return value of '__fpga_mgr_stage_write_init' Info(drivers/staging/fpga/fpga-mgr.c:108): Scanning doc for __fpga_mgr_stage_write Warning(drivers/staging/fpga/fpga-mgr.c:115): No description found for return value of '__fpga_mgr_stage_write' Info(drivers/staging/fpga/fpga-mgr.c:129): Scanning doc for __fpga_mgr_stage_complete Warning(drivers/staging/fpga/fpga-mgr.c:133): No description found for return value of '__fpga_mgr_stage_write_complete' Info(drivers/staging/fpga/fpga-mgr.c:151): Scanning doc for __fpga_mgr_write Warning(drivers/staging/fpga/fpga-mgr.c:158): No description found for return value of '__fpga_mgr_write' Info(drivers/staging/fpga/fpga-mgr.c:173): Scanning doc for fpga_mgr_write Warning(drivers/staging/fpga/fpga-mgr.c:179): No description found for return value of 'fpga_mgr_write' Info(drivers/staging/fpga/fpga-mgr.c:195): Scanning doc for fpga_mgr_firmware_write Warning(drivers/staging/fpga/fpga-mgr.c:204): No description found for return value of 'fpga_mgr_firmware_write' Info(drivers/staging/fpga/fpga-mgr.c:234): Scanning doc for fpga_mgr_name Warning(drivers/staging/fpga/fpga-mgr.c:239): No description found for return value of 'fpga_mgr_name' Info(drivers/staging/fpga/fpga-mgr.c:414): Scanning doc for fpga_mgr_register Warning(drivers/staging/fpga/fpga-mgr.c:423): No description found for return value of 'fpga_mgr_register' Info(drivers/staging/fpga/fpga-mgr.c:480): Scanning doc for fpga_mgr_remove Warning(drivers/staging/fpga/fpga-mgr.c:484): No description found for parameter 'pdev' Warning(drivers/staging/fpga/fpga-mgr.c:484): Excess function parameter 'dev' description in 'fpga_mgr_remove' On 12/17/2014 05:54 PM, atull@xxxxxxxxxxxxxxxxxxxxx wrote: > From: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx> > > Supports standard ops for low level FPGA drivers. > Various manufacturors' FPGAs can be supported by adding low > level drivers. Each driver needs to register its ops > using fpga_mgr_register(). > > Exports methods of doing operations to program FPGAs. These > should be sufficient for individual drivers to request FPGA > programming directly if desired. > > Adds a sysfs interface. The sysfs interface can be compiled out > where desired in production builds. > > Resume is supported by calling low level driver's resume > function, then reloading the firmware image. > > The following are exported as GPL: > * fpga_mgr_reset > Reset the FGPA. > > * fpga_mgr_write > Write a image (in buffer) to the FPGA. > > * fpga_mgr_firmware_write > Request firmware by file name and write it to the FPGA. > > * fpga_mgr_name > Get name of FPGA manager. > > * fpga_mgr_state > Get a state of framework as a string. > > * fpga_mgr_register and fpga_mgr_remove > Register/unregister low level fpga manager driver. > > The following sysfs files are created: > * /sys/class/fpga_manager/<fpga>/name > Name of low level driver. > > * /sys/class/fpga_manager/<fpga>/firmware > Name of FPGA image file to load using firmware class. > $ echo image.rbf > /sys/class/fpga_manager/<fpga>/firmware > > read: read back name of image file previous loaded > $ cat /sys/class/fpga_manager/<fpga>/firmware > > * /sys/class/fpga_manager/<fpga>/reset > reset the FPGA > $ echo 1 > /sys/class/fpga_manager/<fpga>/reset > > * /sys/class/fpga_manager/<fpga>/state > State of fpga framework state machine > > Signed-off-by: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx> > --- > v2: s/mangager/manager/ > check for invalid request_nr > add fpga reset interface > rework the state code > remove FPGA_MGR_FAIL flag > add _ERR states to fpga manager states enum > low level state op now returns a state enum value > initialize framework state from driver state op > remove unused fpga read stuff > merge sysfs.c into fpga-mgr.c > move suspend/resume from bus.c to fpga-mgr.c > > v3: Add struct device to fpga_manager (not as a pointer) > Add to_fpga_manager > Don't get irq in fpga-mgr.c (let low level driver do it) > remove from struct fpga_manager: nr, np, parent > get rid of fpga_mgr_get_new_minor() > simplify fpga_manager_register: > reorder parameters > use dev instead of pdev > get rid of code that used to make more sense when this > was a char driver, don't alloc_chrdev_region > use a mutex instead of flags > > v4: Move to drivers/staging > > v5: Make some things be static > Kconfig: add 'if FPGA' > Makefile: s/fpga-mgr-core.o/fpga-mgr.o/ > clean up includes > use enum fpga_mgr_states instead of int > static const char *state_str > use DEVICE_ATTR_RO/RW/WO and ATTRIBUTE_GROUPS > return -EINVAL instead of BUG_ON > move ida_simple_get after kzalloc > clean up fpga_mgr_remove > fpga-mgr.h: remove '#if IS_ENABLED(CONFIG_FPGA)' > add kernel-doc documentation > Move header to new include/linux/fpga folder > static const struct fpga_mgr_ops > dev_info msg simplified > --- > drivers/staging/Kconfig | 2 + > drivers/staging/Makefile | 1 + > drivers/staging/fpga/Kconfig | 24 ++ > drivers/staging/fpga/Makefile | 10 + > drivers/staging/fpga/fpga-mgr.c | 523 +++++++++++++++++++++++++++++++++++++++ > include/linux/fpga/fpga-mgr.h | 124 ++++++++++ > 6 files changed, 684 insertions(+) > create mode 100644 drivers/staging/fpga/Kconfig > create mode 100644 drivers/staging/fpga/Makefile > create mode 100644 drivers/staging/fpga/fpga-mgr.c > create mode 100644 include/linux/fpga/fpga-mgr.h > > diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig > index 4690ae9..4338a4c 100644 > --- a/drivers/staging/Kconfig > +++ b/drivers/staging/Kconfig > @@ -108,4 +108,6 @@ source "drivers/staging/skein/Kconfig" > > source "drivers/staging/unisys/Kconfig" > > +source "drivers/staging/fpga/Kconfig" > + > endif # STAGING > diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile > index c780a0e..43654a2 100644 > --- a/drivers/staging/Makefile > +++ b/drivers/staging/Makefile > @@ -46,3 +46,4 @@ obj-$(CONFIG_MTD_SPINAND_MT29F) += mt29f_spinand/ > obj-$(CONFIG_GS_FPGABOOT) += gs_fpgaboot/ > obj-$(CONFIG_CRYPTO_SKEIN) += skein/ > obj-$(CONFIG_UNISYSSPAR) += unisys/ > +obj-$(CONFIG_FPGA) += fpga/ > diff --git a/drivers/staging/fpga/Kconfig b/drivers/staging/fpga/Kconfig > new file mode 100644 > index 0000000..89ebafc > --- /dev/null > +++ b/drivers/staging/fpga/Kconfig > @@ -0,0 +1,24 @@ > +# > +# FPGA framework configuration > +# > + > +menu "FPGA devices" > + > +config FPGA > + tristate "FPGA Framework" > + help > + Say Y here if you want support for configuring FPGAs from the > + kernel. The FPGA framework adds a FPGA manager class and FPGA > + manager drivers. > + > +if FPGA > + > +config FPGA_MGR_SYSFS > + bool "FPGA Manager SysFS Interface" > + depends on SYSFS > + help > + FPGA Manager SysFS interface. > + > +endif # FPGA > + > +endmenu > diff --git a/drivers/staging/fpga/Makefile b/drivers/staging/fpga/Makefile > new file mode 100644 > index 0000000..ff6c677 > --- /dev/null > +++ b/drivers/staging/fpga/Makefile > @@ -0,0 +1,10 @@ > +# > +# Makefile for the fpga framework and fpga manager drivers. > +# > + > +fpga-mgr-core-y += fpga-mgr.o remove this line - it is unused. > + > +# Core FPGA Manager Framework > +obj-$(CONFIG_FPGA) += fpga-mgr.o > + > +# FPGA Manager Drivers > diff --git a/drivers/staging/fpga/fpga-mgr.c b/drivers/staging/fpga/fpga-mgr.c > new file mode 100644 > index 0000000..671b114 > --- /dev/null > +++ b/drivers/staging/fpga/fpga-mgr.c > @@ -0,0 +1,523 @@ > +/* > + * FPGA Manager Core > + * > + * Copyright (C) 2013-2014 Altera Corporation > + * > + * With code from the mailing list: > + * Copyright (C) 2013 Xilinx, Inc. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program. If not, see <http://www.gnu.org/licenses/>. > + */ > +#include <linux/delay.h> > +#include <linux/firmware.h> > +#include <linux/fpga/fpga-mgr.h> > +#include <linux/idr.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/pm.h> > +#include <linux/slab.h> > + > +static DEFINE_MUTEX(fpga_mgr_mutex); > +static DEFINE_IDA(fpga_mgr_ida); > +static struct class *fpga_mgr_class; > + > +static LIST_HEAD(fpga_manager_list); > + > +/** > + * fpga_mgr_low_level_state - get FPGA state from low level driver > + * @mgr: fpga manager > + * > + * This will be used to initialize framework state > + */ > +static enum fpga_mgr_states fpga_mgr_low_level_state(struct fpga_manager *mgr) > +{ > + if (!mgr || !mgr->mops || !mgr->mops->state) > + return FPGA_MGR_STATE_UNKNOWN; > + > + return mgr->mops->state(mgr); > +} > + > +/** > + * __fpga_mgr_reset - unlocked version of fpga_mgr_reset > + * @mgr: fpga manager > + */ > +static int __fpga_mgr_reset(struct fpga_manager *mgr) > +{ > + int ret; > + > + if (!mgr->mops->reset) > + return -EINVAL; > + > + ret = mgr->mops->reset(mgr); > + > + mgr->state = fpga_mgr_low_level_state(mgr); > + > + return ret; > +} > + > +/** > + * fpga_mgr_reset - reset the fpga > + * @mgr: fpga manager > + */ > +int fpga_mgr_reset(struct fpga_manager *mgr) > +{ > + int ret; > + > + if (!mutex_trylock(&mgr->lock)) > + return -EBUSY; > + > + ret = __fpga_mgr_reset(mgr); > + > + mutex_unlock(&mgr->lock); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(fpga_mgr_reset); > + > +/** > + * __fpga_mgr_stage_init - prepare fpga for configuration > + * @mgr: fpga manager > + */ > +static int __fpga_mgr_stage_write_init(struct fpga_manager *mgr) > +{ > + int ret; > + > + if (mgr->mops->write_init) { > + mgr->state = FPGA_MGR_STATE_WRITE_INIT; > + ret = mgr->mops->write_init(mgr); > + if (ret) { > + mgr->state = FPGA_MGR_STATE_WRITE_INIT_ERR; > + return ret; > + } > + } > + > + return 0; > +} > + > +/** > + * __fpga_mgr_stage_write - write buffer to fpga > + * @mgr: fpga manager > + * @buf: buffer contain fpga image > + * @count: byte count of buf > + */ > +static int __fpga_mgr_stage_write(struct fpga_manager *mgr, const char *buf, > + size_t count) > +{ > + int ret; > + > + mgr->state = FPGA_MGR_STATE_WRITE; > + ret = mgr->mops->write(mgr, buf, count); > + if (ret) { > + mgr->state = FPGA_MGR_STATE_WRITE_ERR; > + return ret; > + } > + > + return 0; > +} > + > +/** > + * __fpga_mgr_stage_complete - after writing, place fpga in operating state > + * @mgr: fpga manager > + */ > +static int __fpga_mgr_stage_write_complete(struct fpga_manager *mgr) > +{ > + int ret; > + > + if (mgr->mops->write_complete) { > + mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE; > + ret = mgr->mops->write_complete(mgr); > + if (ret) { > + mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE_ERR; > + return ret; > + } > + } > + > + mgr->state = fpga_mgr_low_level_state(mgr); > + > + return 0; > +} > + > +/** > + * __fpga_mgr_write - whole fpga image write cycle > + * @mgr: fpga manager > + * @buf: buffer contain fpga image > + * @count: byte count of buf > + */ > +static int __fpga_mgr_write(struct fpga_manager *mgr, const char *buf, > + size_t count) > +{ > + int ret; > + > + ret = __fpga_mgr_stage_write_init(mgr); > + if (ret) > + return ret; > + > + ret = __fpga_mgr_stage_write(mgr, buf, count); > + if (ret) > + return ret; > + > + return __fpga_mgr_stage_write_complete(mgr); > +} > + > +/** > + * fpga_mgr_write - do complete fpga image write cycle > + * @mgr: fpga manager > + * @buf: buffer contain fpga image > + * @count: byte count of buf > + */ > +int fpga_mgr_write(struct fpga_manager *mgr, const char *buf, size_t count) > +{ > + int ret; > + > + if (!mutex_trylock(&mgr->lock)) > + return -EBUSY; > + > + dev_info(&mgr->dev, "writing buffer to %s\n", mgr->name); > + > + ret = __fpga_mgr_write(mgr, buf, count); > + mutex_unlock(&mgr->lock); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(fpga_mgr_write); > + > +/** > + * fpga_mgr_firmware_write - request firmware and write to fpga > + * @mgr: fpga manager > + * @image_name: name of image file on the firmware search path > + * > + * Grab lock, request firmware, and write out to the FPGA. > + * Update the state before each step to provide info on what step > + * failed if there is a failure. > + */ > +int fpga_mgr_firmware_write(struct fpga_manager *mgr, const char *image_name) > +{ > + const struct firmware *fw; > + int ret; > + > + if (!mutex_trylock(&mgr->lock)) > + return -EBUSY; > + > + dev_info(&mgr->dev, "writing %s to %s\n", image_name, mgr->name); > + > + mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ; > + ret = request_firmware(&fw, image_name, &mgr->dev); > + if (ret) { > + mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ_ERR; > + goto fw_wr_exit; > + } > + > + ret = __fpga_mgr_write(mgr, fw->data, fw->size); > + if (ret) > + goto fw_wr_exit; > + > + strcpy(mgr->image_name, image_name); > + > +fw_wr_exit: > + mutex_unlock(&mgr->lock); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(fpga_mgr_firmware_write); > + > +/** > + * fpga_mgr_name - returns the fpga manager name > + * @mgr: fpga manager > + * @buf: buffer to receive the name > + */ > +int fpga_mgr_name(struct fpga_manager *mgr, char *buf) > +{ > + if (!mgr) > + return -ENODEV; > + > + return sprintf(buf, "%s\n", mgr->name); > +} > +EXPORT_SYMBOL_GPL(fpga_mgr_name); > + > +#if IS_ENABLED(CONFIG_FPGA_MGR_SYSFS) remove this line > +static const char * const state_str[] = { > + [FPGA_MGR_STATE_UNKNOWN] = "unknown", > + [FPGA_MGR_STATE_POWER_OFF] = "power_off", > + [FPGA_MGR_STATE_POWER_UP] = "power_up", > + [FPGA_MGR_STATE_RESET] = "reset", > + > + /* write sequence */ > + [FPGA_MGR_STATE_FIRMWARE_REQ] = "firmware_request", > + [FPGA_MGR_STATE_FIRMWARE_REQ_ERR] = "firmware_request_err", > + [FPGA_MGR_STATE_WRITE_INIT] = "write_init", > + [FPGA_MGR_STATE_WRITE_INIT_ERR] = "write_init_err", > + [FPGA_MGR_STATE_WRITE] = "write", > + [FPGA_MGR_STATE_WRITE_ERR] = "write_err", > + [FPGA_MGR_STATE_WRITE_COMPLETE] = "write_complete", > + [FPGA_MGR_STATE_WRITE_COMPLETE_ERR] = "write_complete_err", > + > + [FPGA_MGR_STATE_OPERATING] = "operating", > +}; > + > +/* > + * class attributes > + */ > +static ssize_t name_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + > + return fpga_mgr_name(mgr, buf); > +} > + > +static ssize_t state_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + > + return sprintf(buf, "%s\n", state_str[mgr->state]); > +} > + > +static ssize_t firmware_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + > + return sprintf(buf, "%s\n", mgr->image_name); > +} > + > +static ssize_t firmware_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + unsigned int len; > + char image_name[NAME_MAX]; > + int ret; > + > + /* lose terminating \n */ > + strcpy(image_name, buf); > + len = strlen(image_name); > + if (image_name[len - 1] == '\n') > + image_name[len - 1] = 0; > + > + ret = fpga_mgr_firmware_write(mgr, image_name); > + if (ret) > + return ret; > + > + return count; > +} > + > +static ssize_t reset_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + unsigned long val; > + int ret; > + > + ret = kstrtoul(buf, 0, &val); > + if (ret) > + return ret; > + > + if (val == 1) { > + ret = fpga_mgr_reset(mgr); > + if (ret) > + return ret; > + } else { > + return -EINVAL; > + } > + > + return count; > +} > + > +static DEVICE_ATTR_RO(name); > +static DEVICE_ATTR_RO(state); > +static DEVICE_ATTR_RW(firmware); > +static DEVICE_ATTR_WO(reset); > + > +static struct attribute *fpga_mgr_attrs[] = { > + &dev_attr_name.attr, > + &dev_attr_state.attr, > + &dev_attr_firmware.attr, > + &dev_attr_reset.attr, > + NULL, > +}; > +ATTRIBUTE_GROUPS(fpga_mgr); > +#else > +#define fpga_mgr_groups NULL > +#endif /* CONFIG_FPGA_MGR_SYSFS */ remove these 3 lines and look below. > + > +static int fpga_mgr_suspend(struct device *dev) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + > + if (!mgr) > + return -ENODEV; > + > + if (mgr->mops->suspend) > + return mgr->mops->suspend(mgr); > + > + return 0; > +} > + > +static int fpga_mgr_resume(struct device *dev) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + int ret = 0; > + > + if (!mgr) > + return -ENODEV; > + > + if (mgr->mops->resume) { > + ret = mgr->mops->resume(mgr); > + if (ret) > + return ret; > + } > + > + if (strlen(mgr->image_name) != 0) > + fpga_mgr_firmware_write(mgr, mgr->image_name); > + > + return 0; > +} > + > +static const struct dev_pm_ops fpga_mgr_dev_pm_ops = { > + .suspend = fpga_mgr_suspend, > + .resume = fpga_mgr_resume, > +}; > + > +static void fpga_mgr_dev_release(struct device *dev) > +{ > + struct fpga_manager *mgr = to_fpga_manager(dev); > + > + dev_dbg(dev, "releasing '%s'\n", mgr->name); > + > + if (mgr->mops->fpga_remove) > + mgr->mops->fpga_remove(mgr); > + > + mgr->mops = NULL; > + > + mutex_lock(&fpga_mgr_mutex); > + list_del(&mgr->list); > + mutex_unlock(&fpga_mgr_mutex); > + > + ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); > + kfree(mgr); > +} > + > +/** > + * fpga_mgr_register - register a low level fpga manager driver > + * @dev: fpga manager device > + * @name: fpga manager name > + * @mops: pointer to structure of fpga manager ops > + * @priv: fpga manager private data > + */ > +int fpga_mgr_register(struct device *dev, const char *name, > + const struct fpga_manager_ops *mops, > + void *priv) > +{ > + struct fpga_manager *mgr; > + int id, ret; > + > + if (!mops || !name || !strlen(name)) > + return -EINVAL; > + > + mgr = kzalloc(sizeof(*mgr), GFP_KERNEL); > + if (!mgr) > + return -ENOMEM; > + > + id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL); > + if (id < 0) > + return id; > + > + mutex_init(&mgr->lock); > + > + mgr->name = name; > + mgr->mops = mops; > + mgr->priv = priv; > + > + /* > + * Initialize framework state by requesting low level driver read state > + * from device. FPGA may be in reset mode or may have been programmed > + * by bootloader or EEPROM. > + */ > + mgr->state = fpga_mgr_low_level_state(mgr); > + > + INIT_LIST_HEAD(&mgr->list); > + mutex_lock(&fpga_mgr_mutex); > + list_add(&mgr->list, &fpga_manager_list); > + mutex_unlock(&fpga_mgr_mutex); > + > + device_initialize(&mgr->dev); > + mgr->dev.class = fpga_mgr_class; > + mgr->dev.parent = dev; > + mgr->dev.of_node = dev->of_node; > + mgr->dev.release = fpga_mgr_dev_release; > + mgr->dev.id = id; > + dev_set_name(&mgr->dev, "%d", id); > + ret = device_add(&mgr->dev); > + if (ret) > + goto error_device; > + > + dev_info(&mgr->dev, "%s registered\n", mgr->name); > + > + return 0; > + > +error_device: > + ida_simple_remove(&fpga_mgr_ida, id); > + kfree(mgr); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(fpga_mgr_register); > + > +/** > + * fpga_mgr_remove - remove a low level fpga manager driver > + * @dev: fpga manager device > + */ > +void fpga_mgr_remove(struct platform_device *pdev) > +{ > + struct list_head *tmp; > + struct fpga_manager *mgr = NULL; > + > + list_for_each(tmp, &fpga_manager_list) { > + mgr = list_entry(tmp, struct fpga_manager, list); > + if (mgr->dev.parent == &pdev->dev) { > + device_unregister(&mgr->dev); > + break; > + } > + } > +} > +EXPORT_SYMBOL_GPL(fpga_mgr_remove); > + > +static int __init fpga_mgr_dev_init(void) > +{ > + pr_info("FPGA Manager framework driver\n"); > + > + fpga_mgr_class = class_create(THIS_MODULE, "fpga_manager"); > + if (IS_ERR(fpga_mgr_class)) > + return PTR_ERR(fpga_mgr_class); > + > + fpga_mgr_class->dev_groups = fpga_mgr_groups; Write this here. if (IS_ENABLED(CONFIG_FPGA_MGR_SYSFS)) { fpga_mgr_class->dev_groups = fpga_mgr_groups; } I have tested it and it is working just fine. You can compile this driver with SYSFS=n The whole code is built and config option just export it. I think it is better than having #if there. Greg: Is there any problem with this solution? BTW: I have pushed my branch to zero day testing system to get better coverage Thanks, Michal -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html