linux-fpga.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/3] Modularization of DFL private feature drivers
@ 2020-08-19  7:45 Xu Yilun
  2020-08-19  7:45 ` [PATCH v7 1/3] fpga: dfl: map feature mmio resources in their own " Xu Yilun
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Xu Yilun @ 2020-08-19  7:45 UTC (permalink / raw)
  To: mdf, linux-fpga, linux-kernel; +Cc: trix, lgoncalv, Xu Yilun

This patchset makes it possible to develop independent driver modules
for DFL private features. It also helps to leverage existing kernel
drivers to enable some IP blocks in DFL.

Patch #1: Release the dfl mmio regions after enumeration, so that private
          feature drivers could request mmio region in their own drivers.
Patch #2: Introduce the dfl bus, then dfl devices could be supported by
          independent dfl drivers.
Patch #3: An example of the dfl driver for N3000 nios private feature.


Main changes from v1:
- Add the new Patch #1, to improve the feature id definition.
- Change the dfl bus uevent format.
- Change the dfl device's sysfs name format.
- refactor dfl_dev_add()
- Add the Patch #4 as an example of the dfl driver.
- A lot of minor fixes for comments from Hao and Tom.

Main changes from v2:
- Add the doc for dfl-n3000-nios driver.
- Minor fixes for comments from Tom.

Main changes from v3:
- improve the dfl devices' uevent format, 4 bits for type & 12 bits for id
- change dfl_device->type to u8
- A dedicate field in struct dfl_feature for dfl device instance.
- error out if dfl_device already exist on dfl_devs_init().
- Move the err log in regmap implementation, and delete
   n3000_nios_writel/readl(), they have nothing to wrapper now.
- Minor fixes and comments improvement.

Main changes from v4:
- The patch "fpga: dfl: change data type of feature id to u16" is already
   applied to for-next
- Unify the naming of some functions in dfl.c
- Fix the output of fec_mode sysfs inf to "no" on 10G configuration, cause
   no FEC mode could be configured for 10G.
- Change the N3000 Nios driver name from "dfl-n3000-nios" to "n3000-nios",
   and also rename some structures and functions from dfl_n3000_nios_* to
   n3000_nios_*
- Minor fixes and comments improvement.

Main changes from v5:
- Fix the output of fec_mode sysfs inf to "not supported" if in 10G,
   or the firmware version major < 3.
- The input param of dfl_devs_add() changes to
   struct dfl_feature_platform_data.
- Minor fixes and improves comments.

Main changes from v6:
- Rebased to 5.9-rc1.
- Improves comments.

Xu Yilun (3):
  fpga: dfl: map feature mmio resources in their own feature drivers
  fpga: dfl: create a dfl bus type to support DFL devices
  fpga: dfl: add support for N3000 Nios private feature

 Documentation/ABI/testing/sysfs-bus-dfl            |  15 +
 .../ABI/testing/sysfs-bus-dfl-devices-n3000-nios   |  21 +
 Documentation/fpga/dfl-n3000-nios.rst              |  80 +++
 Documentation/fpga/index.rst                       |   1 +
 drivers/fpga/Kconfig                               |  11 +
 drivers/fpga/Makefile                              |   2 +
 drivers/fpga/dfl-n3000-nios.c                      | 542 +++++++++++++++++++++
 drivers/fpga/dfl-pci.c                             |  24 +-
 drivers/fpga/dfl.c                                 | 449 ++++++++++++++---
 drivers/fpga/dfl.h                                 |  93 +++-
 10 files changed, 1153 insertions(+), 85 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl
 create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
 create mode 100644 Documentation/fpga/dfl-n3000-nios.rst
 create mode 100644 drivers/fpga/dfl-n3000-nios.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v7 1/3] fpga: dfl: map feature mmio resources in their own feature drivers
  2020-08-19  7:45 [PATCH v7 0/3] Modularization of DFL private feature drivers Xu Yilun
@ 2020-08-19  7:45 ` Xu Yilun
  2020-08-31  0:18   ` Moritz Fischer
  2020-08-19  7:45 ` [PATCH v7 2/3] fpga: dfl: create a dfl bus type to support DFL devices Xu Yilun
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Xu Yilun @ 2020-08-19  7:45 UTC (permalink / raw)
  To: mdf, linux-fpga, linux-kernel
  Cc: trix, lgoncalv, Xu Yilun, Wu Hao, Matthew Gerlach, Russ Weight

This patch makes preparation for modularization of DFL sub feature
drivers.

DFL based FPGA devices may contain some IP blocks which are already
supported by kernel, most of them are supported by platform device
drivers. We could create platform devices for these IP blocks and get them
supported by these drivers.

An important issue is that platform device drivers usually requests mmio
resources on probe. But now DFL mmio is mapped in DFL bus driver (e.g.
dfl-pci) as a whole region. Then platform device drivers for sub features
can't request their own mmio resources again. This is what the patch
trying to resolve.

This patch changes the DFL enumeration. DFL bus driver will unmap mmio
resources after first step enumeration and pass enumeration info to DFL
framework. Then DFL framework will map the mmio resources again, do 2nd
step enumeration, and also unmap the mmio resources. In this way, sub
feature drivers could then request their own mmio resources as needed.

An exception is that mmio resource of FIU headers are still mapped in DFL
bus driver. The FIU headers have some fundamental functions (sriov set,
port enable/disable) needed for DFL bus devices and other sub features.
They should not be unmapped as long as DFL bus device is alive.

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>
Reviewed-by: Tom Rix <trix@redhat.com>
Acked-by: Wu Hao <hao.wu@intel.com>
---
v2: delete dfl_binfo_shift() cause no shift is possible for FIU parsing.
    Only map bar0 for first phase enumeration in dfl-pci, we can find
      all dfl mmio resource info in bar0.
    Some minor fixes for comments from Hao & Tom.
v3: no change
v4: minor fixes for Hao's comments.
v5: improve some comments.
    unify the naming of some functions.
v6: no change.
v7: no change.
---
 drivers/fpga/dfl-pci.c |  24 +++----
 drivers/fpga/dfl.c     | 187 ++++++++++++++++++++++++++++++++++---------------
 drivers/fpga/dfl.h     |   7 +-
 3 files changed, 141 insertions(+), 77 deletions(-)

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index e220bec..a2203d0 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -31,12 +31,12 @@ struct cci_drvdata {
 	struct dfl_fpga_cdev *cdev;	/* container device */
 };
 
-static void __iomem *cci_pci_ioremap_bar(struct pci_dev *pcidev, int bar)
+static void __iomem *cci_pci_ioremap_bar0(struct pci_dev *pcidev)
 {
-	if (pcim_iomap_regions(pcidev, BIT(bar), DRV_NAME))
+	if (pcim_iomap_regions(pcidev, BIT(0), DRV_NAME))
 		return NULL;
 
-	return pcim_iomap_table(pcidev)[bar];
+	return pcim_iomap_table(pcidev)[0];
 }
 
 static int cci_pci_alloc_irq(struct pci_dev *pcidev)
@@ -156,8 +156,8 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 			goto irq_free_exit;
 	}
 
-	/* start to find Device Feature List from Bar 0 */
-	base = cci_pci_ioremap_bar(pcidev, 0);
+	/* start to find Device Feature List in Bar 0 */
+	base = cci_pci_ioremap_bar0(pcidev);
 	if (!base) {
 		ret = -ENOMEM;
 		goto irq_free_exit;
@@ -172,7 +172,7 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 		start = pci_resource_start(pcidev, 0);
 		len = pci_resource_len(pcidev, 0);
 
-		dfl_fpga_enum_info_add_dfl(info, start, len, base);
+		dfl_fpga_enum_info_add_dfl(info, start, len);
 
 		/*
 		 * find more Device Feature Lists (e.g. Ports) per information
@@ -196,26 +196,24 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 			 */
 			bar = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
 			offset = FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
-			base = cci_pci_ioremap_bar(pcidev, bar);
-			if (!base)
-				continue;
-
 			start = pci_resource_start(pcidev, bar) + offset;
 			len = pci_resource_len(pcidev, bar) - offset;
 
-			dfl_fpga_enum_info_add_dfl(info, start, len,
-						   base + offset);
+			dfl_fpga_enum_info_add_dfl(info, start, len);
 		}
 	} else if (dfl_feature_is_port(base)) {
 		start = pci_resource_start(pcidev, 0);
 		len = pci_resource_len(pcidev, 0);
 
-		dfl_fpga_enum_info_add_dfl(info, start, len, base);
+		dfl_fpga_enum_info_add_dfl(info, start, len);
 	} else {
 		ret = -ENODEV;
 		goto irq_free_exit;
 	}
 
+	/* release I/O mappings for next step enumeration */
+	pcim_iounmap_regions(pcidev, BIT(0));
+
 	/* start enumeration with prepared enumeration information */
 	cdev = dfl_fpga_feature_devs_enumerate(info);
 	if (IS_ERR(cdev)) {
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 18575d9..52cafa2 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -250,6 +250,8 @@ int dfl_fpga_check_port_id(struct platform_device *pdev, void *pport_id)
 }
 EXPORT_SYMBOL_GPL(dfl_fpga_check_port_id);
 
+#define is_header_feature(feature) ((feature)->id == FEATURE_ID_FIU_HEADER)
+
 /**
  * dfl_fpga_dev_feature_uinit - uinit for sub features of dfl feature device
  * @pdev: feature device.
@@ -273,8 +275,22 @@ static int dfl_feature_instance_init(struct platform_device *pdev,
 				     struct dfl_feature *feature,
 				     struct dfl_feature_driver *drv)
 {
+	void __iomem *base;
 	int ret = 0;
 
+	if (!is_header_feature(feature)) {
+		base = devm_platform_ioremap_resource(pdev,
+						      feature->resource_index);
+		if (IS_ERR(base)) {
+			dev_err(&pdev->dev,
+				"ioremap failed for feature 0x%x!\n",
+				feature->id);
+			return PTR_ERR(base);
+		}
+
+		feature->ioaddr = base;
+	}
+
 	if (drv->ops->init) {
 		ret = drv->ops->init(pdev, feature);
 		if (ret)
@@ -427,7 +443,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
  * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
  *	       this device.
  * @feature_dev: current feature device.
- * @ioaddr: header register region address of feature device in enumeration.
+ * @ioaddr: header register region address of current FIU in enumeration.
+ * @start: register resource start of current FIU.
+ * @len: max register resource length of current FIU.
  * @sub_features: a sub features linked list for feature device in enumeration.
  * @feature_num: number of sub features for feature device in enumeration.
  */
@@ -439,6 +457,8 @@ struct build_feature_devs_info {
 
 	struct platform_device *feature_dev;
 	void __iomem *ioaddr;
+	resource_size_t start;
+	resource_size_t len;
 	struct list_head sub_features;
 	int feature_num;
 };
@@ -484,10 +504,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
 	struct dfl_feature_platform_data *pdata;
 	struct dfl_feature_info *finfo, *p;
 	enum dfl_id_type type;
-	int ret, index = 0;
-
-	if (!fdev)
-		return 0;
+	int ret, index = 0, res_idx = 0;
 
 	type = feature_dev_id_type(fdev);
 	if (WARN_ON_ONCE(type >= DFL_ID_MAX))
@@ -530,16 +547,32 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
 
 	/* fill features and resource information for feature dev */
 	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
-		struct dfl_feature *feature = &pdata->features[index];
+		struct dfl_feature *feature = &pdata->features[index++];
 		struct dfl_feature_irq_ctx *ctx;
 		unsigned int i;
 
 		/* save resource information for each feature */
 		feature->dev = fdev;
 		feature->id = finfo->fid;
