diff mbox series

[4/4] fpga: dfl: move dfl bus related APIs to include/linux/dfl.h

Message ID 20201103072104.12361-5-mdf@kernel.org (mailing list archive)
State Superseded, archived
Headers show
Series DFL Module support | expand

Commit Message

Moritz Fischer Nov. 3, 2020, 7:21 a.m. UTC 
From: Xu Yilun <yilun.xu@intel.com>

Now the dfl drivers could be made as independent modules and put in
different folders according to their functionalities. In order for
scattered dfl device drivers to include dfl bus APIs, move the
dfl bus APIs to a new header file in the public folder.

[mdf@kernel.org: Fixed up header guards to match filename]
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Reviewed-by: Tom Rix <trix@redhat.com>
Acked-by: Wu Hao <hao.wu@intel.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 MAINTAINERS         |  1 +
 drivers/fpga/dfl.c  |  1 +
 drivers/fpga/dfl.h  | 72 -------------------------------------
 include/linux/dfl.h | 86 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 72 deletions(-)
 create mode 100644 include/linux/dfl.h

Comments

Greg Kroah-Hartman Nov. 3, 2020, 7:43 a.m. UTC | #1 
On Mon, Nov 02, 2020 at 11:21:04PM -0800, Moritz Fischer wrote:
> From: Xu Yilun <yilun.xu@intel.com>
> 
> Now the dfl drivers could be made as independent modules and put in
> different folders according to their functionalities. In order for
> scattered dfl device drivers to include dfl bus APIs, move the
> dfl bus APIs to a new header file in the public folder.
> 
> [mdf@kernel.org: Fixed up header guards to match filename]
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Reviewed-by: Tom Rix <trix@redhat.com>
> Acked-by: Wu Hao <hao.wu@intel.com>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> ---
>  MAINTAINERS         |  1 +
>  drivers/fpga/dfl.c  |  1 +
>  drivers/fpga/dfl.h  | 72 -------------------------------------
>  include/linux/dfl.h | 86 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 88 insertions(+), 72 deletions(-)
>  create mode 100644 include/linux/dfl.h

Why move this if there is no in-kernel users?

greg k-h
Xu Yilun Nov. 3, 2020, 10:43 a.m. UTC | #2 
On Tue, Nov 03, 2020 at 08:43:07AM +0100, Greg KH wrote:
> On Mon, Nov 02, 2020 at 11:21:04PM -0800, Moritz Fischer wrote:
> > From: Xu Yilun <yilun.xu@intel.com>
> > 
> > Now the dfl drivers could be made as independent modules and put in
> > different folders according to their functionalities. In order for
> > scattered dfl device drivers to include dfl bus APIs, move the
> > dfl bus APIs to a new header file in the public folder.
> > 
> > [mdf@kernel.org: Fixed up header guards to match filename]
> > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > Reviewed-by: Tom Rix <trix@redhat.com>
> > Acked-by: Wu Hao <hao.wu@intel.com>
> > Signed-off-by: Moritz Fischer <mdf@kernel.org>
> > ---
> >  MAINTAINERS         |  1 +
> >  drivers/fpga/dfl.c  |  1 +
> >  drivers/fpga/dfl.h  | 72 -------------------------------------
> >  include/linux/dfl.h | 86 +++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 88 insertions(+), 72 deletions(-)
> >  create mode 100644 include/linux/dfl.h
> 
> Why move this if there is no in-kernel users?

The DFL emif driver in driver/memory is the first user, see:

https://lore.kernel.org/linux-fpga/20201027105545.GB20676@kozik-lap/T/#m6b72f043ecf266c6305bf43db88cddcaf3f9f73d

It is not in this patchset, but the memory controller maintainer is already
acked this patch.

Thanks,
Yilun

