[PATCH v5 0/9] FPGA DFL updates

[PATCH v5 0/9] FPGA DFL updates

[Wu Hao]

Hi Greg,

This is v5 patchset which adds more features to FPGA DFL. Marjor changes
against v4 are sysfs related code rework to address comments on v4.

Please help to take a look. Thanks!

Main changes from v4:
  - convert code to use dev_groups for sysfs entries (#2, #3, #4, #6, #8).
  - clean up for empty init function after remove sysfs add/remove (#1).
  - introduce is_visible for sysfs groups (#3, #4, #6, #8).
  - remove revision sysfs entries (#4, #6, #8).
  - improve naming on shared functions (#5).
  - reorganize sysfs entries for port and fme error reporting (#6, #8).

Main changes from v3:
  - drop avx512 partail reconfiguration patch for now.
  - split dfl_fpga_cdev_config_port to 2 functions *_release/assign_port
    (#1).
  - split __dfl_fpga_cdev_config_port_vf into 2 functions with locking
    added (#2).
  - improve description in sysfs doc to avoid misunderstanding (#3).
  - switch to boolean in sysfs entry store function (#3).
  - remove dev_dbg in init/uinit callback function (#7, #9, #11).
  - remove uinit callback which does does nothing (#8, #9)

Main changes from v2:
  - update kernel version/date in sysfs doc (patch #4, #5, #8, #10, #11).
  - add back Documentation patch (patch #12).

Main changes from v1:
  - remove DRV/MODULE_VERSION modifications. (patch #1, #3, #4, #6)
  - remove argsz from new ioctls. (patch #2)
  - replace sysfs_create/remove_* with device_add/remove_* for sysfs entries.
    (patch #5, #8, #11)

Wu Hao (9):
  fpga: dfl: make init callback optional
  fpga: dfl: fme: convert platform_driver to use dev_groups
  fpga: dfl: afu: convert platform_driver to use dev_groups
  fpga: dfl: afu: add userclock sysfs interfaces.
  fpga: dfl: afu: expose __afu_port_enable/disable function.
  fpga: dfl: afu: add error reporting support.
  fpga: dfl: afu: add STP (SignalTap) support
  fpga: dfl: fme: add global error reporting support
  Documentation: fpga: dfl: add descriptions for virtualization and new
    interfaces.

 Documentation/ABI/testing/sysfs-platform-dfl-fme  |  62 ++++
 Documentation/ABI/testing/sysfs-platform-dfl-port |  53 ++++
 Documentation/fpga/dfl.rst                        | 105 +++++++
 drivers/fpga/Makefile                             |   3 +-
 drivers/fpga/dfl-afu-error.c                      | 230 ++++++++++++++
 drivers/fpga/dfl-afu-main.c                       | 230 +++++++++++---
 drivers/fpga/dfl-afu.h                            |   9 +
 drivers/fpga/dfl-fme-error.c                      | 359 ++++++++++++++++++++++
 drivers/fpga/dfl-fme-main.c                       |  42 +--
 drivers/fpga/dfl-fme.h                            |   3 +
 drivers/fpga/dfl.c                                |  10 +-
 drivers/fpga/dfl.h                                |   9 +
 12 files changed, 1041 insertions(+), 74 deletions(-)
 create mode 100644 drivers/fpga/dfl-afu-error.c
 create mode 100644 drivers/fpga/dfl-fme-error.c

-- 
1.8.3.1

[PATCH v5 1/9] fpga: dfl: make init callback optional

[Wu Hao]

This patch makes init callback of sub features optional. With
this change, people don't need to prepare any empty init callback.

Signed-off-by: Wu Hao <hao.wu@intel.com>
---
 drivers/fpga/dfl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index c0512af..96a2b82 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -271,11 +271,13 @@ static int dfl_feature_instance_init(struct platform_device *pdev,
 				     struct dfl_feature *feature,
 				     struct dfl_feature_driver *drv)
 {
-	int ret;
+	int ret = 0;
 
-	ret = drv->ops->init(pdev, feature);
-	if (ret)
-		return ret;
+	if (drv->ops->init) {
+		ret = drv->ops->init(pdev, feature);
+		if (ret)
+			return ret;
+	}
 
 	feature->ops = drv->ops;
 
-- 
1.8.3.1

Re: [PATCH v5 1/9] fpga: dfl: make init callback optional

[Moritz Fischer]

Hi,

On Mon, Aug 12, 2019 at 10:49:56AM +0800, Wu Hao wrote:
> This patch makes init callback of sub features optional. With
> this change, people don't need to prepare any empty init callback.
> 
> Signed-off-by: Wu Hao <hao.wu@intel.com>

Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
>  drivers/fpga/dfl.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index c0512af..96a2b82 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -271,11 +271,13 @@ static int dfl_feature_instance_init(struct platform_device *pdev,
>  				     struct dfl_feature *feature,
>  				     struct dfl_feature_driver *drv)
>  {
> -	int ret;
> +	int ret = 0;
>  
> -	ret = drv->ops->init(pdev, feature);
> -	if (ret)
> -		return ret;
> +	if (drv->ops->init) {
> +		ret = drv->ops->init(pdev, feature);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	feature->ops = drv->ops;

You could swap it around maybe like so:

int dfl_feature_instance_init() ...
{
	feature->ops = drv->ops;
	if (drv->ops->init)
		return drv->ops->init(pdev, feature);

	return 0;
}

With the caveat that feature->ops gets always set ...

Your call.

Thanks,
Moritz

Re: [PATCH v5 1/9] fpga: dfl: make init callback optional

[Wu Hao]

On Tue, Aug 20, 2019 at 08:24:06PM -0700, Moritz Fischer wrote:
> Hi,
> 
> On Mon, Aug 12, 2019 at 10:49:56AM +0800, Wu Hao wrote:
> > This patch makes init callback of sub features optional. With
> > this change, people don't need to prepare any empty init callback.
> > 
> > Signed-off-by: Wu Hao <hao.wu@intel.com>
> 
> Acked-by: Moritz Fischer <mdf@kernel.org>
> > ---
> >  drivers/fpga/dfl.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> > index c0512af..96a2b82 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -271,11 +271,13 @@ static int dfl_feature_instance_init(struct platform_device *pdev,
> >  				     struct dfl_feature *feature,
> >  				     struct dfl_feature_driver *drv)
> >  {
> > -	int ret;
> > +	int ret = 0;
> >  
> > -	ret = drv->ops->init(pdev, feature);
> > -	if (ret)
> > -		return ret;
> > +	if (drv->ops->init) {
> > +		ret = drv->ops->init(pdev, feature);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	feature->ops = drv->ops;
> 
> You could swap it around maybe like so:
> 
> int dfl_feature_instance_init() ...
> {
> 	feature->ops = drv->ops;
> 	if (drv->ops->init)
> 		return drv->ops->init(pdev, feature);
> 
> 	return 0;
> }
> 
> With the caveat that feature->ops gets always set ...
> 
> Your call.

Hi Moritz,

Thanks a lot for the review and comments. It does simplify the code,
will modify it.

Thanks
Hao

> 
> Thanks,
> Moritz

[PATCH v5 2/9] fpga: dfl: fme: convert platform_driver to use dev_groups

[Wu Hao]

This patch takes advantage of driver core which helps to create
and remove sysfs attribute files, so there is no need to register
sysfs entries manually in dfl-fme platform river code.

Signed-off-by: Wu Hao <hao.wu@intel.com>
---
 drivers/fpga/dfl-fme-main.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index f033f1c..bf8114d 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -129,30 +129,6 @@ static ssize_t socket_id_show(struct device *dev,
 };
 ATTRIBUTE_GROUPS(fme_hdr);
 
-static int fme_hdr_init(struct platform_device *pdev,
-			struct dfl_feature *feature)
-{
-	void __iomem *base = feature->ioaddr;
-	int ret;
-
-	dev_dbg(&pdev->dev, "FME HDR Init.\n");
-	dev_dbg(&pdev->dev, "FME cap %llx.\n",
-		(unsigned long long)readq(base + FME_HDR_CAP));
-
-	ret = device_add_groups(&pdev->dev, fme_hdr_groups);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-static void fme_hdr_uinit(struct platform_device *pdev,
-			  struct dfl_feature *feature)
-{
-	dev_dbg(&pdev->dev, "FME HDR UInit.\n");
-	device_remove_groups(&pdev->dev, fme_hdr_groups);
-}
-
 static long fme_hdr_ioctl_release_port(struct dfl_feature_platform_data *pdata,
 				       unsigned long arg)
 {
@@ -199,8 +175,6 @@ static long fme_hdr_ioctl(struct platform_device *pdev,
 };
 
 static const struct dfl_feature_ops fme_hdr_ops = {
-	.init = fme_hdr_init,
-	.uinit = fme_hdr_uinit,
 	.ioctl = fme_hdr_ioctl,
 };
 
@@ -361,7 +335,8 @@ static int fme_remove(struct platform_device *pdev)
 
 static struct platform_driver fme_driver = {
 	.driver	= {
-		.name    = DFL_FPGA_FEATURE_DEV_FME,
+		.name       = DFL_FPGA_FEATURE_DEV_FME,
+		.dev_groups = fme_hdr_groups,
 	},
 	.probe   = fme_probe,
 	.remove  = fme_remove,
-- 
1.8.3.1

Re: [PATCH v5 2/9] fpga: dfl: fme: convert platform_driver to use dev_groups

[Moritz Fischer]

Hi Hao,

On Mon, Aug 12, 2019 at 10:49:57AM +0800, Wu Hao wrote:
> This patch takes advantage of driver core which helps to create
> and remove sysfs attribute files, so there is no need to register
> sysfs entries manually in dfl-fme platform river code.
Nit: s/river/driver
> 
> Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
>  drivers/fpga/dfl-fme-main.c | 29 ++---------------------------
>  1 file changed, 2 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> index f033f1c..bf8114d 100644
> --- a/drivers/fpga/dfl-fme-main.c
> +++ b/drivers/fpga/dfl-fme-main.c
> @@ -129,30 +129,6 @@ static ssize_t socket_id_show(struct device *dev,
>  };
>  ATTRIBUTE_GROUPS(fme_hdr);
>  
> -static int fme_hdr_init(struct platform_device *pdev,
> -			struct dfl_feature *feature)
> -{
> -	void __iomem *base = feature->ioaddr;
> -	int ret;
> -
> -	dev_dbg(&pdev->dev, "FME HDR Init.\n");
> -	dev_dbg(&pdev->dev, "FME cap %llx.\n",
> -		(unsigned long long)readq(base + FME_HDR_CAP));
> -
> -	ret = device_add_groups(&pdev->dev, fme_hdr_groups);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static void fme_hdr_uinit(struct platform_device *pdev,
> -			  struct dfl_feature *feature)
> -{
> -	dev_dbg(&pdev->dev, "FME HDR UInit.\n");
> -	device_remove_groups(&pdev->dev, fme_hdr_groups);
> -}
> -
>  static long fme_hdr_ioctl_release_port(struct dfl_feature_platform_data *pdata,
>  				       unsigned long arg)
>  {
> @@ -199,8 +175,6 @@ static long fme_hdr_ioctl(struct platform_device *pdev,
>  };
>  
>  static const struct dfl_feature_ops fme_hdr_ops = {
> -	.init = fme_hdr_init,
> -	.uinit = fme_hdr_uinit,
>  	.ioctl = fme_hdr_ioctl,
>  };
>  
> @@ -361,7 +335,8 @@ static int fme_remove(struct platform_device *pdev)
>  
>  static struct platform_driver fme_driver = {
>  	.driver	= {
> -		.name    = DFL_FPGA_FEATURE_DEV_FME,
> +		.name       = DFL_FPGA_FEATURE_DEV_FME,
> +		.dev_groups = fme_hdr_groups,
>  	},
>  	.probe   = fme_probe,
>  	.remove  = fme_remove,
> -- 
> 1.8.3.1
> 
Thanks,
Moritz

[PATCH v5 3/9] fpga: dfl: afu: convert platform_driver to use dev_groups

[Wu Hao]

This patch takes advantage of driver core which helps to create
and remove sysfs attribute files, so there is no need to register
sysfs entries manually in dfl-afu platform river code.

Signed-off-by: Wu Hao <hao.wu@intel.com>
---
 drivers/fpga/dfl-afu-main.c | 69 +++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index e50c45e..e955149 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -282,24 +282,17 @@ static int port_get_id(struct platform_device *pdev)
 	&dev_attr_power_state.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(port_hdr);
+
+static const struct attribute_group port_hdr_group = {
+	.attrs = port_hdr_attrs,
+};
 
 static int port_hdr_init(struct platform_device *pdev,
 			 struct dfl_feature *feature)
 {
-	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
-
 	port_reset(pdev);
 
-	return device_add_groups(&pdev->dev, port_hdr_groups);
-}
-
-static void port_hdr_uinit(struct platform_device *pdev,
-			   struct dfl_feature *feature)
-{
-	dev_dbg(&pdev->dev, "PORT HDR UInit.\n");
-
-	device_remove_groups(&pdev->dev, port_hdr_groups);
+	return 0;
 }
 
 static long
@@ -330,7 +323,6 @@ static void port_hdr_uinit(struct platform_device *pdev,
 
 static const struct dfl_feature_ops port_hdr_ops = {
 	.init = port_hdr_init,
-	.uinit = port_hdr_uinit,
 	.ioctl = port_hdr_ioctl,
 };
 
@@ -361,32 +353,37 @@ static void port_hdr_uinit(struct platform_device *pdev,
 	&dev_attr_afu_id.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(port_afu);
 
-static int port_afu_init(struct platform_device *pdev,
-			 struct dfl_feature *feature)
+static umode_t port_afu_attrs_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
 {
-	struct resource *res = &pdev->resource[feature->resource_index];
-	int ret;
-
-	dev_dbg(&pdev->dev, "PORT AFU Init.\n");
+	struct device *dev = kobj_to_dev(kobj);
 
-	ret = afu_mmio_region_add(dev_get_platdata(&pdev->dev),
-				  DFL_PORT_REGION_INDEX_AFU, resource_size(res),
-				  res->start, DFL_PORT_REGION_READ |
-				  DFL_PORT_REGION_WRITE | DFL_PORT_REGION_MMAP);
-	if (ret)
-		return ret;
+	/*
+	 * sysfs entries are visible only if related private feature is
+	 * enumerated.
+	 */
+	if (!dfl_get_feature_by_id(dev, PORT_FEATURE_ID_AFU))
+		return 0;
 
-	return device_add_groups(&pdev->dev, port_afu_groups);
+	return attr->mode;
 }
 
-static void port_afu_uinit(struct platform_device *pdev,
-			   struct dfl_feature *feature)
+static const struct attribute_group port_afu_group = {
+	.attrs      = port_afu_attrs,
+	.is_visible = port_afu_attrs_visible,
+};
+
+static int port_afu_init(struct platform_device *pdev,
+			 struct dfl_feature *feature)
 {
-	dev_dbg(&pdev->dev, "PORT AFU UInit.\n");
+	struct resource *res = &pdev->resource[feature->resource_index];
 
-	device_remove_groups(&pdev->dev, port_afu_groups);
+	return afu_mmio_region_add(dev_get_platdata(&pdev->dev),
+				   DFL_PORT_REGION_INDEX_AFU,
+				   resource_size(res), res->start,
+				   DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ |
+				   DFL_PORT_REGION_WRITE);
 }
 
 static const struct dfl_feature_id port_afu_id_table[] = {
@@ -396,7 +393,6 @@ static void port_afu_uinit(struct platform_device *pdev,
 
 static const struct dfl_feature_ops port_afu_ops = {
 	.init = port_afu_init,
-	.uinit = port_afu_uinit,
 };
 
 static struct dfl_feature_driver port_feature_drvs[] = {
@@ -748,9 +744,16 @@ static int afu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct attribute_group *afu_dev_groups[] = {
+	&port_hdr_group,
+	&port_afu_group,
+	NULL
+};
+
 static struct platform_driver afu_driver = {
 	.driver	= {
-		.name    = DFL_FPGA_FEATURE_DEV_PORT,
+		.name	    = DFL_FPGA_FEATURE_DEV_PORT,
+		.dev_groups = afu_dev_groups,
 	},
 	.probe   = afu_probe,
 	.remove  = afu_remove,
-- 
1.8.3.1

Re: [PATCH v5 3/9] fpga: dfl: afu: convert platform_driver to use dev_groups

[Moritz Fischer]

Hi Hao,

On Mon, Aug 12, 2019 at 10:49:58AM +0800, Wu Hao wrote:
> This patch takes advantage of driver core which helps to create
> and remove sysfs attribute files, so there is no need to register
> sysfs entries manually in dfl-afu platform river code.
Same nit: s/river/driver
> 
> Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
>  drivers/fpga/dfl-afu-main.c | 69 +++++++++++++++++++++++----------------------
>  1 file changed, 36 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index e50c45e..e955149 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -282,24 +282,17 @@ static int port_get_id(struct platform_device *pdev)
>  	&dev_attr_power_state.attr,
>  	NULL,
>  };
> -ATTRIBUTE_GROUPS(port_hdr);
> +
> +static const struct attribute_group port_hdr_group = {
> +	.attrs = port_hdr_attrs,
> +};
>  
>  static int port_hdr_init(struct platform_device *pdev,
>  			 struct dfl_feature *feature)
>  {
> -	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
> -
>  	port_reset(pdev);
>  
> -	return device_add_groups(&pdev->dev, port_hdr_groups);
> -}
> -
> -static void port_hdr_uinit(struct platform_device *pdev,
> -			   struct dfl_feature *feature)
> -{
> -	dev_dbg(&pdev->dev, "PORT HDR UInit.\n");
> -
> -	device_remove_groups(&pdev->dev, port_hdr_groups);
> +	return 0;
>  }
>  
>  static long
> @@ -330,7 +323,6 @@ static void port_hdr_uinit(struct platform_device *pdev,
>  
>  static const struct dfl_feature_ops port_hdr_ops = {
>  	.init = port_hdr_init,
> -	.uinit = port_hdr_uinit,
>  	.ioctl = port_hdr_ioctl,
>  };
>  
> @@ -361,32 +353,37 @@ static void port_hdr_uinit(struct platform_device *pdev,
>  	&dev_attr_afu_id.attr,
>  	NULL
>  };
> -ATTRIBUTE_GROUPS(port_afu);
>  
> -static int port_afu_init(struct platform_device *pdev,
> -			 struct dfl_feature *feature)
> +static umode_t port_afu_attrs_visible(struct kobject *kobj,
> +				      struct attribute *attr, int n)
>  {
> -	struct resource *res = &pdev->resource[feature->resource_index];
> -	int ret;
> -
> -	dev_dbg(&pdev->dev, "PORT AFU Init.\n");
> +	struct device *dev = kobj_to_dev(kobj);
>  
> -	ret = afu_mmio_region_add(dev_get_platdata(&pdev->dev),
> -				  DFL_PORT_REGION_INDEX_AFU, resource_size(res),
> -				  res->start, DFL_PORT_REGION_READ |
> -				  DFL_PORT_REGION_WRITE | DFL_PORT_REGION_MMAP);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * sysfs entries are visible only if related private feature is
> +	 * enumerated.
> +	 */
> +	if (!dfl_get_feature_by_id(dev, PORT_FEATURE_ID_AFU))
> +		return 0;
>  
> -	return device_add_groups(&pdev->dev, port_afu_groups);
> +	return attr->mode;
>  }
>  
> -static void port_afu_uinit(struct platform_device *pdev,
> -			   struct dfl_feature *feature)
> +static const struct attribute_group port_afu_group = {
> +	.attrs      = port_afu_attrs,
> +	.is_visible = port_afu_attrs_visible,
> +};
> +
> +static int port_afu_init(struct platform_device *pdev,
> +			 struct dfl_feature *feature)
>  {
> -	dev_dbg(&pdev->dev, "PORT AFU UInit.\n");
Thanks.
> +	struct resource *res = &pdev->resource[feature->resource_index];
>  
> -	device_remove_groups(&pdev->dev, port_afu_groups);
> +	return afu_mmio_region_add(dev_get_platdata(&pdev->dev),
> +				   DFL_PORT_REGION_INDEX_AFU,
> +				   resource_size(res), res->start,
> +				   DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ |
> +				   DFL_PORT_REGION_WRITE);
>  }
>  
>  static const struct dfl_feature_id port_afu_id_table[] = {
> @@ -396,7 +393,6 @@ static void port_afu_uinit(struct platform_device *pdev,
>  
>  static const struct dfl_feature_ops port_afu_ops = {
>  	.init = port_afu_init,
> -	.uinit = port_afu_uinit,
>  };
>  
>  static struct dfl_feature_driver port_feature_drvs[] = {
> @@ -748,9 +744,16 @@ static int afu_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct attribute_group *afu_dev_groups[] = {
> +	&port_hdr_group,
> +	&port_afu_group,
> +	NULL
> +};
> +
>  static struct platform_driver afu_driver = {
>  	.driver	= {
> -		.name    = DFL_FPGA_FEATURE_DEV_PORT,
> +		.name	    = DFL_FPGA_FEATURE_DEV_PORT,
> +		.dev_groups = afu_dev_groups,
>  	},
>  	.probe   = afu_probe,
>  	.remove  = afu_remove,
> -- 
> 1.8.3.1
> 

Thanks,
Moritz

Re: [PATCH v5 3/9] fpga: dfl: afu: convert platform_driver to use dev_groups

[Wu Hao]

On Thu, Aug 22, 2019 at 08:07:01AM -0700, Moritz Fischer wrote:
> Hi Hao,
> 
> On Mon, Aug 12, 2019 at 10:49:58AM +0800, Wu Hao wrote:
> > This patch takes advantage of driver core which helps to create
> > and remove sysfs attribute files, so there is no need to register
> > sysfs entries manually in dfl-afu platform river code.
> Same nit: s/river/driver
> > 
> > Signed-off-by: Wu Hao <hao.wu@intel.com>
> Acked-by: Moritz Fischer <mdf@kernel.org>


Hi Moritz

Thanks a lot for the review. : )

Have you got a chance to look into the other patches in this patchset?

Thanks
Hao

> > ---
> >  drivers/fpga/dfl-afu-main.c | 69 +++++++++++++++++++++++----------------------
> >  1 file changed, 36 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> > index e50c45e..e955149 100644
> > --- a/drivers/fpga/dfl-afu-main.c
> > +++ b/drivers/fpga/dfl-afu-main.c
> > @@ -282,24 +282,17 @@ static int port_get_id(struct platform_device *pdev)
> >  	&dev_attr_power_state.attr,
> >  	NULL,
> >  };
> > -ATTRIBUTE_GROUPS(port_hdr);
> > +
> > +static const struct attribute_group port_hdr_group = {
> > +	.attrs = port_hdr_attrs,
> > +};
> >  
> >  static int port_hdr_init(struct platform_device *pdev,
> >  			 struct dfl_feature *feature)
> >  {
> > -	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
> > -
> >  	port_reset(pdev);
> >  
> > -	return device_add_groups(&pdev->dev, port_hdr_groups);
> > -}
> > -
> > -static void port_hdr_uinit(struct platform_device *pdev,
> > -			   struct dfl_feature *feature)
> > -{
> > -	dev_dbg(&pdev->dev, "PORT HDR UInit.\n");
> > -
> > -	device_remove_groups(&pdev->dev, port_hdr_groups);
> > +	return 0;
> >  }
> >  
> >  static long
> > @@ -330,7 +323,6 @@ static void port_hdr_uinit(struct platform_device *pdev,
> >  
> >  static const struct dfl_feature_ops port_hdr_ops = {
> >  	.init = port_hdr_init,
> > -	.uinit = port_hdr_uinit,
> >  	.ioctl = port_hdr_ioctl,
> >  };
> >  
> > @@ -361,32 +353,37 @@ static void port_hdr_uinit(struct platform_device *pdev,
> >  	&dev_attr_afu_id.attr,
> >  	NULL
> >  };
> > -ATTRIBUTE_GROUPS(port_afu);
> >  
> > -static int port_afu_init(struct platform_device *pdev,
> > -			 struct dfl_feature *feature)
> > +static umode_t port_afu_attrs_visible(struct kobject *kobj,
> > +				      struct attribute *attr, int n)
> >  {
> > -	struct resource *res = &pdev->resource[feature->resource_index];
> > -	int ret;
> > -
> > -	dev_dbg(&pdev->dev, "PORT AFU Init.\n");
> > +	struct device *dev = kobj_to_dev(kobj);
> >  
> > -	ret = afu_mmio_region_add(dev_get_platdata(&pdev->dev),
> > -				  DFL_PORT_REGION_INDEX_AFU, resource_size(res),
> > -				  res->start, DFL_PORT_REGION_READ |
> > -				  DFL_PORT_REGION_WRITE | DFL_PORT_REGION_MMAP);
> > -	if (ret)
> > -		return ret;
> > +	/*
> > +	 * sysfs entries are visible only if related private feature is
> > +	 * enumerated.
> > +	 */
> > +	if (!dfl_get_feature_by_id(dev, PORT_FEATURE_ID_AFU))
> > +		return 0;
> >  
> > -	return device_add_groups(&pdev->dev, port_afu_groups);
> > +	return attr->mode;
> >  }
> >  
> > -static void port_afu_uinit(struct platform_device *pdev,
> > -			   struct dfl_feature *feature)
> > +static const struct attribute_group port_afu_group = {
> > +	.attrs      = port_afu_attrs,
> > +	.is_visible = port_afu_attrs_visible,
> > +};
> > +
> > +static int port_afu_init(struct platform_device *pdev,
> > +			 struct dfl_feature *feature)
> >  {
> > -	dev_dbg(&pdev->dev, "PORT AFU UInit.\n");
> Thanks.
> > +	struct resource *res = &pdev->resource[feature->resource_index];
> >  
> > -	device_remove_groups(&pdev->dev, port_afu_groups);
> > +	return afu_mmio_region_add(dev_get_platdata(&pdev->dev),
> > +				   DFL_PORT_REGION_INDEX_AFU,
> > +				   resource_size(res), res->start,
> > +				   DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ |
> > +				   DFL_PORT_REGION_WRITE);
> >  }
> >  
> >  static const struct dfl_feature_id port_afu_id_table[] = {
> > @@ -396,7 +393,6 @@ static void port_afu_uinit(struct platform_device *pdev,
> >  
> >  static const struct dfl_feature_ops port_afu_ops = {
> >  	.init = port_afu_init,
> > -	.uinit = port_afu_uinit,
> >  };
> >  
> >  static struct dfl_feature_driver port_feature_drvs[] = {
> > @@ -748,9 +744,16 @@ static int afu_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > +static const struct attribute_group *afu_dev_groups[] = {
> > +	&port_hdr_group,
> > +	&port_afu_group,
> > +	NULL
> > +};
> > +
> >  static struct platform_driver afu_driver = {
> >  	.driver	= {
> > -		.name    = DFL_FPGA_FEATURE_DEV_PORT,
> > +		.name	    = DFL_FPGA_FEATURE_DEV_PORT,
> > +		.dev_groups = afu_dev_groups,
> >  	},
> >  	.probe   = afu_probe,
> >  	.remove  = afu_remove,
> > -- 
> > 1.8.3.1
> > 
> 
> Thanks,
> Moritz

[PATCH v5 4/9] fpga: dfl: afu: add userclock sysfs interfaces.

[Wu Hao]

This patch introduces userclock sysfs interfaces for AFU, user
could use these interfaces for clock setting to AFU.

Please note that, this is only working for port header feature
with revision 0, for later revisions, userclock setting is moved
to a separated private feature, so one revision sysfs interface
is exposed to userspace application for this purpose too.

Signed-off-by: Ananda Ravuri <ananda.ravuri@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v2: rebased, and switched to use device_add/remove_groups for sysfs
v3: update kernel version and date in sysfs doc
v4: rebased.
v5: drop sysfs add/remove in init/uinit callback.
    add missed locking for sysfs.
    use is_visible to decide if hardware supports userclk or not.
    remove revision sysfs entry.
---
 Documentation/ABI/testing/sysfs-platform-dfl-port |  28 ++++++
 drivers/fpga/dfl-afu-main.c                       | 111 +++++++++++++++++++++-
 drivers/fpga/dfl.h                                |   9 ++
 3 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
index 1ab3e6f..c2660e4 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-port
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
@@ -46,3 +46,31 @@ Contact:	Wu Hao <hao.wu@intel.com>
 Description:	Read-write. Read or set AFU latency tolerance reporting value.
 		Set ltr to 1 if the AFU can tolerate latency >= 40us or set it
 		to 0 if it is latency sensitive.
+
+What:		/sys/bus/platform/devices/dfl-port.0/userclk_freqcmd
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Write-only. User writes command to this interface to set
+		userclock to AFU.
+
+What:		/sys/bus/platform/devices/dfl-port.0/userclk_freqsts
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. Read this file to get the status of issued command
+		to userclck_freqcmd.
+
+What:		/sys/bus/platform/devices/dfl-port.0/userclk_freqcntrcmd
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Write-only. User writes command to this interface to set
+		userclock counter.
+
+What:		/sys/bus/platform/devices/dfl-port.0/userclk_freqcntrsts
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. Read this file to get the status of issued command
+		to userclck_freqcntrcmd.
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index e955149..f0b45f2 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -274,17 +274,126 @@ static int port_get_id(struct platform_device *pdev)
 }
 static DEVICE_ATTR_RO(power_state);
 
+static ssize_t
+userclk_freqcmd_store(struct device *dev, struct device_attribute *attr,
+		      const char *buf, size_t count)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	u64 userclk_freq_cmd;
+	void __iomem *base;
+
+	if (kstrtou64(buf, 0, &userclk_freq_cmd))
+		return -EINVAL;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	mutex_lock(&pdata->lock);
+	writeq(userclk_freq_cmd, base + PORT_HDR_USRCLK_CMD0);
+	mutex_unlock(&pdata->lock);
+
+	return count;
+}
+static DEVICE_ATTR_WO(userclk_freqcmd);
+
+static ssize_t
+userclk_freqcntrcmd_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	u64 userclk_freqcntr_cmd;
+	void __iomem *base;
+
+	if (kstrtou64(buf, 0, &userclk_freqcntr_cmd))
+		return -EINVAL;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	mutex_lock(&pdata->lock);
+	writeq(userclk_freqcntr_cmd, base + PORT_HDR_USRCLK_CMD1);
+	mutex_unlock(&pdata->lock);
+
+	return count;
+}
+static DEVICE_ATTR_WO(userclk_freqcntrcmd);
+
+static ssize_t
+userclk_freqsts_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	u64 userclk_freqsts;
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	mutex_lock(&pdata->lock);
+	userclk_freqsts = readq(base + PORT_HDR_USRCLK_STS0);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)userclk_freqsts);
+}
+static DEVICE_ATTR_RO(userclk_freqsts);
+
+static ssize_t
+userclk_freqcntrsts_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	u64 userclk_freqcntrsts;
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	mutex_lock(&pdata->lock);
+	userclk_freqcntrsts = readq(base + PORT_HDR_USRCLK_STS1);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)userclk_freqcntrsts);
+}
+static DEVICE_ATTR_RO(userclk_freqcntrsts);
+
 static struct attribute *port_hdr_attrs[] = {
 	&dev_attr_id.attr,
 	&dev_attr_ltr.attr,
 	&dev_attr_ap1_event.attr,
 	&dev_attr_ap2_event.attr,
 	&dev_attr_power_state.attr,
+	&dev_attr_userclk_freqcmd.attr,
+	&dev_attr_userclk_freqcntrcmd.attr,
+	&dev_attr_userclk_freqsts.attr,
+	&dev_attr_userclk_freqcntrsts.attr,
 	NULL,
 };
 
+static umode_t port_hdr_attrs_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	umode_t mode = attr->mode;
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	if (dfl_feature_revision(base) > 0) {
+		/*
+		 * userclk sysfs interfaces are only visible in case port
+		 * revision is 0, as hardware with revision >0 doesn't
+		 * support this.
+		 */
+		if (attr == &dev_attr_userclk_freqcmd.attr ||
+		    attr == &dev_attr_userclk_freqcntrcmd.attr ||
+		    attr == &dev_attr_userclk_freqsts.attr ||
+		    attr == &dev_attr_userclk_freqcntrsts.attr)
+			mode = 0;
+	}
+
+	return mode;
+}
+
 static const struct attribute_group port_hdr_group = {
-	.attrs = port_hdr_attrs,
+	.attrs      = port_hdr_attrs,
+	.is_visible = port_hdr_attrs_visible,
 };
 
 static int port_hdr_init(struct platform_device *pdev,
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 856ea4e..9f0e656 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -120,6 +120,10 @@
 #define PORT_HDR_CAP		0x30
 #define PORT_HDR_CTRL		0x38
 #define PORT_HDR_STS		0x40
+#define PORT_HDR_USRCLK_CMD0	0x50
+#define PORT_HDR_USRCLK_CMD1	0x58
+#define PORT_HDR_USRCLK_STS0	0x60
+#define PORT_HDR_USRCLK_STS1	0x68
 
 /* Port Capability Register Bitfield */
 #define PORT_CAP_PORT_NUM	GENMASK_ULL(1, 0)	/* ID of this port */
@@ -355,6 +359,11 @@ static inline bool dfl_feature_is_port(void __iomem *base)
 		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
 }
 
+static inline u8 dfl_feature_revision(void __iomem *base)
+{
+	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
+}
+
 /**
  * struct dfl_fpga_enum_info - DFL FPGA enumeration information
  *
-- 
1.8.3.1

[PATCH v5 5/9] fpga: dfl: afu: expose __afu_port_enable/disable function.

[Wu Hao]

As these two functions are used by other private features within the
same driver module but different driver files. e.g. in error reporting
private feature, it requires to clear errors when port is in reset.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v2: rebased
v5: add afu prefix to __port_enable/disable function to keep
    alignment with other func exposed across different afu files.
---
 drivers/fpga/dfl-afu-main.c | 26 +++++++++++++++-----------
 drivers/fpga/dfl-afu.h      |  4 ++++
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index f0b45f2..449185c 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -22,14 +22,17 @@
 #include "dfl-afu.h"
 
 /**
- * port_enable - enable a port
+ * __afu_port_enable - enable a port by clear reset
  * @pdev: port platform device.
  *
  * Enable Port by clear the port soft reset bit, which is set by default.
  * The AFU is unable to respond to any MMIO access while in reset.
- * port_enable function should only be used after port_disable function.
+ * __afu_port_enable function should only be used after __afu_port_disable
+ * function.
+ *
+ * The caller needs to hold lock for protection.
  */
-static void port_enable(struct platform_device *pdev)
+void __afu_port_enable(struct platform_device *pdev)
 {
 	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	void __iomem *base;
@@ -52,13 +55,14 @@ static void port_enable(struct platform_device *pdev)
 #define RST_POLL_TIMEOUT 1000 /* us */
 
 /**
- * port_disable - disable a port
+ * __afu_port_disable - disable a port by hold reset
  * @pdev: port platform device.
  *
- * Disable Port by setting the port soft reset bit, it puts the port into
- * reset.
+ * Disable Port by setting the port soft reset bit, it puts the port into reset.
+ *
+ * The caller needs to hold lock for protection.
  */
-static int port_disable(struct platform_device *pdev)
+int __afu_port_disable(struct platform_device *pdev)
 {
 	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	void __iomem *base;
@@ -104,9 +108,9 @@ static int __port_reset(struct platform_device *pdev)
 {
 	int ret;
 
-	ret = port_disable(pdev);
+	ret = __afu_port_disable(pdev);
 	if (!ret)
-		port_enable(pdev);
+		__afu_port_enable(pdev);
 
 	return ret;
 }
@@ -799,9 +803,9 @@ static int port_enable_set(struct platform_device *pdev, bool enable)
 
 	mutex_lock(&pdata->lock);
 	if (enable)
-		port_enable(pdev);
+		__afu_port_enable(pdev);
 	else
-		ret = port_disable(pdev);
+		ret = __afu_port_disable(pdev);
 	mutex_unlock(&pdata->lock);
 
 	return ret;
diff --git a/drivers/fpga/dfl-afu.h b/drivers/fpga/dfl-afu.h
index 0c7630a..83683f2 100644
--- a/drivers/fpga/dfl-afu.h
+++ b/drivers/fpga/dfl-afu.h
@@ -79,6 +79,10 @@ struct dfl_afu {
 	struct dfl_feature_platform_data *pdata;
 };
 
+/* hold pdata->lock when call __afu_port_enable/disable */
+void __afu_port_enable(struct platform_device *pdev);
+int __afu_port_disable(struct platform_device *pdev);
+
 void afu_mmio_region_init(struct dfl_feature_platform_data *pdata);
 int afu_mmio_region_add(struct dfl_feature_platform_data *pdata,
 			u32 region_index, u64 region_size, u64 phys, u32 flags);
-- 
1.8.3.1

[PATCH v5 6/9] fpga: dfl: afu: add error reporting support.

[Wu Hao]

Error reporting is one important private feature, it reports error
detected on port and accelerated function unit (AFU). It introduces
several sysfs interfaces to allow userspace to check and clear
errors detected by hardware.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v2: switch to device_add/remove_group for sysfs.
v3: update kernel version and date in sysfs doc
v4: remove dev_dbg in init/uinit callback function.
v5: rework init/uinit function and improve naming.
    update sysfs entries:
      remove revision sysfs entry.
      merge WO "clear" sysfs to RO "errors" to keep alignment with
      latest changes in fme error reporting support.
    expose error related sysfs entries via dev_groups.
---
 Documentation/ABI/testing/sysfs-platform-dfl-port |  25 +++
 drivers/fpga/Makefile                             |   1 +
 drivers/fpga/dfl-afu-error.c                      | 230 ++++++++++++++++++++++
 drivers/fpga/dfl-afu-main.c                       |   5 +
 drivers/fpga/dfl-afu.h                            |   5 +
 5 files changed, 266 insertions(+)
 create mode 100644 drivers/fpga/dfl-afu-error.c

diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
index c2660e4..6565826 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-port
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
@@ -74,3 +74,28 @@ KernelVersion:	5.4
 Contact:	Wu Hao <hao.wu@intel.com>
 Description:	Read-only. Read this file to get the status of issued command
 		to userclck_freqcntrcmd.
+
+What:		/sys/bus/platform/devices/dfl-port.0/errors/errors
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-Write. Read this file to get errors detected on port and
+		Accelerated Function Unit (AFU). Write error code to this file
+		to clear errors. Write fails with -EINVAL if input parsing
+		fails or input error code doesn't match. Write fails with
+		-EBUSY or -ETIMEDOUT if error can't be cleared as hardware
+		in low power state (-EBUSY) or not respoding (-ETIMEDOUT).
+
+What:		/sys/bus/platform/devices/dfl-port.0/errors/first_error
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. Read this file to get the first error detected by
+		hardware.
+
+What:		/sys/bus/platform/devices/dfl-port.0/errors/first_malformed_req
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. Read this file to get the first malformed request
+		captured by hardware.
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 312b937..7255891 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_FPGA_DFL_AFU)		+= dfl-afu.o
 
 dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
 dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
+dfl-afu-objs += dfl-afu-error.o
 
 # Drivers for FPGAs which implement DFL
 obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
new file mode 100644
index 0000000..c1467ae
--- /dev/null
+++ b/drivers/fpga/dfl-afu-error.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for FPGA Accelerated Function Unit (AFU) Error Reporting
+ *
+ * Copyright 2019 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Wu Hao <hao.wu@linux.intel.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *   Joseph Grecco <joe.grecco@intel.com>
+ *   Enno Luebbers <enno.luebbers@intel.com>
+ *   Tim Whisonant <tim.whisonant@intel.com>
+ *   Ananda Ravuri <ananda.ravuri@intel.com>
+ *   Mitchel Henry <henry.mitchel@intel.com>
+ */
+
+#include <linux/uaccess.h>
+
+#include "dfl-afu.h"
+
+#define PORT_ERROR_MASK		0x8
+#define PORT_ERROR		0x10
+#define PORT_FIRST_ERROR	0x18
+#define PORT_MALFORMED_REQ0	0x20
+#define PORT_MALFORMED_REQ1	0x28
+
+#define ERROR_MASK		GENMASK_ULL(63, 0)
+
+/* mask or unmask port errors by the error mask register. */
+static void __afu_port_err_mask(struct device *dev, bool mask)
+{
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
+
+	writeq(mask ? ERROR_MASK : 0, base + PORT_ERROR_MASK);
+}
+
+static void afu_port_err_mask(struct device *dev, bool mask)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+
+	mutex_lock(&pdata->lock);
+	__afu_port_err_mask(dev, mask);
+	mutex_unlock(&pdata->lock);
+}
+
+/* clear port errors. */
+static int afu_port_err_clear(struct device *dev, u64 err)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	void __iomem *base_err, *base_hdr;
+	int ret = -EBUSY;
+	u64 v;
+
+	base_err = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
+	base_hdr = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	mutex_lock(&pdata->lock);
+
+	/*
+	 * clear Port Errors
+	 *
+	 * - Check for AP6 State
+	 * - Halt Port by keeping Port in reset
+	 * - Set PORT Error mask to all 1 to mask errors
+	 * - Clear all errors
+	 * - Set Port mask to all 0 to enable errors
+	 * - All errors start capturing new errors
+	 * - Enable Port by pulling the port out of reset
+	 */
+
+	/* if device is still in AP6 power state, can not clear any error. */
+	v = readq(base_hdr + PORT_HDR_STS);
+	if (FIELD_GET(PORT_STS_PWR_STATE, v) == PORT_STS_PWR_STATE_AP6) {
+		dev_err(dev, "Could not clear errors, device in AP6 state.\n");
+		goto done;
+	}
+
+	/* Halt Port by keeping Port in reset */
+	ret = __afu_port_disable(pdev);
+	if (ret)
+		goto done;
+
+	/* Mask all errors */
+	__afu_port_err_mask(dev, true);
+
+	/* Clear errors if err input matches with current port errors.*/
+	v = readq(base_err + PORT_ERROR);
+
+	if (v == err) {
+		writeq(v, base_err + PORT_ERROR);
+
+		v = readq(base_err + PORT_FIRST_ERROR);
+		writeq(v, base_err + PORT_FIRST_ERROR);
+	} else {
+		ret = -EINVAL;
+	}
+
+	/* Clear mask */
+	__afu_port_err_mask(dev, false);
+
+	/* Enable the Port by clear the reset */
+	__afu_port_enable(pdev);
+
+done:
+	mutex_unlock(&pdata->lock);
+	return ret;
+}
+
+static ssize_t errors_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 error;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
+
+	mutex_lock(&pdata->lock);
+	error = readq(base + PORT_ERROR);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)error);
+}
+
+static ssize_t errors_store(struct device *dev, struct device_attribute *attr,
+			    const char *buff, size_t count)
+{
+	u64 value;
+	int ret;
+
+	if (kstrtou64(buff, 0, &value))
+		return -EINVAL;
+
+	ret = afu_port_err_clear(dev, value);
+
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(errors);
+
+static ssize_t first_error_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 error;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
+
+	mutex_lock(&pdata->lock);
+	error = readq(base + PORT_FIRST_ERROR);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)error);
+}
+static DEVICE_ATTR_RO(first_error);
+
+static ssize_t first_malformed_req_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 req0, req1;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
+
+	mutex_lock(&pdata->lock);
+	req0 = readq(base + PORT_MALFORMED_REQ0);
+	req1 = readq(base + PORT_MALFORMED_REQ1);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%016llx%016llx\n",
+		       (unsigned long long)req1, (unsigned long long)req0);
+}
+static DEVICE_ATTR_RO(first_malformed_req);
+
+static struct attribute *port_err_attrs[] = {
+	&dev_attr_errors.attr,
+	&dev_attr_first_error.attr,
+	&dev_attr_first_malformed_req.attr,
+	NULL,
+};
+
+static umode_t port_err_attrs_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+
+	/*
+	 * sysfs entries are visible only if related private feature is
+	 * enumerated.
+	 */
+	if (!dfl_get_feature_by_id(dev, PORT_FEATURE_ID_ERROR))
+		return 0;
+
+	return attr->mode;
+}
+
+const struct attribute_group port_err_group = {
+	.name       = "errors",
+	.attrs      = port_err_attrs,
+	.is_visible = port_err_attrs_visible,
+};
+
+static int port_err_init(struct platform_device *pdev,
+			 struct dfl_feature *feature)
+{
+	afu_port_err_mask(&pdev->dev, false);
+
+	return 0;
+}
+
+static void port_err_uinit(struct platform_device *pdev,
+			   struct dfl_feature *feature)
+{
+	afu_port_err_mask(&pdev->dev, true);
+}
+
+const struct dfl_feature_id port_err_id_table[] = {
+	{.id = PORT_FEATURE_ID_ERROR,},
+	{0,}
+};
+
+const struct dfl_feature_ops port_err_ops = {
+	.init = port_err_init,
+	.uinit = port_err_uinit,
+};
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 449185c..e11352a 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -518,6 +518,10 @@ static int port_afu_init(struct platform_device *pdev,
 		.ops = &port_afu_ops,
 	},
 	{
+		.id_table = port_err_id_table,
+		.ops = &port_err_ops,
+	},
+	{
 		.ops = NULL,
 	}
 };
@@ -860,6 +864,7 @@ static int afu_remove(struct platform_device *pdev)
 static const struct attribute_group *afu_dev_groups[] = {
 	&port_hdr_group,
 	&port_afu_group,
+	&port_err_group,
 	NULL
 };
 
diff --git a/drivers/fpga/dfl-afu.h b/drivers/fpga/dfl-afu.h
index 83683f2..576e949 100644
--- a/drivers/fpga/dfl-afu.h
+++ b/drivers/fpga/dfl-afu.h
@@ -101,4 +101,9 @@ int afu_dma_map_region(struct dfl_feature_platform_data *pdata,
 struct dfl_afu_dma_region *
 afu_dma_region_find(struct dfl_feature_platform_data *pdata,
 		    u64 iova, u64 size);
+
+extern const struct dfl_feature_ops port_err_ops;
+extern const struct dfl_feature_id port_err_id_table[];
+extern const struct attribute_group port_err_group;
+
 #endif /* __DFL_AFU_H */
-- 
1.8.3.1

[PATCH v5 7/9] fpga: dfl: afu: add STP (SignalTap) support

[Wu Hao]

STP (SignalTap) is one of the private features under the port for
debugging. This patch adds private feature driver support for it
to allow userspace applications to mmap related mmio region and
provide STP service.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v4: remove uinit callback which does nothing.
    remove dev_dbg in init callback function.
---
 drivers/fpga/dfl-afu-main.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index e11352a..e4a34dc 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -508,6 +508,27 @@ static int port_afu_init(struct platform_device *pdev,
 	.init = port_afu_init,
 };
 
+static int port_stp_init(struct platform_device *pdev,
+			 struct dfl_feature *feature)
+{
+	struct resource *res = &pdev->resource[feature->resource_index];
+
+	return afu_mmio_region_add(dev_get_platdata(&pdev->dev),
+				   DFL_PORT_REGION_INDEX_STP,
+				   resource_size(res), res->start,
+				   DFL_PORT_REGION_MMAP | DFL_PORT_REGION_READ |
+				   DFL_PORT_REGION_WRITE);
+}
+
+static const struct dfl_feature_id port_stp_id_table[] = {
+	{.id = PORT_FEATURE_ID_STP,},
+	{0,}
+};
+
+static const struct dfl_feature_ops port_stp_ops = {
+	.init = port_stp_init,
+};
+
 static struct dfl_feature_driver port_feature_drvs[] = {
 	{
 		.id_table = port_hdr_id_table,
@@ -522,6 +543,10 @@ static int port_afu_init(struct platform_device *pdev,
 		.ops = &port_err_ops,
 	},
 	{
+		.id_table = port_stp_id_table,
+		.ops = &port_stp_ops,
+	},
+	{
 		.ops = NULL,
 	}
 };
-- 
1.8.3.1

[PATCH v5 8/9] fpga: dfl: fme: add global error reporting support

[Wu Hao]

This patch adds support for global error reporting for FPGA
Management Engine (FME), it introduces sysfs interfaces to
report different error detected by the hardware, and allow
user to clear errors or inject error for testing purpose.

Signed-off-by: Luwei Kang <luwei.kang@intel.com>
Signed-off-by: Ananda Ravuri <ananda.ravuri@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
v2: switch to device_add/remove_groups for sysfs.
v3: update kernel version and date in sysfs doc
v4: rebase, remove dev_dbg in init/uinit callback.
v5: reorganize sysfs entries:
      remove "fme-errors" sub folder and related sysfs entries to
      upper level folder.
      merge WO "clear" to RO "errors" to keep alignment with error
      sysfs entries in upper level folder.
      remove revision sysfs entry.
    add missed locking in sysfs entries.
    expose sysfs group with is_visible via dev_groups.
---
 Documentation/ABI/testing/sysfs-platform-dfl-fme |  62 ++++
 drivers/fpga/Makefile                            |   2 +-
 drivers/fpga/dfl-fme-error.c                     | 359 +++++++++++++++++++++++
 drivers/fpga/dfl-fme-main.c                      |  17 +-
 drivers/fpga/dfl-fme.h                           |   3 +
 5 files changed, 440 insertions(+), 3 deletions(-)
 create mode 100644 drivers/fpga/dfl-fme-error.c

diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-fme b/Documentation/ABI/testing/sysfs-platform-dfl-fme
index 65372aa..72634d3 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-fme
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-fme
@@ -44,3 +44,65 @@ Description:	Read-only. It returns socket_id to indicate which socket
 		this FPGA belongs to, only valid for integrated solution.
 		User only needs this information, in case standard numa node
 		can't provide correct information.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/pcie0_errors
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-Write. Read this file for errors detected on pcie0 link.
+		Write this file to clear errors logged in pcie0_errors. Write
+		fails with -EINVAL if input parsing fails or input error code
+		doesn't match.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/pcie1_errors
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-Write. Read this file for errors detected on pcie1 link.
+		Write this file to clear errors logged in pcie1_errors. Write
+		fails with -EINVAL if input parsing fails or input error code
+		doesn't match.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/nonfatal_errors
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. It returns non-fatal errors detected.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/catfatal_errors
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. It returns catastrophic and fatal errors detected.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/inject_errors
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-Write. Read this file to check errors injected. Write this
+		file to inject errors for testing purpose. Write fails with
+		-EINVAL if input parsing fails or input inject error code isn't
+		supported.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/fme_errors
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-Write. Read this file to get errors detected on FME.
+		Write this file to clear errors logged in fme_errors. Write
+		fials with -EINVAL if input parsing fails or input error code
+		doesn't match.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/first_error
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. Read this file to get the first error detected by
+		hardware.
+
+What:		/sys/bus/platform/devices/dfl-fme.0/errors/next_error
+Date:		August 2019
+KernelVersion:  5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. Read this file to get the second error detected by
+		hardware.
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 7255891..4865b74 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_FPGA_DFL_FME_BRIDGE)	+= dfl-fme-br.o
 obj-$(CONFIG_FPGA_DFL_FME_REGION)	+= dfl-fme-region.o
 obj-$(CONFIG_FPGA_DFL_AFU)		+= dfl-afu.o
 
-dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
+dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o dfl-fme-error.o
 dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
 dfl-afu-objs += dfl-afu-error.o
 
diff --git a/drivers/fpga/dfl-fme-error.c b/drivers/fpga/dfl-fme-error.c
new file mode 100644
index 0000000..f897d41
--- /dev/null
+++ b/drivers/fpga/dfl-fme-error.c
@@ -0,0 +1,359 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for FPGA Management Engine Error Management
+ *
+ * Copyright 2019 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Kang Luwei <luwei.kang@intel.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *   Wu Hao <hao.wu@intel.com>
+ *   Joseph Grecco <joe.grecco@intel.com>
+ *   Enno Luebbers <enno.luebbers@intel.com>
+ *   Tim Whisonant <tim.whisonant@intel.com>
+ *   Ananda Ravuri <ananda.ravuri@intel.com>
+ *   Mitchel, Henry <henry.mitchel@intel.com>
+ */
+
+#include <linux/uaccess.h>
+
+#include "dfl.h"
+#include "dfl-fme.h"
+
+#define FME_ERROR_MASK		0x8
+#define FME_ERROR		0x10
+#define MBP_ERROR		BIT_ULL(6)
+#define PCIE0_ERROR_MASK	0x18
+#define PCIE0_ERROR		0x20
+#define PCIE1_ERROR_MASK	0x28
+#define PCIE1_ERROR		0x30
+#define FME_FIRST_ERROR		0x38
+#define FME_NEXT_ERROR		0x40
+#define RAS_NONFAT_ERROR_MASK	0x48
+#define RAS_NONFAT_ERROR	0x50
+#define RAS_CATFAT_ERROR_MASK	0x58
+#define RAS_CATFAT_ERROR	0x60
+#define RAS_ERROR_INJECT	0x68
+#define INJECT_ERROR_MASK	GENMASK_ULL(2, 0)
+
+#define ERROR_MASK		GENMASK_ULL(63, 0)
+
+static ssize_t pcie0_errors_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 value;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	value = readq(base + PCIE0_ERROR);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+
+static ssize_t pcie0_errors_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	int ret = 0;
+	u64 v, val;
+
+	if (kstrtou64(buf, 0, &val))
+		return -EINVAL;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	writeq(GENMASK_ULL(63, 0), base + PCIE0_ERROR_MASK);
+
+	v = readq(base + PCIE0_ERROR);
+	if (val == v)
+		writeq(v, base + PCIE0_ERROR);
+	else
+		ret = -EINVAL;
+
+	writeq(0ULL, base + PCIE0_ERROR_MASK);
+	mutex_unlock(&pdata->lock);
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(pcie0_errors);
+
+static ssize_t pcie1_errors_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 value;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	value = readq(base + PCIE1_ERROR);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+
+static ssize_t pcie1_errors_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	int ret = 0;
+	u64 v, val;
+
+	if (kstrtou64(buf, 0, &val))
+		return -EINVAL;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	writeq(GENMASK_ULL(63, 0), base + PCIE1_ERROR_MASK);
+
+	v = readq(base + PCIE1_ERROR);
+	if (val == v)
+		writeq(v, base + PCIE1_ERROR);
+	else
+		ret = -EINVAL;
+
+	writeq(0ULL, base + PCIE1_ERROR_MASK);
+	mutex_unlock(&pdata->lock);
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(pcie1_errors);
+
+static ssize_t nonfatal_errors_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)readq(base + RAS_NONFAT_ERROR));
+}
+static DEVICE_ATTR_RO(nonfatal_errors);
+
+static ssize_t catfatal_errors_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)readq(base + RAS_CATFAT_ERROR));
+}
+static DEVICE_ATTR_RO(catfatal_errors);
+
+static ssize_t inject_errors_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 v;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	v = readq(base + RAS_ERROR_INJECT);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)FIELD_GET(INJECT_ERROR_MASK, v));
+}
+
+static ssize_t inject_errors_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u8 inject_error;
+	u64 v;
+
+	if (kstrtou8(buf, 0, &inject_error))
+		return -EINVAL;
+
+	if (inject_error & ~INJECT_ERROR_MASK)
+		return -EINVAL;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	v = readq(base + RAS_ERROR_INJECT);
+	v &= ~INJECT_ERROR_MASK;
+	v |= FIELD_PREP(INJECT_ERROR_MASK, inject_error);
+	writeq(v, base + RAS_ERROR_INJECT);
+	mutex_unlock(&pdata->lock);
+
+	return count;
+}
+static DEVICE_ATTR_RW(inject_errors);
+
+static ssize_t fme_errors_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 value;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	value = readq(base + FME_ERROR);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+
+static ssize_t fme_errors_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 v, val;
+	int ret = 0;
+
+	if (kstrtou64(buf, 0, &val))
+		return -EINVAL;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	writeq(GENMASK_ULL(63, 0), base + FME_ERROR_MASK);
+
+	v = readq(base + FME_ERROR);
+	if (val == v)
+		writeq(v, base + FME_ERROR);
+	else
+		ret = -EINVAL;
+
+	/* Workaround: disable MBP_ERROR if feature revision is 0 */
+	writeq(dfl_feature_revision(base) ? 0ULL : MBP_ERROR,
+	       base + FME_ERROR_MASK);
+	mutex_unlock(&pdata->lock);
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(fme_errors);
+
+static ssize_t first_error_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 value;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	value = readq(base + FME_FIRST_ERROR);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+static DEVICE_ATTR_RO(first_error);
+
+static ssize_t next_error_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 value;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+	value = readq(base + FME_NEXT_ERROR);
+	mutex_unlock(&pdata->lock);
+
+	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+}
+static DEVICE_ATTR_RO(next_error);
+
+static struct attribute *fme_global_err_attrs[] = {
+	&dev_attr_pcie0_errors.attr,
+	&dev_attr_pcie1_errors.attr,
+	&dev_attr_nonfatal_errors.attr,
+	&dev_attr_catfatal_errors.attr,
+	&dev_attr_inject_errors.attr,
+	&dev_attr_fme_errors.attr,
+	&dev_attr_first_error.attr,
+	&dev_attr_next_error.attr,
+	NULL,
+};
+
+static umode_t fme_global_err_attrs_visible(struct kobject *kobj,
+					    struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+
+	/*
+	 * sysfs entries are visible only if related private feature is
+	 * enumerated.
+	 */
+	if (!dfl_get_feature_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR))
+		return 0;
+
+	return attr->mode;
+}
+
+const struct attribute_group fme_global_err_group = {
+	.name       = "errors",
+	.attrs      = fme_global_err_attrs,
+	.is_visible = fme_global_err_attrs_visible,
+};
+
+static void fme_err_mask(struct device *dev, bool mask)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+	mutex_lock(&pdata->lock);
+
+	/* Workaround: keep MBP_ERROR always masked if revision is 0 */
+	if (dfl_feature_revision(base))
+		writeq(mask ? ERROR_MASK : 0, base + FME_ERROR_MASK);
+	else
+		writeq(mask ? ERROR_MASK : MBP_ERROR, base + FME_ERROR_MASK);
+
+	writeq(mask ? ERROR_MASK : 0, base + PCIE0_ERROR_MASK);
+	writeq(mask ? ERROR_MASK : 0, base + PCIE1_ERROR_MASK);
+	writeq(mask ? ERROR_MASK : 0, base + RAS_NONFAT_ERROR_MASK);
+	writeq(mask ? ERROR_MASK : 0, base + RAS_CATFAT_ERROR_MASK);
+
+	mutex_unlock(&pdata->lock);
+}
+
+static int fme_global_err_init(struct platform_device *pdev,
+			       struct dfl_feature *feature)
+{
+	fme_err_mask(&pdev->dev, false);
+
+	return 0;
+}
+
+static void fme_global_err_uinit(struct platform_device *pdev,
+				 struct dfl_feature *feature)
+{
+	fme_err_mask(&pdev->dev, true);
+}
+
+const struct dfl_feature_id fme_global_err_id_table[] = {
+	{.id = FME_FEATURE_ID_GLOBAL_ERR,},
+	{0,}
+};
+
+const struct dfl_feature_ops fme_global_err_ops = {
+	.init = fme_global_err_init,
+	.uinit = fme_global_err_uinit,
+};
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index bf8114d..4d78e18 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -127,7 +127,10 @@ static ssize_t socket_id_show(struct device *dev,
 	&dev_attr_socket_id.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(fme_hdr);
+
+static const struct attribute_group fme_hdr_group = {
+	.attrs = fme_hdr_attrs,
+};
 
 static long fme_hdr_ioctl_release_port(struct dfl_feature_platform_data *pdata,
 				       unsigned long arg)
@@ -188,6 +191,10 @@ static long fme_hdr_ioctl(struct platform_device *pdev,
 		.ops = &fme_pr_mgmt_ops,
 	},
 	{
+		.id_table = fme_global_err_id_table,
+		.ops = &fme_global_err_ops,
+	},
+	{
 		.ops = NULL,
 	},
 };
@@ -333,10 +340,16 @@ static int fme_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct attribute_group *fme_dev_groups[] = {
+	&fme_hdr_group,
+	&fme_global_err_group,
+	NULL
+};
+
 static struct platform_driver fme_driver = {
 	.driver	= {
 		.name       = DFL_FPGA_FEATURE_DEV_FME,
-		.dev_groups = fme_hdr_groups,
+		.dev_groups = fme_dev_groups,
 	},
 	.probe   = fme_probe,
 	.remove  = fme_remove,
diff --git a/drivers/fpga/dfl-fme.h b/drivers/fpga/dfl-fme.h
index e4131e8..6685c8e 100644
--- a/drivers/fpga/dfl-fme.h
+++ b/drivers/fpga/dfl-fme.h
@@ -35,5 +35,8 @@ struct dfl_fme {
 
 extern const struct dfl_feature_ops fme_pr_mgmt_ops;
 extern const struct dfl_feature_id fme_pr_mgmt_id_table[];
+extern const struct dfl_feature_ops fme_global_err_ops;
+extern const struct dfl_feature_id fme_global_err_id_table[];
+extern const struct attribute_group fme_global_err_group;
 
 #endif /* __DFL_FME_H */
-- 
1.8.3.1

[PATCH v5 9/9] Documentation: fpga: dfl: add descriptions for virtualization and new interfaces.

[Wu Hao]

This patch adds virtualization support description for DFL based
FPGA devices (based on PCIe SRIOV), and introductions to new
interfaces added by new dfl private feature drivers.

[mdf@kernel.org: Fixed up to make it work with new reStructuredText docs]
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 Documentation/fpga/dfl.rst | 105 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index 2f125ab..6fa483f 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -87,6 +87,8 @@ The following functions are exposed through ioctls:
 - Get driver API version (DFL_FPGA_GET_API_VERSION)
 - Check for extensions (DFL_FPGA_CHECK_EXTENSION)
 - Program bitstream (DFL_FPGA_FME_PORT_PR)
+- Assign port to PF (DFL_FPGA_FME_PORT_ASSIGN)
+- Release port from PF (DFL_FPGA_FME_PORT_RELEASE)
 
 More functions are exposed through sysfs
 (/sys/class/fpga_region/regionX/dfl-fme.n/):
@@ -102,6 +104,10 @@ More functions are exposed through sysfs
      one FPGA device may have more than one port, this sysfs interface indicates
      how many ports the FPGA device has.
 
+ Global error reporting management (errors/)
+     error reporting sysfs interfaces allow user to read errors detected by the
+     hardware, and clear the logged errors.
+
 
 FIU - PORT
 ==========
@@ -143,6 +149,10 @@ More functions are exposed through sysfs:
  Read Accelerator GUID (afu_id)
      afu_id indicates which PR bitstream is programmed to this AFU.
 
+ Error reporting (errors/)
+     error reporting sysfs interfaces allow user to read port/afu errors
+     detected by the hardware, and clear the logged errors.
+
 
 DFL Framework Overview
 ======================
@@ -218,6 +228,101 @@ the compat_id exposed by the target FPGA region. This check is usually done by
 userspace before calling the reconfiguration IOCTL.
 
 
+FPGA virtualization - PCIe SRIOV
+================================
+This section describes the virtualization support on DFL based FPGA device to
+enable accessing an accelerator from applications running in a virtual machine
+(VM). This section only describes the PCIe based FPGA device with SRIOV support.
+
+Features supported by the particular FPGA device are exposed through Device
+Feature Lists, as illustrated below:
+
+::
+
+    +-------------------------------+  +-------------+
+    |              PF               |  |     VF      |
+    +-------------------------------+  +-------------+
+        ^            ^         ^              ^
+        |            |         |              |
+  +-----|------------|---------|--------------|-------+
+  |     |            |         |              |       |
+  |  +-----+     +-------+ +-------+      +-------+   |
+  |  | FME |     | Port0 | | Port1 |      | Port2 |   |
+  |  +-----+     +-------+ +-------+      +-------+   |
+  |                  ^         ^              ^       |
+  |                  |         |              |       |
+  |              +-------+ +------+       +-------+   |
+  |              |  AFU  | |  AFU |       |  AFU  |   |
+  |              +-------+ +------+       +-------+   |
+  |                                                   |
+  |            DFL based FPGA PCIe Device             |
+  +---------------------------------------------------+
+
+FME is always accessed through the physical function (PF).
+
+Ports (and related AFUs) are accessed via PF by default, but could be exposed
+through virtual function (VF) devices via PCIe SRIOV. Each VF only contains
+1 Port and 1 AFU for isolation. Users could assign individual VFs (accelerators)
+created via PCIe SRIOV interface, to virtual machines.
+
+The driver organization in virtualization case is illustrated below:
+::
+
+    +-------++------++------+             |
+    | FME   || FME  || FME  |             |
+    | FPGA  || FPGA || FPGA |             |
+    |Manager||Bridge||Region|             |
+    +-------++------++------+             |
+    +-----------------------+  +--------+ |             +--------+
+    |          FME          |  |  AFU   | |             |  AFU   |
+    |         Module        |  | Module | |             | Module |
+    +-----------------------+  +--------+ |             +--------+
+          +-----------------------+       |       +-----------------------+
+          | FPGA Container Device |       |       | FPGA Container Device |
+          |  (FPGA Base Region)   |       |       |  (FPGA Base Region)   |
+          +-----------------------+       |       +-----------------------+
+            +------------------+          |         +------------------+
+            | FPGA PCIE Module |          | Virtual | FPGA PCIE Module |
+            +------------------+   Host   | Machine +------------------+
+   -------------------------------------- | ------------------------------
+             +---------------+            |          +---------------+
+             | PCI PF Device |            |          | PCI VF Device |
+             +---------------+            |          +---------------+
+
+FPGA PCIe device driver is always loaded first once a FPGA PCIe PF or VF device
+is detected. It:
+
+* Finishes enumeration on both FPGA PCIe PF and VF device using common
+  interfaces from DFL framework.
+* Supports SRIOV.
+
+The FME device driver plays a management role in this driver architecture, it
+provides ioctls to release Port from PF and assign Port to PF. After release
+a port from PF, then it's safe to expose this port through a VF via PCIe SRIOV
+sysfs interface.
+
+To enable accessing an accelerator from applications running in a VM, the
+respective AFU's port needs to be assigned to a VF using the following steps:
+
+#. The PF owns all AFU ports by default. Any port that needs to be
+   reassigned to a VF must first be released through the
+   DFL_FPGA_FME_PORT_RELEASE ioctl on the FME device.
+
+#. Once N ports are released from PF, then user can use command below
+   to enable SRIOV and VFs. Each VF owns only one Port with AFU.
+
+   ::
+
+      echo N > $PCI_DEVICE_PATH/sriov_numvfs
+
+#. Pass through the VFs to VMs
+
+#. The AFU under VF is accessible from applications in VM (using the
+   same driver inside the VF).
+
+Note that an FME can't be assigned to a VF, thus PR and other management
+functions are only available via the PF.
+
 Device enumeration
 ==================
 This section introduces how applications enumerate the fpga device from
-- 
1.8.3.1

Re: [PATCH v5 0/9] FPGA DFL updates

[Wu Hao]

On Mon, Aug 12, 2019 at 10:49:55AM +0800, Wu Hao wrote:
> Hi Greg,
> 
> This is v5 patchset which adds more features to FPGA DFL. Marjor changes
> against v4 are sysfs related code rework to address comments on v4.
> 
> Please help to take a look. Thanks!

Hi Greg,

Did you get a chance to take a look at this new version patchset? :)

Thanks
Hao

> 
> Main changes from v4:
>   - convert code to use dev_groups for sysfs entries (#2, #3, #4, #6, #8).
>   - clean up for empty init function after remove sysfs add/remove (#1).
>   - introduce is_visible for sysfs groups (#3, #4, #6, #8).
>   - remove revision sysfs entries (#4, #6, #8).
>   - improve naming on shared functions (#5).
>   - reorganize sysfs entries for port and fme error reporting (#6, #8).
> 
> Main changes from v3:
>   - drop avx512 partail reconfiguration patch for now.
>   - split dfl_fpga_cdev_config_port to 2 functions *_release/assign_port
>     (#1).
>   - split __dfl_fpga_cdev_config_port_vf into 2 functions with locking
>     added (#2).
>   - improve description in sysfs doc to avoid misunderstanding (#3).
>   - switch to boolean in sysfs entry store function (#3).
>   - remove dev_dbg in init/uinit callback function (#7, #9, #11).
>   - remove uinit callback which does does nothing (#8, #9)
> 
> Main changes from v2:
>   - update kernel version/date in sysfs doc (patch #4, #5, #8, #10, #11).
>   - add back Documentation patch (patch #12).
> 
> Main changes from v1:
>   - remove DRV/MODULE_VERSION modifications. (patch #1, #3, #4, #6)
>   - remove argsz from new ioctls. (patch #2)
>   - replace sysfs_create/remove_* with device_add/remove_* for sysfs entries.
>     (patch #5, #8, #11)
> 
> Wu Hao (9):
>   fpga: dfl: make init callback optional
>   fpga: dfl: fme: convert platform_driver to use dev_groups
>   fpga: dfl: afu: convert platform_driver to use dev_groups
>   fpga: dfl: afu: add userclock sysfs interfaces.
>   fpga: dfl: afu: expose __afu_port_enable/disable function.
>   fpga: dfl: afu: add error reporting support.
>   fpga: dfl: afu: add STP (SignalTap) support
>   fpga: dfl: fme: add global error reporting support
>   Documentation: fpga: dfl: add descriptions for virtualization and new
>     interfaces.
> 
>  Documentation/ABI/testing/sysfs-platform-dfl-fme  |  62 ++++
>  Documentation/ABI/testing/sysfs-platform-dfl-port |  53 ++++
>  Documentation/fpga/dfl.rst                        | 105 +++++++
>  drivers/fpga/Makefile                             |   3 +-
>  drivers/fpga/dfl-afu-error.c                      | 230 ++++++++++++++
>  drivers/fpga/dfl-afu-main.c                       | 230 +++++++++++---
>  drivers/fpga/dfl-afu.h                            |   9 +
>  drivers/fpga/dfl-fme-error.c                      | 359 ++++++++++++++++++++++
>  drivers/fpga/dfl-fme-main.c                       |  42 +--
>  drivers/fpga/dfl-fme.h                            |   3 +
>  drivers/fpga/dfl.c                                |  10 +-
>  drivers/fpga/dfl.h                                |   9 +
>  12 files changed, 1041 insertions(+), 74 deletions(-)
>  create mode 100644 drivers/fpga/dfl-afu-error.c
>  create mode 100644 drivers/fpga/dfl-fme-error.c
> 
> -- 
> 1.8.3.1

Re: [PATCH v5 0/9] FPGA DFL updates

[Greg KH]

On Mon, Aug 19, 2019 at 01:31:33PM +0800, Wu Hao wrote:
> On Mon, Aug 12, 2019 at 10:49:55AM +0800, Wu Hao wrote:
> > Hi Greg,
> > 
> > This is v5 patchset which adds more features to FPGA DFL. Marjor changes
> > against v4 are sysfs related code rework to address comments on v4.
> > 
> > Please help to take a look. Thanks!
> 
> Hi Greg,
> 
> Did you get a chance to take a look at this new version patchset? :)

I'm not the FPGA maintainer, what about the review from the other one
first?  :)

thanks,

greg k-h

Re: [PATCH v5 0/9] FPGA DFL updates

[Wu Hao]

On Mon, Aug 19, 2019 at 10:51:24PM +0200, Greg KH wrote:
> On Mon, Aug 19, 2019 at 01:31:33PM +0800, Wu Hao wrote:
> > On Mon, Aug 12, 2019 at 10:49:55AM +0800, Wu Hao wrote:
> > > Hi Greg,
> > > 
> > > This is v5 patchset which adds more features to FPGA DFL. Marjor changes
> > > against v4 are sysfs related code rework to address comments on v4.
> > > 
> > > Please help to take a look. Thanks!
> > 
> > Hi Greg,
> > 
> > Did you get a chance to take a look at this new version patchset? :)
> 
> I'm not the FPGA maintainer, what about the review from the other one
> first?  :)


Sure! :)


Hi Moritz

Could you please help review these patches? Thanks! :)

Thanks
Hao

> 
> thanks,
> 
> greg k-h