-		feature->resource_index = index;
-		feature->ioaddr = finfo->ioaddr;
-		fdev->resource[index++] = finfo->mmio_res;
+
+		/*
+		 * the FIU header feature has some fundamental functions (sriov
+		 * set, port enable/disable) needed for the dfl bus device and
+		 * other sub features. So its mmio resource should be mapped by
+		 * DFL bus device. And we should not assign it to feature
+		 * devices (dfl-fme/afu) again.
+		 */
+		if (is_header_feature(feature)) {
+			feature->resource_index = -1;
+			feature->ioaddr =
+				devm_ioremap_resource(binfo->dev,
+						      &finfo->mmio_res);
+			if (IS_ERR(feature->ioaddr))
+				return PTR_ERR(feature->ioaddr);
+		} else {
+			feature->resource_index = res_idx;
+			fdev->resource[res_idx++] = finfo->mmio_res;
+		}
 
 		if (finfo->nr_irqs) {
 			ctx = devm_kcalloc(binfo->dev, finfo->nr_irqs,
@@ -582,19 +615,13 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
 
 static int
 build_info_create_dev(struct build_feature_devs_info *binfo,
-		      enum dfl_id_type type, void __iomem *ioaddr)
+		      enum dfl_id_type type)
 {
 	struct platform_device *fdev;
-	int ret;
 
 	if (type >= DFL_ID_MAX)
 		return -EINVAL;
 
-	/* we will create a new device, commit current device first */
-	ret = build_info_commit_dev(binfo);
-	if (ret)
-		return ret;
-
 	/*
 	 * we use -ENODEV as the initialization indicator which indicates
 	 * whether the id need to be reclaimed
@@ -605,7 +632,7 @@ build_info_create_dev(struct build_feature_devs_info *binfo,
 
 	binfo->feature_dev = fdev;
 	binfo->feature_num = 0;
-	binfo->ioaddr = ioaddr;
+
 	INIT_LIST_HEAD(&binfo->sub_features);
 
 	fdev->id = dfl_id_alloc(type, &fdev->dev);
@@ -747,18 +774,17 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo,
  */
 static int
 create_feature_instance(struct build_feature_devs_info *binfo,
-			struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst,
-			resource_size_t size, u16 fid)
+			resource_size_t ofst, resource_size_t size, u16 fid)
 {
 	unsigned int irq_base, nr_irqs;
 	struct dfl_feature_info *finfo;
 	int ret;
 
 	/* read feature size and id if inputs are invalid */
-	size = size ? size : feature_size(dfl->ioaddr + ofst);
-	fid = fid ? fid : feature_id(dfl->ioaddr + ofst);
+	size = size ? size : feature_size(binfo->ioaddr + ofst);
+	fid = fid ? fid : feature_id(binfo->ioaddr + ofst);
 
-	if (dfl->len - ofst < size)
+	if (binfo->len - ofst < size)
 		return -EINVAL;
 
 	ret = parse_feature_irqs(binfo, ofst, fid, &irq_base, &nr_irqs);
@@ -770,12 +796,11 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 		return -ENOMEM;
 
 	finfo->fid = fid;
-	finfo->mmio_res.start = dfl->start + ofst;
+	finfo->mmio_res.start = binfo->start + ofst;
 	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
 	finfo->mmio_res.flags = IORESOURCE_MEM;
 	finfo->irq_base = irq_base;
 	finfo->nr_irqs = nr_irqs;
-	finfo->ioaddr = dfl->ioaddr + ofst;
 
 	list_add_tail(&finfo->node, &binfo->sub_features);
 	binfo->feature_num++;
@@ -784,7 +809,6 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 }
 
 static int parse_feature_port_afu(struct build_feature_devs_info *binfo,
-				  struct dfl_fpga_enum_dfl *dfl,
 				  resource_size_t ofst)
 {
 	u64 v = readq(binfo->ioaddr + PORT_HDR_CAP);
@@ -792,21 +816,22 @@ static int parse_feature_port_afu(struct build_feature_devs_info *binfo,
 
 	WARN_ON(!size);
 
-	return create_feature_instance(binfo, dfl, ofst, size, FEATURE_ID_AFU);
+	return create_feature_instance(binfo, ofst, size, FEATURE_ID_AFU);
 }
 
+#define is_feature_dev_detected(binfo) (!!(binfo)->feature_dev)
+
 static int parse_feature_afu(struct build_feature_devs_info *binfo,
-			     struct dfl_fpga_enum_dfl *dfl,
 			     resource_size_t ofst)
 {
-	if (!binfo->feature_dev) {
+	if (!is_feature_dev_detected(binfo)) {
 		dev_err(binfo->dev, "this AFU does not belong to any FIU.\n");
 		return -EINVAL;
 	}
 
 	switch (feature_dev_id_type(binfo->feature_dev)) {
 	case PORT_ID:
-		return parse_feature_port_afu(binfo, dfl, ofst);
+		return parse_feature_port_afu(binfo, ofst);
 	default:
 		dev_info(binfo->dev, "AFU belonging to FIU %s is not supported yet.\n",
 			 binfo->feature_dev->name);
@@ -815,8 +840,39 @@ static int parse_feature_afu(struct build_feature_devs_info *binfo,
 	return 0;
 }
 
+static int build_info_prepare(struct build_feature_devs_info *binfo,
+			      resource_size_t start, resource_size_t len)
+{
+	struct device *dev = binfo->dev;
+	void __iomem *ioaddr;
+
+	if (!devm_request_mem_region(dev, start, len, dev_name(dev))) {
+		dev_err(dev, "request region fail, start:%pa, len:%pa\n",
+			&start, &len);
+		return -EBUSY;
+	}
+
+	ioaddr = devm_ioremap(dev, start, len);
+	if (!ioaddr) {
+		dev_err(dev, "ioremap region fail, start:%pa, len:%pa\n",
+			&start, &len);
+		return -ENOMEM;
+	}
+
+	binfo->start = start;
+	binfo->len = len;
+	binfo->ioaddr = ioaddr;
+
+	return 0;
+}
+
+static void build_info_complete(struct build_feature_devs_info *binfo)
+{
+	devm_iounmap(binfo->dev, binfo->ioaddr);
+	devm_release_mem_region(binfo->dev, binfo->start, binfo->len);
+}
+
 static int parse_feature_fiu(struct build_feature_devs_info *binfo,
-			     struct dfl_fpga_enum_dfl *dfl,
 			     resource_size_t ofst)
 {
 	int ret = 0;
@@ -824,27 +880,39 @@ static int parse_feature_fiu(struct build_feature_devs_info *binfo,
 	u16 id;
 	u64 v;
 
-	v = readq(dfl->ioaddr + ofst + DFH);
+	if (is_feature_dev_detected(binfo)) {
+		build_info_complete(binfo);
+
+		ret = build_info_commit_dev(binfo);
+		if (ret)
+			return ret;
+
+		ret = build_info_prepare(binfo, binfo->start + ofst,
+					 binfo->len - ofst);
+		if (ret)
+			return ret;
+	}
+
+	v = readq(binfo->ioaddr + DFH);
 	id = FIELD_GET(DFH_ID, v);
 
 	/* create platform device for dfl feature dev */
-	ret = build_info_create_dev(binfo, dfh_id_to_type(id),
-				    dfl->ioaddr + ofst);
+	ret = build_info_create_dev(binfo, dfh_id_to_type(id));
 	if (ret)
 		return ret;
 
-	ret = create_feature_instance(binfo, dfl, ofst, 0, 0);
+	ret = create_feature_instance(binfo, 0, 0, 0);
 	if (ret)
 		return ret;
 	/*
 	 * find and parse FIU's child AFU via its NEXT_AFU register.
 	 * please note that only Port has valid NEXT_AFU pointer per spec.
 	 */
-	v = readq(dfl->ioaddr + ofst + NEXT_AFU);
+	v = readq(binfo->ioaddr + NEXT_AFU);
 
 	offset = FIELD_GET(NEXT_AFU_NEXT_DFH_OFST, v);
 	if (offset)
-		return parse_feature_afu(binfo, dfl, ofst + offset);
+		return parse_feature_afu(binfo, offset);
 
 	dev_dbg(binfo->dev, "No AFUs detected on FIU %d\n", id);
 
@@ -852,41 +920,39 @@ static int parse_feature_fiu(struct build_feature_devs_info *binfo,
 }
 
 static int parse_feature_private(struct build_feature_devs_info *binfo,
-				 struct dfl_fpga_enum_dfl *dfl,
 				 resource_size_t ofst)
 {
-	if (!binfo->feature_dev) {
+	if (!is_feature_dev_detected(binfo)) {
 		dev_err(binfo->dev, "the private feature 0x%x does not belong to any AFU.\n",
-			feature_id(dfl->ioaddr + ofst));
+			feature_id(binfo->ioaddr + ofst));
 		return -EINVAL;
 	}
 
-	return create_feature_instance(binfo, dfl, ofst, 0, 0);
+	return create_feature_instance(binfo, ofst, 0, 0);
 }
 
 /**
  * parse_feature - parse a feature on given device feature list
  *
  * @binfo: build feature devices information.
- * @dfl: device feature list to parse
- * @ofst: offset to feature header on this device feature list
+ * @ofst: offset to current FIU header
  */
 static int parse_feature(struct build_feature_devs_info *binfo,
-			 struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst)
+			 resource_size_t ofst)
 {
 	u64 v;
 	u32 type;
 
-	v = readq(dfl->ioaddr + ofst + DFH);
+	v = readq(binfo->ioaddr + ofst + DFH);
 	type = FIELD_GET(DFH_TYPE, v);
 
 	switch (type) {
 	case DFH_TYPE_AFU:
-		return parse_feature_afu(binfo, dfl, ofst);
+		return parse_feature_afu(binfo, ofst);
 	case DFH_TYPE_PRIVATE:
-		return parse_feature_private(binfo, dfl, ofst);
+		return parse_feature_private(binfo, ofst);
 	case DFH_TYPE_FIU:
-		return parse_feature_fiu(binfo, dfl, ofst);
+		return parse_feature_fiu(binfo, ofst);
 	default:
 		dev_info(binfo->dev,
 			 "Feature Type %x is not supported.\n", type);
@@ -896,14 +962,17 @@ static int parse_feature(struct build_feature_devs_info *binfo,
 }
 
 static int parse_feature_list(struct build_feature_devs_info *binfo,
-			      struct dfl_fpga_enum_dfl *dfl)
+			      resource_size_t start, resource_size_t len)
 {
-	void __iomem *start = dfl->ioaddr;
-	void __iomem *end = dfl->ioaddr + dfl->len;
+	resource_size_t end = start + len;
 	int ret = 0;
 	u32 ofst = 0;
 	u64 v;
 
+	ret = build_info_prepare(binfo, start, len);
+	if (ret)
+		return ret;
+
 	/* walk through the device feature list via DFH's next DFH pointer. */
 	for (; start < end; start += ofst) {
 		if (end - start < DFH_SIZE) {
@@ -911,11 +980,11 @@ static int parse_feature_list(struct build_feature_devs_info *binfo,
 			return -EINVAL;
 		}
 
-		ret = parse_feature(binfo, dfl, start - dfl->ioaddr);
+		ret = parse_feature(binfo, start - binfo->start);
 		if (ret)
 			return ret;
 
-		v = readq(start + DFH);
+		v = readq(binfo->ioaddr + start - binfo->start + DFH);
 		ofst = FIELD_GET(DFH_NEXT_HDR_OFST, v);
 
 		/* stop parsing if EOL(End of List) is set or offset is 0 */
@@ -924,7 +993,12 @@ static int parse_feature_list(struct build_feature_devs_info *binfo,
 	}
 
 	/* commit current feature device when reach the end of list */
-	return build_info_commit_dev(binfo);
+	build_info_complete(binfo);
+
+	if (is_feature_dev_detected(binfo))
+		ret = build_info_commit_dev(binfo);
+
+	return ret;
 }
 
 struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev)
@@ -977,7 +1051,6 @@ EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_free);
  * @info: ptr to dfl_fpga_enum_info
  * @start: mmio resource address of the device feature list.
  * @len: mmio resource length of the device feature list.
- * @ioaddr: mapped mmio resource address of the device feature list.
  *
  * One FPGA device may have one or more Device Feature Lists (DFLs), use this
  * function to add information of each DFL to common data structure for next
@@ -986,8 +1059,7 @@ EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_free);
  * Return: 0 on success, negative error code otherwise.
  */
 int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
-			       resource_size_t start, resource_size_t len,
-			       void __iomem *ioaddr)
+			       resource_size_t start, resource_size_t len)
 {
 	struct dfl_fpga_enum_dfl *dfl;
 
@@ -997,7 +1069,6 @@ int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
 
 	dfl->start = start;
 	dfl->len = len;
-	dfl->ioaddr = ioaddr;
 
 	list_add_tail(&dfl->node, &info->dfls);
 
@@ -1120,7 +1191,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
 	 * Lists.
 	 */
 	list_for_each_entry(dfl, &info->dfls, node) {
-		ret = parse_feature_list(binfo, dfl);
+		ret = parse_feature_list(binfo, dfl->start, dfl->len);
 		if (ret) {
 			remove_feature_devs(cdev);
 			build_info_free(binfo);
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index bc61942..5973769 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -441,22 +441,17 @@ struct dfl_fpga_enum_info {
  *
  * @start: base address of this device feature list.
  * @len: size of this device feature list.
- * @ioaddr: mapped base address of this device feature list.
  * @node: node in list of device feature lists.
  */
 struct dfl_fpga_enum_dfl {
 	resource_size_t start;
 	resource_size_t len;
-
-	void __iomem *ioaddr;
-
 	struct list_head node;
 };
 
 struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev);
 int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
-			       resource_size_t start, resource_size_t len,
-			       void __iomem *ioaddr);
+			       resource_size_t start, resource_size_t len);
 int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
 			       unsigned int nr_irqs, int *irq_table);
 void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v7 2/3] fpga: dfl: create a dfl bus type to support DFL devices
  2020-08-19  7:45 [PATCH v7 0/3] Modularization of DFL private feature drivers Xu Yilun
  2020-08-19  7:45 ` [PATCH v7 1/3] fpga: dfl: map feature mmio resources in their own " Xu Yilun
@ 2020-08-19  7:45 ` Xu Yilun
  2020-09-05  0:32   ` Moritz Fischer
  2020-08-19  7:45 ` [PATCH v7 3/3] fpga: dfl: add support for N3000 Nios private feature Xu Yilun
  2020-08-20  4:16 ` [PATCH v7 0/3] Modularization of DFL private feature drivers Moritz Fischer
  3 siblings, 1 reply; 9+ messages in thread
From: Xu Yilun @ 2020-08-19  7:45 UTC (permalink / raw)
  To: mdf, linux-fpga, linux-kernel
  Cc: trix, lgoncalv, Xu Yilun, Wu Hao, Matthew Gerlach, Russ Weight

A new bus type "dfl" is introduced for private features which are not
initialized by DFL feature drivers (dfl-fme & dfl-afu drivers). So these
private features could be handled by separate driver modules.

DFL feature drivers (dfl-fme, dfl-port) will create DFL devices on
enumeration. DFL drivers could be registered on this bus to match these
DFL devices. They are matched by dfl type & feature_id.

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>
Reviewed-by: Tom Rix <trix@redhat.com>
Acked-by: Wu Hao <hao.wu@intel.com>
---
v2: change the bus uevent format.
    change the dfl device's sysfs name format.
    refactor dfl_dev_add().
    minor fixes for comments from Hao and Tom.
v3: no change.
v4: improve the uevent format, 4 bits for type & 12 bits for id.
    change dfl_device->type to u8.
    A dedicate field in struct dfl_feature for dfl device instance.
    error out if dfl_device already exist on dfl_devs_init().
v5: minor fixes for Hao's comments
v6: the input param of dfl_devs_add() changes to struct
    dfl_feature_platform_data.
    improve the comments.
v7: no change.
---
 Documentation/ABI/testing/sysfs-bus-dfl |  15 ++
 drivers/fpga/dfl.c                      | 262 +++++++++++++++++++++++++++++++-
 drivers/fpga/dfl.h                      |  86 +++++++++++
 3 files changed, 355 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl

diff --git a/Documentation/ABI/testing/sysfs-bus-dfl b/Documentation/ABI/testing/sysfs-bus-dfl
new file mode 100644
index 0000000..23543be
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-dfl
@@ -0,0 +1,15 @@
+What:		/sys/bus/dfl/devices/dfl_dev.X/type
+Date:		Aug 2020
+KernelVersion:	5.10
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:	Read-only. It returns type of DFL FIU of the device. Now DFL
+		supports 2 FIU types, 0 for FME, 1 for PORT.
+		Format: 0x%x
+
+What:		/sys/bus/dfl/devices/dfl_dev.X/feature_id
+Date:		Aug 2020
+KernelVersion:	5.10
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:	Read-only. It returns feature identifier local to its DFL FIU
+		type.
+		Format: 0x%x
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 52cafa2..c5ba4ac9 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -30,12 +30,6 @@ static DEFINE_MUTEX(dfl_id_mutex);
  * index to dfl_chardevs table. If no chardev support just set devt_type
  * as one invalid index (DFL_FPGA_DEVT_MAX).
  */
-enum dfl_id_type {
-	FME_ID,		/* fme id allocation and mapping */
-	PORT_ID,	/* port id allocation and mapping */
-	DFL_ID_MAX,
-};
-
 enum dfl_fpga_devt_type {
 	DFL_FPGA_DEVT_FME,
 	DFL_FPGA_DEVT_PORT,
@@ -250,6 +244,244 @@ int dfl_fpga_check_port_id(struct platform_device *pdev, void *pport_id)
 }
 EXPORT_SYMBOL_GPL(dfl_fpga_check_port_id);
 
+static DEFINE_IDA(dfl_device_ida);
+
+static const struct dfl_device_id *
+dfl_match_one_device(const struct dfl_device_id *id, struct dfl_device *ddev)
+{
+	if (id->type == ddev->type && id->feature_id == ddev->feature_id)
+		return id;
+
+	return NULL;
+}
+
+static int dfl_bus_match(struct device *dev, struct device_driver *drv)
+{
+	struct dfl_device *ddev = to_dfl_dev(dev);
+	struct dfl_driver *ddrv = to_dfl_drv(drv);
+	const struct dfl_device_id *id_entry = ddrv->id_table;
+
+	if (id_entry) {
+		while (id_entry->feature_id) {
+			if (dfl_match_one_device(id_entry, ddev)) {
+				ddev->id_entry = id_entry;
+				return 1;
+			}
+			id_entry++;
+		}
+	}
+
+	return 0;
+}
+
+static int dfl_bus_probe(struct device *dev)
+{
+	struct dfl_device *ddev = to_dfl_dev(dev);
+	struct dfl_driver *ddrv = to_dfl_drv(dev->driver);
+
+	return ddrv->probe(ddev);
+}
+
+static int dfl_bus_remove(struct device *dev)
+{
+	struct dfl_device *ddev = to_dfl_dev(dev);
+	struct dfl_driver *ddrv = to_dfl_drv(dev->driver);
+
+	if (ddrv->remove)
+		ddrv->remove(ddev);
+
+	return 0;
+}
+
+static int dfl_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct dfl_device *ddev = to_dfl_dev(dev);
+
+	/* The type has 4 valid bits and feature_id has 12 valid bits */
+	return add_uevent_var(env, "MODALIAS=dfl:t%01Xf%03X",
+			      ddev->type, ddev->feature_id);
+}
+
+/* show dfl info fields */
+#define dfl_info_attr(field, format_string)				\
+static ssize_t								\
+field##_show(struct device *dev, struct device_attribute *attr,		\
+	     char *buf)							\
+{									\
+	struct dfl_device *ddev = to_dfl_dev(dev);			\
+									\
+	return sprintf(buf, format_string, ddev->field);		\
+}									\
+static DEVICE_ATTR_RO(field)
+
+dfl_info_attr(type, "0x%x\n");
+dfl_info_attr(feature_id, "0x%x\n");
+
+static struct attribute *dfl_dev_attrs[] = {
+	&dev_attr_type.attr,
+	&dev_attr_feature_id.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(dfl_dev);
+
+static struct bus_type dfl_bus_type = {
+	.name		= "dfl",
+	.match		= dfl_bus_match,
+	.probe		= dfl_bus_probe,
+	.remove		= dfl_bus_remove,
+	.uevent		= dfl_bus_uevent,
+	.dev_groups	= dfl_dev_groups,
+};
+
+static void release_dfl_dev(struct device *dev)
+{
+	struct dfl_device *ddev = to_dfl_dev(dev);
+
+	if (ddev->mmio_res.parent)
+		release_resource(&ddev->mmio_res);
+
+	ida_simple_remove(&dfl_device_ida, ddev->id);
+	kfree(ddev->irqs);
+	kfree(ddev);
+}
+
+static struct dfl_device *
+dfl_dev_add(struct dfl_feature_platform_data *pdata,
+	    struct dfl_feature *feature)
+{
+	struct platform_device *pdev = pdata->dev;
+	struct resource *parent_res;
+	struct dfl_device *ddev;
+	int id, i, ret;
+
+	ddev = kzalloc(sizeof(*ddev), GFP_KERNEL);
+	if (!ddev)
+		return ERR_PTR(-ENOMEM);
+
+	id = ida_simple_get(&dfl_device_ida, 0, 0, GFP_KERNEL);
+	if (id < 0) {
+		dev_err(&pdev->dev, "unable to get id\n");
+		kfree(ddev);
+		return ERR_PTR(id);
+	}
+
+	/* freeing resources by put_device() after device_initialize() */
+	device_initialize(&ddev->dev);
+	ddev->dev.parent = &pdev->dev;
+	ddev->dev.bus = &dfl_bus_type;
+	ddev->dev.release = release_dfl_dev;
+	ddev->id = id;
+	ret = dev_set_name(&ddev->dev, "dfl_dev.%d", id);
+	if (ret)
+		goto put_dev;
+
+	ddev->type = feature_dev_id_type(pdev);
+	ddev->feature_id = feature->id;
+	ddev->cdev = pdata->dfl_cdev;
+
+	/* add mmio resource */
+	parent_res = &pdev->resource[feature->resource_index];
+	ddev->mmio_res.flags = IORESOURCE_MEM;
+	ddev->mmio_res.start = parent_res->start;
+	ddev->mmio_res.end = parent_res->end;
+	ddev->mmio_res.name = dev_name(&ddev->dev);
+	ret = insert_resource(parent_res, &ddev->mmio_res);
+	if (ret) {
+		dev_err(&pdev->dev, "%s failed to claim resource: %pR\n",
+			dev_name(&ddev->dev), &ddev->mmio_res);
+		goto put_dev;
+	}
+
+	/* then add irq resource */
+	if (feature->nr_irqs) {
+		ddev->irqs = kcalloc(feature->nr_irqs,
+				     sizeof(*ddev->irqs), GFP_KERNEL);
+		if (!ddev->irqs) {
+			ret = -ENOMEM;
+			goto put_dev;
+		}
+
+		for (i = 0; i < feature->nr_irqs; i++)
+			ddev->irqs[i] = feature->irq_ctx[i].irq;
+
+		ddev->num_irqs = feature->nr_irqs;
+	}
+
+	ret = device_add(&ddev->dev);
+	if (ret)
+		goto put_dev;
+
+	dev_info(&pdev->dev, "add dfl_dev: %s\n", dev_name(&ddev->dev));
+	return ddev;
+
+put_dev:
+	/* calls release_dfl_dev() which does the clean up  */
+	put_device(&ddev->dev);
+	return ERR_PTR(ret);
+}
+
+static void dfl_devs_remove(struct dfl_feature_platform_data *pdata)
+{
+	struct dfl_feature *feature;
+
+	dfl_fpga_dev_for_each_feature(pdata, feature) {
+		if (feature->ddev) {
+			device_unregister(&feature->ddev->dev);
+			feature->ddev = NULL;
+		}
+	}
+}
+
+static int dfl_devs_add(struct dfl_feature_platform_data *pdata)
+{
+	struct dfl_feature *feature;
+	struct dfl_device *ddev;
+	int ret;
+
+	dfl_fpga_dev_for_each_feature(pdata, feature) {
+		if (feature->ioaddr)
+			continue;
+
+		if (feature->ddev) {
+			ret = -EEXIST;
+			goto err;
+		}
+
+		ddev = dfl_dev_add(pdata, feature);
+		if (IS_ERR(ddev)) {
+			ret = PTR_ERR(ddev);
+			goto err;
+		}
+
+		feature->ddev = ddev;
+	}
+
+	return 0;
+
+err:
+	dfl_devs_remove(pdata);
+	return ret;
+}
+
+int __dfl_driver_register(struct dfl_driver *dfl_drv, struct module *owner)
+{
+	if (!dfl_drv || !dfl_drv->probe || !dfl_drv->id_table)
+		return -EINVAL;
+
+	dfl_drv->drv.owner = owner;
+	dfl_drv->drv.bus = &dfl_bus_type;
+
+	return driver_register(&dfl_drv->drv);
+}
+EXPORT_SYMBOL(__dfl_driver_register);
+
+void dfl_driver_unregister(struct dfl_driver *dfl_drv)
+{
+	driver_unregister(&dfl_drv->drv);
+}
+EXPORT_SYMBOL(dfl_driver_unregister);
+
 #define is_header_feature(feature) ((feature)->id == FEATURE_ID_FIU_HEADER)
 
 /**
@@ -261,12 +493,15 @@ void dfl_fpga_dev_feature_uinit(struct platform_device *pdev)
 	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct dfl_feature *feature;
 
-	dfl_fpga_dev_for_each_feature(pdata, feature)
+	dfl_devs_remove(pdata);
+
+	dfl_fpga_dev_for_each_feature(pdata, feature) {
 		if (feature->ops) {
 			if (feature->ops->uinit)
 				feature->ops->uinit(pdev, feature);
 			feature->ops = NULL;
 		}
+	}
 }
 EXPORT_SYMBOL_GPL(dfl_fpga_dev_feature_uinit);
 
@@ -347,6 +582,10 @@ int dfl_fpga_dev_feature_init(struct platform_device *pdev,
 		drv++;
 	}
 
+	ret = dfl_devs_add(pdata);
+	if (ret)
+		goto exit;
+
 	return 0;
 exit:
 	dfl_fpga_dev_feature_uinit(pdev);
@@ -1284,11 +1523,17 @@ static int __init dfl_fpga_init(void)
 {
 	int ret;
 
+	ret = bus_register(&dfl_bus_type);
+	if (ret)
+		return ret;
+
 	dfl_ids_init();
 
 	ret = dfl_chardev_init();
-	if (ret)
+	if (ret) {
 		dfl_ids_destroy();
+		bus_unregister(&dfl_bus_type);
+	}
 
 	return ret;
 }
@@ -1626,6 +1871,7 @@ static void __exit dfl_fpga_exit(void)
 {
 	dfl_chardev_uinit();
 	dfl_ids_destroy();
+	bus_unregister(&dfl_bus_type);
 }
 
 module_init(dfl_fpga_init);
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 5973769..5dc758f 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -236,6 +236,7 @@ struct dfl_feature_irq_ctx {
  * @irq_ctx: interrupt context list.
  * @nr_irqs: number of interrupt contexts.
  * @ops: ops of this sub feature.
+ * @ddev: ptr to the dfl device of this sub feature.
  * @priv: priv data of this feature.
  */
 struct dfl_feature {
@@ -246,6 +247,7 @@ struct dfl_feature {
 	struct dfl_feature_irq_ctx *irq_ctx;
 	unsigned int nr_irqs;
 	const struct dfl_feature_ops *ops;
+	struct dfl_device *ddev;
 	void *priv;
 };
 
@@ -514,4 +516,88 @@ 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_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.
+ * @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 */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v7 3/3] fpga: dfl: add support for N3000 Nios private feature
  2020-08-19  7:45 [PATCH v7 0/3] Modularization of DFL private feature drivers Xu Yilun
  2020-08-19  7:45 ` [PATCH v7 1/3] fpga: dfl: map feature mmio resources in their own " Xu Yilun
  2020-08-19  7:45 ` [PATCH v7 2/3] fpga: dfl: create a dfl bus type to support DFL devices Xu Yilun
@ 2020-08-19  7:45 ` Xu Yilun
  2020-09-05  0:45   ` Moritz Fischer
  2020-08-20  4:16 ` [PATCH v7 0/3] Modularization of DFL private feature drivers Moritz Fischer
  3 siblings, 1 reply; 9+ messages in thread
From: Xu Yilun @ 2020-08-19  7:45 UTC (permalink / raw)
  To: mdf, linux-fpga, linux-kernel
  Cc: trix, lgoncalv, Xu Yilun, Wu Hao, Matthew Gerlach, Russ Weight

This patch adds support for the Nios handshake private feature on Intel
PAC (Programmable Acceleration Card) N3000.

The Nios is the embedded processor on the FPGA card. This private feature
provides a handshake interface to FPGA Nois firmware, which receives
retimer configuration command from host and executes via an internal SPI
master (spi-altera). When Nios finishes the configuration, host takes over
the ownership of the SPI master to control an Intel MAX10 BMC (Board
Management Controller) Chip on the SPI bus.

For Nios firmware handshake part, this driver requests the retimer
configuration for Nios with parameters from module param, and adds some
sysfs nodes for user to query the onboard retimer's working mode and
Nios firmware version.

For SPI part, this driver adds a spi-altera platform device as well as
the MAX10 BMC spi slave info. A spi-altera driver will be matched to
handle the following SPI work.

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>
Reviewed-by: Tom Rix <trix@redhat.com>
Acked-by: Wu Hao <hao.wu@intel.com>
---
v3: Add the doc for this driver
    Minor fixes for comments from Tom
v4: Move the err log in regmap implementation, and delete
     n3000_nios_writel/readl(), they have nothing to wrapper now.
    Some minor fixes and comments improvement.
v5: Fix the output of fec_mode sysfs inf to "no" on 10G configuration,
     cause no FEC mode could be configured for 10G.
    Rename the dfl_n3000_nios_* to n3000_nios_*
    Improves comments.
v6: Fix the output of fec_mode sysfs inf to "not supported" if in 10G,
     or the firmware version major < 3.
    Minor fixes and improves comments.
v7: Improves comments.
---
 .../ABI/testing/sysfs-bus-dfl-devices-n3000-nios   |  21 +
 Documentation/fpga/dfl-n3000-nios.rst              |  80 +++
 Documentation/fpga/index.rst                       |   1 +
 drivers/fpga/Kconfig                               |  11 +
 drivers/fpga/Makefile                              |   2 +
 drivers/fpga/dfl-n3000-nios.c                      | 542 +++++++++++++++++++++
 6 files changed, 657 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
 create mode 100644 Documentation/fpga/dfl-n3000-nios.rst
 create mode 100644 drivers/fpga/dfl-n3000-nios.c

diff --git a/Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios b/Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
new file mode 100644
index 0000000..ce5b474
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
@@ -0,0 +1,21 @@
+What:		/sys/bus/dfl/devices/dfl_dev.X/fec_mode
+Date:		Aug 2020
+KernelVersion:	5.10
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:	Read-only. It returns the FEC mode of the 25G links of the
+		ethernet retimers configured by NIOS firmware. "rs" for Reed
+		Solomon FEC, "kr" for Fire Code FEC, "no" for NO FEC.
+		"not supported" if the FEC mode setting is not supported, this
+		happens when the Nios firmware version major < 3, or no link is
+		configured to 25G. The FEC mode could be set by module
+		parameters, but it could only be set once after the board
+		powers up.
+		Format: string
+
+What:		/sys/bus/dfl/devices/dfl_dev.X/nios_fw_version
+Date:		Aug 2020
+KernelVersion:	5.10
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:	Read-only. It returns the version of the NIOS firmware in FPGA.
+		Its format is "major.minor.patch".
+		Format: %x.%x.%x
diff --git a/Documentation/fpga/dfl-n3000-nios.rst b/Documentation/fpga/dfl-n3000-nios.rst
new file mode 100644
index 0000000..72dd600
--- /dev/null
+++ b/Documentation/fpga/dfl-n3000-nios.rst
@@ -0,0 +1,80 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================================
+N3000 Nios Private Feature Driver
+=================================
+
+The N3000 Nios driver supports for the Nios handshake private feature on Intel
+PAC (Programmable Acceleration Card) N3000.
+
+The Nios is the embedded processor in the FPGA, it will configure the 2 onboard
+ethernet retimers on power up. This private feature provides a handshake
+interface to FPGA Nios firmware, which receives the ethernet retimer
+configuration command from host and does the configuration via an internal SPI
+master (spi-altera). When Nios finishes the configuration, host takes over the
+ownership of the SPI master to control an Intel MAX10 BMC (Board Management
+Controller) Chip on the SPI bus.
+
+So the driver does 2 major tasks on probe, uses the Nios firmware to configure
+the ethernet retimer, and then creates a spi master platform device with the
+MAX10 device info in spi_board_info.
+
+
+Configuring the ethernet retimer
+================================
+
+The Intel PAC N3000 is a FPGA based SmartNIC platform which could be programmed
+to various configurations (with different link numbers and speeds, e.g. 8x10G,
+4x25G ...). And the retimer chips should also be configured correspondingly by
+Nios firmware. There are 2 retimer chips on the board, each of them supports 4
+links. For example, in 8x10G configuration, the 2 retimer chips are both set to
+4x10G mode, while in 4x25G configuration, retimer A is set to 4x25G and retimer
+B is in reset. For now, the Nios firmware only supports 10G and 25G mode
+setting for the retimer chips.
+
+For all 25G links, their FEC (Forward Error Correction) mode could be further
+configured by Nios firmware for user's requirement. For 10G links, they don't
+have the FEC mode at all, the firmware ignores the FEC mode setting for them.
+The FEC setting is not supported if the firmware version major < 3.
+
+The retimer configuration can only be done once after the board powers up, the
+Nios firmware will not accept second configuration afterward. So it is not
+proper for the driver to create a RW sysfs node for the FEC mode. A better way
+is that the driver accepts a module parameter for the FEC mode, and does the
+retimer configuration on driver probe, it also creates a RO sysfs node for the
+FEC mode query.
+
+Module Parameters
+=================
+
+The N3000 Nios driver supports the following module parameters:
+
+* fec_mode: string
+  Require the Nios firmware to set the FEC mode for all 25G links of the
+  ethernet retimers. The Nios firmware configures all these links with the same
+  FEC mode. The possible values of fec_mode could be:
+
+  - "rs": Reed Solomon FEC (default)
+  - "kr": Fire Code FEC
+  - "no": No FEC
+
+  Since the firmware doesn't accept second configuration, The FEC mode will not
+  be changed if the module is reloaded with a different parameter value.
+
+  The parameter has no effect for 10G links. It has no effect to all the links
+  if firmware version major < 3.
+
+
+Sysfs Attributes
+================
+
+The driver creates the sysfs file in /sys/bus/dfl/devices/dfl_dev.X/
+
+* fec_mode:
+  Read-only. It shows the FEC mode of the 25G links of the ethernet retimers.
+  The possible values are the same as the fec_mode module parameter. An extra
+  value "not supported" is returned if firmware version major < 3, or no link
+  is configured to 25G.
+
+* nios_fw_version:
+  Read-only. It shows the version of the Nios firmware in FPGA.
diff --git a/Documentation/fpga/index.rst b/Documentation/fpga/index.rst
index f80f956..5fd3c37 100644
--- a/Documentation/fpga/index.rst
+++ b/Documentation/fpga/index.rst
@@ -8,6 +8,7 @@ fpga
     :maxdepth: 1
 
     dfl
+    dfl-n3000-nios
 
 .. only::  subproject and html
 
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 7cd5a29..38c7130 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -191,6 +191,17 @@ config FPGA_DFL_AFU
 	  to the FPGA infrastructure via a Port. There may be more than one
 	  Port/AFU per DFL based FPGA device.
 
+config FPGA_DFL_NIOS_INTEL_PAC_N3000
+        tristate "FPGA DFL NIOS Driver for Intel PAC N3000"
+        depends on FPGA_DFL
+        select REGMAP
+        help
+	  This is the driver for the N3000 Nios private feature on Intel
+	  PAC (Programmable Acceleration Card) N3000. It communicates
+	  with the embedded Nios processor to configure the retimers on
+	  the card. It also instantiates the SPI master (spi-altera) for
+	  the card's BMC (Board Management Controller) control.
+
 config FPGA_DFL_PCI
 	tristate "FPGA DFL PCIe Device Driver"
 	depends on PCI && FPGA_DFL
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index d8e21df..18dc9885 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -44,5 +44,7 @@ dfl-fme-objs += dfl-fme-perf.o
 dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
 dfl-afu-objs += dfl-afu-error.o
 
+obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
+
 # Drivers for FPGAs which implement DFL
 obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
diff --git a/drivers/fpga/dfl-n3000-nios.c b/drivers/fpga/dfl-n3000-nios.c
new file mode 100644
index 0000000..c73ce38
--- /dev/null
+++ b/drivers/fpga/dfl-n3000-nios.c
@@ -0,0 +1,542 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DFL device driver for Nios private feature on Intel PAC (Programmable
+ * Acceleration Card) N3000
+ *
+ * Copyright (C) 2019-2020 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Wu Hao <hao.wu@intel.com>
+ *   Xu Yilun <yilun.xu@intel.com>
+ */
+#include <linux/bitfield.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/stddef.h>
+#include <linux/spi/altera.h>
+#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");
+
+/* N3000 Nios private feature registers */
+#define NIOS_SPI_PARAM			0x8
+#define PARAM_SHIFT_MODE_MSK		BIT_ULL(1)
+#define PARAM_SHIFT_MODE_MSB		0
+#define PARAM_SHIFT_MODE_LSB		1
+#define PARAM_DATA_WIDTH		GENMASK_ULL(7, 2)
+#define PARAM_NUM_CS			GENMASK_ULL(13, 8)
+#define PARAM_CLK_POL			BIT_ULL(14)
+#define PARAM_CLK_PHASE			BIT_ULL(15)
+#define PARAM_PERIPHERAL_ID		GENMASK_ULL(47, 32)
+
+#define NIOS_SPI_CTRL			0x10
+#define CTRL_WR_DATA			GENMASK_ULL(31, 0)
+#define CTRL_ADDR			GENMASK_ULL(44, 32)
+#define CTRL_CMD_MSK			GENMASK_ULL(63, 62)
+#define CTRL_CMD_NOP			0
+#define CTRL_CMD_RD			1
+#define CTRL_CMD_WR			2
+
+#define NIOS_SPI_STAT			0x18
+#define STAT_RD_DATA			GENMASK_ULL(31, 0)
+#define STAT_RW_VAL			BIT_ULL(32)
+
+/* Nios handshake registers, indirect access */
+#define NIOS_INIT			0x1000
+#define NIOS_INIT_DONE			BIT(0)
+#define NIOS_INIT_START			BIT(1)
+/* Mode for retimer A, link 0, the same below */
+#define REQ_FEC_MODE_A0_MSK		GENMASK(9, 8)
+#define REQ_FEC_MODE_A1_MSK		GENMASK(11, 10)
+#define REQ_FEC_MODE_A2_MSK		GENMASK(13, 12)
+#define REQ_FEC_MODE_A3_MSK		GENMASK(15, 14)
+#define REQ_FEC_MODE_B0_MSK		GENMASK(17, 16)
+#define REQ_FEC_MODE_B1_MSK		GENMASK(19, 18)
+#define REQ_FEC_MODE_B2_MSK		GENMASK(21, 20)
+#define REQ_FEC_MODE_B3_MSK		GENMASK(23, 22)
+#define REQ_FEC_MODE_NO			0x0
+#define REQ_FEC_MODE_KR			0x1
+#define REQ_FEC_MODE_RS			0x2
+
+#define NIOS_FW_VERSION			0x1004
+#define NIOS_FW_VERSION_PATCH		GENMASK(23, 20)
+#define NIOS_FW_VERSION_MINOR		GENMASK(27, 24)
+#define NIOS_FW_VERSION_MAJOR		GENMASK(31, 28)
+
+/* The retimers we use on Intel PAC N3000 is Parkvale, abbreviated to PKVL */
+#define PKVL_A_MODE_STS			0x1020
+#define PKVL_B_MODE_STS			0x1024
+#define PKVL_MODE_STS_GROUP_MSK		GENMASK(15, 8)
+#define PKVL_MODE_STS_GROUP_OK		0x0
+#define PKVL_MODE_STS_ID_MSK		GENMASK(7, 0)
+/* When GROUP MASK field == GROUP_OK  */
+#define PKVL_MODE_ID_RESET		0x0
+#define PKVL_MODE_ID_4X10G		0x1
+#define PKVL_MODE_ID_4X25G		0x2
+#define PKVL_MODE_ID_2X25G		0x3
+#define PKVL_MODE_ID_2X25G_2X10G	0x4
+#define PKVL_MODE_ID_1X25G		0x5
+
+#define NS_REGBUS_WAIT_TIMEOUT		10000		/* loop count */
+#define NIOS_INIT_TIMEOUT		10000000	/* usec */
+#define NIOS_INIT_TIME_INTV		100000		/* usec */
+
+struct n3000_nios {
+	void __iomem *base;
+	struct regmap *regmap;
+	struct device *dev;
+	struct platform_device *altera_spi;
+};
+
+static ssize_t nios_fw_version_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct n3000_nios *ns = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(ns->regmap, NIOS_FW_VERSION, &val);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%x.%x.%x\n",
+		       (u8)FIELD_GET(NIOS_FW_VERSION_MAJOR, val),
+		       (u8)FIELD_GET(NIOS_FW_VERSION_MINOR, val),
+		       (u8)FIELD_GET(NIOS_FW_VERSION_PATCH, val));
+}
+static DEVICE_ATTR_RO(nios_fw_version);
+
+#define IS_MODE_STATUS_OK(mode_stat)				\
+	(FIELD_GET(PKVL_MODE_STS_GROUP_MSK, (mode_stat)) ==	\
+	 PKVL_MODE_STS_GROUP_OK)
+
+#define IS_RETIMER_FEC_SUPPORTED(retimer_mode)	\
+	((retimer_mode) != PKVL_MODE_ID_RESET &&	\
+	 (retimer_mode) != PKVL_MODE_ID_4X10G)
+
+static int get_retimer_mode(struct n3000_nios *ns, unsigned int mode_stat_reg,
+			    unsigned int *retimer_mode)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(ns->regmap, mode_stat_reg, &val);
+	if (ret)
+		return ret;
+
+	if (!IS_MODE_STATUS_OK(val))
+		return -EFAULT;
+
+	*retimer_mode = FIELD_GET(PKVL_MODE_STS_ID_MSK, val);
+
+	return 0;
+}
+
+static ssize_t fec_mode_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct n3000_nios *ns = dev_get_drvdata(dev);
+	unsigned int val, retimer_a_mode, retimer_b_mode, fec_mode;
+	int ret;
+
+	/* FEC mode setting is not supported in early FW versions */
+	ret = regmap_read(ns->regmap, NIOS_FW_VERSION, &val);
+	if (ret)
+		return ret;
+
+	if (FIELD_GET(NIOS_FW_VERSION_MAJOR, val) < 3)
+		return sprintf(buf, "not supported\n");
+
+	/* If no 25G links, FEC mode setting is not supported either */
+	ret = get_retimer_mode(ns, PKVL_A_MODE_STS, &retimer_a_mode);
+	if (ret)
+		return ret;
+
+	ret = get_retimer_mode(ns, PKVL_B_MODE_STS, &retimer_b_mode);
+	if (ret)
+		return ret;
+
+	if (!IS_RETIMER_FEC_SUPPORTED(retimer_a_mode) &&
+	    !IS_RETIMER_FEC_SUPPORTED(retimer_b_mode))
+		return sprintf(buf, "not supported\n");
+
+	/* get the valid FEC mode for 25G links */
+	ret = regmap_read(ns->regmap, NIOS_INIT, &val);
+	if (ret)
+		return ret;
+
+	/*
+	 * FEC mode should always be the same for all links, as we set them
+	 * in this way.
+	 */
+	fec_mode = FIELD_GET(REQ_FEC_MODE_A0_MSK, val);
+	if (fec_mode != FIELD_GET(REQ_FEC_MODE_A1_MSK, val) ||
+	    fec_mode != FIELD_GET(REQ_FEC_MODE_A2_MSK, val) ||
+	    fec_mode != FIELD_GET(REQ_FEC_MODE_A3_MSK, val) ||
+	    fec_mode != FIELD_GET(REQ_FEC_MODE_B0_MSK, val) ||
+	    fec_mode != FIELD_GET(REQ_FEC_MODE_B1_MSK, val) ||
+	    fec_mode != FIELD_GET(REQ_FEC_MODE_B2_MSK, val) ||
+	    fec_mode != FIELD_GET(REQ_FEC_MODE_B3_MSK, val))
+		return -EFAULT;
+
+	switch (fec_mode) {
+	case REQ_FEC_MODE_NO:
+		return sprintf(buf, "no\n");
+	case REQ_FEC_MODE_KR:
+		return sprintf(buf, "kr\n");
+	case REQ_FEC_MODE_RS:
+		return sprintf(buf, "rs\n");
+	}
+
+	return -EFAULT;
+}
+static DEVICE_ATTR_RO(fec_mode);
+
+static struct attribute *n3000_nios_attrs[] = {
+	&dev_attr_nios_fw_version.attr,
+	&dev_attr_fec_mode.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(n3000_nios);
+
+static int init_error_detected(struct n3000_nios *ns)
+{
+	unsigned int val;
+
+	if (regmap_read(ns->regmap, PKVL_A_MODE_STS, &val))
+		return true;
+
+	if (!IS_MODE_STATUS_OK(val))
+		return true;
+
+	if (regmap_read(ns->regmap, PKVL_B_MODE_STS, &val))
+		return true;
+
+	if (!IS_MODE_STATUS_OK(val))
+		return true;
+
+	return false;
+}
+
+static void dump_error_stat(struct n3000_nios *ns)
+{
+	unsigned int val;
+
+	if (regmap_read(ns->regmap, PKVL_A_MODE_STS, &val))
+		return;
+
+	dev_err(ns->dev, "PKVL_A_MODE_STS 0x%x\n", val);
+
+	if (regmap_read(ns->regmap, PKVL_B_MODE_STS, &val))
+		return;
+
+	dev_err(ns->dev, "PKVL_B_MODE_STS 0x%x\n", val);
+}
+
+static int n3000_nios_init_done_check(struct n3000_nios *ns)
+{
+	struct device *dev = ns->dev;
+	unsigned int val, mode;
+	int ret;
+
+	/*
+	 * this SPI is shared by Nios core inside FPGA, Nios will use this SPI
+	 * master to do some one time initialization after power up, and then
+	 * release the control to OS. driver needs to poll on INIT_DONE to
+	 * see when driver could take the control.
+	 *
+	 * Please note that after Nios firmware version 3.0.0, INIT_START is
+	 * introduced, so driver needs to trigger START firstly and then check
+	 * INIT_DONE.
+	 */
+
+	ret = regmap_read(ns->regmap, NIOS_FW_VERSION, &val);
+	if (ret)
+		return ret;
+
+	/*
+	 * If Nios version register is totally uninitialized(== 0x0), then the
+	 * Nios firmware is missing. So host could take control of SPI master
+	 * safely, but initialization work for Nios is not done. To restore the
+	 * card, we need to reprogram a new Nios firmware via the BMC chip on
+	 * SPI bus. So the driver doesn't error out, it continues to create the
+	 * spi controller device and spi_board_info for BMC.
+	 */
+	if (val == 0) {
+		dev_err(dev, "Nios version reg = 0x%x, skip INIT_DONE check, but the retimer may be uninitialized\n",
+			val);
+		return 0;
+	}
+
+	if (FIELD_GET(NIOS_FW_VERSION_MAJOR, val) >= 3) {
+		/* read NIOS_INIT to check if retimer initialization is done */
+		ret = regmap_read(ns->regmap, NIOS_INIT, &val);
+		if (ret)
+			return ret;
+
+		/* check if retimers are initialized already */
+		if (val & NIOS_INIT_DONE || val & NIOS_INIT_START)
+			goto nios_init_done;
+
+		/* configure FEC mode per module param */
+		val = NIOS_INIT_START;
+
+		/*
+		 * When the retimer is to be set to 10G mode, there is no FEC
+		 * mode setting, so the REQ_FEC_MODE field will be ignored by
+		 * Nios firmware in this case. But we should still fill the FEC
+		 * mode field cause host could not get the retimer working mode
+		 * until the Nios init is done.
+		 */
+		if (!strcmp(fec_mode, "no"))
+			mode = REQ_FEC_MODE_NO;
+		else if (!strcmp(fec_mode, "kr"))
+			mode = REQ_FEC_MODE_KR;
+		else if (!strcmp(fec_mode, "rs"))
+			mode = REQ_FEC_MODE_RS;
+		else
+			return -EINVAL;
+
+		/* set the same FEC mode for all links */
+		val |= FIELD_PREP(REQ_FEC_MODE_A0_MSK, mode) |
+		       FIELD_PREP(REQ_FEC_MODE_A1_MSK, mode) |
+		       FIELD_PREP(REQ_FEC_MODE_A2_MSK, mode) |
+		       FIELD_PREP(REQ_FEC_MODE_A3_MSK, mode) |
+		       FIELD_PREP(REQ_FEC_MODE_B0_MSK, mode) |
+		       FIELD_PREP(REQ_FEC_MODE_B1_MSK, mode) |
+		       FIELD_PREP(REQ_FEC_MODE_B2_MSK, mode) |
+		       FIELD_PREP(REQ_FEC_MODE_B3_MSK, mode);
+
+		ret = regmap_write(ns->regmap, NIOS_INIT, val);
+		if (ret)
+			return ret;
+	}
+
+nios_init_done:
+	/* polls on NIOS_INIT_DONE */
+	ret = regmap_read_poll_timeout(ns->regmap, NIOS_INIT, val,
+				       val & NIOS_INIT_DONE,
+				       NIOS_INIT_TIME_INTV,
+				       NIOS_INIT_TIMEOUT);
+	if (ret) {
+		dev_err(dev, "NIOS_INIT_DONE %s\n",
+			(ret == -ETIMEDOUT) ? "timed out" : "check error");
+		goto dump_sts;
+	}
+
+	/*
+	 * After INIT_DONE is detected, it still needs to check if any error
+	 * detected.
+	 */
+	if (init_error_detected(ns)) {
+		/*
+		 * If the retimer configuration is failed, the Nios firmware
+		 * will still release the spi controller for host to
+		 * communicate with the BMC. It makes possible for people to
+		 * reprogram a new Nios firmware and restore the card. So the
+		 * driver doesn't error out, it continues to create the spi
+		 * controller device and spi_board_info for BMC.
+		 */
+		dev_err(dev, "NIOS_INIT_DONE OK, but err found during init\n");
+		goto dump_sts;
+	}
+	return 0;
+
+dump_sts:
+	dump_error_stat(ns);
+
+	return ret;
+}
+
+struct spi_board_info m10_n3000_info = {
+	.modalias = "m10-n3000",
+	.max_speed_hz = 12500000,
+	.bus_num = 0,
+	.chip_select = 0,
+};
+
+static int create_altera_spi_controller(struct n3000_nios *ns)
+{
+	struct altera_spi_platform_data pdata = { 0 };
+	struct platform_device_info pdevinfo = { 0 };
+	void __iomem *base = ns->base;
+	u64 v;
+
+	v = readq(base + NIOS_SPI_PARAM);
+
+	pdata.mode_bits = SPI_CS_HIGH;
+	if (FIELD_GET(PARAM_CLK_POL, v))
+		pdata.mode_bits |= SPI_CPOL;
+	if (FIELD_GET(PARAM_CLK_PHASE, v))
+		pdata.mode_bits |= SPI_CPHA;
+
+	pdata.num_chipselect = FIELD_GET(PARAM_NUM_CS, v);
+	pdata.bits_per_word_mask =
+		SPI_BPW_RANGE_MASK(1, FIELD_GET(PARAM_DATA_WIDTH, v));
+
+	pdata.num_devices = 1;
+	pdata.devices = &m10_n3000_info;
+
+	dev_dbg(ns->dev, "%s cs %u bpm 0x%x mode 0x%x\n", __func__,
+		pdata.num_chipselect, pdata.bits_per_word_mask,
+		pdata.mode_bits);
+
+	pdevinfo.name = "subdev_spi_altera";
+	pdevinfo.id = PLATFORM_DEVID_AUTO;
+	pdevinfo.parent = ns->dev;
+	pdevinfo.data = &pdata;
+	pdevinfo.size_data = sizeof(pdata);
+
+	ns->altera_spi = platform_device_register_full(&pdevinfo);
+	return PTR_ERR_OR_ZERO(ns->altera_spi);
+}
+
+static void destroy_altera_spi_controller(struct n3000_nios *ns)
+{
+	platform_device_unregister(ns->altera_spi);
+}
+
+/* ns is the abbreviation of nios_spi */
+static int ns_poll_stat_timeout(void __iomem *base, u64 *v)
+{
+	int loops = NS_REGBUS_WAIT_TIMEOUT;
+
+	/*
+	 * We don't use the time based timeout here for performance.
+	 *
+	 * The regbus read/write is on the critical path of Intel PAC N3000
+	 * image programing. The time based timeout checking will add too much
+	 * overhead on it. Usually the state changes in 1 or 2 loops on the
+	 * test server, and we set 10000 times loop here for safety.
+	 */
+	do {
+		*v = readq(base + NIOS_SPI_STAT);
+		if (*v & STAT_RW_VAL)
+			break;
+		cpu_relax();
+	} while (--loops);
+
+	return loops ? 0 : -ETIMEDOUT;
+}
+
+static int ns_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct n3000_nios *ns = context;
+	u64 v = 0;
+	int ret;
+
+	v |= FIELD_PREP(CTRL_CMD_MSK, CTRL_CMD_WR);
+	v |= FIELD_PREP(CTRL_ADDR, reg);
+	v |= FIELD_PREP(CTRL_WR_DATA, val);
+	writeq(v, ns->base + NIOS_SPI_CTRL);
+
+	ret = ns_poll_stat_timeout(ns->base, &v);
+	if (ret)
+		dev_err(ns->dev, "fail to write reg 0x%x val 0x%x: %d\n",
+			reg, val, ret);
+
+	return ret;
+}
+
+static int ns_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct n3000_nios *ns = context;
+	u64 v = 0;
+	int ret;
+
+	v |= FIELD_PREP(CTRL_CMD_MSK, CTRL_CMD_RD);
+	v |= FIELD_PREP(CTRL_ADDR, reg);
+	writeq(v, ns->base + NIOS_SPI_CTRL);
+
+	ret = ns_poll_stat_timeout(ns->base, &v);
+	if (ret)
+		dev_err(ns->dev, "fail to read reg 0x%x: %d\n", reg, ret);
+	else
+		*val = FIELD_GET(STAT_RD_DATA, v);
+
+	return ret;
+}
+
+static const struct regmap_config ns_regbus_cfg = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+
+	.reg_write = ns_reg_write,
+	.reg_read = ns_reg_read,
+};
+
+static int n3000_nios_probe(struct dfl_device *dfl_dev)
+{
+	struct device *dev = &dfl_dev->dev;
+	struct n3000_nios *ns;
+	int ret;
+
+	ns = devm_kzalloc(dev, sizeof(*ns), GFP_KERNEL);
+	if (!ns)
+		return -ENOMEM;
+
+	dev_set_drvdata(&dfl_dev->dev, ns);
+
+	ns->dev = dev;
+
+	ns->base = devm_ioremap_resource(&dfl_dev->dev, &dfl_dev->mmio_res);
+	if (IS_ERR(ns->base))
+		return PTR_ERR(ns->base);
+
+	ns->regmap = devm_regmap_init(dev, NULL, ns, &ns_regbus_cfg);
+	if (IS_ERR(ns->regmap))
+		return PTR_ERR(ns->regmap);
+
+	ret = n3000_nios_init_done_check(ns);
+	if (ret)
+		return ret;
+
+	ret = create_altera_spi_controller(ns);
+	if (ret)
+		dev_err(dev, "altera spi controller create failed: %d\n", ret);
+
+	return ret;
+}
+
+static void n3000_nios_remove(struct dfl_device *dfl_dev)
+{
+	struct n3000_nios *ns = dev_get_drvdata(&dfl_dev->dev);
+
+	destroy_altera_spi_controller(ns);
+}
+
+#define FME_FEATURE_ID_N3000_NIOS	0xd
+
+static const struct dfl_device_id n3000_nios_ids[] = {
+	{ FME_ID, FME_FEATURE_ID_N3000_NIOS },
+	{ }
+};
+
+static struct dfl_driver n3000_nios_driver = {
+	.drv	= {
+		.name       = "n3000-nios",
+		.dev_groups = n3000_nios_groups,
+	},
+	.id_table = n3000_nios_ids,
+	.probe   = n3000_nios_probe,
+	.remove  = n3000_nios_remove,
+};
+
+module_dfl_driver(n3000_nios_driver);
+
+MODULE_DEVICE_TABLE(dfl, n3000_nios_ids);
+MODULE_DESCRIPTION("Driver for Nios private feature on Intel PAC N3000");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v7 0/3] Modularization of DFL private feature drivers
  2020-08-19  7:45 [PATCH v7 0/3] Modularization of DFL private feature drivers Xu Yilun
                   ` (2 preceding siblings ...)
  2020-08-19  7:45 ` [PATCH v7 3/3] fpga: dfl: add support for N3000 Nios private feature Xu Yilun
