Skip to contentSkip to conversation

Submission of XillyUSB driver

PATCH In review [email protected] 2 participants 6 messages May 26, 2021 – May 28, 2021

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

v5 cover Submission of XillyUSB driver

From: Eli Billauer <[email protected]>

This is a resubmission of the XillyUSB driver, which is the USB variant of the existing Xillybus driver.

Because these driver share some API related functions, this submission consists of two patches:

(1) A patch moving away Xillybus' class related functions to a separate module file.
(2) A patch adding the new XillyUSB driver, based upon this new separate module.

Thanks,
Eli

Eli Billauer (2):
char: xillybus: Move class-related functions to new xillybus_class.c
char: xillybus: Add driver for XillyUSB (Xillybus variant for USB)

 drivers/char/xillybus/Kconfig          |   22 +-
 drivers/char/xillybus/Makefile         |    2 +
 drivers/char/xillybus/xillybus.h       |   10 +-
 drivers/char/xillybus/xillybus_class.c |  263 +++
 drivers/char/xillybus/xillybus_class.h |   30 +
 drivers/char/xillybus/xillybus_core.c  |  181 +-
 drivers/char/xillybus/xillyusb.c       | 2260 ++++++++++++++++++++++++
 7 files changed, 2599 insertions(+), 169 deletions(-)
 create mode 100644 drivers/char/xillybus/xillybus_class.c
 create mode 100644 drivers/char/xillybus/xillybus_class.h
 create mode 100644 drivers/char/xillybus/xillyusb.c
Signature · 1 line
2.17.1
author maintainer
#1 lore raw

PATCH v5 1/2 char: xillybus: Move class-related functions to new xillybus_class.c

From: Eli Billauer <[email protected]>

This patch is a preparation for adding another related driver, XillyUSB. In order to share some code between the existing Xillybus driver and the one to be added, some functions are moved to xillybus_class.c

XILLYBUS_CLASS is added to Kconfig and is common to all drivers in this group. The relation with the existing XILLYBUS symbol is "select" rather than "depends on" XILLYBUS_CLASS, or else "make olddefconfig" will silently turn off XILLYBUS, which is currently enabled in several distributions.

XILLYBUS_CLASS doesn't depend on anything else, hence using it with "select" poses no risk for a broken configuration.

After the future addition of the XillyUSB module, the tree of symbols will be as follows:

XILLYBUS_CLASS --+-- XILLYBUS --+-- XILLYBUS_PCIE

                 |              |
                 |              +-- XILLYBUS_OF
                 |
                 +-- XILLYUSB

XILLYBUS is for drivers based upon memory registers + DMA-based interfaces, and it's combined with XILLYBUS_PCIE and/or XILLYBUS_OF.

XILLYUSB is for the USB variant only.

Or a more detailed, bottom-up outline:

* CONFIG_XILLYBUS_PCIE -> xillybus_pcie.c: Functions related to PCIe.
* CONFIG_XILLYBUS_OF -> xillybus_of.c: Functions related to Xillybus as a peripheral on an FPGA / Processor combo chip.
* CONFIG_XILLYBUS -> xillybus_core.c: Functions that are common to the two above, mainly access to the peripheral with memory-mapped registers and DMA.
* CONFIG_XILLYUSB -> xillyusb.c: The driver for the USB variant, accesses the peripheral through the USB framework.
* CONFIG_XILLYBUS_CLASS -> xillybus_class.c: The new module, which contains the class and API parts that would otherwise appear both in xillybus_core.c and xillyusb.c. Contains utility functions for the two latter.

And since I'm at it, comments on the module names are added in the Kconfig's help part.

The functions are exported with the non-GPL EXPORT_SYMBOL (a matter of taste).

---

Notes:
Changelog:

v5:
- xillybus_class.h to local directory, as the intended XillyUSB module won't go to staging.
- Add "The module will be called" notes in Kconfig.

v4:
- Fix error code return value bugs in xillybus_init_chrdev() as detected by Smatch test robot, and reported by Dan Carpenter.

This patch did not exist prior to v3.

 drivers/char/xillybus/Kconfig          |   8 +-
 drivers/char/xillybus/Makefile         |   1 +
 drivers/char/xillybus/xillybus.h       |  10 +-
 drivers/char/xillybus/xillybus_class.c | 263 +++++++++++++++++++++++++
 drivers/char/xillybus/xillybus_class.h |  30 +++
 drivers/char/xillybus/xillybus_core.c  | 181 +++--------------
 6 files changed, 324 insertions(+), 169 deletions(-)
 create mode 100644 drivers/char/xillybus/xillybus_class.c
 create mode 100644 drivers/char/xillybus/xillybus_class.h