> 
> greg k-h
Greg Kroah-Hartman Nov. 3, 2020, 11:08 a.m. UTC | #3 
On Tue, Nov 03, 2020 at 06:43:17PM +0800, Xu Yilun wrote:
> On Tue, Nov 03, 2020 at 08:43:07AM +0100, Greg KH wrote:
> > On Mon, Nov 02, 2020 at 11:21:04PM -0800, Moritz Fischer wrote:
> > > From: Xu Yilun <yilun.xu@intel.com>
> > > 
> > > Now the dfl drivers could be made as independent modules and put in
> > > different folders according to their functionalities. In order for
> > > scattered dfl device drivers to include dfl bus APIs, move the
> > > dfl bus APIs to a new header file in the public folder.
> > > 
> > > [mdf@kernel.org: Fixed up header guards to match filename]
> > > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > Reviewed-by: Tom Rix <trix@redhat.com>
> > > Acked-by: Wu Hao <hao.wu@intel.com>
> > > Signed-off-by: Moritz Fischer <mdf@kernel.org>
> > > ---
> > >  MAINTAINERS         |  1 +
> > >  drivers/fpga/dfl.c  |  1 +
> > >  drivers/fpga/dfl.h  | 72 -------------------------------------
> > >  include/linux/dfl.h | 86 +++++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 88 insertions(+), 72 deletions(-)
> > >  create mode 100644 include/linux/dfl.h
> > 
> > Why move this if there is no in-kernel users?
> 
> The DFL emif driver in driver/memory is the first user, see:
> 
> https://lore.kernel.org/linux-fpga/20201027105545.GB20676@kozik-lap/T/#m6b72f043ecf266c6305bf43db88cddcaf3f9f73d
> 
> It is not in this patchset, but the memory controller maintainer is already
> acked this patch.

How am I, or anyone else, supposed to know this?

Again, don't include patches that are not actually used, that's a huge
red flag to any reviewer and it just makes them grumpy and sad and
less-likely to ever want to review code from the submitter again...

{sigh}

greg k-h
Xu Yilun Nov. 4, 2020, 1:19 a.m. UTC | #4 
On Tue, Nov 03, 2020 at 12:08:17PM +0100, Greg KH wrote:
> On Tue, Nov 03, 2020 at 06:43:17PM +0800, Xu Yilun wrote:
> > On Tue, Nov 03, 2020 at 08:43:07AM +0100, Greg KH wrote:
> > > On Mon, Nov 02, 2020 at 11:21:04PM -0800, Moritz Fischer wrote:
> > > > From: Xu Yilun <yilun.xu@intel.com>
> > > > 
> > > > Now the dfl drivers could be made as independent modules and put in
> > > > different folders according to their functionalities. In order for
> > > > scattered dfl device drivers to include dfl bus APIs, move the
> > > > dfl bus APIs to a new header file in the public folder.
> > > > 
> > > > [mdf@kernel.org: Fixed up header guards to match filename]
> > > > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > > Reviewed-by: Tom Rix <trix@redhat.com>
> > > > Acked-by: Wu Hao <hao.wu@intel.com>
> > > > Signed-off-by: Moritz Fischer <mdf@kernel.org>
> > > > ---
> > > >  MAINTAINERS         |  1 +
> > > >  drivers/fpga/dfl.c  |  1 +
> > > >  drivers/fpga/dfl.h  | 72 -------------------------------------
> > > >  include/linux/dfl.h | 86 +++++++++++++++++++++++++++++++++++++++++++++
> > > >  4 files changed, 88 insertions(+), 72 deletions(-)
> > > >  create mode 100644 include/linux/dfl.h
> > > 
> > > Why move this if there is no in-kernel users?
> > 
> > The DFL emif driver in driver/memory is the first user, see:
> > 
> > https://lore.kernel.org/linux-fpga/20201027105545.GB20676@kozik-lap/T/#m6b72f043ecf266c6305bf43db88cddcaf3f9f73d
> > 
> > It is not in this patchset, but the memory controller maintainer is already
> > acked this patch.
> 
> How am I, or anyone else, supposed to know this?
> 
> Again, don't include patches that are not actually used, that's a huge
> red flag to any reviewer and it just makes them grumpy and sad and
> less-likely to ever want to review code from the submitter again...