@ 2020-08-20  4:16 ` Moritz Fischer
  3 siblings, 0 replies; 9+ messages in thread
From: Moritz Fischer @ 2020-08-20  4:16 UTC (permalink / raw)
  To: Xu Yilun; +Cc: mdf, linux-fpga, linux-kernel, trix, lgoncalv

Hi Xu,

On Wed, Aug 19, 2020 at 03:45:18PM +0800, Xu Yilun wrote:
> This patchset makes it possible to develop independent driver modules
> for DFL private features. It also helps to leverage existing kernel
> drivers to enable some IP blocks in DFL.
> 
> Patch #1: Release the dfl mmio regions after enumeration, so that private
>           feature drivers could request mmio region in their own drivers.
> Patch #2: Introduce the dfl bus, then dfl devices could be supported by
>           independent dfl drivers.
> Patch #3: An example of the dfl driver for N3000 nios private feature.
> 
> 
> Main changes from v1:
> - Add the new Patch #1, to improve the feature id definition.
> - Change the dfl bus uevent format.
> - Change the dfl device's sysfs name format.
> - refactor dfl_dev_add()
> - Add the Patch #4 as an example of the dfl driver.
> - A lot of minor fixes for comments from Hao and Tom.
> 
> Main changes from v2:
> - Add the doc for dfl-n3000-nios driver.
> - Minor fixes for comments from Tom.
> 
> Main changes from v3:
> - improve the dfl devices' uevent format, 4 bits for type & 12 bits for id
> - change dfl_device->type to u8
> - A dedicate field in struct dfl_feature for dfl device instance.
> - error out if dfl_device already exist on dfl_devs_init().
> - Move the err log in regmap implementation, and delete
>    n3000_nios_writel/readl(), they have nothing to wrapper now.
> - Minor fixes and comments improvement.
> 
> Main changes from v4:
> - The patch "fpga: dfl: change data type of feature id to u16" is already
>    applied to for-next
> - Unify the naming of some functions in dfl.c
> - Fix the output of fec_mode sysfs inf to "no" on 10G configuration, cause
>    no FEC mode could be configured for 10G.
> - Change the N3000 Nios driver name from "dfl-n3000-nios" to "n3000-nios",
>    and also rename some structures and functions from dfl_n3000_nios_* to
>    n3000_nios_*
> - Minor fixes and comments improvement.
> 
> Main changes from v5:
> - Fix the output of fec_mode sysfs inf to "not supported" if in 10G,
>    or the firmware version major < 3.
> - The input param of dfl_devs_add() changes to
>    struct dfl_feature_platform_data.
> - Minor fixes and improves comments.
> 
> Main changes from v6:
> - Rebased to 5.9-rc1.
> - Improves comments.
> 
> Xu Yilun (3):
>   fpga: dfl: map feature mmio resources in their own feature drivers
>   fpga: dfl: create a dfl bus type to support DFL devices
>   fpga: dfl: add support for N3000 Nios private feature
> 
>  Documentation/ABI/testing/sysfs-bus-dfl            |  15 +
>  .../ABI/testing/sysfs-bus-dfl-devices-n3000-nios   |  21 +
>  Documentation/fpga/dfl-n3000-nios.rst              |  80 +++
>  Documentation/fpga/index.rst                       |   1 +
>  drivers/fpga/Kconfig                               |  11 +
>  drivers/fpga/Makefile                              |   2 +
>  drivers/fpga/dfl-n3000-nios.c                      | 542 +++++++++++++++++++++
>  drivers/fpga/dfl-pci.c                             |  24 +-
>  drivers/fpga/dfl.c                                 | 449 ++++++++++++++---
>  drivers/fpga/dfl.h                                 |  93 +++-
>  10 files changed, 1153 insertions(+), 85 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
>  create mode 100644 Documentation/fpga/dfl-n3000-nios.rst
>  create mode 100644 drivers/fpga/dfl-n3000-nios.c
> 
> -- 
> 2.7.4
> 

I'll get to your series this week, sorry for the delay.

Thanks,
Moritz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7 1/3] fpga: dfl: map feature mmio resources in their own feature drivers
  2020-08-19  7:45 ` [PATCH v7 1/3] fpga: dfl: map feature mmio resources in their own " Xu Yilun