drivers/char/xillybus/Kconfig +6 -2
index 130dbdce858f..7cc4d719ec4f 100644
@@ -3,10 +3,14 @@
 # Xillybus devices
 #
 
+config XILLYBUS_CLASS
+	tristate
+
 config XILLYBUS
 	tristate "Xillybus generic FPGA interface"
 	depends on PCI || OF
 	select CRC32
+	select XILLYBUS_CLASS
 	help
 	  Xillybus is a generic interface for peripherals designed on
 	  programmable logic (FPGA). The driver probes the hardware for
@@ -21,7 +25,7 @@ config XILLYBUS_PCIE
 	depends on PCI_MSI
 	help
 	  Set to M if you want Xillybus to use PCI Express for communicating
-	  with the FPGA.
+	  with the FPGA. The module will be called xillybus_pcie.
 
 config XILLYBUS_OF
 	tristate "Xillybus over Device Tree"
@@ -29,6 +33,6 @@ config XILLYBUS_OF
 	help
 	  Set to M if you want Xillybus to find its resources from the
 	  Open Firmware Flattened Device Tree. If the target is an embedded
-	  system, say M.
+	  system, say M. The module will be called xillybus_of.
 
 endif # if XILLYBUS
drivers/char/xillybus/Makefile +1 -0
index 099e9a3585fc..591615264591 100644
@@ -3,6 +3,7 @@
 # Makefile for Xillybus driver
 #
 
+obj-$(CONFIG_XILLYBUS_CLASS)	+= xillybus_class.o
 obj-$(CONFIG_XILLYBUS)		+= xillybus_core.o
 obj-$(CONFIG_XILLYBUS_PCIE)	+= xillybus_pcie.o
 obj-$(CONFIG_XILLYBUS_OF)	+= xillybus_of.o