Sorry, I didn't explain it clearly.

The DFL emif driver was in this patchset before v11. All the maintainers
are on the maillist, see the v10:
https://lore.kernel.org/linux-fpga/20201015162812.GA251058@epycbox.lan/T/#m64f61e205ed0ea1fd65e30e24bea55da40f8881d

In v10, the first 4 patches are DFL bus related, the same as you can see
in this pull request. The 5th is the dfl-n3000-nios driver and the 6th is
the dfl-emif, they are the in-kernel users of dfl bus.

In v11, I didn't include the first 4 patches because Moritz has already
queued them to his branch, I just send the remaining ones in order to
save time for the reviewers.

Thanks,
Yilun

> 
> {sigh}
> 
> greg k-h
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index e73636b75f29..9bbb378308a8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6886,6 +6886,7 @@  S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-dfl
 F:	Documentation/fpga/dfl.rst
 F:	drivers/fpga/dfl*
+F:	include/linux/dfl.h
 F:	include/uapi/linux/fpga-dfl.h
 
 FPGA MANAGER FRAMEWORK
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 5a6ba3b2fa05..511b20ff35a3 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -10,6 +10,7 @@ 
  *   Wu Hao <hao.wu@intel.com>
  *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
  */
+#include <linux/dfl.h>
 #include <linux/fpga-dfl.h>
 #include <linux/module.h>
 #include <linux/uaccess.h>
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 549c7900dcfd..2b82c96ba56c 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -517,76 +517,4 @@  long dfl_feature_ioctl_set_irq(struct platform_device *pdev,
 			       struct dfl_feature *feature,
 			       unsigned long arg);
 
-/**
- * enum dfl_id_type - define the DFL FIU types
- */
-enum dfl_id_type {
-	FME_ID = 0,
-	PORT_ID = 1,
-	DFL_ID_MAX,
-};
-
-/**
- * struct dfl_device - represent an dfl device on dfl bus
- *
- * @dev: generic device interface.
- * @id: id of the dfl device.
- * @type: type of DFL FIU of the device. See enum dfl_id_type.
- * @feature_id: feature identifier local to its DFL FIU type.
- * @mmio_res: mmio resource of this dfl device.
- * @irqs: list of Linux IRQ numbers of this dfl device.
- * @num_irqs: number of IRQs supported by this dfl device.
- * @cdev: pointer to DFL FPGA container device this dfl device belongs to.
- * @id_entry: matched id entry in dfl driver's id table.
- */
-struct dfl_device {
-	struct device dev;
-	int id;
-	u16 type;
-	u16 feature_id;
-	struct resource mmio_res;
-	int *irqs;
-	unsigned int num_irqs;
-	struct dfl_fpga_cdev *cdev;
-	const struct dfl_device_id *id_entry;
-};
-
-/**
- * struct dfl_driver - represent an dfl device driver
- *
- * @drv: driver model structure.
- * @id_table: pointer to table of device IDs the driver is interested in.
- *	      { } member terminated.
- * @probe: mandatory callback for device binding.
- * @remove: callback for device unbinding.
- */
-struct dfl_driver {
-	struct device_driver drv;
-	const struct dfl_device_id *id_table;
-
-	int (*probe)(struct dfl_device *dfl_dev);
-	void (*remove)(struct dfl_device *dfl_dev);
-};
-
-#define to_dfl_dev(d) container_of(d, struct dfl_device, dev)
-#define to_dfl_drv(d) container_of(d, struct dfl_driver, drv)
-
-/*
- * use a macro to avoid include chaining to get THIS_MODULE.
- */
-#define dfl_driver_register(drv) \
-	__dfl_driver_register(drv, THIS_MODULE)
-int __dfl_driver_register(struct dfl_driver *dfl_drv, struct module *owner);
-void dfl_driver_unregister(struct dfl_driver *dfl_drv);
-
-/*
- * module_dfl_driver() - Helper macro for drivers that don't do
- * anything special in module init/exit.  This eliminates a lot of
- * boilerplate.  Each module may only use this macro once, and
- * calling it replaces module_init() and module_exit().
- */
-#define module_dfl_driver(__dfl_driver) \
-	module_driver(__dfl_driver, dfl_driver_register, \
-		      dfl_driver_unregister)
-
 #endif /* __FPGA_DFL_H */