@ 2020-08-31  0:18   ` Moritz Fischer
  0 siblings, 0 replies; 9+ messages in thread
From: Moritz Fischer @ 2020-08-31  0:18 UTC (permalink / raw)
  To: Xu Yilun
  Cc: mdf, linux-fpga, linux-kernel, trix, lgoncalv, Wu Hao,
	Matthew Gerlach, Russ Weight

On Wed, Aug 19, 2020 at 03:45:19PM +0800, Xu Yilun wrote:
> This patch makes preparation for modularization of DFL sub feature
> drivers.
> 
> DFL based FPGA devices may contain some IP blocks which are already
> supported by kernel, most of them are supported by platform device
> drivers. We could create platform devices for these IP blocks and get them
> supported by these drivers.
> 
> An important issue is that platform device drivers usually requests mmio
> resources on probe. But now DFL mmio is mapped in DFL bus driver (e.g.
> dfl-pci) as a whole region. Then platform device drivers for sub features
> can't request their own mmio resources again. This is what the patch
> trying to resolve.
> 
> This patch changes the DFL enumeration. DFL bus driver will unmap mmio
> resources after first step enumeration and pass enumeration info to DFL
> framework. Then DFL framework will map the mmio resources again, do 2nd
> step enumeration, and also unmap the mmio resources. In this way, sub
> feature drivers could then request their own mmio resources as needed.
> 
> An exception is that mmio resource of FIU headers are still mapped in DFL
> bus driver. The FIU headers have some fundamental functions (sriov set,
> port enable/disable) needed for DFL bus devices and other sub features.
> They should not be unmapped as long as DFL bus device is alive.
> 
> 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>
> Reviewed-by: Tom Rix <trix@redhat.com>
> Acked-by: Wu Hao <hao.wu@intel.com>
> ---
> v2: delete dfl_binfo_shift() cause no shift is possible for FIU parsing.
>     Only map bar0 for first phase enumeration in dfl-pci, we can find
>       all dfl mmio resource info in bar0.
>     Some minor fixes for comments from Hao & Tom.
> v3: no change
> v4: minor fixes for Hao's comments.
> v5: improve some comments.
>     unify the naming of some functions.
> v6: no change.
> v7: no change.
> ---
>  drivers/fpga/dfl-pci.c |  24 +++----
>  drivers/fpga/dfl.c     | 187 ++++++++++++++++++++++++++++++++++---------------
>  drivers/fpga/dfl.h     |   7 +-
>  3 files changed, 141 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index e220bec..a2203d0 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -31,12 +31,12 @@ struct cci_drvdata {
>  	struct dfl_fpga_cdev *cdev;	/* container device */
>  };
>  
> -static void __iomem *cci_pci_ioremap_bar(struct pci_dev *pcidev, int bar)
> +static void __iomem *cci_pci_ioremap_bar0(struct pci_dev *pcidev)
>  {
> -	if (pcim_iomap_regions(pcidev, BIT(bar), DRV_NAME))
> +	if (pcim_iomap_regions(pcidev, BIT(0), DRV_NAME))
>  		return NULL;
>  
> -	return pcim_iomap_table(pcidev)[bar];
> +	return pcim_iomap_table(pcidev)[0];
>  }
>  
>  static int cci_pci_alloc_irq(struct pci_dev *pcidev)
> @@ -156,8 +156,8 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  			goto irq_free_exit;
>  	}
>  
> -	/* start to find Device Feature List from Bar 0 */
> -	base = cci_pci_ioremap_bar(pcidev, 0);
> +	/* start to find Device Feature List in Bar 0 */
> +	base = cci_pci_ioremap_bar0(pcidev);
>  	if (!base) {
>  		ret = -ENOMEM;
>  		goto irq_free_exit;
> @@ -172,7 +172,7 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  		start = pci_resource_start(pcidev, 0);
>  		len = pci_resource_len(pcidev, 0);
>  
> -		dfl_fpga_enum_info_add_dfl(info, start, len, base);
> +		dfl_fpga_enum_info_add_dfl(info, start, len);
>  
>  		/*
>  		 * find more Device Feature Lists (e.g. Ports) per information
> @@ -196,26 +196,24 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  			 */
>  			bar = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
>  			offset = FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
> -			base = cci_pci_ioremap_bar(pcidev, bar);
> -			if (!base)
> -				continue;
> -
>  			start = pci_resource_start(pcidev, bar) + offset;
>  			len = pci_resource_len(pcidev, bar) - offset;
>  
> -			dfl_fpga_enum_info_add_dfl(info, start, len,
> -						   base + offset);
> +			dfl_fpga_enum_info_add_dfl(info, start, len);
>  		}
>  	} else if (dfl_feature_is_port(base)) {
>  		start = pci_resource_start(pcidev, 0);
>  		len = pci_resource_len(pcidev, 0);
>  
> -		dfl_fpga_enum_info_add_dfl(info, start, len, base);
> +		dfl_fpga_enum_info_add_dfl(info, start, len);
>  	} else {
>  		ret = -ENODEV;
>  		goto irq_free_exit;
>  	}
>  
> +	/* release I/O mappings for next step enumeration */
> +	pcim_iounmap_regions(pcidev, BIT(0));
> +
>  	/* start enumeration with prepared enumeration information */
>  	cdev = dfl_fpga_feature_devs_enumerate(info);
>  	if (IS_ERR(cdev)) {
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 18575d9..52cafa2 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -250,6 +250,8 @@ int dfl_fpga_check_port_id(struct platform_device *pdev, void *pport_id)
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_check_port_id);
>  
> +#define is_header_feature(feature) ((feature)->id == FEATURE_ID_FIU_HEADER)
> +
>  /**
>   * dfl_fpga_dev_feature_uinit - uinit for sub features of dfl feature device
>   * @pdev: feature device.
> @@ -273,8 +275,22 @@ static int dfl_feature_instance_init(struct platform_device *pdev,
>  				     struct dfl_feature *feature,
>  				     struct dfl_feature_driver *drv)
>  {
> +	void __iomem *base;
>  	int ret = 0;
>  
> +	if (!is_header_feature(feature)) {
> +		base = devm_platform_ioremap_resource(pdev,
> +						      feature->resource_index);
> +		if (IS_ERR(base)) {
> +			dev_err(&pdev->dev,
> +				"ioremap failed for feature 0x%x!\n",
> +				feature->id);
> +			return PTR_ERR(base);
> +		}
> +
> +		feature->ioaddr = base;
> +	}
> +
>  	if (drv->ops->init) {
>  		ret = drv->ops->init(pdev, feature);
>  		if (ret)
> @@ -427,7 +443,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
>   * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
>   *	       this device.
>   * @feature_dev: current feature device.
> - * @ioaddr: header register region address of feature device in enumeration.
> + * @ioaddr: header register region address of current FIU in enumeration.
> + * @start: register resource start of current FIU.
> + * @len: max register resource length of current FIU.
>   * @sub_features: a sub features linked list for feature device in enumeration.
>   * @feature_num: number of sub features for feature device in enumeration.
>   */
> @@ -439,6 +457,8 @@ struct build_feature_devs_info {
>  
>  	struct platform_device *feature_dev;
>  	void __iomem *ioaddr;
> +	resource_size_t start;
> +	resource_size_t len;
>  	struct list_head sub_features;
>  	int feature_num;
>  };
> @@ -484,10 +504,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  	struct dfl_feature_platform_data *pdata;
>  	struct dfl_feature_info *finfo, *p;
>  	enum dfl_id_type type;
> -	int ret, index = 0;
> -
> -	if (!fdev)
> -		return 0;
> +	int ret, index = 0, res_idx = 0;
>  
>  	type = feature_dev_id_type(fdev);
>  	if (WARN_ON_ONCE(type >= DFL_ID_MAX))
> @@ -530,16 +547,32 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  
>  	/* fill features and resource information for feature dev */
>  	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
> -		struct dfl_feature *feature = &pdata->features[index];
> +		struct dfl_feature *feature = &pdata->features[index++];
>  		struct dfl_feature_irq_ctx *ctx;
>  		unsigned int i;
>  
>  		/* save resource information for each feature */
>  		feature->dev = fdev;
>  		feature->id = finfo->fid;
> -		feature->resource_index = index;
> -		feature->ioaddr = finfo->ioaddr;
> -		fdev->resource[index++] = finfo->mmio_res;
> +
> +		/*
> +		 * the FIU header feature has some fundamental functions (sriov
> +		 * set, port enable/disable) needed for the dfl bus device and
> +		 * other sub features. So its mmio resource should be mapped by
> +		 * DFL bus device. And we should not assign it to feature
> +		 * devices (dfl-fme/afu) again.
> +		 */
> +		if (is_header_feature(feature)) {
> +			feature->resource_index = -1;
> +			feature->ioaddr =
> +				devm_ioremap_resource(binfo->dev,
> +						      &finfo->mmio_res);
> +			if (IS_ERR(feature->ioaddr))
> +				return PTR_ERR(feature->ioaddr);
> +		} else {
> +			feature->resource_index = res_idx;
> +			fdev->resource[res_idx++] = finfo->mmio_res;
> +		}
>  
>  		if (finfo->nr_irqs) {
>  			ctx = devm_kcalloc(binfo->dev, finfo->nr_irqs,
> @@ -582,19 +615,13 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  
>  static int
>  build_info_create_dev(struct build_feature_devs_info *binfo,
> -		      enum dfl_id_type type, void __iomem *ioaddr)
> +		      enum dfl_id_type type)
>  {
>  	struct platform_device *fdev;
> -	int ret;
>  
>  	if (type >= DFL_ID_MAX)
>  		return -EINVAL;
>  
> -	/* we will create a new device, commit current device first */
> -	ret = build_info_commit_dev(binfo);
> -	if (ret)
> -		return ret;
> -
>  	/*
>  	 * we use -ENODEV as the initialization indicator which indicates
>  	 * whether the id need to be reclaimed
> @@ -605,7 +632,7 @@ build_info_create_dev(struct build_feature_devs_info *binfo,
>  
>  	binfo->feature_dev = fdev;
>  	binfo->feature_num = 0;
> -	binfo->ioaddr = ioaddr;
> +
>  	INIT_LIST_HEAD(&binfo->sub_features);
>  
>  	fdev->id = dfl_id_alloc(type, &fdev->dev);
> @@ -747,18 +774,17 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo,
>   */
>  static int
>  create_feature_instance(struct build_feature_devs_info *binfo,
> -			struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst,
> -			resource_size_t size, u16 fid)
> +			resource_size_t ofst, resource_size_t size, u16 fid)
>  {
>  	unsigned int irq_base, nr_irqs;
>  	struct dfl_feature_info *finfo;
>  	int ret;
>  
>  	/* read feature size and id if inputs are invalid */
> -	size = size ? size : feature_size(dfl->ioaddr + ofst);
> -	fid = fid ? fid : feature_id(dfl->ioaddr + ofst);
> +	size = size ? size : feature_size(binfo->ioaddr + ofst);
> +	fid = fid ? fid : feature_id(binfo->ioaddr + ofst);
>  
> -	if (dfl->len - ofst < size)
> +	if (binfo->len - ofst < size)
>  		return -EINVAL;
>  
>  	ret = parse_feature_irqs(binfo, ofst, fid, &irq_base, &nr_irqs);
> @@ -770,12 +796,11 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  		return -ENOMEM;
>  
>  	finfo->fid = fid;
> -	finfo->mmio_res.start = dfl->start + ofst;
> +	finfo->mmio_res.start = binfo->start + ofst;
>  	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>  	finfo->mmio_res.flags = IORESOURCE_MEM;
>  	finfo->irq_base = irq_base;
>  	finfo->nr_irqs = nr_irqs;
> -	finfo->ioaddr = dfl->ioaddr + ofst;
>  
>  	list_add_tail(&finfo->node, &binfo->sub_features);
>  	binfo->feature_num++;
> @@ -784,7 +809,6 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  }
>  
>  static int parse_feature_port_afu(struct build_feature_devs_info *binfo,
> -				  struct dfl_fpga_enum_dfl *dfl,
>  				  resource_size_t ofst)
>  {
>  	u64 v = readq(binfo->ioaddr + PORT_HDR_CAP);
> @@ -792,21 +816,22 @@ static int parse_feature_port_afu(struct build_feature_devs_info *binfo,
>  
>  	WARN_ON(!size);
>  
> -	return create_feature_instance(binfo, dfl, ofst, size, FEATURE_ID_AFU);
> +	return create_feature_instance(binfo, ofst, size, FEATURE_ID_AFU);
>  }
>  
> +#define is_feature_dev_detected(binfo) (!!(binfo)->feature_dev)
> +
>  static int parse_feature_afu(struct build_feature_devs_info *binfo,
> -			     struct dfl_fpga_enum_dfl *dfl,
>  			     resource_size_t ofst)
>  {
> -	if (!binfo->feature_dev) {
> +	if (!is_feature_dev_detected(binfo)) {
>  		dev_err(binfo->dev, "this AFU does not belong to any FIU.\n");
>  		return -EINVAL;
>  	}
>  
>  	switch (feature_dev_id_type(binfo->feature_dev)) {
>  	case PORT_ID:
> -		return parse_feature_port_afu(binfo, dfl, ofst);
> +		return parse_feature_port_afu(binfo, ofst);
>  	default:
>  		dev_info(binfo->dev, "AFU belonging to FIU %s is not supported yet.\n",
>  			 binfo->feature_dev->name);
> @@ -815,8 +840,39 @@ static int parse_feature_afu(struct build_feature_devs_info *binfo,
>  	return 0;
>  }
>  
> +static int build_info_prepare(struct build_feature_devs_info *binfo,
> +			      resource_size_t start, resource_size_t len)
> +{
> +	struct device *dev = binfo->dev;
> +	void __iomem *ioaddr;
> +
> +	if (!devm_request_mem_region(dev, start, len, dev_name(dev))) {
> +		dev_err(dev, "request region fail, start:%pa, len:%pa\n",
> +			&start, &len);
> +		return -EBUSY;
> +	}
> +
> +	ioaddr = devm_ioremap(dev, start, len);
> +	if (!ioaddr) {
> +		dev_err(dev, "ioremap region fail, start:%pa, len:%pa\n",
> +			&start, &len);
> +		return -ENOMEM;
> +	}
> +
> +	binfo->start = start;
> +	binfo->len = len;
> +	binfo->ioaddr = ioaddr;
> +
> +	return 0;
> +}
> +
> +static void build_info_complete(struct build_feature_devs_info *binfo)
> +{
> +	devm_iounmap(binfo->dev, binfo->ioaddr);
> +	devm_release_mem_region(binfo->dev, binfo->start, binfo->len);
> +}
> +
>  static int parse_feature_fiu(struct build_feature_devs_info *binfo,
> -			     struct dfl_fpga_enum_dfl *dfl,
>  			     resource_size_t ofst)
>  {
>  	int ret = 0;
> @@ -824,27 +880,39 @@ static int parse_feature_fiu(struct build_feature_devs_info *binfo,
>  	u16 id;
>  	u64 v;
>  
> -	v = readq(dfl->ioaddr + ofst + DFH);
> +	if (is_feature_dev_detected(binfo)) {
> +		build_info_complete(binfo);
> +
> +		ret = build_info_commit_dev(binfo);
> +		if (ret)
> +			return ret;
> +
> +		ret = build_info_prepare(binfo, binfo->start + ofst,
> +					 binfo->len - ofst);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	v = readq(binfo->ioaddr + DFH);
>  	id = FIELD_GET(DFH_ID, v);
>  
>  	/* create platform device for dfl feature dev */
> -	ret = build_info_create_dev(binfo, dfh_id_to_type(id),
> -				    dfl->ioaddr + ofst);
> +	ret = build_info_create_dev(binfo, dfh_id_to_type(id));
>  	if (ret)
>  		return ret;
>  
> -	ret = create_feature_instance(binfo, dfl, ofst, 0, 0);
> +	ret = create_feature_instance(binfo, 0, 0, 0);
>  	if (ret)
>  		return ret;
>  	/*
>  	 * find and parse FIU's child AFU via its NEXT_AFU register.
>  	 * please note that only Port has valid NEXT_AFU pointer per spec.
>  	 */
> -	v = readq(dfl->ioaddr + ofst + NEXT_AFU);
> +	v = readq(binfo->ioaddr + NEXT_AFU);
>  
>  	offset = FIELD_GET(NEXT_AFU_NEXT_DFH_OFST, v);
>  	if (offset)
> -		return parse_feature_afu(binfo, dfl, ofst + offset);
> +		return parse_feature_afu(binfo, offset);
>  
>  	dev_dbg(binfo->dev, "No AFUs detected on FIU %d\n", id);
>  
> @@ -852,41 +920,39 @@ static int parse_feature_fiu(struct build_feature_devs_info *binfo,
>  }
>  
>  static int parse_feature_private(struct build_feature_devs_info *binfo,
> -				 struct dfl_fpga_enum_dfl *dfl,
>  				 resource_size_t ofst)
>  {
> -	if (!binfo->feature_dev) {
> +	if (!is_feature_dev_detected(binfo)) {
>  		dev_err(binfo->dev, "the private feature 0x%x does not belong to any AFU.\n",
> -			feature_id(dfl->ioaddr + ofst));
> +			feature_id(binfo->ioaddr + ofst));
>  		return -EINVAL;
>  	}
>  
> -	return create_feature_instance(binfo, dfl, ofst, 0, 0);
> +	return create_feature_instance(binfo, ofst, 0, 0);
>  }
>  
>  /**
>   * parse_feature - parse a feature on given device feature list
>   *
>   * @binfo: build feature devices information.
> - * @dfl: device feature list to parse
> - * @ofst: offset to feature header on this device feature list
> + * @ofst: offset to current FIU header
>   */
>  static int parse_feature(struct build_feature_devs_info *binfo,
> -			 struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst)
> +			 resource_size_t ofst)
>  {
>  	u64 v;
>  	u32 type;
>  
> -	v = readq(dfl->ioaddr + ofst + DFH);
> +	v = readq(binfo->ioaddr + ofst + DFH);
>  	type = FIELD_GET(DFH_TYPE, v);
>  
>  	switch (type) {
>  	case DFH_TYPE_AFU:
> -		return parse_feature_afu(binfo, dfl, ofst);
> +		return parse_feature_afu(binfo, ofst);
>  	case DFH_TYPE_PRIVATE:
> -		return parse_feature_private(binfo, dfl, ofst);
> +		return parse_feature_private(binfo, ofst);
>  	case DFH_TYPE_FIU:
> -		return parse_feature_fiu(binfo, dfl, ofst);
> +		return parse_feature_fiu(binfo, ofst);
>  	default:
>  		dev_info(binfo->dev,
>  			 "Feature Type %x is not supported.\n", type);
> @@ -896,14 +962,17 @@ static int parse_feature(struct build_feature_devs_info *binfo,
>  }
>  
>  static int parse_feature_list(struct build_feature_devs_info *binfo,
> -			      struct dfl_fpga_enum_dfl *dfl)
> +			      resource_size_t start, resource_size_t len)
>  {
> -	void __iomem *start = dfl->ioaddr;
> -	void __iomem *end = dfl->ioaddr + dfl->len;
> +	resource_size_t end = start + len;
>  	int ret = 0;
>  	u32 ofst = 0;
>  	u64 v;
>  
> +	ret = build_info_prepare(binfo, start, len);
> +	if (ret)
> +		return ret;
> +
>  	/* walk through the device feature list via DFH's next DFH pointer. */
>  	for (; start < end; start += ofst) {
>  		if (end - start < DFH_SIZE) {
> @@ -911,11 +980,11 @@ static int parse_feature_list(struct build_feature_devs_info *binfo,
>  			return -EINVAL;
>  		}
>  
> -		ret = parse_feature(binfo, dfl, start - dfl->ioaddr);
> +		ret = parse_feature(binfo, start - binfo->start);
>  		if (ret)
>  			return ret;
>  
> -		v = readq(start + DFH);
> +		v = readq(binfo->ioaddr + start - binfo->start + DFH);
>  		ofst = FIELD_GET(DFH_NEXT_HDR_OFST, v);
>  
>  		/* stop parsing if EOL(End of List) is set or offset is 0 */
> @@ -924,7 +993,12 @@ static int parse_feature_list(struct build_feature_devs_info *binfo,
>  	}
>  
>  	/* commit current feature device when reach the end of list */
> -	return build_info_commit_dev(binfo);
> +	build_info_complete(binfo);
> +
> +	if (is_feature_dev_detected(binfo))
> +		ret = build_info_commit_dev(binfo);
> +
> +	return ret;
>  }
>  
>  struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev)
> @@ -977,7 +1051,6 @@ EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_free);
>   * @info: ptr to dfl_fpga_enum_info
>   * @start: mmio resource address of the device feature list.
>   * @len: mmio resource length of the device feature list.
> - * @ioaddr: mapped mmio resource address of the device feature list.
>   *
>   * One FPGA device may have one or more Device Feature Lists (DFLs), use this
>   * function to add information of each DFL to common data structure for next
> @@ -986,8 +1059,7 @@ EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_free);
>   * Return: 0 on success, negative error code otherwise.
>   */
>  int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
> -			       resource_size_t start, resource_size_t len,
> -			       void __iomem *ioaddr)
> +			       resource_size_t start, resource_size_t len)
>  {
>  	struct dfl_fpga_enum_dfl *dfl;
>  
> @@ -997,7 +1069,6 @@ int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
>  
>  	dfl->start = start;
>  	dfl->len = len;
> -	dfl->ioaddr = ioaddr;
>  
>  	list_add_tail(&dfl->node, &info->dfls);
>  
> @@ -1120,7 +1191,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>  	 * Lists.
>  	 */
>  	list_for_each_entry(dfl, &info->dfls, node) {
> -		ret = parse_feature_list(binfo, dfl);
> +		ret = parse_feature_list(binfo, dfl->start, dfl->len);
>  		if (ret) {
>  			remove_feature_devs(cdev);
>  			build_info_free(binfo);
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index bc61942..5973769 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -441,22 +441,17 @@ struct dfl_fpga_enum_info {
>   *
>   * @start: base address of this device feature list.
>   * @len: size of this device feature list.
> - * @ioaddr: mapped base address of this device feature list.
>   * @node: node in list of device feature lists.
>   */
>  struct dfl_fpga_enum_dfl {
>  	resource_size_t start;
>  	resource_size_t len;
> -
> -	void __iomem *ioaddr;
> -
>  	struct list_head node;
>  };
>  
>  struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev);
>  int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
> -			       resource_size_t start, resource_size_t len,
> -			       void __iomem *ioaddr);
> +			       resource_size_t start, resource_size_t len);
>  int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
>  			       unsigned int nr_irqs, int *irq_table);
>  void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
> -- 
> 2.7.4
> 

Applied to for-next,

Thanks

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7 2/3] fpga: dfl: create a dfl bus type to support DFL devices
  2020-08-19  7:45 ` [PATCH v7 2/3] fpga: dfl: create a dfl bus type to support DFL devices Xu Yilun