drivers/char/xillybus/xillybus.h +2 -8
index 8e3ed4d1bb7f..c63ffc56637c 100644
@@ -30,7 +30,8 @@ struct xilly_buffer {
 
 struct xilly_idt_handle {
 	unsigned char *chandesc;
-	unsigned char *idt;
+	unsigned char *names;
+	int names_len;
 	int entries;
 };
 
@@ -94,7 +95,6 @@ struct xilly_endpoint {
 	struct device *dev;
 	struct xilly_endpoint_hardware *ephw;
 
-	struct list_head ep_list;
 	int dma_using_dac; /* =1 if 64-bit DMA is used, =0 otherwise. */
 	__iomem void *registers;
 	int fatal_error;
@@ -102,12 +102,6 @@ struct xilly_endpoint {
 	struct mutex register_mutex;
 	wait_queue_head_t ep_wait;
 
-	/* Channels and message handling */
-	struct cdev cdev;
-
-	int major;
-	int lowest_minor; /* Highest minor = lowest_minor + num_channels - 1 */
-
 	int num_channels; /* EXCLUDING message buffer */
 	struct xilly_channel **channels;
 	int msg_counter;
drivers/char/xillybus/xillybus_class.c new file +263 -0

263 lines not on this page · raw message

drivers/char/xillybus/xillybus_class.h new file +30 -0
new file mode 100644
index 000000000000..5dbfdfc95c65
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2021 Xillybus Ltd, http://www.xillybus.com
+ *
+ * Header file for the Xillybus class
+ */
+
+#ifndef __XILLYBUS_CLASS_H
+#define __XILLYBUS_CLASS_H
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+
+int xillybus_init_chrdev(struct device *dev,
+			 const struct file_operations *fops,
+			 struct module *owner,
+			 void *private_data,
+			 unsigned char *idt, unsigned int len,
+			 int num_nodes,
+			 const char *prefix, bool enumerate);
+
+void xillybus_cleanup_chrdev(void *private_data,
+			     struct device *dev);
+
+int xillybus_find_inode(struct inode *inode,
+			void **private_data, int *index);
+
+#endif /* __XILLYBUS_CLASS_H */
drivers/char/xillybus/xillybus_core.c +22 -159

266 lines not on this page · raw message

Signature · 1 line
2.17.1
author maintainer
#2 lore raw

PATCH v5 2/2 char: xillybus: Add driver for XillyUSB (Xillybus variant for USB)

In reply to [email protected]

From: Eli Billauer <[email protected]>

The XillyUSB driver is the USB variant for the Xillybus FPGA IP core. Even though it presents a nearly identical API on the FPGA and host, it's almost a complete rewrite of the driver: The framework for exchanging data on a USB bus is fundamentally different from doing the same with a PCIe interface, which leaves very little in common between the existing driver and the new one for XillyUSB.

---

Notes:
Changelog:

v5:
- Move xillyusb.c back from staging to char/xillybus/
- Turn previously lockless FIFO into one relying on a lock
- Set driver's soft_unbind flag, so that the device is notified when the driver is unloaded + due changes for killing URBs as required
- Some refactoring of locks for better granularity
- Avoid using pr_err and pr_warn, replace with dev_* counterparts
- Bump MODULE_VERSION to 1.1
- Remove module_params
- Fix a few minor bugs regarding error handling flow
- Various coding style fixes and comment clarifications

v4:
(No changes)

v3:
- Move to staging
- Rely on xillybus_class for device file operations
- Fix no return value bug in xillyusb_discovery()
- Add module parameters for URB buffer size and count

v2:
- Add comment in Kconfig file, saying XILLYUSB really doesn't depend on XILLYBUS (following comment by Randy Dunlap)
- Use SEEK_* predefined constants instead of numbers

 drivers/char/xillybus/Kconfig    |   14 +
 drivers/char/xillybus/Makefile   |    1 +
 drivers/char/xillybus/xillyusb.c | 2260 ++++++++++++++++++++++++++++++
 3 files changed, 2275 insertions(+)
 create mode 100644 drivers/char/xillybus/xillyusb.c
drivers/char/xillybus/Kconfig +14 -0
index 7cc4d719ec4f..a8036dad437e 100644
@@ -36,3 +36,17 @@ config XILLYBUS_OF
 	  system, say M. The module will be called xillybus_of.
 
 endif # if XILLYBUS
+
+# XILLYUSB doesn't depend on XILLYBUS
+
+config XILLYUSB
+	tristate "XillyUSB: Xillybus generic FPGA interface for USB"
+	depends on USB
+	select CRC32
+	select XILLYBUS_CLASS
+	help
+	  XillyUSB is the Xillybus variant which uses USB for communicating
+	  with the FPGA.
+
+	  Set to M if you want Xillybus to use USB for communicating with
+	  the FPGA. The module will be called xillyusb.
drivers/char/xillybus/Makefile +1 -0
index 591615264591..16f31d03209d 100644
@@ -7,3 +7,4 @@ obj-$(CONFIG_XILLYBUS_CLASS)	+= xillybus_class.o
 obj-$(CONFIG_XILLYBUS)		+= xillybus_core.o
 obj-$(CONFIG_XILLYBUS_PCIE)	+= xillybus_pcie.o
 obj-$(CONFIG_XILLYBUS_OF)	+= xillybus_of.o
+obj-$(CONFIG_XILLYUSB)		+= xillyusb.o
drivers/char/xillybus/xillyusb.c new file +2260 -0

2,260 lines not on this page · raw message

Signature · 1 line
2.17.1
maintainer
#3 lore raw

Re: [PATCH v5 2/2] char: xillybus: Add driver for XillyUSB (Xillybus variant for USB)

Quoted 15 lines · On Wed, May 26, 2021 at 01:03:11PM +0300, [email protected] wrote:

From: Eli Billauer <[email protected]>

The XillyUSB driver is the USB variant for the Xillybus FPGA IP core. Even though it presents a nearly identical API on the FPGA and host, it's almost a complete rewrite of the driver: The framework for exchanging data on a USB bus is fundamentally different from doing the same with a PCIe interface, which leaves very little in common between the existing driver and the new one for XillyUSB.

---

Notes:
Changelog:

Quoted 8 lines

v5:
- Move xillyusb.c back from staging to char/xillybus/
- Turn previously lockless FIFO into one relying on a lock
- Set driver's soft_unbind flag, so that the device is notified when the driver is unloaded + due changes for killing URBs as required
- Some refactoring of locks for better granularity
- Avoid using pr_err and pr_warn, replace with dev_* counterparts
- Bump MODULE_VERSION to 1.1

Nit, you should just remove the MODULE_VERSION() stuff, it makes no sense once it is in the kernel tree to have them as they then mean nothing.

But that can be an add-on patch, let me review these...

maintainer
#4 lore raw

Re: [PATCH v5 2/2] char: xillybus: Add driver for XillyUSB (Xillybus variant for USB)

In reply to [email protected]
Quoted 74 lines · On Wed, May 26, 2021 at 01:03:11PM +0300, [email protected] wrote:

From: Eli Billauer <[email protected]>

The XillyUSB driver is the USB variant for the Xillybus FPGA IP core. Even though it presents a nearly identical API on the FPGA and host, it's almost a complete rewrite of the driver: The framework for exchanging data on a USB bus is fundamentally different from doing the same with a PCIe interface, which leaves very little in common between the existing driver and the new one for XillyUSB.

---

Notes:
Changelog:

v5:
- Move xillyusb.c back from staging to char/xillybus/
- Turn previously lockless FIFO into one relying on a lock
- Set driver's soft_unbind flag, so that the device is notified when the driver is unloaded + due changes for killing URBs as required
- Some refactoring of locks for better granularity
- Avoid using pr_err and pr_warn, replace with dev_* counterparts
- Bump MODULE_VERSION to 1.1
- Remove module_params
- Fix a few minor bugs regarding error handling flow
- Various coding style fixes and comment clarifications

v4:
(No changes)

v3:
- Move to staging
- Rely on xillybus_class for device file operations
- Fix no return value bug in xillyusb_discovery()
- Add module parameters for URB buffer size and count

v2:
- Add comment in Kconfig file, saying XILLYUSB really doesn't depend on XILLYBUS (following comment by Randy Dunlap)
- Use SEEK_* predefined constants instead of numbers

 drivers/char/xillybus/Kconfig    |   14 +
 drivers/char/xillybus/Makefile   |    1 +
 drivers/char/xillybus/xillyusb.c | 2260 ++++++++++++++++++++++++++++++
 3 files changed, 2275 insertions(+)
 create mode 100644 drivers/char/xillybus/xillyusb.c
drivers/char/xillybus/Kconfig +14 -0
index 7cc4d719ec4f..a8036dad437e 100644
@@ -36,3 +36,17 @@ config XILLYBUS_OF
 	  system, say M. The module will be called xillybus_of.
 
 endif # if XILLYBUS
+
+# XILLYUSB doesn't depend on XILLYBUS
+
+config XILLYUSB
+	tristate "XillyUSB: Xillybus generic FPGA interface for USB"
+	depends on USB
+	select CRC32
+	select XILLYBUS_CLASS
+	help
+	  XillyUSB is the Xillybus variant which uses USB for communicating
+	  with the FPGA.
+
+	  Set to M if you want Xillybus to use USB for communicating with
+	  the FPGA. The module will be called xillyusb.
drivers/char/xillybus/Makefile +0 -0
index 591615264591..16f31d03209d 100644

drivers/char/xillybus/Makefile obj-$(CONFIG_XILLYBUS_CLASS) += xillybus_class.o · line 10

Quoted 4 lines
drivers/char/xillybus/Makefile +1 -0
@@ -7,3 +7,4 @@ obj-$(CONFIG_XILLYBUS_CLASS)	+= xillybus_class.o
 obj-$(CONFIG_XILLYBUS)		+= xillybus_core.o
 obj-$(CONFIG_XILLYBUS_PCIE)	+= xillybus_pcie.o
 obj-$(CONFIG_XILLYBUS_OF)	+= xillybus_of.o
+obj-$(CONFIG_XILLYUSB)		+= xillyusb.o

This doesn't quite work.

Even if I have not selected CONFIG_XILLYBUS I get asked aboug CONFIG_XILLYUSB. And if I say 'm' to CONFIG_XILLYUSB, nothing gets built.

So I'll take this now, but you need to send a follow-up patch to fix these dependencies up to work correctly.

thanks,

greg k-h

author maintainer
#5 lore raw

Re: [PATCH v5 2/2] char: xillybus: Add driver for XillyUSB (Xillybus variant for USB)

Quoted 10 lines · Greg KH
Quoted 10 lines
drivers/char/xillybus/Makefile +0 -0
Quoted 8 lines

index 591615264591..16f31d03209d 100644
--- a/drivers/char/xillybus/Makefile
+++ b/drivers/char/xillybus/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_XILLYBUS_CLASS) += xillybus_class.o
obj-$(CONFIG_XILLYBUS) += xillybus_core.o
obj-$(CONFIG_XILLYBUS_PCIE) += xillybus_pcie.o
obj-$(CONFIG_XILLYBUS_OF) += xillybus_of.o
+obj-$(CONFIG_XILLYUSB) += xillyusb.o

Quoted 4 lines

This doesn't quite work.

Even if I have not selected CONFIG_XILLYBUS I get asked aboug CONFIG_XILLYUSB.

The symbol tree is:

XILLYBUS_CLASS --+-- XILLYBUS --+-- XILLYBUS_PCIE

                 |              |
                 |              +-- XILLYBUS_OF
                 |
                 +-- XILLYUSB

Hence not selecting CONFIG_XILLYBUS doesn't eliminate CONFIG_XILLYUSB. The behavior you described is correct: CONFIG_XILLYUSB doesn't depend on CONFIG_XILLYBUS.

Quoted 4 lines

And if I say 'm' to CONFIG_XILLYUSB, nothing gets
built.

Sorry, my bad. The directory is currently invoked based upon CONFIG_XILLYBUS, which was correct before, but now it should be CONFIG_XILLYBUS_CLASS. I'll submit a patch fixing that promptly.

Plus another patch removing MODULE_VERSION() from all C files, per your earlier comment.

Thanks and regards,
Eli