diff --git a/include/linux/dfl.h b/include/linux/dfl.h
new file mode 100644
index 000000000000..6cc10982351a
--- /dev/null
+++ b/include/linux/dfl.h
@@ -0,0 +1,86 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for DFL driver and device API
+ *
+ * Copyright (C) 2020 Intel Corporation, Inc.
+ */
+
+#ifndef __LINUX_DFL_H
+#define __LINUX_DFL_H
+
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+
+/**
+ * enum dfl_id_type - define the DFL FIU types
+ */
+enum dfl_id_type {
+	FME_ID = 0,
+	PORT_ID = 1,
+	DFL_ID_MAX,
+};
+
+/**
+ * struct dfl_device - represent an dfl device on dfl bus
+ *
+ * @dev: generic device interface.
+ * @id: id of the dfl device.
+ * @type: type of DFL FIU of the device. See enum dfl_id_type.
+ * @feature_id: feature identifier local to its DFL FIU type.
+ * @mmio_res: mmio resource of this dfl device.
+ * @irqs: list of Linux IRQ numbers of this dfl device.
+ * @num_irqs: number of IRQs supported by this dfl device.
+ * @cdev: pointer to DFL FPGA container device this dfl device belongs to.
+ * @id_entry: matched id entry in dfl driver's id table.
+ */
+struct dfl_device {
+	struct device dev;
+	int id;
+	u16 type;
+	u16 feature_id;
+	struct resource mmio_res;
+	int *irqs;
+	unsigned int num_irqs;
+	struct dfl_fpga_cdev *cdev;
+	const struct dfl_device_id *id_entry;
+};
+
+/**
+ * struct dfl_driver - represent an dfl device driver
+ *
+ * @drv: driver model structure.
+ * @id_table: pointer to table of device IDs the driver is interested in.
+ *	      { } member terminated.
+ * @probe: mandatory callback for device binding.
+ * @remove: callback for device unbinding.
+ */
+struct dfl_driver {
+	struct device_driver drv;
+	const struct dfl_device_id *id_table;
+
+	int (*probe)(struct dfl_device *dfl_dev);
+	void (*remove)(struct dfl_device *dfl_dev);
+};
+
+#define to_dfl_dev(d) container_of(d, struct dfl_device, dev)
+#define to_dfl_drv(d) container_of(d, struct dfl_driver, drv)
+
+/*
+ * use a macro to avoid include chaining to get THIS_MODULE.
+ */
+#define dfl_driver_register(drv) \
+	__dfl_driver_register(drv, THIS_MODULE)
+int __dfl_driver_register(struct dfl_driver *dfl_drv, struct module *owner);
+void dfl_driver_unregister(struct dfl_driver *dfl_drv);
+
+/*
+ * module_dfl_driver() - Helper macro for drivers that don't do
+ * anything special in module init/exit.  This eliminates a lot of
+ * boilerplate.  Each module may only use this macro once, and
+ * calling it replaces module_init() and module_exit().
+ */
+#define module_dfl_driver(__dfl_driver) \
+	module_driver(__dfl_driver, dfl_driver_register, \
+		      dfl_driver_unregister)
+
+#endif /* __LINUX_DFL_H */