@ 2020-09-05  0:32   ` Moritz Fischer
  0 siblings, 0 replies; 9+ messages in thread
From: Moritz Fischer @ 2020-09-05  0:32 UTC (permalink / raw)
  To: Xu Yilun
  Cc: mdf, linux-fpga, linux-kernel, trix, lgoncalv, Wu Hao,
	Matthew Gerlach, Russ Weight

Hi Xu,

On Wed, Aug 19, 2020 at 03:45:20PM +0800, Xu Yilun wrote:
> A new bus type "dfl" is introduced for private features which are not
> initialized by DFL feature drivers (dfl-fme & dfl-afu drivers). So these
> private features could be handled by separate driver modules.
> 
> DFL feature drivers (dfl-fme, dfl-port) will create DFL devices on
> enumeration. DFL drivers could be registered on this bus to match these
> DFL devices. They are matched by dfl type & feature_id.
> 
> 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>
> Reviewed-by: Tom Rix <trix@redhat.com>
> Acked-by: Wu Hao <hao.wu@intel.com>
> ---
> v2: change the bus uevent format.
>     change the dfl device's sysfs name format.
>     refactor dfl_dev_add().
>     minor fixes for comments from Hao and Tom.
> v3: no change.
> v4: improve the uevent format, 4 bits for type & 12 bits for id.
>     change dfl_device->type to u8.
>     A dedicate field in struct dfl_feature for dfl device instance.
>     error out if dfl_device already exist on dfl_devs_init().
> v5: minor fixes for Hao's comments
> v6: the input param of dfl_devs_add() changes to struct
>     dfl_feature_platform_data.
>     improve the comments.
> v7: no change.
> ---
>  Documentation/ABI/testing/sysfs-bus-dfl |  15 ++
>  drivers/fpga/dfl.c                      | 262 +++++++++++++++++++++++++++++++-
>  drivers/fpga/dfl.h                      |  86 +++++++++++
>  3 files changed, 355 insertions(+), 8 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-dfl b/Documentation/ABI/testing/sysfs-bus-dfl
> new file mode 100644
> index 0000000..23543be
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-dfl
> @@ -0,0 +1,15 @@
> +What:		/sys/bus/dfl/devices/dfl_dev.X/type
> +Date:		Aug 2020
> +KernelVersion:	5.10
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:	Read-only. It returns type of DFL FIU of the device. Now DFL
> +		supports 2 FIU types, 0 for FME, 1 for PORT.
> +		Format: 0x%x
> +
> +What:		/sys/bus/dfl/devices/dfl_dev.X/feature_id
> +Date:		Aug 2020
> +KernelVersion:	5.10
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:	Read-only. It returns feature identifier local to its DFL FIU
> +		type.
> +		Format: 0x%x
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 52cafa2..c5ba4ac9 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -30,12 +30,6 @@ static DEFINE_MUTEX(dfl_id_mutex);
>   * index to dfl_chardevs table. If no chardev support just set devt_type
>   * as one invalid index (DFL_FPGA_DEVT_MAX).
>   */
> -enum dfl_id_type {
> -	FME_ID,		/* fme id allocation and mapping */
> -	PORT_ID,	/* port id allocation and mapping */
> -	DFL_ID_MAX,
> -};
> -
>  enum dfl_fpga_devt_type {
>  	DFL_FPGA_DEVT_FME,
>  	DFL_FPGA_DEVT_PORT,
> @@ -250,6 +244,244 @@ int dfl_fpga_check_port_id(struct platform_device *pdev, void *pport_id)
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_check_port_id);
>  
> +static DEFINE_IDA(dfl_device_ida);
> +
> +static const struct dfl_device_id *
> +dfl_match_one_device(const struct dfl_device_id *id, struct dfl_device *ddev)
> +{
> +	if (id->type == ddev->type && id->feature_id == ddev->feature_id)
> +		return id;
> +
> +	return NULL;
> +}
> +
> +static int dfl_bus_match(struct device *dev, struct device_driver *drv)
> +{
> +	struct dfl_device *ddev = to_dfl_dev(dev);
> +	struct dfl_driver *ddrv = to_dfl_drv(drv);
> +	const struct dfl_device_id *id_entry = ddrv->id_table;
> +
> +	if (id_entry) {
> +		while (id_entry->feature_id) {
> +			if (dfl_match_one_device(id_entry, ddev)) {
> +				ddev->id_entry = id_entry;
> +				return 1;
> +			}
> +			id_entry++;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int dfl_bus_probe(struct device *dev)
> +{
> +	struct dfl_device *ddev = to_dfl_dev(dev);
> +	struct dfl_driver *ddrv = to_dfl_drv(dev->driver);
Can you swap those for reverse x-mas tree where possible?

        struct dfl_driver *ddrv = to_dfl_drv(dev->driver);
        struct dfl_device *ddev = to_dfl_dev(dev);
	...

> +
> +	return ddrv->probe(ddev);
> +}
> +
> +static int dfl_bus_remove(struct device *dev)
> +{
> +	struct dfl_device *ddev = to_dfl_dev(dev);
> +	struct dfl_driver *ddrv = to_dfl_drv(dev->driver);

Same here.
> +
> +	if (ddrv->remove)
> +		ddrv->remove(ddev);
> +
> +	return 0;
> +}
> +
> +static int dfl_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
> +{
> +	struct dfl_device *ddev = to_dfl_dev(dev);
> +
> +	/* The type has 4 valid bits and feature_id has 12 valid bits */
> +	return add_uevent_var(env, "MODALIAS=dfl:t%01Xf%03X",
> +			      ddev->type, ddev->feature_id);
> +}
> +
> +/* show dfl info fields */
> +#define dfl_info_attr(field, format_string)				\
> +static ssize_t								\
> +field##_show(struct device *dev, struct device_attribute *attr,		\
> +	     char *buf)							\
> +{									\
> +	struct dfl_device *ddev = to_dfl_dev(dev);			\
> +									\
> +	return sprintf(buf, format_string, ddev->field);		\
> +}									\
> +static DEVICE_ATTR_RO(field)
> +
> +dfl_info_attr(type, "0x%x\n");
> +dfl_info_attr(feature_id, "0x%x\n");
Can you either caplitalize those or if it's just two fields, keep them
without a macro?
> +
> +static struct attribute *dfl_dev_attrs[] = {
> +	&dev_attr_type.attr,
> +	&dev_attr_feature_id.attr,
> +	NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(dfl_dev);
> +
> +static struct bus_type dfl_bus_type = {
> +	.name		= "dfl",
> +	.match		= dfl_bus_match,
> +	.probe		= dfl_bus_probe,
> +	.remove		= dfl_bus_remove,
> +	.uevent		= dfl_bus_uevent,
> +	.dev_groups	= dfl_dev_groups,
> +};
> +
> +static void release_dfl_dev(struct device *dev)
> +{
> +	struct dfl_device *ddev = to_dfl_dev(dev);
> +
> +	if (ddev->mmio_res.parent)
> +		release_resource(&ddev->mmio_res);
> +
> +	ida_simple_remove(&dfl_device_ida, ddev->id);
> +	kfree(ddev->irqs);
> +	kfree(ddev);
> +}
> +
> +static struct dfl_device *
> +dfl_dev_add(struct dfl_feature_platform_data *pdata,
> +	    struct dfl_feature *feature)
> +{
> +	struct platform_device *pdev = pdata->dev;
> +	struct resource *parent_res;
> +	struct dfl_device *ddev;
> +	int id, i, ret;
> +
> +	ddev = kzalloc(sizeof(*ddev), GFP_KERNEL);
> +	if (!ddev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	id = ida_simple_get(&dfl_device_ida, 0, 0, GFP_KERNEL);
> +	if (id < 0) {
> +		dev_err(&pdev->dev, "unable to get id\n");
> +		kfree(ddev);
> +		return ERR_PTR(id);
> +	}
> +
> +	/* freeing resources by put_device() after device_initialize() */
> +	device_initialize(&ddev->dev);
> +	ddev->dev.parent = &pdev->dev;
> +	ddev->dev.bus = &dfl_bus_type;
> +	ddev->dev.release = release_dfl_dev;
> +	ddev->id = id;
> +	ret = dev_set_name(&ddev->dev, "dfl_dev.%d", id);
> +	if (ret)
> +		goto put_dev;
> +
> +	ddev->type = feature_dev_id_type(pdev);
> +	ddev->feature_id = feature->id;
> +	ddev->cdev = pdata->dfl_cdev;
> +
> +	/* add mmio resource */
> +	parent_res = &pdev->resource[feature->resource_index];
> +	ddev->mmio_res.flags = IORESOURCE_MEM;
> +	ddev->mmio_res.start = parent_res->start;
> +	ddev->mmio_res.end = parent_res->end;
> +	ddev->mmio_res.name = dev_name(&ddev->dev);
> +	ret = insert_resource(parent_res, &ddev->mmio_res);
> +	if (ret) {
> +		dev_err(&pdev->dev, "%s failed to claim resource: %pR\n",
> +			dev_name(&ddev->dev), &ddev->mmio_res);
> +		goto put_dev;
> +	}
> +
> +	/* then add irq resource */
> +	if (feature->nr_irqs) {
> +		ddev->irqs = kcalloc(feature->nr_irqs,
> +				     sizeof(*ddev->irqs), GFP_KERNEL);
> +		if (!ddev->irqs) {
> +			ret = -ENOMEM;
> +			goto put_dev;
> +		}
> +
> +		for (i = 0; i < feature->nr_irqs; i++)
> +			ddev->irqs[i] = feature->irq_ctx[i].irq;
> +
> +		ddev->num_irqs = feature->nr_irqs;
> +	}
> +
> +	ret = device_add(&ddev->dev);
> +	if (ret)
> +		goto put_dev;
> +
> +	dev_info(&pdev->dev, "add dfl_dev: %s\n", dev_name(&ddev->dev));
Consider making this dev_dbg().
> +	return ddev;
> +
> +put_dev:
> +	/* calls release_dfl_dev() which does the clean up  */
> +	put_device(&ddev->dev);
> +	return ERR_PTR(ret);
> +}
> +
> +static void dfl_devs_remove(struct dfl_feature_platform_data *pdata)
> +{
> +	struct dfl_feature *feature;
> +
> +	dfl_fpga_dev_for_each_feature(pdata, feature) {
> +		if (feature->ddev) {
> +			device_unregister(&feature->ddev->dev);
> +			feature->ddev = NULL;
> +		}
> +	}
> +}
> +
> +static int dfl_devs_add(struct dfl_feature_platform_data *pdata)
> +{
> +	struct dfl_feature *feature;
> +	struct dfl_device *ddev;
> +	int ret;
> +
> +	dfl_fpga_dev_for_each_feature(pdata, feature) {
> +		if (feature->ioaddr)
> +			continue;
> +
> +		if (feature->ddev) {
> +			ret = -EEXIST;
> +			goto err;
> +		}
> +
> +		ddev = dfl_dev_add(pdata, feature);
> +		if (IS_ERR(ddev)) {
> +			ret = PTR_ERR(ddev);
> +			goto err;
> +		}
> +
> +		feature->ddev = ddev;
> +	}
> +
> +	return 0;
> +
> +err:
> +	dfl_devs_remove(pdata);
> +	return ret;
> +}
> +
> +int __dfl_driver_register(struct dfl_driver *dfl_drv, struct module *owner)
> +{
> +	if (!dfl_drv || !dfl_drv->probe || !dfl_drv->id_table)
> +		return -EINVAL;
> +
> +	dfl_drv->drv.owner = owner;
> +	dfl_drv->drv.bus = &dfl_bus_type;
> +
> +	return driver_register(&dfl_drv->drv);
> +}
> +EXPORT_SYMBOL(__dfl_driver_register);
> +
> +void dfl_driver_unregister(struct dfl_driver *dfl_drv)
> +{
> +	driver_unregister(&dfl_drv->drv);
> +}
> +EXPORT_SYMBOL(dfl_driver_unregister);
> +
>  #define is_header_feature(feature) ((feature)->id == FEATURE_ID_FIU_HEADER)
>  
>  /**
> @@ -261,12 +493,15 @@ void dfl_fpga_dev_feature_uinit(struct platform_device *pdev)
>  	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
>  	struct dfl_feature *feature;
>  
> -	dfl_fpga_dev_for_each_feature(pdata, feature)
> +	dfl_devs_remove(pdata);
> +
> +	dfl_fpga_dev_for_each_feature(pdata, feature) {
>  		if (feature->ops) {
>  			if (feature->ops->uinit)
>  				feature->ops->uinit(pdev, feature);
>  			feature->ops = NULL;
>  		}
> +	}
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_dev_feature_uinit);
>  
> @@ -347,6 +582,10 @@ int dfl_fpga_dev_feature_init(struct platform_device *pdev,
>  		drv++;
>  	}
>  
> +	ret = dfl_devs_add(pdata);
> +	if (ret)
> +		goto exit;
> +
>  	return 0;
>  exit:
>  	dfl_fpga_dev_feature_uinit(pdev);
> @@ -1284,11 +1523,17 @@ static int __init dfl_fpga_init(void)
>  {
>  	int ret;
>  
> +	ret = bus_register(&dfl_bus_type);
> +	if (ret)
> +		return ret;
> +
>  	dfl_ids_init();
>  
>  	ret = dfl_chardev_init();
> -	if (ret)
> +	if (ret) {
>  		dfl_ids_destroy();
> +		bus_unregister(&dfl_bus_type);
> +	}
>  
>  	return ret;
>  }
> @@ -1626,6 +1871,7 @@ static void __exit dfl_fpga_exit(void)
>  {
>  	dfl_chardev_uinit();
>  	dfl_ids_destroy();
> +	bus_unregister(&dfl_bus_type);
>  }
>  
>  module_init(dfl_fpga_init);
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 5973769..5dc758f 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -236,6 +236,7 @@ struct dfl_feature_irq_ctx {
>   * @irq_ctx: interrupt context list.
>   * @nr_irqs: number of interrupt contexts.
>   * @ops: ops of this sub feature.
> + * @ddev: ptr to the dfl device of this sub feature.
>   * @priv: priv data of this feature.
>   */
>  struct dfl_feature {
> @@ -246,6 +247,7 @@ struct dfl_feature {
>  	struct dfl_feature_irq_ctx *irq_ctx;
>  	unsigned int nr_irqs;
>  	const struct dfl_feature_ops *ops;
> +	struct dfl_device *ddev;
>  	void *priv;
>  };
>  
> @@ -514,4 +516,88 @@ 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_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.
> + * @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 */
> -- 
> 2.7.4
> 

Thanks,
Moritz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7 3/3] fpga: dfl: add support for N3000 Nios private feature
  2020-08-19  7:45 ` [PATCH v7 3/3] fpga: dfl: add support for N3000 Nios private feature Xu Yilun
