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@xxxxxxxxxx>

>
> 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@xxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx>
>   */
>  #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 */