[PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Xu Yilun]

In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
patch moves struct dfl_device_id to mod_devicetable.h

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 drivers/fpga/dfl.h              | 13 +------------
 include/linux/mod_devicetable.h | 12 ++++++++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 5dc758f..d5b0760 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -26,6 +26,7 @@
 #include <linux/slab.h>
 #include <linux/uuid.h>
 #include <linux/fpga/fpga-region.h>
+#include <linux/mod_devicetable.h>
 
 /* maximum supported number of ports */
 #define MAX_DFL_FPGA_PORT_NUM 4
@@ -526,18 +527,6 @@ enum dfl_id_type {
 };
 
 /**
- * struct dfl_device_id -  dfl device identifier
- * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
- * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
- * @driver_data: driver specific data.
- */
-struct dfl_device_id {
-	u8 type;
-	u16 feature_id;
-	unsigned long driver_data;
-};
-
-/**
  * struct dfl_device - represent an dfl device on dfl bus
  *
  * @dev: generic device interface.
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 5b08a47..407d8dc 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -838,4 +838,16 @@ struct mhi_device_id {
 	kernel_ulong_t driver_data;
 };
 
+/**
+ * struct dfl_device_id -  dfl device identifier
+ * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
+ * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
+ * @driver_data: driver specific data.
+ */
+struct dfl_device_id {
+	__u8 type;
+	__u16 feature_id;
+	kernel_ulong_t driver_data;
+};
+
 #endif /* LINUX_MOD_DEVICETABLE_H */
-- 
2.7.4

[PATCH 2/3] dfl: add dfl bus support to MODULE_DEVICE_TABLE()

[Xu Yilun]

Device Feature List (DFL) is a linked list of feature headers within the
device MMIO space. It is used by FPGA to enumerate multiple sub features
within it. Each feature can be uniquely identified by DFL type and
feature id, which can be read out from feature headers.

A dfl bus helps DFL framework modularize DFL device drivers for different
sub features. The dfl bus matches its devices and drivers by DFL type and
feature id.

This patch add dfl bus support to MODULE_DEVICE_TABLE() by adding info
about struct dfl_device_id in devicetable-offsets.c and add a dfl entry
point in file2alias.c.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 scripts/mod/devicetable-offsets.c |  4 ++++
 scripts/mod/file2alias.c          | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index 27007c1..d8350ee 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -243,5 +243,9 @@ int main(void)
 	DEVID(mhi_device_id);
 	DEVID_FIELD(mhi_device_id, chan);
 
+	DEVID(dfl_device_id);
+	DEVID_FIELD(dfl_device_id, type);
+	DEVID_FIELD(dfl_device_id, feature_id);
+
 	return 0;
 }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 2417dd1..0753fc2 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1368,6 +1368,18 @@ static int do_mhi_entry(const char *filename, void *symval, char *alias)
 	return 1;
 }
 