@ 2020-09-05  0:45   ` Moritz Fischer
  2020-09-07  6:13     ` Xu Yilun
  0 siblings, 1 reply; 9+ messages in thread
From: Moritz Fischer @ 2020-09-05  0:45 UTC (permalink / raw)
  To: Xu Yilun
  Cc: mdf, linux-fpga, linux-kernel, trix, lgoncalv, Wu Hao,
	Matthew Gerlach, Russ Weight

Hi Xu,

On Wed, Aug 19, 2020 at 03:45:21PM +0800, Xu Yilun wrote:
> This patch adds support for the Nios handshake private feature on Intel
> PAC (Programmable Acceleration Card) N3000.
> 
> The Nios is the embedded processor on the FPGA card. This private feature
> provides a handshake interface to FPGA Nois firmware, which receives
FPGA *NIOS* or *Nios* I think ;-)
> retimer configuration command from host and executes via an internal SPI
> master (spi-altera). When Nios finishes the configuration, host takes over
> the ownership of the SPI master to control an Intel MAX10 BMC (Board
> Management Controller) Chip on the SPI bus.
> 
> For Nios firmware handshake part, this driver requests the retimer
> configuration for Nios with parameters from module param, and adds some
> sysfs nodes for user to query the onboard retimer's working mode and
> Nios firmware version.
> 
> For SPI part, this driver adds a spi-altera platform device as well as
> the MAX10 BMC spi slave info. A spi-altera driver will be matched to
> handle the following SPI work.
> 
> 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>
> Reviewed-by: Tom Rix <trix@redhat.com>
> Acked-by: Wu Hao <hao.wu@intel.com>
> ---
> v3: Add the doc for this driver
>     Minor fixes for comments from Tom
> v4: Move the err log in regmap implementation, and delete
>      n3000_nios_writel/readl(), they have nothing to wrapper now.
>     Some minor fixes and comments improvement.
> v5: Fix the output of fec_mode sysfs inf to "no" on 10G configuration,
>      cause no FEC mode could be configured for 10G.
>     Rename the dfl_n3000_nios_* to n3000_nios_*
>     Improves comments.
> v6: Fix the output of fec_mode sysfs inf to "not supported" if in 10G,
>      or the firmware version major < 3.
>     Minor fixes and improves comments.
> v7: Improves comments.
> ---
>  .../ABI/testing/sysfs-bus-dfl-devices-n3000-nios   |  21 +
>  Documentation/fpga/dfl-n3000-nios.rst              |  80 +++
>  Documentation/fpga/index.rst                       |   1 +
>  drivers/fpga/Kconfig                               |  11 +
>  drivers/fpga/Makefile                              |   2 +
>  drivers/fpga/dfl-n3000-nios.c                      | 542 +++++++++++++++++++++
>  6 files changed, 657 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
>  create mode 100644 Documentation/fpga/dfl-n3000-nios.rst
>  create mode 100644 drivers/fpga/dfl-n3000-nios.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios b/Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
> new file mode 100644
> index 0000000..ce5b474
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-dfl-devices-n3000-nios
> @@ -0,0 +1,21 @@
> +What:		/sys/bus/dfl/devices/dfl_dev.X/fec_mode
> +Date:		Aug 2020
> +KernelVersion:	5.10
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:	Read-only. It returns the FEC mode of the 25G links of the
> +		ethernet retimers configured by NIOS firmware. "rs" for Reed
> +		Solomon FEC, "kr" for Fire Code FEC, "no" for NO FEC.
> +		"not supported" if the FEC mode setting is not supported, this
> +		happens when the Nios firmware version major < 3, or no link is
> +		configured to 25G. The FEC mode could be set by module
> +		parameters, but it could only be set once after the board
> +		powers up.
> +		Format: string
> +
> +What:		/sys/bus/dfl/devices/dfl_dev.X/nios_fw_version
> +Date:		Aug 2020
> +KernelVersion:	5.10
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:	Read-only. It returns the version of the NIOS firmware in FPGA.
> +		Its format is "major.minor.patch".
> +		Format: %x.%x.%x
> diff --git a/Documentation/fpga/dfl-n3000-nios.rst b/Documentation/fpga/dfl-n3000-nios.rst
> new file mode 100644
> index 0000000..72dd600
> --- /dev/null
> +++ b/Documentation/fpga/dfl-n3000-nios.rst
> @@ -0,0 +1,80 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=================================
> +N3000 Nios Private Feature Driver
> +=================================
> +
> +The N3000 Nios driver supports for the Nios handshake private feature on Intel
> +PAC (Programmable Acceleration Card) N3000.
> +
> +The Nios is the embedded processor in the FPGA, it will configure the 2 onboard
> +ethernet retimers on power up. This private feature provides a handshake
> +interface to FPGA Nios firmware, which receives the ethernet retimer
> +configuration command from host and does the configuration via an internal SPI
> +master (spi-altera). When Nios finishes the configuration, host takes over the
> +ownership of the SPI master to control an Intel MAX10 BMC (Board Management
> +Controller) Chip on the SPI bus.
> +
> +So the driver does 2 major tasks on probe, uses the Nios firmware to configure
> +the ethernet retimer, and then creates a spi master platform device with the
> +MAX10 device info in spi_board_info.
> +
> +
> +Configuring the ethernet retimer
> +================================
> +
> +The Intel PAC N3000 is a FPGA based SmartNIC platform which could be programmed
> +to various configurations (with different link numbers and speeds, e.g. 8x10G,
> +4x25G ...). And the retimer chips should also be configured correspondingly by
> +Nios firmware. There are 2 retimer chips on the board, each of them supports 4
> +links. For example, in 8x10G configuration, the 2 retimer chips are both set to
> +4x10G mode, while in 4x25G configuration, retimer A is set to 4x25G and retimer
> +B is in reset. For now, the Nios firmware only supports 10G and 25G mode
> +setting for the retimer chips.
Make sure your API above exposes all of that.
> +
> +For all 25G links, their FEC (Forward Error Correction) mode could be further
> +configured by Nios firmware for user's requirement. For 10G links, they don't
> +have the FEC mode at all, the firmware ignores the FEC mode setting for them.
> +The FEC setting is not supported if the firmware version major < 3.
> +
> +The retimer configuration can only be done once after the board powers up, the
> +Nios firmware will not accept second configuration afterward. So it is not
> +proper for the driver to create a RW sysfs node for the FEC mode. A better way
> +is that the driver accepts a module parameter for the FEC mode, and does the
> +retimer configuration on driver probe, it also creates a RO sysfs node for the
> +FEC mode query.
> +
> +Module Parameters
> +=================
> +
> +The N3000 Nios driver supports the following module parameters:
> +
> +* fec_mode: string
> +  Require the Nios firmware to set the FEC mode for all 25G links of the
> +  ethernet retimers. The Nios firmware configures all these links with the same
> +  FEC mode. The possible values of fec_mode could be:
> +
> +  - "rs": Reed Solomon FEC (default)
> +  - "kr": Fire Code FEC
> +  - "no": No FEC
> +
> +  Since the firmware doesn't accept second configuration, The FEC mode will not
> +  be changed if the module is reloaded with a different parameter value.
> +
> +  The parameter has no effect for 10G links. It has no effect to all the links
> +  if firmware version major < 3.
> +
> +
> +Sysfs Attributes
> +================
> +
> +The driver creates the sysfs file in /sys/bus/dfl/devices/dfl_dev.X/
> +
> +* fec_mode:
> +  Read-only. It shows the FEC mode of the 25G links of the ethernet retimers.
> +  The possible values are the same as the fec_mode module parameter. An extra
> +  value "not supported" is returned if firmware version major < 3, or no link
> +  is configured to 25G.

