fpga: doc: documentation for FPGA debugfs
Subsystems (2)
PATCH 1/2 fpga: doc: documentation for FPGA debugfs
From: Alan Tull <[email protected]>
This patch depends on my recently submitted documentation changes ("docs: fpga: document programming fpgas using regions")
Document the DebugFS interface for the core FPGA Manager framework.
--- Documentation/driver-api/fpga/fpga-mgr.rst | 38 ++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
Documentation/driver-api/fpga/fpga-mgr.rst +38 -0
@@ -125,3 +125,41 @@ API for implementing a new FPGA Manager driver .. kernel-doc:: drivers/fpga/fpga-mgr.c :functions: fpga_mgr_unregister + +FPGA Manager DebugFS +-------------------- + +This interface allows the user to program an FPGA from userspace. However, +bridges and soft IP device driver loading/unloading are not handled. This makes +it really easy to mess things up by doing things like reprogramming the hardware +out from under a driver or reprogramming while a bridge is enabled, causing gunk +to go out on a CPU bus. It should go without saying that this interface is for +debug and development only. Not intended for production use. + +Each FPGA gets its own directory such as <debugfs>/fpga_manager/fpga0 and the +files described below. To program the FPGA, write the ``flags`` and/or +``config_complete_timeout_us`` files (as needed), then use either the +``firmware_name`` or ``image`` file to program. + +* ``flags`` - [RW] flags as defined in fpga-mgr.h. For example:: + + $ echo 1 > /sys/kernel/debug/fpga_manager/fpga0/flags + +* ``config_complete_timeout_us`` - [RW] time out in microseconds to wait for the + FPGA to go to operating state after region has been programmed. Not all + low level drivers use this. For example:: + + $ echo 4 > /sys/kernel/debug/fpga_manager/fpga0/config_complete_timeout_us + +* ``firmware_name`` - [RW] Name of an FPGA image firmware file. Writing + initiates an FPGA programming cycle. Note that the image file must be in a + directory on the firmware search path such as /lib/firmware:: + + $ echo image.rbf > /sys/kernel/debug/fpga_manager/fpga0/firmware_name + +* ``image`` - [WO] Raw FPGA image data. Writing the FPGA image data will + initiate an FPGA programming cycle. Data must be written in one chunk, for + example:: + + $ dd bs=10M if=./image.rbf of=/sys/kernel/debug/fpga_manager/fpga0/image + (where image.rbf < 10M)
Signature · 1 line
2.7.4
PATCH 2/2 fpga: add FPGA manager debugfs
From: Alan Tull <[email protected]>
Implement DebugFS for the FPGA Manager Framework for debugging and developmental use.
Enabled by CONFIG_FPGA_MGR_DEBUG_FS
Each FPGA gets its own directory such as
<debugfs>/fpga_manager/fpga0 and three files:
* [RW] flags = flags as defined in fpga-mgr.h
* [RW] firmware_name = write/read back name of FPGA image firmware file to program
* [WO] image = write-only file for directly writing
fpga image w/o firmware layer
* [RW] config_complete_timeout_us = maximum for the FPGA to go to the operating state after programming
The debugfs is implemented in a separate fpga_mgr_debugfs.c file, but the FPGA manager core is still built as one module. Note the name change from fpga-mgr.ko to fpga_mgr.ko.
--- drivers/fpga/Kconfig | 7 ++ drivers/fpga/Makefile | 4 +- drivers/fpga/fpga-mgr-debugfs.c | 221 ++++++++++++++++++++++++++++++++++++++++ drivers/fpga/fpga-mgr-debugfs.h | 22 ++++ drivers/fpga/fpga-mgr.c | 8 ++ include/linux/fpga/fpga-mgr.h | 3 + 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 drivers/fpga/fpga-mgr-debugfs.c create mode 100644 drivers/fpga/fpga-mgr-debugfs.h
drivers/fpga/Kconfig +7 -0
@@ -9,6 +9,13 @@ menuconfig FPGA kernel. The FPGA framework adds a FPGA manager class and FPGA manager drivers. +config FPGA_MGR_DEBUG_FS + bool "FPGA Manager DebugFS" + depends on FPGA && DEBUG_FS + help + Say Y here if you want to expose a DebugFS interface for the + FPGA Manager Framework. + if FPGA config FPGA_MGR_SOCFPGA
drivers/fpga/Makefile +3 -1
@@ -4,7 +4,9 @@ # # Core FPGA Manager Framework -obj-$(CONFIG_FPGA) += fpga-mgr.o +obj-$(CONFIG_FPGA) += fpga_mgr.o +fpga_mgr-y := fpga-mgr.o +fpga_mgr-$(CONFIG_FPGA_MGR_DEBUG_FS) += fpga-mgr-debugfs.o # FPGA Manager Drivers obj-$(CONFIG_FPGA_MGR_ALTERA_CVP) += altera-cvp.o
drivers/fpga/fpga-mgr-debugfs.c new file +221 -0
221 lines not on this page · raw message
drivers/fpga/fpga-mgr-debugfs.h new file +22 -0
@@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 + +#ifndef _LINUX_FPGA_MGR_DEBUGFS_H +#define _LINUX_FPGA_MGR_DEBUGFS_H + +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr); +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr); +void fpga_mgr_debugfs_init (void ); +void fpga_mgr_debugfs_uninit (void ); + +#else + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) {} +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr) {} +void fpga_mgr_debugfs_init (void ) {} +void fpga_mgr_debugfs_uninit (void ) {} + +#endif /* CONFIG_FPGA_MGR_DEBUG_FS */ + +#endif /*_LINUX_FPGA_MGR_DEBUGFS_H */
drivers/fpga/fpga-mgr.c +8 -0
@@ -17,6 +17,7 @@#include <linux/slab.h> #include <linux/scatterlist.h> #include <linux/highmem.h> +#include "fpga-mgr-debugfs.h" static DEFINE_IDA (fpga_mgr_ida);static struct class *fpga_mgr_class;
@@ -698,6 +699,8 @@ int fpga_mgr_register(struct fpga_manager *mgr)if (ret) goto error_device; +fpga_mgr_debugfs_add (mgr); +dev_info (&mgr->dev,"%s registered\n" , mgr->name);return 0 ;
@@ -722,6 +725,8 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) {dev_info (&mgr->dev,"%s %s\n" , __func__, mgr->name); +fpga_mgr_debugfs_remove (mgr); +/* * If the low level driver provides a method for putting fpga into * a desired state upon unregister, do it.
@@ -748,11 +753,14 @@ static int __init fpga_mgr_class_init(void) fpga_mgr_class->dev_groups = fpga_mgr_groups; fpga_mgr_class->dev_release = fpga_mgr_dev_release; +fpga_mgr_debugfs_init (); +return 0 ; } static void __exit fpga_mgr_class_exit(void) { +fpga_mgr_debugfs_uninit ();class_destroy (fpga_mgr_class);ida_destroy (&fpga_mgr_ida); }
include/linux/fpga/fpga-mgr.h +3 -0
@@ -170,6 +170,9 @@ struct fpga_manager {struct fpga_compat_id *compat_id;const struct fpga_manager_ops *mops;void *priv; +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) +void *debugfs; +#endif };#define to_fpga_manager (d) container_of(d, struct fpga_manager, dev)
Signature · 1 line
2.7.4
Randy Dunlap
reviewerRe: [PATCH 2/2] fpga: add FPGA manager debugfs
Quoted 6 lines · Alan Tull
From: Alan Tull <[email protected]>
Implement DebugFS for the FPGA Manager Framework for debugging and developmental use.
Enabled by
CONFIG_FPGA_MGR_DEBUG_FS
Hi Alan,
for your v2 changes:
Quoted 2 lines
Each FPGA gets its own directory such as
<debugfs>/fpga_manager/fpga0 and three files:
s/three/four/
Quoted 11 lines
* [RW] flags = flags as defined in
fpga-mgr.h
* [RW]firmware_name= write/read back name of FPGA image firmware file to program
* [WO] image = write-only file for directly writing
fpga image w/o firmware layer
* [RW]config_complete_timeout_us= maximum for the FPGA to go to the operating state after programmingThe debugfs is implemented in a separate
fpga_mgr_debugfs.c
debugfs interface is
Quoted 21 lines
file, but the FPGA manager core is still built as one module. Note the name change from fpga-mgr.ko to fpga_mgr.ko.
Signed-off-by Alan Tull <[email protected]>Signed-off-by Matthew Gerlach <[email protected]>--- drivers/fpga/Kconfig | 7 ++ drivers/fpga/Makefile | 4 +- drivers/fpga/fpga-mgr-debugfs.c | 221 ++++++++++++++++++++++++++++++++++++++++ drivers/fpga/fpga-mgr-debugfs.h | 22 ++++ drivers/fpga/fpga-mgr.c | 8 ++ include/linux/fpga/fpga-mgr.h | 3 + 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 drivers/fpga/fpga-mgr-debugfs.c create mode 100644 drivers/fpga/fpga-mgr-debugfs.hdrivers/fpga/Kconfig +0
-0
drivers/fpga/Kconfig menuconfig FPGA · line 17
Quoted 9 lines
drivers/fpga/Kconfig +6
-0@@ -9,6 +9,13 @@ menuconfig FPGA kernel. The FPGA framework adds a FPGA manager class and FPGA manager drivers. +config FPGA_MGR_DEBUG_FS + bool "FPGA Manager DebugFS" + depends on FPGA && DEBUG_FS + help + Say Y here if you want to expose a DebugFS interface for the + FPGA Manager Framework.
Use tab indentation for bool, depends, and help lines.
Use tab +2 spaces for help text.
drivers/fpga/Kconfig menuconfig FPGA · line 21
Quoted 4 lines
drivers/fpga/Kconfig +1
-0+ if FPGA config FPGA_MGR_SOCFPGA
thanks,
Signature · 1 line
~Randy
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
On Wed, Aug 15, 2018 at 5:34 PM, Randy Dunlap <[email protected]> wrote:
Hi Randy,
Quoted 8 lines · Alan Tull
Quoted 6 lines
From: Alan Tull <[email protected]>
Implement DebugFS for the FPGA Manager Framework for debugging and developmental use.
Enabled by
CONFIG_FPGA_MGR_DEBUG_FS
Quoted 8 lines
Hi Alan,
for your v2 changes:
Quoted 2 lines
Each FPGA gets its own directory such as
<debugfs>/fpga_manager/fpga0 and three files:s/three/four/
Yes
Quoted 11 lines
Quoted 11 lines
* [RW] flags = flags as defined in
fpga-mgr.h
* [RW]firmware_name= write/read back name of FPGA image firmware file to program
* [WO] image = write-only file for directly writing
fpga image w/o firmware layer
* [RW]config_complete_timeout_us= maximum for the FPGA to go to the operating state after programming
Quoted 3 lines
Quoted 1 line
The debugfs is implemented in a separate
fpga_mgr_debugfs.cdebugfs interface is
Yes
Quoted 34 lines
Quoted 22 lines
file, but the FPGA manager core is still built as one module. Note the name change from fpga-mgr.ko to fpga_mgr.ko.
Signed-off-by Alan Tull <[email protected]>Signed-off-by Matthew Gerlach <[email protected]>--- drivers/fpga/Kconfig | 7 ++ drivers/fpga/Makefile | 4 +- drivers/fpga/fpga-mgr-debugfs.c | 221 ++++++++++++++++++++++++++++++++++++++++ drivers/fpga/fpga-mgr-debugfs.h | 22 ++++ drivers/fpga/fpga-mgr.c | 8 ++ include/linux/fpga/fpga-mgr.h | 3 + 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 drivers/fpga/fpga-mgr-debugfs.c create mode 100644 drivers/fpga/fpga-mgr-debugfs.hdrivers/fpga/Kconfig +0
-0drivers/fpga/Kconfig menuconfig FPGA · line 17
Quoted 9 lines
drivers/fpga/Kconfig +6
-0@@ -9,6 +9,13 @@ menuconfig FPGA kernel. The FPGA framework adds a FPGA manager class and FPGA manager drivers. +config FPGA_MGR_DEBUG_FS + bool "FPGA Manager DebugFS" + depends on FPGA && DEBUG_FS + help + Say Y here if you want to expose a DebugFS interface for the + FPGA Manager Framework.Use tab indentation for bool, depends, and help lines.
Quoted 1 line
Use tab +2 spaces for help text.
Yes, thanks for catching that. I'll fix these in v2.
Thanks for the review!
Alan
Quoted 9 lines
drivers/fpga/Kconfig menuconfig FPGA · line 21
Quoted 5 lines
drivers/fpga/Kconfig +1
-0+ if FPGA config FPGA_MGR_SOCFPGAthanks,
--
~Randy
Moritz Fischer
maintainerRe: [PATCH 2/2] fpga: add FPGA manager debugfs
In reply to Alan TullHi Alan,
comments inline. While I see how this is useful, I have the suspicion that from the moment this gets merged vendor kernels will just default to use this ...
Quoted 146 lines · Alan Tull
From: Alan Tull <[email protected]>
Implement DebugFS for the FPGA Manager Framework for debugging and developmental use.
Enabled by
CONFIG_FPGA_MGR_DEBUG_FSEach FPGA gets its own directory such as
<debugfs>/fpga_manager/fpga0 and three files:* [RW] flags = flags as defined in
fpga-mgr.h
* [RW]firmware_name= write/read back name of FPGA image firmware file to program
* [WO] image = write-only file for directly writing
fpga image w/o firmware layer
* [RW]config_complete_timeout_us= maximum for the FPGA to go to the operating state after programmingThe debugfs is implemented in a separate
fpga_mgr_debugfs.cfile, but the FPGA manager core is still built as one module. Note the name change from fpga-mgr.ko to fpga_mgr.ko.Signed-off-by Alan Tull <[email protected]>Signed-off-by Matthew Gerlach <[email protected]>--- drivers/fpga/Kconfig | 7 ++ drivers/fpga/Makefile | 4 +- drivers/fpga/fpga-mgr-debugfs.c | 221 ++++++++++++++++++++++++++++++++++++++++ drivers/fpga/fpga-mgr-debugfs.h | 22 ++++ drivers/fpga/fpga-mgr.c | 8 ++ include/linux/fpga/fpga-mgr.h | 3 + 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 drivers/fpga/fpga-mgr-debugfs.c create mode 100644 drivers/fpga/fpga-mgr-debugfs.hdrivers/fpga/Kconfig +7
-0@@ -9,6 +9,13 @@ menuconfig FPGA kernel. The FPGA framework adds a FPGA manager class and FPGA manager drivers. +config FPGA_MGR_DEBUG_FS + bool "FPGA Manager DebugFS" + depends on FPGA && DEBUG_FS + help + Say Y here if you want to expose a DebugFS interface for the + FPGA Manager Framework. + if FPGA config FPGA_MGR_SOCFPGAdrivers/fpga/Makefile +3
-1@@ -4,7 +4,9 @@ # # Core FPGA Manager Framework -obj-$(CONFIG_FPGA) += fpga-mgr.o +obj-$(CONFIG_FPGA) += fpga_mgr.o +fpga_mgr-y := fpga-mgr.o +fpga_mgr-$(CONFIG_FPGA_MGR_DEBUG_FS) += fpga-mgr-debugfs.o # FPGA Manager Drivers obj-$(CONFIG_FPGA_MGR_ALTERA_CVP) += altera-cvp.odrivers/fpga/fpga-mgr-debugfs.c new file +71
-0@@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* FPGA Manager DebugFS +* +* Copyright (C) 2016-2018 Intel Corporation. All rights reserved. +*/ +#include <linux/debugfs.h> +#include <linux/fpga/fpga-mgr.h> +#include <linux/slab.h> +#include <linux/uaccess.h> + +static struct dentry *fpga_mgr_debugfs_root; + +struct fpga_mgr_debugfs { +struct dentry *debugfs_dir; +struct fpga_image_info *info; +}; + +static ssize_t fpga_mgr_firmware_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =devm_kzalloc (dev, count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +devm_kfree (dev, buf); +return -EFAULT ; + } + + buf[count] =0 ; +if (buf[count -1 ] =='\n' ) + buf[count -1 ] =0 ; + +/* Release previous firmware name (if any). Save current one. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name = buf; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret) +dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + +return count; +} + +static ssize_t fpga_mgr_firmware_read_file (struct file *file, +char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +char *buf;
drivers/fpga/fpga-mgr-debugfs.c line 81
Quoted 10 lines
drivers/fpga/fpga-mgr-debugfs.c new file +10
-0+int ret; + +if (!debugfs->info->firmware_name) +return 0 ; + + buf =kzalloc (PAGE_SIZE ,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + + ret =snprintf (buf,PAGE_SIZE ,"%s\n" , debugfs->info->firmware_name);
snip:
---->8->8->8-----
drivers/fpga/fpga-mgr-debugfs.c line 85
Quoted 4 lines
drivers/fpga/fpga-mgr-debugfs.c new file +4
-0+if (ret <0 ) { +kfree (buf); +return ret; + }
---->8->8->8-----
just replace with:
if (ret < 0) goto out;
drivers/fpga/fpga-mgr-debugfs.c line 87
Quoted 2 lines
drivers/fpga/fpga-mgr-debugfs.c new file +2
-0+ + ret =simple_read_from_buffer (user_buf, count, ppos, buf, ret);
out:
Quoted 47 lines
drivers/fpga/fpga-mgr-debugfs.c new file +47
-0+kfree (buf); +return ret; +} + +static const struct file_operations fpga_mgr_firmware_fops = { + .open = simple_open, + .read = fpga_mgr_firmware_read_file, + .write = fpga_mgr_firmware_write_file, + .llseek = default_llseek, +}; + +static ssize_t fpga_mgr_image_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + +dev_info (dev,"writing %zu bytes to %s\n" , count, mgr->name); + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =kzalloc (count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +kfree (buf); +return -EFAULT ; + } + +/* If firmware interface was previously used, forget it. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name =NULL ; + + debugfs->info->buf = buf;
drivers/fpga/fpga-mgr-debugfs.c line 145
Quoted 10 lines
drivers/fpga/fpga-mgr-debugfs.c new file +10
-0+ debugfs->info->count = count; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret) +dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + + debugfs->info->buf =NULL ; + debugfs->info->count =0 ;
Is that ordering between unlock() and setting those correct?
Quoted 166 lines
drivers/fpga/fpga-mgr-debugfs.c new file +76
-0+ +kfree (buf); + +return count; +} + +static const struct file_operations fpga_mgr_image_fops = { + .open = simple_open, + .write = fpga_mgr_image_write_file, + .llseek = default_llseek, +}; + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) +{ +struct device *dev = &mgr->dev; +struct fpga_mgr_debugfs *debugfs; +struct fpga_image_info *info; + +if (!fpga_mgr_debugfs_root) +return ; + + debugfs =kzalloc (sizeof (*debugfs),GFP_KERNEL ); +if (!debugfs) +return ; + + info =fpga_image_info_alloc (dev); +if (!info) { +kfree (debugfs); +return ; + } + debugfs->info = info; + + debugfs->debugfs_dir =debugfs_create_dir (dev_name (dev), + fpga_mgr_debugfs_root); + +debugfs_create_file ("firmware_name" ,0600 , debugfs->debugfs_dir, mgr, + &fpga_mgr_firmware_fops); + +debugfs_create_file ("image" ,0200 , debugfs->debugfs_dir, mgr, + &fpga_mgr_image_fops); + +debugfs_create_u32 ("flags" ,0600 , debugfs->debugfs_dir, &info->flags); + +debugfs_create_u32 ("config_complete_timeout_us" ,0600 , + debugfs->debugfs_dir, + &info->config_complete_timeout_us); + + mgr->debugfs = debugfs; +} + +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr) +{ +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; + +if (!fpga_mgr_debugfs_root) +return ; + +debugfs_remove_recursive (debugfs->debugfs_dir); + +/* this function also frees debugfs->info->firmware_name */ +fpga_image_info_free (debugfs->info); + +kfree (debugfs); +} + +void fpga_mgr_debugfs_init (void ) +{ + fpga_mgr_debugfs_root =debugfs_create_dir ("fpga_manager" ,NULL ); +if (!fpga_mgr_debugfs_root) +pr_warn ("fpga_mgr: Failed to create debugfs root\n" ); +} + +void fpga_mgr_debugfs_uninit (void ) +{ +debugfs_remove_recursive (fpga_mgr_debugfs_root); +}drivers/fpga/fpga-mgr-debugfs.h new file +22
-0@@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 + +#ifndef _LINUX_FPGA_MGR_DEBUGFS_H +#define _LINUX_FPGA_MGR_DEBUGFS_H + +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr); +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr); +void fpga_mgr_debugfs_init (void ); +void fpga_mgr_debugfs_uninit (void ); + +#else + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) {} +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr) {} +void fpga_mgr_debugfs_init (void ) {} +void fpga_mgr_debugfs_uninit (void ) {} + +#endif /* CONFIG_FPGA_MGR_DEBUG_FS */ + +#endif /*_LINUX_FPGA_MGR_DEBUGFS_H */ drivers/fpga/fpga-mgr.c +8
-0@@ -17,6 +17,7 @@#include <linux/slab.h> #include <linux/scatterlist.h> #include <linux/highmem.h> +#include "fpga-mgr-debugfs.h" static DEFINE_IDA (fpga_mgr_ida);static struct class *fpga_mgr_class;@@ -698,6 +699,8 @@ int fpga_mgr_register(struct fpga_manager *mgr)if (ret) goto error_device; +fpga_mgr_debugfs_add (mgr); +dev_info (&mgr->dev,"%s registered\n" , mgr->name);return 0 ;@@ -722,6 +725,8 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) {dev_info (&mgr->dev,"%s %s\n" , __func__, mgr->name); +fpga_mgr_debugfs_remove (mgr); +/* * If the low level driver provides a method for putting fpga into * a desired state upon unregister, do it. @@ -748,11 +753,14 @@ static int __init fpga_mgr_class_init(void) fpga_mgr_class->dev_groups = fpga_mgr_groups; fpga_mgr_class->dev_release = fpga_mgr_dev_release; +fpga_mgr_debugfs_init (); +return 0 ; } static void __exit fpga_mgr_class_exit(void) { +fpga_mgr_debugfs_uninit ();class_destroy (fpga_mgr_class);ida_destroy (&fpga_mgr_ida); }include/linux/fpga/fpga-mgr.h +3
-0@@ -170,6 +170,9 @@ struct fpga_manager {struct fpga_compat_id *compat_id;const struct fpga_manager_ops *mops;void *priv; +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) +void *debugfs; +#endif };#define to_fpga_manager (d) container_of(d, struct fpga_manager, dev)Signature · 1 line
2.7.4
Thanks,
Moritz
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
Quoted 1 line · Moritz Fischer
Hi Alan,
Hi Moritz,
Quoted 3 lines
comments inline. While I see how this is useful, I have the suspicion that from the moment this gets merged vendor kernels will just default to use this ...
Yeah, I have that suspicion as well. That's probably why I sat on this and didn't upstream it for 2 years. But on the other hand, I keep hearing of lots of cases of people implementing this independently anyway. At least if it is debugfs, it makes it clear that it's not intended for production use.
Quoted 164 lines · Alan Tull
Quoted 146 lines
From: Alan Tull <[email protected]>
Implement DebugFS for the FPGA Manager Framework for debugging and developmental use.
Enabled by
CONFIG_FPGA_MGR_DEBUG_FSEach FPGA gets its own directory such as
<debugfs>/fpga_manager/fpga0 and three files:* [RW] flags = flags as defined in
fpga-mgr.h
* [RW]firmware_name= write/read back name of FPGA image firmware file to program
* [WO] image = write-only file for directly writing
fpga image w/o firmware layer
* [RW]config_complete_timeout_us= maximum for the FPGA to go to the operating state after programmingThe debugfs is implemented in a separate
fpga_mgr_debugfs.cfile, but the FPGA manager core is still built as one module. Note the name change from fpga-mgr.ko to fpga_mgr.ko.Signed-off-by Alan Tull <[email protected]>Signed-off-by Matthew Gerlach <[email protected]>--- drivers/fpga/Kconfig | 7 ++ drivers/fpga/Makefile | 4 +- drivers/fpga/fpga-mgr-debugfs.c | 221 ++++++++++++++++++++++++++++++++++++++++ drivers/fpga/fpga-mgr-debugfs.h | 22 ++++ drivers/fpga/fpga-mgr.c | 8 ++ include/linux/fpga/fpga-mgr.h | 3 + 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 drivers/fpga/fpga-mgr-debugfs.c create mode 100644 drivers/fpga/fpga-mgr-debugfs.hdrivers/fpga/Kconfig +7
-0@@ -9,6 +9,13 @@ menuconfig FPGA kernel. The FPGA framework adds a FPGA manager class and FPGA manager drivers. +config FPGA_MGR_DEBUG_FS + bool "FPGA Manager DebugFS" + depends on FPGA && DEBUG_FS + help + Say Y here if you want to expose a DebugFS interface for the + FPGA Manager Framework. + if FPGA config FPGA_MGR_SOCFPGAdrivers/fpga/Makefile +3
-1@@ -4,7 +4,9 @@ # # Core FPGA Manager Framework -obj-$(CONFIG_FPGA) += fpga-mgr.o +obj-$(CONFIG_FPGA) += fpga_mgr.o +fpga_mgr-y := fpga-mgr.o +fpga_mgr-$(CONFIG_FPGA_MGR_DEBUG_FS) += fpga-mgr-debugfs.o # FPGA Manager Drivers obj-$(CONFIG_FPGA_MGR_ALTERA_CVP) += altera-cvp.odrivers/fpga/fpga-mgr-debugfs.c new file +71
-0@@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* FPGA Manager DebugFS +* +* Copyright (C) 2016-2018 Intel Corporation. All rights reserved. +*/ +#include <linux/debugfs.h> +#include <linux/fpga/fpga-mgr.h> +#include <linux/slab.h> +#include <linux/uaccess.h> + +static struct dentry *fpga_mgr_debugfs_root; + +struct fpga_mgr_debugfs { +struct dentry *debugfs_dir; +struct fpga_image_info *info; +}; + +static ssize_t fpga_mgr_firmware_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =devm_kzalloc (dev, count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +devm_kfree (dev, buf); +return -EFAULT ; + } + + buf[count] =0 ; +if (buf[count -1 ] =='\n' ) + buf[count -1 ] =0 ; + +/* Release previous firmware name (if any). Save current one. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name = buf; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret) +dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + +return count; +} + +static ssize_t fpga_mgr_firmware_read_file (struct file *file, +char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +char *buf;drivers/fpga/fpga-mgr-debugfs.c line 81
Quoted 10 lines
drivers/fpga/fpga-mgr-debugfs.c new file +10
-0+int ret; + +if (!debugfs->info->firmware_name) +return 0 ; + + buf =kzalloc (PAGE_SIZE ,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + + ret =snprintf (buf,PAGE_SIZE ,"%s\n" , debugfs->info->firmware_name);snip:
---->8->8->8-----drivers/fpga/fpga-mgr-debugfs.c line 85
Quoted 4 lines
drivers/fpga/fpga-mgr-debugfs.c new file +4
-0+if (ret <0 ) { +kfree (buf); +return ret; + }
Quoted 11 lines
---->8->8->8-----
just replace with:if (ret < 0) goto out;drivers/fpga/fpga-mgr-debugfs.c line 87
Quoted 2 lines
drivers/fpga/fpga-mgr-debugfs.c new file +2
-0+ + ret =simple_read_from_buffer (user_buf, count, ppos, buf, ret);out:
Quoted 3 lines
drivers/fpga/fpga-mgr-debugfs.c new file +3
-0+kfree (buf); +return ret; +}
Yes, that's better. I'll include that in v2.
Quoted 48 lines
Quoted 48 lines
drivers/fpga/fpga-mgr-debugfs.c new file +48
-0+ +static const struct file_operations fpga_mgr_firmware_fops = { + .open = simple_open, + .read = fpga_mgr_firmware_read_file, + .write = fpga_mgr_firmware_write_file, + .llseek = default_llseek, +}; + +static ssize_t fpga_mgr_image_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + +dev_info (dev,"writing %zu bytes to %s\n" , count, mgr->name); + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =kzalloc (count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +kfree (buf); +return -EFAULT ; + } + +/* If firmware interface was previously used, forget it. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name =NULL ; + + debugfs->info->buf = buf; + debugfs->info->count = count; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret)
Quoted 8 lines
drivers/fpga/fpga-mgr-debugfs.c line 145
Quoted 6 lines
drivers/fpga/fpga-mgr-debugfs.c new file +6
-0+dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + + debugfs->info->buf =NULL ; + debugfs->info->count =0 ;Is that ordering between
unlock()and setting those correct?
This order should be alright, but I'll switch it anyway to be more obviously correct.
Quoted 162 lines
Quoted 162 lines
drivers/fpga/fpga-mgr-debugfs.c new file +76
-0+ +kfree (buf); + +return count; +} + +static const struct file_operations fpga_mgr_image_fops = { + .open = simple_open, + .write = fpga_mgr_image_write_file, + .llseek = default_llseek, +}; + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) +{ +struct device *dev = &mgr->dev; +struct fpga_mgr_debugfs *debugfs; +struct fpga_image_info *info; + +if (!fpga_mgr_debugfs_root) +return ; + + debugfs =kzalloc (sizeof (*debugfs),GFP_KERNEL ); +if (!debugfs) +return ; + + info =fpga_image_info_alloc (dev); +if (!info) { +kfree (debugfs); +return ; + } + debugfs->info = info; + + debugfs->debugfs_dir =debugfs_create_dir (dev_name (dev), + fpga_mgr_debugfs_root); + +debugfs_create_file ("firmware_name" ,0600 , debugfs->debugfs_dir, mgr, + &fpga_mgr_firmware_fops); + +debugfs_create_file ("image" ,0200 , debugfs->debugfs_dir, mgr, + &fpga_mgr_image_fops); + +debugfs_create_u32 ("flags" ,0600 , debugfs->debugfs_dir, &info->flags); + +debugfs_create_u32 ("config_complete_timeout_us" ,0600 , + debugfs->debugfs_dir, + &info->config_complete_timeout_us); + + mgr->debugfs = debugfs; +} + +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr) +{ +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; + +if (!fpga_mgr_debugfs_root) +return ; + +debugfs_remove_recursive (debugfs->debugfs_dir); + +/* this function also frees debugfs->info->firmware_name */ +fpga_image_info_free (debugfs->info); + +kfree (debugfs); +} + +void fpga_mgr_debugfs_init (void ) +{ + fpga_mgr_debugfs_root =debugfs_create_dir ("fpga_manager" ,NULL ); +if (!fpga_mgr_debugfs_root) +pr_warn ("fpga_mgr: Failed to create debugfs root\n" ); +} + +void fpga_mgr_debugfs_uninit (void ) +{ +debugfs_remove_recursive (fpga_mgr_debugfs_root); +}drivers/fpga/fpga-mgr-debugfs.h new file +22
-0@@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 + +#ifndef _LINUX_FPGA_MGR_DEBUGFS_H +#define _LINUX_FPGA_MGR_DEBUGFS_H + +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr); +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr); +void fpga_mgr_debugfs_init (void ); +void fpga_mgr_debugfs_uninit (void ); + +#else + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) {} +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr) {} +void fpga_mgr_debugfs_init (void ) {} +void fpga_mgr_debugfs_uninit (void ) {} + +#endif /* CONFIG_FPGA_MGR_DEBUG_FS */ + +#endif /*_LINUX_FPGA_MGR_DEBUGFS_H */ drivers/fpga/fpga-mgr.c +8
-0@@ -17,6 +17,7 @@#include <linux/slab.h> #include <linux/scatterlist.h> #include <linux/highmem.h> +#include "fpga-mgr-debugfs.h" static DEFINE_IDA (fpga_mgr_ida);static struct class *fpga_mgr_class;@@ -698,6 +699,8 @@ int fpga_mgr_register(struct fpga_manager *mgr)if (ret) goto error_device; +fpga_mgr_debugfs_add (mgr); +dev_info (&mgr->dev,"%s registered\n" , mgr->name);return 0 ;@@ -722,6 +725,8 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) {dev_info (&mgr->dev,"%s %s\n" , __func__, mgr->name); +fpga_mgr_debugfs_remove (mgr); +/* * If the low level driver provides a method for putting fpga into * a desired state upon unregister, do it. @@ -748,11 +753,14 @@ static int __init fpga_mgr_class_init(void) fpga_mgr_class->dev_groups = fpga_mgr_groups; fpga_mgr_class->dev_release = fpga_mgr_dev_release; +fpga_mgr_debugfs_init (); +return 0 ; } static void __exit fpga_mgr_class_exit(void) { +fpga_mgr_debugfs_uninit ();class_destroy (fpga_mgr_class);ida_destroy (&fpga_mgr_ida); }include/linux/fpga/fpga-mgr.h +3
-0@@ -170,6 +170,9 @@ struct fpga_manager {struct fpga_compat_id *compat_id;const struct fpga_manager_ops *mops;void *priv; +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) +void *debugfs; +#endif };
Quoted 8 lines
Quoted 4 lines
include/linux/fpga/fpga-mgr.h +0
-1#define to_fpga_manager (d) container_of(d, struct fpga_manager, dev) --2.7.4
Thanks,
Moritz
Thanks for the review!
Alan
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
Hi,
Quoted 9 lines · CEST Alan Tull
Quoted 1 line · Moritz Fischer
Hi Alan,
Hi Moritz,
Quoted 3 lines
comments inline. While I see how this is useful, I have the suspicion that from the moment this gets merged vendor kernels will just default to use this ...
Quoted 5 lines
Yeah, I have that suspicion as well. That's probably why I sat on this and didn't upstream it for 2 years. But on the other hand, I keep hearing of lots of cases of people implementing this independently anyway. At least if it is debugfs, it makes it clear that it's not intended for production use.
I'm one of those guys who implemented this independently.
@Mortiz
I do not see how this can be a bad thing (from what you wrote I guess you prefer another interface). Which interface to use depends on the use case. If you have this suspicion it's, I guess, because such interface it is extremely easy to use.
@Alan
DebugFS can be a first step, but I would go for a normal device in /dev at some point. I do not see why this should not be used in production
Below, again, my code that does the same thing. I already posted this in another thread [1] but probably this is the best place. As I said in the other thread, this is what I did quickly last year to make things working for us (where this kind of interface is a must). Unfortunately, I do not have time to review my own code and improve it.
[1] https://lkml.org/lkml/2018/8/16/107
-----
From d6a6df9515c5e92cc74b8948c3c10ac9cbeec6d2 Mon Sep 17 00:00:00 2001
From: Federico Vaga <[email protected]>
Date: Mon, 20 Nov 2017 14:40:26 +0100
Subject: [PATCH] fpga: program from user-space
--- Documentation/fpga/fpga-mgr.txt | 3 + drivers/fpga/fpga-mgr.c | 261 ++++++++++++++++++++++++++++++++++++ ++ +- include/linux/fpga/fpga-mgr.h | 19 +++ 3 files changed, 281 insertions(+), 2 deletions(-)
Documentation/fpga/fpga- renamed from Documentation/fpga/fpga-mgr.txt +0 -0
mgr.txt
index 78f197f..397dae9 100644
Documentation/fpga/fpga-mgr.txt +0 -0
@@ -197,3 +197,6 @@ to put the FPGA into operating mode. The ops include a .state function which will read the hardware FPGA
manager
and
return a code of type enum fpga_mgr_states. It doesn't result in a change
in
hardware state.
+
+Configuring the FPGA from user-space
+====================================
drivers/fpga/fpga-mgr.c +20 -0
@@ -27,10 +27,56 @@#include <linux/slab.h> #include <linux/scatterlist.h> #include <linux/highmem.h> +#include <linux/spinlock.h> #include <linux/version.h> +#include <linux/cdev.h> +#include <linux/list.h> static DEFINE_IDA (fpga_mgr_ida);static struct class *fpga_mgr_class; +static dev_t fpga_mgr_devt; + +/** +* struct fpga_image_chunck - FPGA configuration chunck +* @data: chunk data +* @size: chunk data size +* @list: for the linked list of chunks +*/ +struct fpga_image_chunk { +void *data; +size_t size; +struct list_head list; +}; +#define CHUNK_MAX_SIZE (PAGE_SIZE) + +/** +* struct fpga_user_load - structure to handle configuration from user-
space
+ * @mgr: pointer to the FPGA manager
+ * @chunk_list: HEAD point of a linked list of FPGA chunks
+ * @n_chunks: number of chunks in the list
+ * @lock: it protects: chunk_list, n_chunks
+ */
+struct fpga_user_load {
+ struct fpga_manager *mgr; + struct list_head chunk_list; + unsigned int n_chunks; + struct spinlock lock;
+};
+
+
+/**
+ * It sets by default a huge timeout for configuration coming from user- space
+ * just to play safe.
+ *
+ * FIXME what about sysfs parameters to adjust it? The flag bit in particular
+ */
+struct fpga_image_info default_user_info = {
+ .flags = 0, + .enable_timeout_us = 10000000, /* 10s */ + .disable_timeout_us = 10000000, /* 10s */ + .config_complete_timeout_us = 10000000, /* 10s */
+};
+
/*
* Call the low level driver's write_init function. This will do the
@@ -310,6 +356,158 @@ int fpga_mgr_firmware_load(struct fpga_manager *mgr,
}
EXPORT_SYMBOL_GPL(fpga_mgr_firmware_load);
+
+static int fpga_mgr_open(struct inode *inode, struct file *file) +{
+ struct fpga_manager *mgr = container_of(inode->i_cdev,
+ struct fpga_manager,
+ cdev);
+ struct fpga_user_load *usr;
+ int ret;
+
+ /* Allow the user-space programming only if user unlocked the FPGA
*/
+ if (!test_bit(FPGA_MGR_FLAG_UNLOCK, mgr->flags)) {
+ dev_info(&mgr->dev, "The FPGA programming is locked\n");
+ return -EPERM;
+ }
+
+ ret = mutex_trylock(&mgr->usr_mutex);
+ if (ret == 0)
+ return -EBUSY;
+
+ usr = kzalloc(sizeof(struct fpga_user_load), GFP_KERNEL);
+ if (!usr) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ usr->mgr = mgr; + spin_lock_init(&usr->lock); + INIT_LIST_HEAD(&usr->chunk_list); + file->private_data = usr;
+
+ return 0;
+
+err_alloc:
+ spin_lock(&mgr->lock); + clear_bit(FPGA_MGR_FLAG_UNLOCK, mgr->flags); + spin_unlock(&mgr->lock); + mutex_unlock(&mgr->usr_mutex); + return ret;
+}
+
+
+static int fpga_mgr_flush(struct file *file, fl_owner_t id) +{
+ struct fpga_user_load *usr = file->private_data; + struct fpga_image_chunk *chunk, *tmp; + struct sg_table sgt; + struct scatterlist *sg; + int err = 0;
+
+ if (!usr->n_chunks)
+ return 0;
+
+ err = sg_alloc_table(&sgt, usr->n_chunks, GFP_KERNEL);
+ if (err)
+ goto out;
+
+ sg = sgt.sgl;
+ list_for_each_entry(chunk, &usr->chunk_list, list) {
+ sg_set_buf(sg, chunk->data, chunk->size); + sg = sg_next(sg); + if (!sg)
+ break;
+ }
+
+ err = fpga_mgr_buf_load_sg(usr->mgr,
+ &default_user_info,
+ &sgt);
+ sg_free_table(&sgt);
+
+out:
+ /* Remove all chunks */
+ spin_lock(&usr->lock);
+ list_for_each_entry_safe(chunk, tmp, &usr->chunk_list, list) {
+ list_del(&chunk->list);
+ kfree(chunk->data);
+ kfree(chunk);
+ usr->n_chunks--;+ }
+ spin_unlock(&usr->lock);
+
+ return err;
+}
+
+
+static int fpga_mgr_close(struct inode *inode, struct file *file) +{
+ struct fpga_user_load *usr = file->private_data;
+ struct fpga_manager *mgr = usr->mgr;
+
+ kfree(usr);
+
+ spin_lock(&mgr->lock); + clear_bit(FPGA_MGR_FLAG_UNLOCK, mgr->flags); + spin_unlock(&mgr->lock);
+
+ mutex_unlock(&mgr->usr_mutex);
+
+ return 0;
+}
+
+
+static ssize_t fpga_mgr_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *offp)
+{
+ struct fpga_user_load *usr = file->private_data; + struct fpga_image_chunk *chunk; + int err;
+
+ chunk = kmalloc(sizeof(struct fpga_image_chunk), GFP_KERNEL);
+ if (!chunk)
+ return -ENOMEM;
+
+ chunk->size = count > CHUNK_MAX_SIZE ? CHUNK_MAX_SIZE : count;
+ chunk->data = kmalloc(chunk->size, GFP_KERNEL);
+ if (!chunk->data) {+ err = -ENOMEM;
+ goto err_buf_alloc;
+ }
+
+ err = copy_from_user(chunk->data, buf, chunk->size);
+ if(err)
+ goto err_buf_copy;
+
+ spin_lock(&usr->lock); + list_add_tail(&chunk->list, &usr->chunk_list); + usr->n_chunks++; + spin_unlock(&usr->lock);
+
+ *offp += count;
+
+ return chunk->size;
+
+err_buf_copy:
+ kfree(chunk->data);
+err_buf_alloc:
+ kfree(chunk);
+ return err;
+}
+
+
+/**
+ * Char device operation
+ */
+static const struct file_operations fpga_mgr_fops = {
+ .owner = THIS_MODULE, + .open = fpga_mgr_open, + .flush = fpga_mgr_flush, + .release = fpga_mgr_close, + .write = fpga_mgr_write,
+};
+
+
static const char * const state_str[] = {
[FPGA_MGR_STATE_UNKNOWN] = "unknown",
[FPGA_MGR_STATE_POWER_OFF] = "power off",
@@ -352,13 +550,43 @@ static ssize_t state_show(struct device *dev,
return sprintf(buf, "%s\n", state_str[mgr->state]);
}
+static ssize_t config_lock_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fpga_manager *mgr = to_fpga_manager(dev);
+
+ if (test_bit(FPGA_MGR_FLAG_UNLOCK, mgr->flags))
+ return sprintf(buf, "unlock\n");
+ return sprintf(buf, "lock\n");
+}
+
+static ssize_t config_lock_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fpga_manager *mgr = to_fpga_manager(dev);
+
+ spin_lock(&mgr->lock);
+ if (strncmp(buf, "lock" , min(4, (int)count)) == 0)
+ clear_bit(FPGA_MGR_FLAG_UNLOCK, mgr->flags);
+ else if (strncmp(buf, "unlock" , min(6, (int)count)) == 0)
+ set_bit(FPGA_MGR_FLAG_UNLOCK, mgr->flags);
+ else
+ count = -EINVAL;
+ spin_unlock(&mgr->lock);
+
+ return count;
+}
+
#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,11,0) static DEVICE_ATTR_RO(name);
static DEVICE_ATTR_RO(state);
+static DEVICE_ATTR_RW(config_lock);
static struct attribute *fpga_mgr_attrs[] = {
&dev_attr_name.attr, &dev_attr_state.attr,
+ &dev_attr_lock.attr,
NULL,
};
ATTRIBUTE_GROUPS(fpga_mgr);
@@ -367,6 +595,9 @@ ATTRIBUTE_GROUPS(fpga_mgr);
static struct device_attribute fpga_mgr_attrs[] = {
__ATTR(name, S_IRUGO, name_show, NULL), __ATTR(state, S_IRUGO, state_show, NULL),
+ __ATTR(config_lock, S_IRUGO | S_IWUSR | S_IRGRP | S_IWGRP,
+ config_lock_show, config_lock_store),
+ __ATTR_NULL,
};
#endif
@@ -507,6 +738,8 @@ int fpga_mgr_register(struct device *dev, const char *name,
}
mutex_init(&mgr->ref_mutex);+ mutex_init(&mgr->usr_mutex);
+ spin_lock_init(&mgr->lock);
mgr->name = name;
mgr->mops = mops;
@@ -524,12 +757,19 @@ int fpga_mgr_register(struct device *dev, const char
*name,
mgr->dev.parent = dev;
mgr->dev.of_node = dev->of_node;
mgr->dev.id = id;
+ mgr->dev.devt = MKDEV(MAJOR(fpga_mgr_devt), id);
dev_set_drvdata(dev, mgr);
ret = dev_set_name(&mgr->dev, "fpga%d", id);
if (ret)
goto error_device;+ cdev_init(&mgr->cdev, &fpga_mgr_fops);
+ mgr->cdev.owner = THIS_MODULE;
+ ret = cdev_add(&mgr->cdev, mgr->dev.devt, 1);
+ if (ret)
+ goto err_cdev;
+
ret = device_add(&mgr->dev);
if (ret)
goto error_device;
@@ -539,6 +779,8 @@ int fpga_mgr_register(struct device *dev, const char
*name,
return 0; error_device:
+ cdev_del(&mgr->cdev);
+err_cdev:
ida_simple_remove(&fpga_mgr_ida, id);
error_kfree:
kfree(mgr);
@@ -572,17 +814,27 @@ static void fpga_mgr_dev_release(struct device *dev)
{
struct fpga_manager *mgr = to_fpga_manager(dev);+ cdev_del(&mgr->cdev);
ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
kfree(mgr);}
static int __init fpga_mgr_class_init(void)
{
+ int err;
+
pr_info("FPGA manager framework\n");+ err = alloc_chrdev_region(&fpga_mgr_devt, 0, FPGA_MGR_MAX_DEV,
+ "fpga_mgr");
+ if (err)
+ return err;
+
fpga_mgr_class = class_create(THIS_MODULE, "fpga_manager");
- if (IS_ERR(fpga_mgr_class))
- return PTR_ERR(fpga_mgr_class);
+ if (IS_ERR(fpga_mgr_class)) {
+ err = PTR_ERR(fpga_mgr_class);
+ goto err_class;
+ }
#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,11,0)
fpga_mgr_class->dev_groups = fpga_mgr_groups;
#else
@@ -591,12 +843,17 @@ static int __init fpga_mgr_class_init(void)
fpga_mgr_class->dev_release = fpga_mgr_dev_release;
return 0;+
+err_class:
+ unregister_chrdev_region(fpga_mgr_devt, FPGA_MGR_MAX_DEV);
+ return err;
}
static void __exit fpga_mgr_class_exit(void)
{
class_destroy(fpga_mgr_class);
ida_destroy(&fpga_mgr_ida);+ unregister_chrdev_region(fpga_mgr_devt, FPGA_MGR_MAX_DEV);
}
MODULE_AUTHOR("Alan Tull <[email protected]>");
include/linux/fpga/fpga-mgr.h +0 -0
@@ -15,8 +15,10 @@* 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/cdev.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/spinlock.h>
#ifndef _LINUX_FPGA_MGR_H
#define _LINUX_FPGA_MGR_H
@@ -24,6 +26,8 @@
struct fpga_manager;
struct sg_table;
+#define FPGA_MGR_MAX_DEV (256)
+
/**
* enum fpga_mgr_states - fpga framework states
* @FPGA_MGR_STATE_UNKNOWN: can't determine state
@@ -118,22 +122,37 @@ struct fpga_manager_ops {
void (*fpga_remove)(struct fpga_manager *mgr);};
+/*
+ * List of status FLAGS for the FPGA manager
+ */
+#define FPGA_MGR_FLAG_BITS (32)
+#define FPGA_MGR_FLAG_UNLOCK BIT(0)
+
/**
* struct fpga_manager - fpga manager structure
* @name: name of low level fpga manager
+ * @cdev: char device interface
* @dev: fpga manager device
* @ref_mutex: only allows one reference to fpga manager
* @state: state of fpga manager
* @mops: pointer to struct of fpga manager ops
* @priv: low level driver private date
+ * @flags: manager status bits
+ * @lock: it protects: flags
+ * @usr_mutex: only allows one user to program the FPGA */
struct fpga_manager {
const char *name;
+ struct cdev cdev;
struct device dev;
struct mutex ref_mutex;
enum fpga_mgr_states state;
const struct fpga_manager_ops *mops;
void *priv;
+
+ DECLARE_BITMAP(flags, FPGA_MGR_FLAG_BITS);
+ struct spinlock lock;
+ struct mutex usr_mutex;};
#define to_fpga_manager(d) container_of(d, struct fpga_manager, dev)
Signature · 6 lines
2.15.0 -- Federico Vaga [BE-CO-HT]
Moritz Fischer
maintainerRe: [PATCH 2/2] fpga: add FPGA manager debugfs
Hi Federico,
Quoted 12 lines · Federico Vaga
Hi,
Quoted 9 lines · CEST Alan Tull
Quoted 1 line · Moritz Fischer
Hi Alan,
Hi Moritz,
Quoted 3 lines
comments inline. While I see how this is useful, I have the suspicion that from the moment this gets merged vendor kernels will just default to use this ...
Quoted 7 lines
Quoted 5 lines
Yeah, I have that suspicion as well. That's probably why I sat on this and didn't upstream it for 2 years. But on the other hand, I keep hearing of lots of cases of people implementing this independently anyway. At least if it is debugfs, it makes it clear that it's not intended for production use.
I'm one of those guys who implemented this independently.
We all have in one way or another ;) Most people on ARM run an out of tree patch using devicetree overlays these days I hope rather than /dev/mem and UIO ... or other vender solutions...
Quoted 6 lines
@Mortiz
I do not see how this can be a bad thing (from what you wrote I guess you prefer another interface). Which interface to use depends on the use case. If you have this suspicion it's, I guess, because such interface it is extremely easy to use.
What happens to a kernel driver doing MMIO with devices while you reload the entire FPGA from userland?
Quoted 4 lines
@Alan
DebugFS can be a first step, but I would go for a normal device in /dev at some point. I do not see why this should not be used in production
I'm not against having a userland interface to reprogram the FPGA, the Intel DFL code is a good example of a sensible one, doing so in a safe manner.
Ideally we'll get around to have a more generic interface, as we get time to work on it.
- Moritz
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
Hi Mortiz,
I'm not 100% into the problem to understand all cases. I'm putting on the table the point of view, mainly, of an user. If you say there are problems here or there I believe you. At the beginning, you did not say that this interface may introduce problems (and I'm interested in those problems since I implemented one and we are using it), but that you fear that it becomes the default (usually, being a default is a good thing).
Since you and Alan are working on this for a long time, you can read each other mind, but I need a more verbose email to understand ^_^'
Of course the interface must be safe, I totally agree. In order to make me understand what are the issues, can you list some of them? And expand your comment about MMIO.
(I just did a dummy backport, and implemented that interface. So, I repeat
myself: I do not have enough experience with this framework to understand all consequences, but I'm interested to know what are the risks behind this interface)
Quoted 48 lines · CEST Moritz Fischer
Hi Federico,
Quoted 12 lines · Federico Vaga
Hi,
Quoted 9 lines · CEST Alan Tull
Quoted 1 line · Moritz Fischer
Hi Alan,
Hi Moritz,
Quoted 3 lines
comments inline. While I see how this is useful, I have the suspicion that from the moment this gets merged vendor kernels will just default to use this ...
Quoted 7 lines
Quoted 5 lines
Yeah, I have that suspicion as well. That's probably why I sat on this and didn't upstream it for 2 years. But on the other hand, I keep hearing of lots of cases of people implementing this independently anyway. At least if it is debugfs, it makes it clear that it's not intended for production use.
I'm one of those guys who implemented this independently.
We all have in one way or another ;) Most people on ARM run an out of tree patch using devicetree overlays these days I hope rather than
/dev/memand UIO ... or other vender solutions...Quoted 5 lines
@Mortiz
I do not see how this can be a bad thing (from what you wrote I guess you prefer another interface). Which interface to use depends on the use case. If you have this suspicion it's, I guess, because such interface it is extremely easy to use.What happens to a kernel driver doing MMIO with devices while you reload the entire FPGA from userland?
Quoted 3 lines
@Alan
DebugFS can be a first step, but I would go for a normal device in /dev at some point. I do not see why this should not be used in productionI'm not against having a userland interface to reprogram the FPGA, the Intel DFL code is a good example of a sensible one, doing so in a safe manner.
Ideally we'll get around to have a more generic interface, as we get time to work on it.
- Moritz
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
Quoted 11 lines · Federico Vaga
Hi Mortiz,
I'm not 100% into the problem to understand all cases. I'm putting on the table the point of view, mainly, of an user. If you say there are problems here or there I believe you. At the beginning, you did not say that this interface may introduce problems (and I'm interested in those problems since I
implemented one and we are using it), but that you fear that it becomes the
default (usually, being a default is a good thing).
Quoted 5 lines
Since you and Alan are working on this for a long time, you can read each other mind, but I need a more verbose email to understand ^_^'
Of course the interface must be safe, I totally agree. In order to make me understand what are the issues, can you list some of them?
Before we repeat what the doc l posted says, could you look at it and comment on what I'm not saying there?
https://lkml.org/lkml/2018/8/15/525
Alan
And expand your
Quoted 67 lines
comment about MMIO.
(I just did a dummy backport, and implemented that interface. So, I repeat
myself: I do not have enough experience with this framework to understand all
consequences, but I'm interested to know what are the risks behind this interface)Quoted 7 lines · CEST Moritz Fischer
Hi Federico,
Quoted 4 lines · Federico Vaga
Hi,
Quoted 1 line · CEST Alan Tull
On Thu, Aug 16, 2018 at 1:59 PM, Moritz Fischer <[email protected]>
Quoted 14 lines · wrote:
Quoted 14 lines
Quoted 13 lines
Quoted 1 line
Hi Alan,
Hi Moritz,
Quoted 3 lines
comments inline. While I see how this is useful, I have the suspicion that from the moment this gets merged vendor kernels will just default to use this ...
Yeah, I have that suspicion as well. That's probably why I sat on this and didn't upstream it for 2 years. But on the other hand, I keep hearing of lots of cases of people implementing this independently anyway. At least if it is debugfs, it makes it clear that it's not intended for production use.
Quoted 3 lines
Quoted 1 line
I'm one of those guys who implemented this independently.
We all have in one way or another ;) Most people on ARM run an out of
tree
Quoted 5 lines
patch using devicetree overlays these days I hope rather than
/dev/memand UIO ... or other vender solutions...Quoted 2 lines
@Mortiz
I do not see how this can be a bad thing (from what you wrote I guessyou
Quoted 1 line
Quoted 1 line
prefer another interface). Which interface to use depends on the use
case.
Quoted 8 lines
Quoted 2 lines
If you have this suspicion it's, I guess, because such interface it is extremely easy to use.
What happens to a kernel driver doing MMIO with devices while you reload the entire FPGA from userland?
Quoted 2 lines
@Alan
DebugFS can be a first step, but I would go for a normal device in/dev at
Quoted 10 lines
Quoted 1 line
some point. I do not see why this should not be used in production
I'm not against having a userland interface to reprogram the FPGA, the Intel DFL code is a good example of a sensible one, doing so in a safe manner.
Ideally we'll get around to have a more generic interface, as we get time to work on it.
- Moritz
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
Quoted 15 lines · CEST Alan Tull
Quoted 14 lines · Federico Vaga
Hi Mortiz,
I'm not 100% into the problem to understand all cases. I'm putting on the table the point of view, mainly, of an user. If you say there are problems here or there I believe you. At the beginning, you did not say that this interface may introduce problems (and I'm interested in those problems since I
implemented one and we are using it), but that you fear that it becomes the
default (usually, being a default is a good thing).Since you and Alan are working on this for a long time, you can read each other mind, but I need a more verbose email to understand ^_^'
Quoted 7 lines
Quoted 2 lines
Of course the interface must be safe, I totally agree. In order to make me understand what are the issues, can you list some of them?
Before we repeat what the doc l posted says, could you look at it and comment on what I'm not saying there?
I read it, but do you mean that the problems you foresee are only the ones listed in there? If this is so, comments:
loading devices
It is true that it is a problem, and probably it is clear to everyone who will try to use such interface: "and now how do I load my devices?". But this is only a possible case, the FPGA may run without device driver and in this case it is perfectly fine for production.
If the answer to the above question is: "ok, let me see where my devices are in the memory ..." well if the machine crashes, it's their problem. This problem exists even without the FPGA manager.
bridge
My understand is that it is optional. Developers are allowed to not implement the bridge's operations. Which probably means that it does not exist or it is not needed.
When an user uses this interface from userspace it shouldn't be hard to detect if the operation is risky or not (bridge enabled/disabled). And if it is risky, the operation fails with EPERM, EBUSY.
I have to say that I'm not familiar with the bridge design, perhaps I'm missing something.
Conclusions
Yes, those are problems but I think they do not justify the label "not for production": in some cases I think is fine.
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
In reply to Alan TullHi Alan, Federico,
Quoted 13 lines · Alan Tull
Quoted 12 lines · Federico Vaga
Hi Mortiz,
I'm not 100% into the problem to understand all cases. I'm putting on the table the point of view, mainly, of an user. If you say there are problems here or there I believe you. At the beginning, you did not say that this interface may introduce problems (and I'm interested in those problems since I
implemented one and we are using it), but that you fear that it becomes the
default (usually, being a default is a good thing).
Quoted 5 lines
Quoted 5 lines
Since you and Alan are working on this for a long time, you can read each other mind, but I need a more verbose email to understand ^_^'
Of course the interface must be safe, I totally agree. In order to make me understand what are the issues, can you list some of them?
Say you have kernel drivers (a network driver in the FPGA, or an I2C controller) for example bound to hardware on a MMIO bus in the the FPGA.
You reprogram the FPGA using the debugfs interface, and the drivers don't get unloaded correctly, the driver will try to access the registers and depending on your system / bus either give you bad values or lock up.
Now userland locked up your system. Bad.
I'm not saying it isn't possible to do this if you're careful, of course you could
first unload the drivers using rrmod and it would work just fine.
I just feel an interface like this might make it easier to create the wrong design.
I've seen plenty of Application notes from vendors where they literally did "cat foo.bin > /dev/fpga" followed by mmap(/dev/mem...).
Quoted 6 lines
Before we repeat what the doc l posted says, could you look at it and comment on what I'm not saying there?
Alan, maybe I didn't express myself well. I'm fine with the debugfs interface as a debug interface, just not for general usage ;-) I think your document is clear on that.
Thanks,
Moritz
Re: [PATCH 2/2] fpga: add FPGA manager debugfs
Hi,
Quoted 4 lines · CEST Moritz Fischer
Hi Alan, Federico,
Quoted 1 line · Alan Tull
On Fri, Aug 17, 2018, 2:00 AM Federico Vaga <[email protected]>
Quoted 18 lines · wrote:
Quoted 12 lines
Quoted 12 lines
Hi Mortiz,
I'm not 100% into the problem to understand all cases. I'm putting on the table the point of view, mainly, of an user. If you say there are problems here or there I believe you. At the beginning, you did not say that this interface may introduce problems (and I'm interested in those problems since I
implemented one and we are using it), but that you fear that it becomes
the
default (usually, being a default is a good thing).Quoted 5 lines
Quoted 5 lines
Since you and Alan are working on this for a long time, you can read each other mind, but I need a more verbose email to understand ^_^'
Of course the interface must be safe, I totally agree. In order to make me understand what are the issues, can you list some of them?
Quoted 6 lines
Say you have kernel drivers (a network driver in the FPGA, or an I2C controller) for example bound to hardware on a MMIO bus in the the FPGA. You reprogram the FPGA using the debugfs interface, and the drivers don't get unloaded correctly, the driver will try to access the registers and depending on your system / bus either give you bad values or lock up. Now userland locked up your system. Bad.
I think I got confused by your reference to the MMIO, but now it sound like it was just a very specific example of a more general problem. Because this is true for any device driver for FPGA soft-IP/IP-core, it is not strictly an MMIO problem. Am I missing something?
I get the problem, I have to fight with **this** problem daily because I'm loading images with:
cat hello.bin > /dev/fpga0
And then, somehow I have to load the device drivers (memory, IRQ, ...). But I will not say publicly what I do (it is a "don't try this at home" thing).
Quoted 3 lines
I'm not saying it isn't possible to do this if you're careful, of course you could
first unload the drivers using rrmod and it would work just fine.
Or having some reference counter on the last loaded FPGA image may work. This way it will be possible to detect if there are users of the current FPGA and inhibit any unwanted FPGA load (like the module counter forbid rmmod when the device is in use). If a device driver is using some FPGA component the reference counter increase. How to do it? Need more studies, but probably this is a safe way that perhaps worth to look at.
Quoted 4 lines
I just feel an interface like this might make it easier to create the wrong design.
I've seen plenty of Application notes from vendors where they literally did "cat foo.bin >/dev/fpga" followed by mmap(/dev/mem...).
Actually, I'm doing worst than this (to compensate the lack of infrastructure). You tried, but you are not scaring me :P
Quoted 12 lines
Quoted 4 lines
Before we repeat what the doc l posted says, could you look at it and comment on what I'm not saying there?
Alan, maybe I didn't express myself well. I'm fine with the debugfs interface as a debug interface, just not for general usage ;-) I think your document is clear on that.
Thanks,
Moritz
Signature · 2 lines
Federico Vaga [BE-CO-HT]
RE: [PATCH 2/2] fpga: add FPGA manager debugfs
In reply to Alan TullFYI...
Regards,
Kedar.
Quoted 393 lines
-----Original Message-----
From: Alan Tull <[email protected]>
Sent: Thursday, August 16, 2018 3:40 AM
To: Moritz Fischer <[email protected]>; Jonathan Corbet <[email protected]>; Randy Dunlap <[email protected]>; Dinh Nguyen <[email protected]>
Cc: Appana Durga Kedareswara Rao <[email protected]>; linux- [email protected]; [email protected]; linux- [email protected]; Alan Tull <[email protected]>; Alan Tull <[email protected]>; Matthew Gerlach <[email protected]>
Subject: [PATCH 2/2] fpga: add FPGA manager debugfsFrom: Alan Tull <[email protected]>
Implement DebugFS for the FPGA Manager Framework for debugging and developmental use.
Enabled by
CONFIG_FPGA_MGR_DEBUG_FSEach FPGA gets its own directory such as
<debugfs>/fpga_manager/fpga0 and three files:* [RW] flags = flags as defined in
fpga-mgr.h
* [RW]firmware_name= write/read back name of FPGA image firmware file to program
* [WO] image = write-only file for directly writing
fpga image w/o firmware layer
* [RW]config_complete_timeout_us= maximum for the FPGA to go to the operating state after programmingThe debugfs is implemented in a separate
fpga_mgr_debugfs.cfile, but the FPGA manager core is still built as one module. Note the name change from fpga-mgr.ko to fpga_mgr.ko.Signed-off-by Alan Tull <[email protected]>Signed-off-by Matthew Gerlach <[email protected]>--- drivers/fpga/Kconfig | 7 ++ drivers/fpga/Makefile | 4 +- drivers/fpga/fpga-mgr-debugfs.c | 221 ++++++++++++++++++++++++++++++++++++++++ drivers/fpga/fpga-mgr-debugfs.h | 22 ++++ drivers/fpga/fpga-mgr.c | 8 ++ include/linux/fpga/fpga-mgr.h | 3 + 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644
drivers/fpga/fpga-mgr-debugfs.ccreate mode 100644drivers/fpga/fpga-mgr-debugfs.hdrivers/fpga/Kconfig index 1ebcef4..600924d renamed from drivers/fpga/Kconfig +0
-0100644
drivers/fpga/Kconfig +7
-0@@ -9,6 +9,13 @@ menuconfig FPGA kernel. The FPGA framework adds a FPGA manager class and FPGA manager drivers. +config FPGA_MGR_DEBUG_FS + bool "FPGA Manager DebugFS" + depends on FPGA && DEBUG_FS + help + Say Y here if you want to expose a DebugFS interface for the + FPGA Manager Framework. + if FPGA config FPGA_MGR_SOCFPGAdrivers/fpga/Makefile index renamed from drivers/fpga/Makefile +0
-07a2d73b..62910cc 100644
drivers/fpga/Makefile +3
-1@@ -4,7 +4,9 @@ # # Core FPGA Manager Framework -obj-$(CONFIG_FPGA) += fpga-mgr.o +obj-$(CONFIG_FPGA) += fpga_mgr.o +fpga_mgr-y := fpga-mgr.o +fpga_mgr-$(CONFIG_FPGA_MGR_DEBUG_FS) += fpga-mgr-debugfs.o # FPGA Manager Drivers obj-$(CONFIG_FPGA_MGR_ALTERA_CVP) += altera-cvp.odrivers/fpga/fpga-mgr-debugfs.c new file +81
-0@@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* FPGA Manager DebugFS +* +* Copyright (C) 2016-2018 Intel Corporation. All rights reserved. +*/ +#include <linux/debugfs.h> +#include <linux/fpga/fpga-mgr.h> +#include <linux/slab.h> +#include <linux/uaccess.h> + +static struct dentry *fpga_mgr_debugfs_root; + +struct fpga_mgr_debugfs { +struct dentry *debugfs_dir; +struct fpga_image_info *info; +}; + +static ssize_t fpga_mgr_firmware_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =devm_kzalloc (dev, count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +devm_kfree (dev, buf); +return -EFAULT ; + } + + buf[count] =0 ; +if (buf[count -1 ] =='\n' ) + buf[count -1 ] =0 ; + +/* Release previous firmware name (if any). Save current one. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name = buf; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret) +dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + +return count; +} + +static ssize_t fpga_mgr_firmware_read_file (struct file *file, +char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +char *buf; +int ret; + +if (!debugfs->info->firmware_name) +return 0 ; + + buf =kzalloc (PAGE_SIZE ,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + + ret = snprintf(buf, PAGE_SIZE, "%s\n", debugfs->info-Quoted 1 line
firmware_name);drivers/fpga/fpga-mgr-debugfs.c new file +99
-0+if (ret <0 ) { +kfree (buf); +return ret; + } + + ret =simple_read_from_buffer (user_buf, count, ppos, buf, ret); +kfree (buf); + +return ret; +} + +static const struct file_operations fpga_mgr_firmware_fops = { + .open = simple_open, + .read = fpga_mgr_firmware_read_file, + .write = fpga_mgr_firmware_write_file, + .llseek = default_llseek, +}; + +static ssize_t fpga_mgr_image_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + +dev_info (dev,"writing %zu bytes to %s\n" , count, mgr->name); + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =kzalloc (count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +kfree (buf); +return -EFAULT ; + } + +/* If firmware interface was previously used, forget it. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name =NULL ; + + debugfs->info->buf = buf; + debugfs->info->count = count; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret) +dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + + debugfs->info->buf =NULL ; + debugfs->info->count =0 ; + +kfree (buf); + +return count; +} + +static const struct file_operations fpga_mgr_image_fops = { + .open = simple_open, + .write = fpga_mgr_image_write_file, + .llseek = default_llseek, +}; + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) { +struct device *dev = &mgr->dev; +struct fpga_mgr_debugfs *debugfs; +struct fpga_image_info *info; + +if (!fpga_mgr_debugfs_root) +return ; + + debugfs =kzalloc (sizeof (*debugfs),GFP_KERNEL ); +if (!debugfs) +return ; + + info =fpga_image_info_alloc (dev); +if (!info) { +kfree (debugfs); +return ; + } + debugfs->info = info; + + debugfs->debugfs_dir =debugfs_create_dir (dev_name (dev), + fpga_mgr_debugfs_root); + + debugfs_create_file("firmware_name", 0600, debugfs->debugfs_dir,mgr,
drivers/fpga/fpga-mgr-debugfs.c new file +6
-0+ &fpga_mgr_firmware_fops); + +debugfs_create_file ("image" ,0200 , debugfs->debugfs_dir, mgr, + &fpga_mgr_image_fops); + + debugfs_create_u32("flags", 0600, debugfs->debugfs_dir, &info-Quoted 1 line
flags);
drivers/fpga/fpga-mgr-debugfs.c new file +32
-0+ +debugfs_create_u32 ("config_complete_timeout_us" ,0600 , + debugfs->debugfs_dir, + &info->config_complete_timeout_us); + + mgr->debugfs = debugfs; +} + +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr) { +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; + +if (!fpga_mgr_debugfs_root) +return ; + +debugfs_remove_recursive (debugfs->debugfs_dir); + +/* this function also frees debugfs->info->firmware_name */ +fpga_image_info_free (debugfs->info); + +kfree (debugfs); +} + +void fpga_mgr_debugfs_init (void ) +{ + fpga_mgr_debugfs_root =debugfs_create_dir ("fpga_manager" ,NULL ); +if (!fpga_mgr_debugfs_root) +pr_warn ("fpga_mgr: Failed to create debugfs root\n" ); } + +void fpga_mgr_debugfs_uninit (void ) +{ +debugfs_remove_recursive (fpga_mgr_debugfs_root); +}drivers/fpga/fpga-mgr-debugfs.h new file +20
-0@@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 + +#ifndef _LINUX_FPGA_MGR_DEBUGFS_H +#define _LINUX_FPGA_MGR_DEBUGFS_H + +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr);void +fpga_mgr_debugfs_remove (struct fpga_manager *mgr);void +fpga_mgr_debugfs_init (void );void fpga_mgr_debugfs_uninit (void ); + +#else + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) {}void +fpga_mgr_debugfs_remove (struct fpga_manager *mgr) {}void +fpga_mgr_debugfs_init (void ) {}void fpga_mgr_debugfs_uninit (void ) {} + +#endif /* CONFIG_FPGA_MGR_DEBUG_FS */ + +#endif /*_LINUX_FPGA_MGR_DEBUGFS_H */ drivers/fpga/fpga-mgr.c index renamed from drivers/fpga/fpga-mgr.c +0
-0c8684bc..66eb6f6 100644
drivers/fpga/fpga-mgr.c +8
-0@@ -17,6 +17,7 @@#include <linux/slab.h> #include <linux/scatterlist.h> #include <linux/highmem.h> +#include "fpga-mgr-debugfs.h" static DEFINE_IDA (fpga_mgr_ida);static struct class *fpga_mgr_class;@@ -698,6 +699,8 @@ int fpga_mgr_register(struct fpga_manager *mgr)if (ret) goto error_device; +fpga_mgr_debugfs_add (mgr); +dev_info (&mgr->dev,"%s registered\n" , mgr->name);return 0 ;@@ -722,6 +725,8 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) {dev_info (&mgr->dev,"%s %s\n" , __func__, mgr->name); +fpga_mgr_debugfs_remove (mgr); +/* * If the low level driver provides a method for putting fpga into * a desired state upon unregister, do it. @@ -748,11 +753,14 @@ static int __init fpga_mgr_class_init(void) fpga_mgr_class->dev_groups = fpga_mgr_groups; fpga_mgr_class->dev_release = fpga_mgr_dev_release; +fpga_mgr_debugfs_init (); +return 0 ; } static void __exit fpga_mgr_class_exit(void) { +fpga_mgr_debugfs_uninit ();class_destroy (fpga_mgr_class);ida_destroy (&fpga_mgr_ida); }include/linux/fpga/fpga-mgr.h +3
-1@@ -170,6 +170,9 @@ struct fpga_manager {struct fpga_compat_id *compat_id;const struct fpga_manager_ops *mops;void *priv; +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) +void *debugfs; +#endif };#define to_fpga_manager (d) container_of(d, struct fpga_manager, dev) --2.7.4
RE: [PATCH 2/2] fpga: add FPGA manager debugfs
Please ignore this mail....
Regards,
Kedar.
Quoted 413 lines
-----Original Message-----
From: Appana Durga Kedareswara Rao
Sent: Tuesday, March 19, 2019 3:58 PM
To: 'Alan Tull' <[email protected]>; Moritz Fischer <[email protected]>; Jonathan Corbet <[email protected]>; Randy Dunlap <[email protected]>; Dinh Nguyen <[email protected]>
Cc: [email protected]; [email protected]; linux- [email protected]; Alan Tull <[email protected]>; Matthew Gerlach <[email protected]>
Subject: RE: [PATCH 2/2] fpga: add FPGA manager debugfsFYI...
Regards,
Kedar.drivers/fpga/fpga-mgr.c void fpga_mgr_unregister(struct fpga_manager *mgr)
Quoted 359 lines
-----Original Message-----
From: Alan Tull <[email protected]>
Sent: Thursday, August 16, 2018 3:40 AM
To: Moritz Fischer <[email protected]>; Jonathan Corbet <[email protected]>; Randy Dunlap <[email protected]>; Dinh Nguyen <[email protected]>
Cc: Appana Durga Kedareswara Rao <[email protected]>; linux- [email protected]; [email protected]; linux- [email protected]; Alan Tull <[email protected]>; Alan Tull <[email protected]>; Matthew Gerlach <[email protected]>
Subject: [PATCH 2/2] fpga: add FPGA manager debugfsFrom: Alan Tull <[email protected]>
Implement DebugFS for the FPGA Manager Framework for debugging and developmental use.
Enabled by
CONFIG_FPGA_MGR_DEBUG_FSEach FPGA gets its own directory such as
<debugfs>/fpga_manager/fpga0 and three files:* [RW] flags = flags as defined in
fpga-mgr.h
* [RW]firmware_name= write/read back name of FPGA image firmware file to program
* [WO] image = write-only file for directly writing
fpga image w/o firmware layer
* [RW]config_complete_timeout_us= maximum for the FPGA to go to the operating state after programmingThe debugfs is implemented in a separate
fpga_mgr_debugfs.cfile, but the FPGA manager core is still built as one module. Note the name change from fpga-mgr.ko to fpga_mgr.ko.Signed-off-by Alan Tull <[email protected]>Signed-off-by Matthew Gerlach <[email protected]>--- drivers/fpga/Kconfig | 7 ++ drivers/fpga/Makefile | 4 +- drivers/fpga/fpga-mgr-debugfs.c | 221 ++++++++++++++++++++++++++++++++++++++++ drivers/fpga/fpga-mgr-debugfs.h | 22 ++++ drivers/fpga/fpga-mgr.c | 8 ++ include/linux/fpga/fpga-mgr.h | 3 + 6 files changed, 264 insertions(+), 1 deletion(-) create mode 100644
drivers/fpga/fpga-mgr-debugfs.ccreate mode 100644drivers/fpga/fpga-mgr-debugfs.hdrivers/fpga/Kconfig index renamed from drivers/fpga/Kconfig +0
-01ebcef4..600924d
100644drivers/fpga/Kconfig +7
-0@@ -9,6 +9,13 @@ menuconfig FPGA kernel. The FPGA framework adds a FPGA manager class and FPGA manager drivers. +config FPGA_MGR_DEBUG_FS + bool "FPGA Manager DebugFS" + depends on FPGA && DEBUG_FS + help + Say Y here if you want to expose a DebugFS interface for the + FPGA Manager Framework. + if FPGA config FPGA_MGR_SOCFPGAdrivers/fpga/Makefile index renamed from drivers/fpga/Makefile +0
-07a2d73b..62910cc 100644
drivers/fpga/Makefile +3
-1@@ -4,7 +4,9 @@ # # Core FPGA Manager Framework -obj-$(CONFIG_FPGA) += fpga-mgr.o +obj-$(CONFIG_FPGA) += fpga_mgr.o +fpga_mgr-y := fpga-mgr.o +fpga_mgr-$(CONFIG_FPGA_MGR_DEBUG_FS) += fpga-mgr-debugfs.o # FPGA Manager Drivers obj-$(CONFIG_FPGA_MGR_ALTERA_CVP) += altera-cvp.o(quoted diff) +0
-0
b/drivers/fpga/fpga-mgr-debugfs.cnew file mode 100644 index 0000000..f2fdf58drivers/fpga/fpga-mgr-debugfs.c new file +81
-0@@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* FPGA Manager DebugFS +* +* Copyright (C) 2016-2018 Intel Corporation. All rights reserved. +*/ +#include <linux/debugfs.h> +#include <linux/fpga/fpga-mgr.h> +#include <linux/slab.h> +#include <linux/uaccess.h> + +static struct dentry *fpga_mgr_debugfs_root; + +struct fpga_mgr_debugfs { +struct dentry *debugfs_dir; +struct fpga_image_info *info; +}; + +static ssize_t fpga_mgr_firmware_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =devm_kzalloc (dev, count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +devm_kfree (dev, buf); +return -EFAULT ; + } + + buf[count] =0 ; +if (buf[count -1 ] =='\n' ) + buf[count -1 ] =0 ; + +/* Release previous firmware name (if any). Save current one. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name = buf; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret) +dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + +return count; +} + +static ssize_t fpga_mgr_firmware_read_file (struct file *file, +char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +char *buf; +int ret; + +if (!debugfs->info->firmware_name) +return 0 ; + + buf =kzalloc (PAGE_SIZE ,GFP_KERNEL ); +if (!buf) +return -ENOMEM ; + + ret = snprintf(buf, PAGE_SIZE, "%s\n", debugfs->info-Quoted 1 line
firmware_name);drivers/fpga/fpga-mgr-debugfs.c new file +99
-0+if (ret <0 ) { +kfree (buf); +return ret; + } + + ret =simple_read_from_buffer (user_buf, count, ppos, buf, ret); +kfree (buf); + +return ret; +} + +static const struct file_operations fpga_mgr_firmware_fops = { + .open = simple_open, + .read = fpga_mgr_firmware_read_file, + .write = fpga_mgr_firmware_write_file, + .llseek = default_llseek, +}; + +static ssize_t fpga_mgr_image_write_file (struct file *file, +const char __user *user_buf, +size_t count,loff_t *ppos) +{ +struct fpga_manager *mgr = file->private_data; +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; +struct device *dev = &mgr->dev; +char *buf; +int ret; + +dev_info (dev,"writing %zu bytes to %s\n" , count, mgr->name); + + ret =fpga_mgr_lock (mgr); +if (ret) { +dev_err (dev,"FPGA manager is busy\n" ); +return -EBUSY ; + } + + buf =kzalloc (count,GFP_KERNEL ); +if (!buf) { +fpga_mgr_unlock (mgr); +return -ENOMEM ; + } + +if (copy_from_user (buf, user_buf, count)) { +fpga_mgr_unlock (mgr); +kfree (buf); +return -EFAULT ; + } + +/* If firmware interface was previously used, forget it. */ +if (debugfs->info->firmware_name) +devm_kfree (dev, debugfs->info->firmware_name); + debugfs->info->firmware_name =NULL ; + + debugfs->info->buf = buf; + debugfs->info->count = count; + + ret =fpga_mgr_load (mgr, debugfs->info); +if (ret) +dev_err (dev,"fpga_mgr_load returned with value %d\n" , ret); + +fpga_mgr_unlock (mgr); + + debugfs->info->buf =NULL ; + debugfs->info->count =0 ; + +kfree (buf); + +return count; +} + +static const struct file_operations fpga_mgr_image_fops = { + .open = simple_open, + .write = fpga_mgr_image_write_file, + .llseek = default_llseek, +}; + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) { +struct device *dev = &mgr->dev; +struct fpga_mgr_debugfs *debugfs; +struct fpga_image_info *info; + +if (!fpga_mgr_debugfs_root) +return ; + + debugfs =kzalloc (sizeof (*debugfs),GFP_KERNEL ); +if (!debugfs) +return ; + + info =fpga_image_info_alloc (dev); +if (!info) { +kfree (debugfs); +return ; + } + debugfs->info = info; + + debugfs->debugfs_dir =debugfs_create_dir (dev_name (dev), + fpga_mgr_debugfs_root); + + debugfs_create_file("firmware_name", 0600, debugfs->debugfs_dir,mgr,
drivers/fpga/fpga-mgr-debugfs.c new file +6
-0+ &fpga_mgr_firmware_fops); + +debugfs_create_file ("image" ,0200 , debugfs->debugfs_dir, mgr, + &fpga_mgr_image_fops); + + debugfs_create_u32("flags", 0600, debugfs->debugfs_dir, &info-Quoted 1 line
flags);
drivers/fpga/fpga-mgr-debugfs.c new file +32
-0+ +debugfs_create_u32 ("config_complete_timeout_us" ,0600 , + debugfs->debugfs_dir, + &info->config_complete_timeout_us); + + mgr->debugfs = debugfs; +} + +void fpga_mgr_debugfs_remove (struct fpga_manager *mgr) { +struct fpga_mgr_debugfs *debugfs = mgr->debugfs; + +if (!fpga_mgr_debugfs_root) +return ; + +debugfs_remove_recursive (debugfs->debugfs_dir); + +/* this function also frees debugfs->info->firmware_name */ +fpga_image_info_free (debugfs->info); + +kfree (debugfs); +} + +void fpga_mgr_debugfs_init (void ) +{ + fpga_mgr_debugfs_root =debugfs_create_dir ("fpga_manager" ,NULL ); +if (!fpga_mgr_debugfs_root) +pr_warn ("fpga_mgr: Failed to create debugfs root\n" ); } + +void fpga_mgr_debugfs_uninit (void ) +{ +debugfs_remove_recursive (fpga_mgr_debugfs_root); +}(quoted diff) +0
-0
b/drivers/fpga/fpga-mgr-debugfs.hnew file mode 100644 index 0000000..17cd3ebdrivers/fpga/fpga-mgr-debugfs.h new file +20
-0@@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 + +#ifndef _LINUX_FPGA_MGR_DEBUGFS_H +#define _LINUX_FPGA_MGR_DEBUGFS_H + +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr);void +fpga_mgr_debugfs_remove (struct fpga_manager *mgr);void +fpga_mgr_debugfs_init (void );void fpga_mgr_debugfs_uninit (void ); + +#else + +void fpga_mgr_debugfs_add (struct fpga_manager *mgr) {}void +fpga_mgr_debugfs_remove (struct fpga_manager *mgr) {}void +fpga_mgr_debugfs_init (void ) {}void fpga_mgr_debugfs_uninit (void ) {} + +#endif /* CONFIG_FPGA_MGR_DEBUG_FS */ + +#endif /*_LINUX_FPGA_MGR_DEBUGFS_H */ drivers/fpga/fpga-mgr.c index renamed from drivers/fpga/fpga-mgr.c +0
-0c8684bc..66eb6f6 100644
drivers/fpga/fpga-mgr.c +3
-0@@ -17,6 +17,7 @@#include <linux/slab.h> #include <linux/scatterlist.h> #include <linux/highmem.h> +#include "fpga-mgr-debugfs.h" static DEFINE_IDA (fpga_mgr_ida);static struct class *fpga_mgr_class;@@ -698,6 +699,8 @@ int fpga_mgr_register(struct fpga_manager *mgr)if (ret) goto error_device; +fpga_mgr_debugfs_add (mgr); +dev_info (&mgr->dev,"%s registered\n" , mgr->name);return 0 ;@@ -722,6 +725,8 @@ void fpga_mgr_unregister(struct fpga_manager *mgr){
Quoted 37 lines
drivers/fpga/fpga-mgr.c +5
-0dev_info (&mgr->dev,"%s %s\n" , __func__, mgr->name); +fpga_mgr_debugfs_remove (mgr); +/* * If the low level driver provides a method for putting fpga into * a desired state upon unregister, do it. @@ -748,11 +753,14 @@ static int __init fpga_mgr_class_init(void) fpga_mgr_class->dev_groups = fpga_mgr_groups; fpga_mgr_class->dev_release = fpga_mgr_dev_release; +fpga_mgr_debugfs_init (); +return 0 ; } static void __exit fpga_mgr_class_exit(void) { +fpga_mgr_debugfs_uninit ();class_destroy (fpga_mgr_class);ida_destroy (&fpga_mgr_ida); }(quoted diff) +0
-0
b/include/linux/fpga/fpga-mgr.hindex 777c502..e8f2159 100644include/linux/fpga/fpga-mgr.h +3
-1@@ -170,6 +170,9 @@ struct fpga_manager {struct fpga_compat_id *compat_id;const struct fpga_manager_ops *mops;void *priv; +#if IS_ENABLED (CONFIG_FPGA_MGR_DEBUG_FS ) +void *debugfs; +#endif };#define to_fpga_manager (d) container_of(d, struct fpga_manager, dev) --2.7.4
Quoted 1 line · Alan Tull
From: Alan Tull <[email protected]>
Ugh. This patchset is a squash-and-cleanup of some really old commits that had my no-longer valid opensource.altera.com email address as the commit author. I'll need to fix that in v2.
Alan
Quoted 9 lines
This patch depends on my recently submitted documentation changes ("docs: fpga: document programming fpgas using regions")
Document the DebugFS interface for the core FPGA Manager framework.
Signed-off-by Alan Tull <[email protected]>Signed-off-by Matthew Gerlach <[email protected]>