+/* Looks like: dfl:tNfN */
+static int do_dfl_entry(const char *filename, void *symval, char *alias)
+{
+	DEF_FIELD(symval, dfl_device_id, type);
+	DEF_FIELD(symval, dfl_device_id, feature_id);
+
+	sprintf(alias, "dfl:t%01Xf%03X", type, feature_id);
+
+	add_wildcard(alias);
+	return 1;
+}
+
 /* Does namelen bytes of name exactly match the symbol? */
 static bool sym_is(const char *name, unsigned namelen, const char *symbol)
 {
@@ -1442,6 +1454,7 @@ static const struct devtable devtable[] = {
 	{"tee", SIZE_tee_client_device_id, do_tee_entry},
 	{"wmi", SIZE_wmi_device_id, do_wmi_entry},
 	{"mhi", SIZE_mhi_device_id, do_mhi_entry},
+	{"dfl", SIZE_dfl_device_id, do_dfl_entry},
 };
 
 /* Create MODULE_ALIAS() statements.
-- 
2.7.4

Re: [PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Tom Rix]

On 9/7/20 10:48 PM, Xu Yilun wrote:
> In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
> patch moves struct dfl_device_id to mod_devicetable.h
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Wu Hao <hao.wu@intel.com>
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
>  drivers/fpga/dfl.h              | 13 +------------
>  include/linux/mod_devicetable.h | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 5dc758f..d5b0760 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -26,6 +26,7 @@
>  #include <linux/slab.h>
>  #include <linux/uuid.h>
>  #include <linux/fpga/fpga-region.h>
> +#include <linux/mod_devicetable.h>
>  
>  /* maximum supported number of ports */
>  #define MAX_DFL_FPGA_PORT_NUM 4
> @@ -526,18 +527,6 @@ enum dfl_id_type {
>  };
>  
>  /**
> - * struct dfl_device_id -  dfl device identifier
> - * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
> - * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
> - * @driver_data: driver specific data.
> - */
> -struct dfl_device_id {
> -	u8 type;
> -	u16 feature_id;
> -	unsigned long driver_data;
> -};
> -
> -/**
>   * struct dfl_device - represent an dfl device on dfl bus
>   *
>   * @dev: generic device interface.
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 5b08a47..407d8dc 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -838,4 +838,16 @@ struct mhi_device_id {
>  	kernel_ulong_t driver_data;
>  };
>  
> +/**
> + * struct dfl_device_id -  dfl device identifier
> + * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
> + * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
> + * @driver_data: driver specific data.
> + */
> +struct dfl_device_id {
> +	__u8 type;
> +	__u16 feature_id;

I thought i saw feature id's going to 64 bit, does this type need to expand ?

Tom 

> +	kernel_ulong_t driver_data;
> +};
> +
>  #endif /* LINUX_MOD_DEVICETABLE_H */

Re: [PATCH 2/3] dfl: add dfl bus support to MODULE_DEVICE_TABLE()

[Tom Rix]

On 9/7/20 10:48 PM, Xu Yilun wrote:
> Device Feature List (DFL) is a linked list of feature headers within the
> device MMIO space. It is used by FPGA to enumerate multiple sub features
Do you mean a linked list in the pci config space ?
> within it. Each feature can be uniquely identified by DFL type and
> feature id, which can be read out from feature headers.
>
> A dfl bus helps DFL framework modularize DFL device drivers for different
> sub features. The dfl bus matches its devices and drivers by DFL type and
> feature id.
>
> This patch add dfl bus support to MODULE_DEVICE_TABLE() by adding info
> about struct dfl_device_id in devicetable-offsets.c and add a dfl entry
> point in file2alias.c.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Wu Hao <hao.wu@intel.com>
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
>  scripts/mod/devicetable-offsets.c |  4 ++++
>  scripts/mod/file2alias.c          | 13 +++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> index 27007c1..d8350ee 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -243,5 +243,9 @@ int main(void)
>  	DEVID(mhi_device_id);
>  	DEVID_FIELD(mhi_device_id, chan);
>  
> +	DEVID(dfl_device_id);
> +	DEVID_FIELD(dfl_device_id, type);
> +	DEVID_FIELD(dfl_device_id, feature_id);
> +
>  	return 0;
>  }
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 2417dd1..0753fc2 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1368,6 +1368,18 @@ static int do_mhi_entry(const char *filename, void *symval, char *alias)
>  	return 1;
>  }
>  
> +/* Looks like: dfl:tNfN */
> +static int do_dfl_entry(const char *filename, void *symval, char *alias)
> +{
> +	DEF_FIELD(symval, dfl_device_id, type);
> +	DEF_FIELD(symval, dfl_device_id, feature_id);
> +
> +	sprintf(alias, "dfl:t%01Xf%03X", type, feature_id);

Is it necessary to constrain the output here ?

If you do not want to use the full width of the types,

a comment would be helpful.

Tom

> +
> +	add_wildcard(alias);
> +	return 1;
> +}
> +
>  /* Does namelen bytes of name exactly match the symbol? */
>  static bool sym_is(const char *name, unsigned namelen, const char *symbol)
>  {
> @@ -1442,6 +1454,7 @@ static const struct devtable devtable[] = {
>  	{"tee", SIZE_tee_client_device_id, do_tee_entry},
>  	{"wmi", SIZE_wmi_device_id, do_wmi_entry},
>  	{"mhi", SIZE_mhi_device_id, do_mhi_entry},
> +	{"dfl", SIZE_dfl_device_id, do_dfl_entry},
>  };
>  
>  /* Create MODULE_ALIAS() statements.

Re: [PATCH 3/3] fpga: dfl: move dfl-bus related APIs to include/linux/fpga/dfl-bus.h

[Tom Rix]

On 9/7/20 10:48 PM, Xu Yilun wrote:
> The patch moves dfl-bus related APIs to include/linux/fpga/dfl-bus.h

Should add a line in the MAINTAINERS under FPGA DFL DRIVERS

F:      include/linux/fpga/dfl-bus.h

Otherwise a straight forward move.

Looks good to me.

Reviewed-by: Tom Rix <trix@redhat.com>

>
> Now the DFL sub feature drivers could be made as independent modules and
> put in different folders according to their functionality. In order for
> scattered sub feature drivers to include dfl bus APIs, move the dfl bus
> APIs to a new header file in the public folder.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
>  drivers/fpga/dfl-n3000-nios.c |  3 +-
>  drivers/fpga/dfl.c            |  1 +
>  drivers/fpga/dfl.h            | 73 ------------------------------------
>  include/linux/fpga/dfl-bus.h  | 86 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 88 insertions(+), 75 deletions(-)
>  create mode 100644 include/linux/fpga/dfl-bus.h
>
> diff --git a/drivers/fpga/dfl-n3000-nios.c b/drivers/fpga/dfl-n3000-nios.c
> index 70b44c3..d5f8b5b 100644
> --- a/drivers/fpga/dfl-n3000-nios.c
> +++ b/drivers/fpga/dfl-n3000-nios.c
> @@ -11,6 +11,7 @@
>   */
>  #include <linux/bitfield.h>
>  #include <linux/errno.h>
> +#include <linux/fpga/dfl-bus.h>
>  #include <linux/io.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/kernel.h>
> @@ -22,8 +23,6 @@
>  #include <linux/spi/spi.h>
>  #include <linux/types.h>
>  
> -#include "dfl.h"
> -
>  static char *fec_mode = "rs";
>  module_param(fec_mode, charp, 0444);
>  MODULE_PARM_DESC(fec_mode, "FEC mode of the ethernet retimer on Intel PAC N3000");
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index b450870..02a6780 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -11,6 +11,7 @@
>   *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
>   */
>  #include <linux/fpga-dfl.h>
> +#include <linux/fpga/dfl-bus.h>
>  #include <linux/module.h>
>  #include <linux/uaccess.h>
>  
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index d5b0760..03f73d9 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -26,7 +26,6 @@
>  #include <linux/slab.h>
>  #include <linux/uuid.h>
>  #include <linux/fpga/fpga-region.h>
> -#include <linux/mod_devicetable.h>
>  
>  /* maximum supported number of ports */
>  #define MAX_DFL_FPGA_PORT_NUM 4
> @@ -517,76 +516,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,
> -	PORT_ID,
> -	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: 16 bits 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;
> -	u8 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/fpga/dfl-bus.h b/include/linux/fpga/dfl-bus.h
> new file mode 100644
> index 0000000..2a2b283
> --- /dev/null
> +++ b/include/linux/fpga/dfl-bus.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 __FPGA_DFL_BUS_H
> +#define __FPGA_DFL_BUS_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,
> +	PORT_ID,
> +	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: 16 bits 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;
> +	u8 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_BUS_H */

Re: [PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Xu Yilun]

On Wed, Sep 09, 2020 at 05:55:33AM -0700, Tom Rix wrote:
> 
> On 9/7/20 10:48 PM, Xu Yilun wrote:
> > In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
> > patch moves struct dfl_device_id to mod_devicetable.h
> >
> > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > Signed-off-by: Wu Hao <hao.wu@intel.com>
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > ---
> >  drivers/fpga/dfl.h              | 13 +------------
> >  include/linux/mod_devicetable.h | 12 ++++++++++++
> >  2 files changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > index 5dc758f..d5b0760 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -26,6 +26,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/uuid.h>
> >  #include <linux/fpga/fpga-region.h>
> > +#include <linux/mod_devicetable.h>
> >  
> >  /* maximum supported number of ports */
> >  #define MAX_DFL_FPGA_PORT_NUM 4
> > @@ -526,18 +527,6 @@ enum dfl_id_type {
> >  };
> >  
> >  /**
> > - * struct dfl_device_id -  dfl device identifier
> > - * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
> > - * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
> > - * @driver_data: driver specific data.
> > - */
> > -struct dfl_device_id {
> > -	u8 type;
> > -	u16 feature_id;
> > -	unsigned long driver_data;
> > -};
> > -
> > -/**
> >   * struct dfl_device - represent an dfl device on dfl bus
> >   *
> >   * @dev: generic device interface.
> > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> > index 5b08a47..407d8dc 100644
> > --- a/include/linux/mod_devicetable.h
> > +++ b/include/linux/mod_devicetable.h
> > @@ -838,4 +838,16 @@ struct mhi_device_id {
> >  	kernel_ulong_t driver_data;
> >  };
> >  
> > +/**
> > + * struct dfl_device_id -  dfl device identifier
> > + * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
> > + * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
> > + * @driver_data: driver specific data.
> > + */
> > +struct dfl_device_id {
> > +	__u8 type;
> > +	__u16 feature_id;
> 
> I thought i saw feature id's going to 64 bit, does this type need to expand ?

Feature id is a 12bit field in DFL spec. Previously we define it u64
cause we are considering it may expand sometime. But now seems GUID will
be used in future design.

And the header file will be used by modpost, which is not supporting u64
now. So it is not necessary we use u64 for extra work. A patch is
already applied for this change - "change data type of feature id to u16"

Thanks,
Yilun

> 
> Tom 
> 
> > +	kernel_ulong_t driver_data;
> > +};
> > +
> >  #endif /* LINUX_MOD_DEVICETABLE_H */

Re: [PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Tom Rix]

On 9/10/20 1:41 AM, Xu Yilun wrote:
> On Wed, Sep 09, 2020 at 05:55:33AM -0700, Tom Rix wrote:
>> On 9/7/20 10:48 PM, Xu Yilun wrote:
>>> In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
>>> patch moves struct dfl_device_id to mod_devicetable.h
>>>
>>> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
>>> Signed-off-by: Wu Hao <hao.wu@intel.com>
>>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>>> ---
>>>  drivers/fpga/dfl.h              | 13 +------------
>>>  include/linux/mod_devicetable.h | 12 ++++++++++++
>>>  2 files changed, 13 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
>>> index 5dc758f..d5b0760 100644
>>> --- a/drivers/fpga/dfl.h
>>> +++ b/drivers/fpga/dfl.h
>>> @@ -26,6 +26,7 @@
>>>  #include <linux/slab.h>
>>>  #include <linux/uuid.h>
>>>  #include <linux/fpga/fpga-region.h>
>>> +#include <linux/mod_devicetable.h>
>>>  
>>>  /* maximum supported number of ports */
>>>  #define MAX_DFL_FPGA_PORT_NUM 4
>>> @@ -526,18 +527,6 @@ enum dfl_id_type {
>>>  };
>>>  
>>>  /**
>>> - * struct dfl_device_id -  dfl device identifier
>>> - * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
>>> - * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
>>> - * @driver_data: driver specific data.
>>> - */
>>> -struct dfl_device_id {
>>> -	u8 type;
>>> -	u16 feature_id;
>>> -	unsigned long driver_data;
>>> -};
>>> -
>>> -/**
>>>   * struct dfl_device - represent an dfl device on dfl bus
>>>   *
>>>   * @dev: generic device interface.
>>> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
>>> index 5b08a47..407d8dc 100644
>>> --- a/include/linux/mod_devicetable.h
>>> +++ b/include/linux/mod_devicetable.h
>>> @@ -838,4 +838,16 @@ struct mhi_device_id {
>>>  	kernel_ulong_t driver_data;
>>>  };
>>>  
>>> +/**
>>> + * struct dfl_device_id -  dfl device identifier
>>> + * @type: contains 4 bits DFL FIU type of the device. See enum dfl_id_type.
>>> + * @feature_id: contains 12 bits feature identifier local to its DFL FIU type.
>>> + * @driver_data: driver specific data.
>>> + */
>>> +struct dfl_device_id {
>>> +	__u8 type;
>>> +	__u16 feature_id;
>> I thought i saw feature id's going to 64 bit, does this type need to expand ?
> Feature id is a 12bit field in DFL spec. Previously we define it u64
> cause we are considering it may expand sometime. But now seems GUID will
> be used in future design.
>
> And the header file will be used by modpost, which is not supporting u64
> now. So it is not necessary we use u64 for extra work. A patch is
> already applied for this change - "change data type of feature id to u16"
>
> Thanks,
> Yilun

Thanks for the explanation.

Reviewed-by: Tom Rix <trix@redhat.com>

>
>> Tom 
>>
>>> +	kernel_ulong_t driver_data;
>>> +};
>>> +
>>>  #endif /* LINUX_MOD_DEVICETABLE_H */

Re: [PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Moritz Fischer]

Hao,

On Fri, Sep 11, 2020 at 06:50:18AM +0000, Wu, Hao wrote:
> > On 9/10/20 1:41 AM, Xu Yilun wrote:
> > > On Wed, Sep 09, 2020 at 05:55:33AM -0700, Tom Rix wrote:
> > >> On 9/7/20 10:48 PM, Xu Yilun wrote:
> > >>> In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
> > >>> patch moves struct dfl_device_id to mod_devicetable.h
> > >>>
> > >>> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > >>> Signed-off-by: Wu Hao <hao.wu@intel.com>
> > >>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > >>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > >>> ---
> > >>>  drivers/fpga/dfl.h              | 13 +------------
> > >>>  include/linux/mod_devicetable.h | 12 ++++++++++++
> > >>>  2 files changed, 13 insertions(+), 12 deletions(-)
> > >>>
> > >>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > >>> index 5dc758f..d5b0760 100644
> > >>> --- a/drivers/fpga/dfl.h
> > >>> +++ b/drivers/fpga/dfl.h
> > >>> @@ -26,6 +26,7 @@
> > >>>  #include <linux/slab.h>
> > >>>  #include <linux/uuid.h>
> > >>>  #include <linux/fpga/fpga-region.h>
> > >>> +#include <linux/mod_devicetable.h>
> 
> Please fix the order for the new one, other places to good to me.
> Acked-by: Wu Hao <hao.wu@intel.com>  for DFL related changes after this fix.

Do you mean the order of the includes, like alphabeticized? If it's
minor I can fix it up when applying.

Thanks,
Moritz

Re: [PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Xu Yilun]

On Mon, Sep 14, 2020 at 02:32:32PM -0700, Moritz Fischer wrote:
> Hao,
> 
> On Fri, Sep 11, 2020 at 06:50:18AM +0000, Wu, Hao wrote:
> > > On 9/10/20 1:41 AM, Xu Yilun wrote:
> > > > On Wed, Sep 09, 2020 at 05:55:33AM -0700, Tom Rix wrote:
> > > >> On 9/7/20 10:48 PM, Xu Yilun wrote:
> > > >>> In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
> > > >>> patch moves struct dfl_device_id to mod_devicetable.h
> > > >>>
> > > >>> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > >>> Signed-off-by: Wu Hao <hao.wu@intel.com>
> > > >>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > >>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > > >>> ---
> > > >>>  drivers/fpga/dfl.h              | 13 +------------
> > > >>>  include/linux/mod_devicetable.h | 12 ++++++++++++
> > > >>>  2 files changed, 13 insertions(+), 12 deletions(-)
> > > >>>
> > > >>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > > >>> index 5dc758f..d5b0760 100644
> > > >>> --- a/drivers/fpga/dfl.h
> > > >>> +++ b/drivers/fpga/dfl.h
> > > >>> @@ -26,6 +26,7 @@
> > > >>>  #include <linux/slab.h>
> > > >>>  #include <linux/uuid.h>
> > > >>>  #include <linux/fpga/fpga-region.h>
> > > >>> +#include <linux/mod_devicetable.h>
> > 
> > Please fix the order for the new one, other places to good to me.
> > Acked-by: Wu Hao <hao.wu@intel.com>  for DFL related changes after this fix.
> 
> Do you mean the order of the includes, like alphabeticized? If it's
> minor I can fix it up when applying.

I think it is the alphabetic of head files. I could sent v2 patchset for
this fix as well as other fixes.

Thanks,
Yilun.

> 
> Thanks,
> Moritz

Re: [PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Moritz Fischer]

On Tue, Sep 15, 2020 at 10:55:17AM +0800, Xu Yilun wrote:
> On Mon, Sep 14, 2020 at 02:32:32PM -0700, Moritz Fischer wrote:
> > Hao,
> > 
> > On Fri, Sep 11, 2020 at 06:50:18AM +0000, Wu, Hao wrote:
> > > > On 9/10/20 1:41 AM, Xu Yilun wrote:
> > > > > On Wed, Sep 09, 2020 at 05:55:33AM -0700, Tom Rix wrote:
> > > > >> On 9/7/20 10:48 PM, Xu Yilun wrote:
> > > > >>> In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
> > > > >>> patch moves struct dfl_device_id to mod_devicetable.h
> > > > >>>
> > > > >>> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > > >>> Signed-off-by: Wu Hao <hao.wu@intel.com>
> > > > >>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > > >>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > > > >>> ---
> > > > >>>  drivers/fpga/dfl.h              | 13 +------------
> > > > >>>  include/linux/mod_devicetable.h | 12 ++++++++++++
> > > > >>>  2 files changed, 13 insertions(+), 12 deletions(-)
> > > > >>>
> > > > >>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > > > >>> index 5dc758f..d5b0760 100644
> > > > >>> --- a/drivers/fpga/dfl.h
> > > > >>> +++ b/drivers/fpga/dfl.h
> > > > >>> @@ -26,6 +26,7 @@
> > > > >>>  #include <linux/slab.h>
> > > > >>>  #include <linux/uuid.h>
> > > > >>>  #include <linux/fpga/fpga-region.h>
> > > > >>> +#include <linux/mod_devicetable.h>
> > > 
> > > Please fix the order for the new one, other places to good to me.
> > > Acked-by: Wu Hao <hao.wu@intel.com>  for DFL related changes after this fix.
> > 
> > Do you mean the order of the includes, like alphabeticized? If it's
> > minor I can fix it up when applying.
> 
> I think it is the alphabetic of head files. I could sent v2 patchset for
> this fix as well as other fixes.

Alright, works for me. Can you do that?

- Moritz

Re: [PATCH 1/3] fpga: dfl: move dfl_device_id to mod_devicetable.h

[Xu Yilun]

On Mon, Sep 14, 2020 at 08:10:14PM -0700, Moritz Fischer wrote:
> On Tue, Sep 15, 2020 at 10:55:17AM +0800, Xu Yilun wrote:
> > On Mon, Sep 14, 2020 at 02:32:32PM -0700, Moritz Fischer wrote:
> > > Hao,
> > > 
> > > On Fri, Sep 11, 2020 at 06:50:18AM +0000, Wu, Hao wrote:
> > > > > On 9/10/20 1:41 AM, Xu Yilun wrote:
> > > > > > On Wed, Sep 09, 2020 at 05:55:33AM -0700, Tom Rix wrote:
> > > > > >> On 9/7/20 10:48 PM, Xu Yilun wrote:
> > > > > >>> In order to support MODULE_DEVICE_TABLE() for dfl device driver, this
> > > > > >>> patch moves struct dfl_device_id to mod_devicetable.h
> > > > > >>>
> > > > > >>> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > > > > >>> Signed-off-by: Wu Hao <hao.wu@intel.com>
> > > > > >>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > > > >>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > > > > >>> ---
> > > > > >>>  drivers/fpga/dfl.h              | 13 +------------
> > > > > >>>  include/linux/mod_devicetable.h | 12 ++++++++++++
> > > > > >>>  2 files changed, 13 insertions(+), 12 deletions(-)
> > > > > >>>
> > > > > >>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > > > > >>> index 5dc758f..d5b0760 100644
> > > > > >>> --- a/drivers/fpga/dfl.h
> > > > > >>> +++ b/drivers/fpga/dfl.h
> > > > > >>> @@ -26,6 +26,7 @@
> > > > > >>>  #include <linux/slab.h>
> > > > > >>>  #include <linux/uuid.h>
> > > > > >>>  #include <linux/fpga/fpga-region.h>
> > > > > >>> +#include <linux/mod_devicetable.h>
> > > > 
> > > > Please fix the order for the new one, other places to good to me.
> > > > Acked-by: Wu Hao <hao.wu@intel.com>  for DFL related changes after this fix.
> > > 
> > > Do you mean the order of the includes, like alphabeticized? If it's
> > > minor I can fix it up when applying.
> > 
> > I think it is the alphabetic of head files. I could sent v2 patchset for
> > this fix as well as other fixes.
> 
> Alright, works for me. Can you do that?

Yes, I'm working on it. :)

Thanks,
Yilun

> 
> - Moritz