This seems somewhat duplicate, runs the risk to get out of sync with the
ABI doc?

> +
> +* nios_fw_version:
> +  Read-only. It shows the version of the Nios firmware in FPGA.
> diff --git a/Documentation/fpga/index.rst b/Documentation/fpga/index.rst
> index f80f956..5fd3c37 100644
> --- a/Documentation/fpga/index.rst
> +++ b/Documentation/fpga/index.rst
> @@ -8,6 +8,7 @@ fpga
>      :maxdepth: 1
>  
>      dfl
> +    dfl-n3000-nios
>  
>  .. only::  subproject and html
>  
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 7cd5a29..38c7130 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -191,6 +191,17 @@ config FPGA_DFL_AFU
>  	  to the FPGA infrastructure via a Port. There may be more than one
>  	  Port/AFU per DFL based FPGA device.
>  
> +config FPGA_DFL_NIOS_INTEL_PAC_N3000
> +        tristate "FPGA DFL NIOS Driver for Intel PAC N3000"
> +        depends on FPGA_DFL
> +        select REGMAP
> +        help
> +	  This is the driver for the N3000 Nios private feature on Intel
> +	  PAC (Programmable Acceleration Card) N3000. It communicates
> +	  with the embedded Nios processor to configure the retimers on
> +	  the card. It also instantiates the SPI master (spi-altera) for
> +	  the card's BMC (Board Management Controller) control.
> +
>  config FPGA_DFL_PCI
>  	tristate "FPGA DFL PCIe Device Driver"
>  	depends on PCI && FPGA_DFL
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index d8e21df..18dc9885 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -44,5 +44,7 @@ dfl-fme-objs += dfl-fme-perf.o
>  dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
>  dfl-afu-objs += dfl-afu-error.o
>  
> +obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
> +
>  # Drivers for FPGAs which implement DFL
>  obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
> diff --git a/drivers/fpga/dfl-n3000-nios.c b/drivers/fpga/dfl-n3000-nios.c
> new file mode 100644
> index 0000000..c73ce38
> --- /dev/null
> +++ b/drivers/fpga/dfl-n3000-nios.c
> @@ -0,0 +1,542 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DFL device driver for Nios private feature on Intel PAC (Programmable
> + * Acceleration Card) N3000
> + *
> + * Copyright (C) 2019-2020 Intel Corporation, Inc.
> + *
> + * Authors:
> + *   Wu Hao <hao.wu@intel.com>
> + *   Xu Yilun <yilun.xu@intel.com>
> + */
> +#include <linux/bitfield.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/stddef.h>
> +#include <linux/spi/altera.h>
> +#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");
> +
> +/* N3000 Nios private feature registers */
> +#define NIOS_SPI_PARAM			0x8
> +#define PARAM_SHIFT_MODE_MSK		BIT_ULL(1)
> +#define PARAM_SHIFT_MODE_MSB		0
> +#define PARAM_SHIFT_MODE_LSB		1
> +#define PARAM_DATA_WIDTH		GENMASK_ULL(7, 2)
> +#define PARAM_NUM_CS			GENMASK_ULL(13, 8)
> +#define PARAM_CLK_POL			BIT_ULL(14)
> +#define PARAM_CLK_PHASE			BIT_ULL(15)
> +#define PARAM_PERIPHERAL_ID		GENMASK_ULL(47, 32)
> +
> +#define NIOS_SPI_CTRL			0x10
> +#define CTRL_WR_DATA			GENMASK_ULL(31, 0)
> +#define CTRL_ADDR			GENMASK_ULL(44, 32)
> +#define CTRL_CMD_MSK			GENMASK_ULL(63, 62)
> +#define CTRL_CMD_NOP			0
> +#define CTRL_CMD_RD			1
> +#define CTRL_CMD_WR			2
> +
> +#define NIOS_SPI_STAT			0x18
> +#define STAT_RD_DATA			GENMASK_ULL(31, 0)
> +#define STAT_RW_VAL			BIT_ULL(32)
> +
> +/* Nios handshake registers, indirect access */
> +#define NIOS_INIT			0x1000
> +#define NIOS_INIT_DONE			BIT(0)
> +#define NIOS_INIT_START			BIT(1)
> +/* Mode for retimer A, link 0, the same below */
> +#define REQ_FEC_MODE_A0_MSK		GENMASK(9, 8)
> +#define REQ_FEC_MODE_A1_MSK		GENMASK(11, 10)
> +#define REQ_FEC_MODE_A2_MSK		GENMASK(13, 12)
> +#define REQ_FEC_MODE_A3_MSK		GENMASK(15, 14)
> +#define REQ_FEC_MODE_B0_MSK		GENMASK(17, 16)
> +#define REQ_FEC_MODE_B1_MSK		GENMASK(19, 18)
> +#define REQ_FEC_MODE_B2_MSK		GENMASK(21, 20)
> +#define REQ_FEC_MODE_B3_MSK		GENMASK(23, 22)
> +#define REQ_FEC_MODE_NO			0x0
> +#define REQ_FEC_MODE_KR			0x1
> +#define REQ_FEC_MODE_RS			0x2
> +
> +#define NIOS_FW_VERSION			0x1004
> +#define NIOS_FW_VERSION_PATCH		GENMASK(23, 20)
> +#define NIOS_FW_VERSION_MINOR		GENMASK(27, 24)
> +#define NIOS_FW_VERSION_MAJOR		GENMASK(31, 28)
> +
> +/* The retimers we use on Intel PAC N3000 is Parkvale, abbreviated to PKVL */
> +#define PKVL_A_MODE_STS			0x1020
> +#define PKVL_B_MODE_STS			0x1024
> +#define PKVL_MODE_STS_GROUP_MSK		GENMASK(15, 8)
> +#define PKVL_MODE_STS_GROUP_OK		0x0
> +#define PKVL_MODE_STS_ID_MSK		GENMASK(7, 0)
> +/* When GROUP MASK field == GROUP_OK  */
> +#define PKVL_MODE_ID_RESET		0x0
> +#define PKVL_MODE_ID_4X10G		0x1
> +#define PKVL_MODE_ID_4X25G		0x2
> +#define PKVL_MODE_ID_2X25G		0x3
> +#define PKVL_MODE_ID_2X25G_2X10G	0x4
> +#define PKVL_MODE_ID_1X25G		0x5
> +
> +#define NS_REGBUS_WAIT_TIMEOUT		10000		/* loop count */
> +#define NIOS_INIT_TIMEOUT		10000000	/* usec */
> +#define NIOS_INIT_TIME_INTV		100000		/* usec */
> +
> +struct n3000_nios {
> +	void __iomem *base;
> +	struct regmap *regmap;
> +	struct device *dev;
> +	struct platform_device *altera_spi;
> +};
> +
> +static ssize_t nios_fw_version_show(struct device *dev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	struct n3000_nios *ns = dev_get_drvdata(dev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(ns->regmap, NIOS_FW_VERSION, &val);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%x.%x.%x\n",
> +		       (u8)FIELD_GET(NIOS_FW_VERSION_MAJOR, val),
> +		       (u8)FIELD_GET(NIOS_FW_VERSION_MINOR, val),
> +		       (u8)FIELD_GET(NIOS_FW_VERSION_PATCH, val));
> +}
> +static DEVICE_ATTR_RO(nios_fw_version);
> +
> +#define IS_MODE_STATUS_OK(mode_stat)				\
> +	(FIELD_GET(PKVL_MODE_STS_GROUP_MSK, (mode_stat)) ==	\
> +	 PKVL_MODE_STS_GROUP_OK)
> +
> +#define IS_RETIMER_FEC_SUPPORTED(retimer_mode)	\
> +	((retimer_mode) != PKVL_MODE_ID_RESET &&	\
> +	 (retimer_mode) != PKVL_MODE_ID_4X10G)
> +
> +static int get_retimer_mode(struct n3000_nios *ns, unsigned int mode_stat_reg,
> +			    unsigned int *retimer_mode)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(ns->regmap, mode_stat_reg, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (!IS_MODE_STATUS_OK(val))
> +		return -EFAULT;
> +
> +	*retimer_mode = FIELD_GET(PKVL_MODE_STS_ID_MSK, val);
> +
> +	return 0;
> +}
> +
> +static ssize_t fec_mode_show(struct device *dev,
> +			     struct device_attribute *attr, char *buf)
> +{
> +	struct n3000_nios *ns = dev_get_drvdata(dev);
> +	unsigned int val, retimer_a_mode, retimer_b_mode, fec_mode;
Can you make this reverse x-mas tree if possible?
> +	int ret;
> +
> +	/* FEC mode setting is not supported in early FW versions */
> +	ret = regmap_read(ns->regmap, NIOS_FW_VERSION, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (FIELD_GET(NIOS_FW_VERSION_MAJOR, val) < 3)
> +		return sprintf(buf, "not supported\n");
> +
> +	/* If no 25G links, FEC mode setting is not supported either */
> +	ret = get_retimer_mode(ns, PKVL_A_MODE_STS, &retimer_a_mode);
> +	if (ret)
> +		return ret;
> +
> +	ret = get_retimer_mode(ns, PKVL_B_MODE_STS, &retimer_b_mode);
> +	if (ret)
> +		return ret;
> +
> +	if (!IS_RETIMER_FEC_SUPPORTED(retimer_a_mode) &&
> +	    !IS_RETIMER_FEC_SUPPORTED(retimer_b_mode))
> +		return sprintf(buf, "not supported\n");
> +
> +	/* get the valid FEC mode for 25G links */
> +	ret = regmap_read(ns->regmap, NIOS_INIT, &val);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * FEC mode should always be the same for all links, as we set them
> +	 * in this way.
> +	 */
> +	fec_mode = FIELD_GET(REQ_FEC_MODE_A0_MSK, val);
> +	if (fec_mode != FIELD_GET(REQ_FEC_MODE_A1_MSK, val) ||
> +	    fec_mode != FIELD_GET(REQ_FEC_MODE_A2_MSK, val) ||
> +	    fec_mode != FIELD_GET(REQ_FEC_MODE_A3_MSK, val) ||
> +	    fec_mode != FIELD_GET(REQ_FEC_MODE_B0_MSK, val) ||
> +	    fec_mode != FIELD_GET(REQ_FEC_MODE_B1_MSK, val) ||
> +	    fec_mode != FIELD_GET(REQ_FEC_MODE_B2_MSK, val) ||
> +	    fec_mode != FIELD_GET(REQ_FEC_MODE_B3_MSK, val))
> +		return -EFAULT;
> +
> +	switch (fec_mode) {
> +	case REQ_FEC_MODE_NO:
> +		return sprintf(buf, "no\n");
> +	case REQ_FEC_MODE_KR:
> +		return sprintf(buf, "kr\n");
> +	case REQ_FEC_MODE_RS:
> +		return sprintf(buf, "rs\n");
> +	}
> +
> +	return -EFAULT;
> +}
> +static DEVICE_ATTR_RO(fec_mode);
> +
> +static struct attribute *n3000_nios_attrs[] = {
> +	&dev_attr_nios_fw_version.attr,
> +	&dev_attr_fec_mode.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(n3000_nios);
> +
> +static int init_error_detected(struct n3000_nios *ns)
Do you want to make the function either bool, or the return values ints?
> +{
> +	unsigned int val;
> +
> +	if (regmap_read(ns->regmap, PKVL_A_MODE_STS, &val))
> +		return true;
> +
> +	if (!IS_MODE_STATUS_OK(val))
> +		return true;
> +
> +	if (regmap_read(ns->regmap, PKVL_B_MODE_STS, &val))
> +		return true;
> +
> +	if (!IS_MODE_STATUS_OK(val))
> +		return true;
> +
> +	return false;
> +}
> +
> +static void dump_error_stat(struct n3000_nios *ns)
> +{
> +	unsigned int val;
> +
> +	if (regmap_read(ns->regmap, PKVL_A_MODE_STS, &val))
> +		return;
> +
> +	dev_err(ns->dev, "PKVL_A_MODE_STS 0x%x\n", val);
> +
> +	if (regmap_read(ns->regmap, PKVL_B_MODE_STS, &val))
> +		return;
> +
> +	dev_err(ns->dev, "PKVL_B_MODE_STS 0x%x\n", val);
> +}
> +
> +static int n3000_nios_init_done_check(struct n3000_nios *ns)
> +{
> +	struct device *dev = ns->dev;
> +	unsigned int val, mode;
> +	int ret;
> +
> +	/*
> +	 * this SPI is shared by Nios core inside FPGA, Nios will use this SPI
> +	 * master to do some one time initialization after power up, and then
> +	 * release the control to OS. driver needs to poll on INIT_DONE to
> +	 * see when driver could take the control.
> +	 *
> +	 * Please note that after Nios firmware version 3.0.0, INIT_START is
> +	 * introduced, so driver needs to trigger START firstly and then check
> +	 * INIT_DONE.
> +	 */
> +
> +	ret = regmap_read(ns->regmap, NIOS_FW_VERSION, &val);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * If Nios version register is totally uninitialized(== 0x0), then the
> +	 * Nios firmware is missing. So host could take control of SPI master
> +	 * safely, but initialization work for Nios is not done. To restore the
> +	 * card, we need to reprogram a new Nios firmware via the BMC chip on
> +	 * SPI bus. So the driver doesn't error out, it continues to create the
> +	 * spi controller device and spi_board_info for BMC.
> +	 */
> +	if (val == 0) {
> +		dev_err(dev, "Nios version reg = 0x%x, skip INIT_DONE check, but the retimer may be uninitialized\n",
This looks long, any chance you can linebreak this?
> +			val);
> +		return 0;
> +	}
> +
> +	if (FIELD_GET(NIOS_FW_VERSION_MAJOR, val) >= 3) {
> +		/* read NIOS_INIT to check if retimer initialization is done */
> +		ret = regmap_read(ns->regmap, NIOS_INIT, &val);
> +		if (ret)
> +			return ret;
> +
> +		/* check if retimers are initialized already */
> +		if (val & NIOS_INIT_DONE || val & NIOS_INIT_START)
> +			goto nios_init_done;
> +
> +		/* configure FEC mode per module param */
> +		val = NIOS_INIT_START;
> +
> +		/*
> +		 * When the retimer is to be set to 10G mode, there is no FEC
> +		 * mode setting, so the REQ_FEC_MODE field will be ignored by
> +		 * Nios firmware in this case. But we should still fill the FEC
> +		 * mode field cause host could not get the retimer working mode
> +		 * until the Nios init is done.
> +		 */
> +		if (!strcmp(fec_mode, "no"))
> +			mode = REQ_FEC_MODE_NO;
> +		else if (!strcmp(fec_mode, "kr"))
> +			mode = REQ_FEC_MODE_KR;
> +		else if (!strcmp(fec_mode, "rs"))
> +			mode = REQ_FEC_MODE_RS;
> +		else
> +			return -EINVAL;
> +
> +		/* set the same FEC mode for all links */
> +		val |= FIELD_PREP(REQ_FEC_MODE_A0_MSK, mode) |
> +		       FIELD_PREP(REQ_FEC_MODE_A1_MSK, mode) |
> +		       FIELD_PREP(REQ_FEC_MODE_A2_MSK, mode) |
> +		       FIELD_PREP(REQ_FEC_MODE_A3_MSK, mode) |
> +		       FIELD_PREP(REQ_FEC_MODE_B0_MSK, mode) |
> +		       FIELD_PREP(REQ_FEC_MODE_B1_MSK, mode) |
> +		       FIELD_PREP(REQ_FEC_MODE_B2_MSK, mode) |
> +		       FIELD_PREP(REQ_FEC_MODE_B3_MSK, mode);
> +
> +		ret = regmap_write(ns->regmap, NIOS_INIT, val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +nios_init_done:
> +	/* polls on NIOS_INIT_DONE */
> +	ret = regmap_read_poll_timeout(ns->regmap, NIOS_INIT, val,
> +				       val & NIOS_INIT_DONE,
> +				       NIOS_INIT_TIME_INTV,
> +				       NIOS_INIT_TIMEOUT);
> +	if (ret) {
> +		dev_err(dev, "NIOS_INIT_DONE %s\n",
> +			(ret == -ETIMEDOUT) ? "timed out" : "check error");
> +		goto dump_sts;
> +	}
> +
> +	/*
> +	 * After INIT_DONE is detected, it still needs to check if any error
> +	 * detected.
> +	 */
> +	if (init_error_detected(ns)) {
> +		/*
> +		 * If the retimer configuration is failed, the Nios firmware
> +		 * will still release the spi controller for host to
> +		 * communicate with the BMC. It makes possible for people to
> +		 * reprogram a new Nios firmware and restore the card. So the
> +		 * driver doesn't error out, it continues to create the spi
> +		 * controller device and spi_board_info for BMC.
> +		 */
> +		dev_err(dev, "NIOS_INIT_DONE OK, but err found during init\n");
> +		goto dump_sts;
> +	}
> +	return 0;
> +
> +dump_sts:
> +	dump_error_stat(ns);
> +
> +	return ret;
> +}
> +
> +struct spi_board_info m10_n3000_info = {
> +	.modalias = "m10-n3000",
Did I miss the patch for this, or did this only go to the SPI folks?
> +	.max_speed_hz = 12500000,
> +	.bus_num = 0,
> +	.chip_select = 0,
> +};

> +
> +static int create_altera_spi_controller(struct n3000_nios *ns)
> +{
> +	struct altera_spi_platform_data pdata = { 0 };
> +	struct platform_device_info pdevinfo = { 0 };
> +	void __iomem *base = ns->base;
> +	u64 v;
> +
> +	v = readq(base + NIOS_SPI_PARAM);
> +
> +	pdata.mode_bits = SPI_CS_HIGH;
> +	if (FIELD_GET(PARAM_CLK_POL, v))
> +		pdata.mode_bits |= SPI_CPOL;
> +	if (FIELD_GET(PARAM_CLK_PHASE, v))
> +		pdata.mode_bits |= SPI_CPHA;
> +
> +	pdata.num_chipselect = FIELD_GET(PARAM_NUM_CS, v);
> +	pdata.bits_per_word_mask =
> +		SPI_BPW_RANGE_MASK(1, FIELD_GET(PARAM_DATA_WIDTH, v));
> +
> +	pdata.num_devices = 1;
> +	pdata.devices = &m10_n3000_info;
> +
> +	dev_dbg(ns->dev, "%s cs %u bpm 0x%x mode 0x%x\n", __func__,
> +		pdata.num_chipselect, pdata.bits_per_word_mask,
> +		pdata.mode_bits);
> +
> +	pdevinfo.name = "subdev_spi_altera";
> +	pdevinfo.id = PLATFORM_DEVID_AUTO;
> +	pdevinfo.parent = ns->dev;
> +	pdevinfo.data = &pdata;
> +	pdevinfo.size_data = sizeof(pdata);
> +
> +	ns->altera_spi = platform_device_register_full(&pdevinfo);
> +	return PTR_ERR_OR_ZERO(ns->altera_spi);
> +}
> +
> +static void destroy_altera_spi_controller(struct n3000_nios *ns)
> +{
> +	platform_device_unregister(ns->altera_spi);
> +}
> +
> +/* ns is the abbreviation of nios_spi */
> +static int ns_poll_stat_timeout(void __iomem *base, u64 *v)
> +{
> +	int loops = NS_REGBUS_WAIT_TIMEOUT;
> +
> +	/*
> +	 * We don't use the time based timeout here for performance.
> +	 *
> +	 * The regbus read/write is on the critical path of Intel PAC N3000
> +	 * image programing. The time based timeout checking will add too much
> +	 * overhead on it. Usually the state changes in 1 or 2 loops on the
> +	 * test server, and we set 10000 times loop here for safety.
> +	 */
> +	do {
> +		*v = readq(base + NIOS_SPI_STAT);
> +		if (*v & STAT_RW_VAL)
> +			break;
> +		cpu_relax();
> +	} while (--loops);
> +
> +	return loops ? 0 : -ETIMEDOUT;
> +}
> +
> +static int ns_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct n3000_nios *ns = context;
> +	u64 v = 0;
> +	int ret;
> +
> +	v |= FIELD_PREP(CTRL_CMD_MSK, CTRL_CMD_WR);
> +	v |= FIELD_PREP(CTRL_ADDR, reg);
> +	v |= FIELD_PREP(CTRL_WR_DATA, val);
> +	writeq(v, ns->base + NIOS_SPI_CTRL);
> +
> +	ret = ns_poll_stat_timeout(ns->base, &v);
> +	if (ret)
> +		dev_err(ns->dev, "fail to write reg 0x%x val 0x%x: %d\n",
> +			reg, val, ret);
> +
> +	return ret;
> +}
> +
> +static int ns_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct n3000_nios *ns = context;
> +	u64 v = 0;
> +	int ret;
> +
> +	v |= FIELD_PREP(CTRL_CMD_MSK, CTRL_CMD_RD);
> +	v |= FIELD_PREP(CTRL_ADDR, reg);
> +	writeq(v, ns->base + NIOS_SPI_CTRL);
> +
> +	ret = ns_poll_stat_timeout(ns->base, &v);
> +	if (ret)
> +		dev_err(ns->dev, "fail to read reg 0x%x: %d\n", reg, ret);
> +	else
> +		*val = FIELD_GET(STAT_RD_DATA, v);
> +
> +	return ret;
> +}
> +
> +static const struct regmap_config ns_regbus_cfg = {
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.fast_io = true,
> +
> +	.reg_write = ns_reg_write,
> +	.reg_read = ns_reg_read,
> +};
> +
> +static int n3000_nios_probe(struct dfl_device *dfl_dev)
> +{
> +	struct device *dev = &dfl_dev->dev;
> +	struct n3000_nios *ns;
> +	int ret;
> +
> +	ns = devm_kzalloc(dev, sizeof(*ns), GFP_KERNEL);
> +	if (!ns)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&dfl_dev->dev, ns);
> +
> +	ns->dev = dev;
> +
> +	ns->base = devm_ioremap_resource(&dfl_dev->dev, &dfl_dev->mmio_res);
> +	if (IS_ERR(ns->base))
> +		return PTR_ERR(ns->base);
> +
> +	ns->regmap = devm_regmap_init(dev, NULL, ns, &ns_regbus_cfg);
> +	if (IS_ERR(ns->regmap))
> +		return PTR_ERR(ns->regmap);
> +
> +	ret = n3000_nios_init_done_check(ns);
> +	if (ret)
> +		return ret;
> +
> +	ret = create_altera_spi_controller(ns);
> +	if (ret)
> +		dev_err(dev, "altera spi controller create failed: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static void n3000_nios_remove(struct dfl_device *dfl_dev)
> +{
> +	struct n3000_nios *ns = dev_get_drvdata(&dfl_dev->dev);
> +
> +	destroy_altera_spi_controller(ns);
> +}
> +
> +#define FME_FEATURE_ID_N3000_NIOS	0xd
> +
> +static const struct dfl_device_id n3000_nios_ids[] = {
> +	{ FME_ID, FME_FEATURE_ID_N3000_NIOS },
> +	{ }
> +};
> +
> +static struct dfl_driver n3000_nios_driver = {
> +	.drv	= {
> +		.name       = "n3000-nios",
> +		.dev_groups = n3000_nios_groups,
> +	},
> +	.id_table = n3000_nios_ids,
> +	.probe   = n3000_nios_probe,
> +	.remove  = n3000_nios_remove,
> +};
> +
> +module_dfl_driver(n3000_nios_driver);
> +
> +MODULE_DEVICE_TABLE(dfl, n3000_nios_ids);
> +MODULE_DESCRIPTION("Driver for Nios private feature on Intel PAC N3000");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 
Thanks,
Moritz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v7 3/3] fpga: dfl: add support for N3000 Nios private feature
  2020-09-05  0:45   ` Moritz Fischer
@ 2020-09-07  6:13     ` Xu Yilun
  0 siblings, 0 replies; 9+ messages in thread
From: Xu Yilun @ 2020-09-07  6:13 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: linux-fpga, linux-kernel, trix, lgoncalv, Wu Hao,
	Matthew Gerlach, Russ Weight

On Fri, Sep 04, 2020 at 05:45:12PM -0700, Moritz Fischer wrote:
> Hi Xu,
> 
> On Wed, Aug 19, 2020 at 03:45:21PM +0800, Xu Yilun wrote:
> > This patch adds support for the Nios handshake private feature on Intel
> > PAC (Programmable Acceleration Card) N3000.
> > 
> > The Nios is the embedded processor on the FPGA card. This private feature
> > provides a handshake interface to FPGA Nois firmware, which receives
> FPGA *NIOS* or *Nios* I think ;-)

Thanks for catching the misspelling.

> > +Configuring the ethernet retimer
> > +================================
> > +
> > +The Intel PAC N3000 is a FPGA based SmartNIC platform which could be programmed
> > +to various configurations (with different link numbers and speeds, e.g. 8x10G,
> > +4x25G ...). And the retimer chips should also be configured correspondingly by
> > +Nios firmware. There are 2 retimer chips on the board, each of them supports 4
> > +links. For example, in 8x10G configuration, the 2 retimer chips are both set to
> > +4x10G mode, while in 4x25G configuration, retimer A is set to 4x25G and retimer
> > +B is in reset. For now, the Nios firmware only supports 10G and 25G mode
> > +setting for the retimer chips.
> Make sure your API above exposes all of that.

Yes. I could add the sysfs interfaces for query of the retimer mode.

> > +Sysfs Attributes
> > +================
> > +
> > +The driver creates the sysfs file in /sys/bus/dfl/devices/dfl_dev.X/
> > +
> > +* fec_mode:
> > +  Read-only. It shows the FEC mode of the 25G links of the ethernet retimers.
> > +  The possible values are the same as the fec_mode module parameter. An extra
> > +  value "not supported" is returned if firmware version major < 3, or no link
> > +  is configured to 25G.
> 
> This seems somewhat duplicate, runs the risk to get out of sync with the
> ABI doc?

Yes. I could just reference the file in Documentation/ABI

> > +static ssize_t fec_mode_show(struct device *dev,
> > +			     struct device_attribute *attr, char *buf)
> > +{
> > +	struct n3000_nios *ns = dev_get_drvdata(dev);
> > +	unsigned int val, retimer_a_mode, retimer_b_mode, fec_mode;
> Can you make this reverse x-mas tree if possible?

Yes, I'll change it.

> > +static int init_error_detected(struct n3000_nios *ns)
> Do you want to make the function either bool, or the return values ints?

I can make it bool.

> > +{
> > +	unsigned int val;
> > +
> > +	if (regmap_read(ns->regmap, PKVL_A_MODE_STS, &val))
> > +		return true;
> > +
> > +	if (!IS_MODE_STATUS_OK(val))
> > +		return true;
> > +
> > +	if (regmap_read(ns->regmap, PKVL_B_MODE_STS, &val))
> > +		return true;
> > +
> > +	if (!IS_MODE_STATUS_OK(val))
> > +		return true;
> > +
> > +	return false;
> > +}

> > +	if (val == 0) {
> > +		dev_err(dev, "Nios version reg = 0x%x, skip INIT_DONE check, but the retimer may be uninitialized\n",
> This looks long, any chance you can linebreak this?

I tried to break the line by
	dev_err(dev, "XXX"
		"XXX", val);

But checkpatch says "WARNING: quoted string split across lines".

I see checkpatch allows long lines for logging functions, so could we
keep it unchanged?

> > +struct spi_board_info m10_n3000_info = {
> > +	.modalias = "m10-n3000",
> Did I miss the patch for this, or did this only go to the SPI folks?

I didn't send the MAX 10 patchset in fpga domain, It goes to the SPI &
MFD maintainers, please see:
	https://lkml.org/lkml/2020/8/19/172

The regmap-spi-avmm has been applied for linux-next. The MAX 10 base
driver is under review.

> > +	.max_speed_hz = 12500000,
> > +	.bus_num = 0,
> > +	.chip_select = 0,
> > +};

Thanks,
Yilun.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-09-07  6:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-19  7:45 [PATCH v7 0/3] Modularization of DFL private feature drivers Xu Yilun
2020-08-19  7:45 ` [PATCH v7 1/3] fpga: dfl: map feature mmio resources in their own " Xu Yilun
2020-08-31  0:18   ` Moritz Fischer
2020-08-19  7:45 ` [PATCH v7 2/3] fpga: dfl: create a dfl bus type to support DFL devices Xu Yilun
2020-09-05  0:32   ` Moritz Fischer
2020-08-19  7:45 ` [PATCH v7 3/3] fpga: dfl: add support for N3000 Nios private feature Xu Yilun
2020-09-05  0:45   ` Moritz Fischer
2020-09-07  6:13     ` Xu Yilun
2020-08-20  4:16 ` [PATCH v7 0/3] Modularization of DFL private feature drivers Moritz Fischer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).