[v2] fpga: dfl: fme: adding reserved bits for revision of FME/Port error

Commit Message

Zhang, Tianfei April 19, 2022, 3:03 a.m. UTC

From: Tianfei zhang <[e-mail]>

There are 2 different register layouts for FME/Port error
registers. The new production like Intel PAC N6000 plans to
deploy the new register format which it is more reasonable.

To distinguish the register layouts,we provide another sysfs
node for revision info, but it is a bad method that using one
sysfs node's value to determine the usage of other sysfs node.

This patch introduces 4 reserved bits (Bit[59:56]) which
are reserved by HW, dfl driver appends the FME/Port error
revision info on those bits for attribution on the readout
value.

Signed-off-by: Tianfei zhang <[e-mail]>
---
v2:
 - add documentation for this change.
 - fix the reverse xmas tree declaration.
---
 .../ABI/testing/sysfs-platform-dfl-fme        | 37 ++++++++++++-------
 drivers/fpga/dfl-fme-error.c                  | 36 +++++++++++++++---
 2 files changed, 54 insertions(+), 19 deletions(-)

Comments

Wu, Hao May 5, 2022, 6:47 a.m. UTC | #1

> -----Original Message-----
> From: Zhang, Tianfei <[e-mail]>
> Sent: Tuesday, April 19, 2022 11:04 AM
> To: Wu, Hao <[e-mail]>; [e-mail]; [e-mail]; Xu, Yilun
> <[e-mail]>; [e-mail]
> Cc: Zhang, Tianfei <[e-mail]>
> Subject: [PATCH v2] fpga: dfl: fme: adding reserved bits for revision of FME/Port
> error
> 
> From: Tianfei zhang <[e-mail]>
> 
> There are 2 different register layouts for FME/Port error
> registers. The new production like Intel PAC N6000 plans to
> deploy the new register format which it is more reasonable.
> 

Can you explain a little more why there would be different register
layouts? There will only be two layouts or more?
Intel PAC N6000 "plans" to have new register format? Not finalized yet?
So anyone could define its own error register layout, is my understanding correct?

> To distinguish the register layouts,we provide another sysfs
> node for revision info, but it is a bad method that using one
> sysfs node's value to determine the usage of other sysfs node.
> 
> This patch introduces 4 reserved bits (Bit[59:56]) which
> are reserved by HW, dfl driver appends the FME/Port error
> revision info on those bits for attribution on the readout
> value.

Will UUID be more reasonable than just 4bits value? If we 
consider this is a FPGA, anyone could implement its own
error register layout, then just reserve 4bits will not be
enough? I hope we don't need to modify the code and
design again...

> 
> Signed-off-by: Tianfei zhang <[e-mail]>
> ---
> v2:
>  - add documentation for this change.
>  - fix the reverse xmas tree declaration.
> ---
>  .../ABI/testing/sysfs-platform-dfl-fme        | 37 ++++++++++++-------
>  drivers/fpga/dfl-fme-error.c                  | 36 +++++++++++++++---
>  2 files changed, 54 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-fme
> b/Documentation/ABI/testing/sysfs-platform-dfl-fme
> index d6ab34e81b9b..b886568d6071 100644
> --- a/Documentation/ABI/testing/sysfs-platform-dfl-fme
> +++ b/Documentation/ABI/testing/sysfs-platform-dfl-fme
> @@ -50,46 +50,57 @@ Date:		August 2019
>  KernelVersion:  5.4
>  Contact:	Wu Hao <[e-mail]>
>  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.
> +		The readout value has embedded 4 bits revision attribution
> +		in Bit[59:56] which reserved by hardware. Write this file to
> +		clear errors logged in pcie0_errors. Clean Bit[59:56] before
> +		write this file. 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 <[e-mail]>
>  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.
> +		The readout value has embedded 4 bits revision attribution
> +		in Bit[59:56] which reserved by hardware. Write this file to
> +		clear errors logged in pcie1_errors. Clean Bit[59:56] before
> +		write this file. 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 <[e-mail]>
> -Description:	Read-only. It returns non-fatal errors detected.
> +Description:	Read-only. It returns non-fatal errors detected. The readout
> +		value has embedded 4 bits revision attribution in Bit[59:56]
> +		which reserved by hardware.
> 
>  What:		/sys/bus/platform/devices/dfl-fme.0/errors/catfatal_errors
>  Date:		August 2019
>  KernelVersion:  5.4
>  Contact:	Wu Hao <[e-mail]>
>  Description:	Read-only. It returns catastrophic and fatal errors detected.
> +		The readout value has embedded 4 bits revision attribution in
> +		Bit[59:56] which reserved by hardware.
> 
>  What:		/sys/bus/platform/devices/dfl-fme.0/errors/inject_errors
>  Date:		August 2019
>  KernelVersion:  5.4
>  Contact:	Wu Hao <[e-mail]>
> -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.
> +Description:	Read-Write. Read this file to check errors injected. The readout
> +		value has embedded 4 bits revision attribution which reserved
> by
> +		hardware. Write this file to inject errors for testing purpose.
> +		Clean Bit[59:56] before write this file. 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 <[e-mail]>
> -Description:	Read-Write. Read this file to get errors detected on FME.
> -		Write this file to clear errors logged in fme_errors. Write
> +Description:	Read-Write. Read this file to get errors detected on FME. The
> +		readout value has embedded 4 bits revision attribution which
> +		reserved by hardware. Write this file to clear errors logged
> +		in fme_errors. Clean Bit[59:56] before write this file. Write
>  		fials with -EINVAL if input parsing fails or input error code
>  		doesn't match.
> 
> diff --git a/drivers/fpga/dfl-fme-error.c b/drivers/fpga/dfl-fme-error.c
> index 51c2892ec06d..a440bc09938d 100644
> --- a/drivers/fpga/dfl-fme-error.c
> +++ b/drivers/fpga/dfl-fme-error.c
> @@ -39,6 +39,22 @@
> 
>  #define ERROR_MASK		GENMASK_ULL(63, 0)
> 
> +/* Bit[59:56] was reserved by software for error revision */
> +#define ERROR_SW_REVISION_MASK GENMASK_ULL(59, 56)
> +
> +static u64 set_error_revision(struct device *dev, u64 value)
> +{
> +	void __iomem *base;
> +	u64 revision;
> +	u64 dfh;
> +
> +	base = dfl_get_feature_ioaddr_by_id(dev,
> FME_FEATURE_ID_GLOBAL_ERR);
> +	dfh = readq(base);
> +	revision = FIELD_GET(DFH_REVISION, dfh);
> +
> +	return value | FIELD_PREP(ERROR_SW_REVISION_MASK, revision);
> +}
> +
>  static ssize_t pcie0_errors_show(struct device *dev,
>  				 struct device_attribute *attr, char *buf)
>  {
> @@ -52,7 +68,8 @@ static ssize_t pcie0_errors_show(struct device *dev,
>  	value = readq(base + PCIE0_ERROR);
>  	mutex_unlock(&pdata->lock);
> 
> -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> +	return sprintf(buf, "0x%llx\n",
> +		       (unsigned long long)set_error_revision(dev, value));
>  }
> 
>  static ssize_t pcie0_errors_store(struct device *dev,
> @@ -97,7 +114,8 @@ static ssize_t pcie1_errors_show(struct device *dev,
>  	value = readq(base + PCIE1_ERROR);
>  	mutex_unlock(&pdata->lock);
> 
> -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> +	return sprintf(buf, "0x%llx\n",
> +		       (unsigned long long)set_error_revision(dev, value));
>  }
> 
>  static ssize_t pcie1_errors_store(struct device *dev,
> @@ -133,11 +151,13 @@ static ssize_t nonfatal_errors_show(struct device
> *dev,
>  				    struct device_attribute *attr, char *buf)
>  {
>  	void __iomem *base;
> +	u64 value;
> 
>  	base = dfl_get_feature_ioaddr_by_id(dev,
> FME_FEATURE_ID_GLOBAL_ERR);
> +	value = readq(base + RAS_NONFAT_ERROR);
> 
>  	return sprintf(buf, "0x%llx\n",
> -		       (unsigned long long)readq(base + RAS_NONFAT_ERROR));
> +		       (unsigned long long)set_error_revision(dev, value));
>  }
>  static DEVICE_ATTR_RO(nonfatal_errors);
> 
> @@ -145,11 +165,13 @@ static ssize_t catfatal_errors_show(struct device *dev,
>  				    struct device_attribute *attr, char *buf)
>  {
>  	void __iomem *base;
> +	u64 value;
> 
>  	base = dfl_get_feature_ioaddr_by_id(dev,
> FME_FEATURE_ID_GLOBAL_ERR);
> +	value = readq(base + RAS_CATFAT_ERROR);
> 
>  	return sprintf(buf, "0x%llx\n",
> -		       (unsigned long long)readq(base + RAS_CATFAT_ERROR));
> +		       (unsigned long long)set_error_revision(dev, value));
>  }
>  static DEVICE_ATTR_RO(catfatal_errors);
> 
> @@ -165,9 +187,10 @@ static ssize_t inject_errors_show(struct device *dev,
>  	mutex_lock(&pdata->lock);
>  	v = readq(base + RAS_ERROR_INJECT);
>  	mutex_unlock(&pdata->lock);
> +	v = FIELD_GET(INJECT_ERROR_MASK, v);
> 
>  	return sprintf(buf, "0x%llx\n",
> -		       (unsigned long long)FIELD_GET(INJECT_ERROR_MASK, v));
> +		       (unsigned long long)set_error_revision(dev, v));
>  }
> 
>  static ssize_t inject_errors_store(struct device *dev,
> @@ -211,7 +234,8 @@ static ssize_t fme_errors_show(struct device *dev,
>  	value = readq(base + FME_ERROR);
>  	mutex_unlock(&pdata->lock);
> 
> -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> +	return sprintf(buf, "0x%llx\n",
> +		       (unsigned long long)set_error_revision(dev, value));
>  }
> 
>  static ssize_t fme_errors_store(struct device *dev,
> --
> 2.26.2

Zhang, Tianfei May 16, 2022, 3:01 a.m. UTC | #2

> -----Original Message-----
> From: Wu, Hao <[e-mail]>
> Sent: Thursday, May 5, 2022 2:47 PM
> To: Zhang, Tianfei <[e-mail]>; [e-mail];
> [e-mail]; Xu, Yilun <[e-mail]>; [e-mail]
> Subject: RE: [PATCH v2] fpga: dfl: fme: adding reserved bits for revision of
> FME/Port error
> 
> > -----Original Message-----
> > From: Zhang, Tianfei <[e-mail]>
> > Sent: Tuesday, April 19, 2022 11:04 AM
> > To: Wu, Hao <[e-mail]>; [e-mail]; [e-mail]; Xu,
> > Yilun <[e-mail]>; [e-mail]
> > Cc: Zhang, Tianfei <[e-mail]>
> > Subject: [PATCH v2] fpga: dfl: fme: adding reserved bits for revision
> > of FME/Port error
> >
> > From: Tianfei zhang <[e-mail]>
> >
> > There are 2 different register layouts for FME/Port error registers.
> > The new production like Intel PAC N6000 plans to deploy the new
> > register format which it is more reasonable.
> >
> 
> Can you explain a little more why there would be different register layouts?

There is a historical problem for it, the older PAC card using the older register layout, but for now, we will use the newer register layout for OFS PAC card.

> There will only be two layouts or more?

Yes, we have two layouts now.

> Intel PAC N6000 "plans" to have new register format? Not finalized yet?

N6000 and all of PAC card in the future will use the new register layout for FME/Port error register.

> So anyone could define its own error register layout, is my understanding
> correct?

Yes, we suggested that they are follow our OFS spec.

> 
> > To distinguish the register layouts,we provide another sysfs node for
> > revision info, but it is a bad method that using one sysfs node's
> > value to determine the usage of other sysfs node.
> >
> > This patch introduces 4 reserved bits (Bit[59:56]) which are reserved
> > by HW, dfl driver appends the FME/Port error revision info on those
> > bits for attribution on the readout value.
> 
> Will UUID be more reasonable than just 4bits value? If we consider this is a
> FPGA, anyone could implement its own error register layout, then just reserve
> 4bits will not be enough? I hope we don't need to modify the code and design
> again...

The revision was came from DFH_REVISION field in DFH register which is 4-bits, I think is enough now.
We suggested that the customers follow our register layout design.
In the DFH v1 spec, we will consider use the UUID but this spec has not finalized now.

> 
> >
> > Signed-off-by: Tianfei zhang <[e-mail]>
> > ---
> > v2:
> >  - add documentation for this change.
> >  - fix the reverse xmas tree declaration.
> > ---
> >  .../ABI/testing/sysfs-platform-dfl-fme        | 37 ++++++++++++-------
> >  drivers/fpga/dfl-fme-error.c                  | 36 +++++++++++++++---
> >  2 files changed, 54 insertions(+), 19 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > b/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > index d6ab34e81b9b..b886568d6071 100644
> > --- a/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > +++ b/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > @@ -50,46 +50,57 @@ Date:		August 2019
> >  KernelVersion:  5.4
> >  Contact:	Wu Hao <[e-mail]>
> >  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.
> > +		The readout value has embedded 4 bits revision attribution
> > +		in Bit[59:56] which reserved by hardware. Write this file to
> > +		clear errors logged in pcie0_errors. Clean Bit[59:56] before
> > +		write this file. 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 <[e-mail]>
> >  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.
> > +		The readout value has embedded 4 bits revision attribution
> > +		in Bit[59:56] which reserved by hardware. Write this file to
> > +		clear errors logged in pcie1_errors. Clean Bit[59:56] before
> > +		write this file. 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 <[e-mail]>
> > -Description:	Read-only. It returns non-fatal errors detected.
> > +Description:	Read-only. It returns non-fatal errors detected. The readout
> > +		value has embedded 4 bits revision attribution in Bit[59:56]
> > +		which reserved by hardware.
> >
> >  What:		/sys/bus/platform/devices/dfl-
> fme.0/errors/catfatal_errors
> >  Date:		August 2019
> >  KernelVersion:  5.4
> >  Contact:	Wu Hao <[e-mail]>
> >  Description:	Read-only. It returns catastrophic and fatal errors detected.
> > +		The readout value has embedded 4 bits revision attribution in
> > +		Bit[59:56] which reserved by hardware.
> >
> >  What:		/sys/bus/platform/devices/dfl-
> fme.0/errors/inject_errors
> >  Date:		August 2019
> >  KernelVersion:  5.4
> >  Contact:	Wu Hao <[e-mail]>
> > -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.
> > +Description:	Read-Write. Read this file to check errors injected. The readout
> > +		value has embedded 4 bits revision attribution which reserved
> > by
> > +		hardware. Write this file to inject errors for testing purpose.
> > +		Clean Bit[59:56] before write this file. 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 <[e-mail]>
> > -Description:	Read-Write. Read this file to get errors detected on FME.
> > -		Write this file to clear errors logged in fme_errors. Write
> > +Description:	Read-Write. Read this file to get errors detected on FME. The
> > +		readout value has embedded 4 bits revision attribution which
> > +		reserved by hardware. Write this file to clear errors logged
> > +		in fme_errors. Clean Bit[59:56] before write this file. Write
> >  		fials with -EINVAL if input parsing fails or input error code
> >  		doesn't match.
> >
> > diff --git a/drivers/fpga/dfl-fme-error.c
> > b/drivers/fpga/dfl-fme-error.c index 51c2892ec06d..a440bc09938d 100644
> > --- a/drivers/fpga/dfl-fme-error.c
> > +++ b/drivers/fpga/dfl-fme-error.c
> > @@ -39,6 +39,22 @@
> >
> >  #define ERROR_MASK		GENMASK_ULL(63, 0)
> >
> > +/* Bit[59:56] was reserved by software for error revision */ #define
> > +ERROR_SW_REVISION_MASK GENMASK_ULL(59, 56)
> > +
> > +static u64 set_error_revision(struct device *dev, u64 value) {
> > +	void __iomem *base;
> > +	u64 revision;
> > +	u64 dfh;
> > +
> > +	base = dfl_get_feature_ioaddr_by_id(dev,
> > FME_FEATURE_ID_GLOBAL_ERR);
> > +	dfh = readq(base);
> > +	revision = FIELD_GET(DFH_REVISION, dfh);
> > +
> > +	return value | FIELD_PREP(ERROR_SW_REVISION_MASK, revision); }
> > +
> >  static ssize_t pcie0_errors_show(struct device *dev,
> >  				 struct device_attribute *attr, char *buf)  { @@
> -52,7 +68,8 @@
> > static ssize_t pcie0_errors_show(struct device *dev,
> >  	value = readq(base + PCIE0_ERROR);
> >  	mutex_unlock(&pdata->lock);
> >
> > -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> > +	return sprintf(buf, "0x%llx\n",
> > +		       (unsigned long long)set_error_revision(dev, value));
> >  }
> >
> >  static ssize_t pcie0_errors_store(struct device *dev, @@ -97,7 +114,8
> > @@ static ssize_t pcie1_errors_show(struct device *dev,
> >  	value = readq(base + PCIE1_ERROR);
> >  	mutex_unlock(&pdata->lock);
> >
> > -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> > +	return sprintf(buf, "0x%llx\n",
> > +		       (unsigned long long)set_error_revision(dev, value));
> >  }
> >
> >  static ssize_t pcie1_errors_store(struct device *dev, @@ -133,11
> > +151,13 @@ static ssize_t nonfatal_errors_show(struct device *dev,
> >  				    struct device_attribute *attr, char *buf)  {
> >  	void __iomem *base;
> > +	u64 value;
> >
> >  	base = dfl_get_feature_ioaddr_by_id(dev,
> > FME_FEATURE_ID_GLOBAL_ERR);
> > +	value = readq(base + RAS_NONFAT_ERROR);
> >
> >  	return sprintf(buf, "0x%llx\n",
> > -		       (unsigned long long)readq(base + RAS_NONFAT_ERROR));
> > +		       (unsigned long long)set_error_revision(dev, value));
> >  }
> >  static DEVICE_ATTR_RO(nonfatal_errors);
> >
> > @@ -145,11 +165,13 @@ static ssize_t catfatal_errors_show(struct device
> *dev,
> >  				    struct device_attribute *attr, char *buf)
> >  {
> >  	void __iomem *base;
> > +	u64 value;
> >
> >  	base = dfl_get_feature_ioaddr_by_id(dev,
> > FME_FEATURE_ID_GLOBAL_ERR);
> > +	value = readq(base + RAS_CATFAT_ERROR);
> >
> >  	return sprintf(buf, "0x%llx\n",
> > -		       (unsigned long long)readq(base + RAS_CATFAT_ERROR));
> > +		       (unsigned long long)set_error_revision(dev, value));
> >  }
> >  static DEVICE_ATTR_RO(catfatal_errors);
> >
> > @@ -165,9 +187,10 @@ static ssize_t inject_errors_show(struct device *dev,
> >  	mutex_lock(&pdata->lock);
> >  	v = readq(base + RAS_ERROR_INJECT);
> >  	mutex_unlock(&pdata->lock);
> > +	v = FIELD_GET(INJECT_ERROR_MASK, v);
> >
> >  	return sprintf(buf, "0x%llx\n",
> > -		       (unsigned long long)FIELD_GET(INJECT_ERROR_MASK, v));
> > +		       (unsigned long long)set_error_revision(dev, v));
> >  }
> >
> >  static ssize_t inject_errors_store(struct device *dev,
> > @@ -211,7 +234,8 @@ static ssize_t fme_errors_show(struct device *dev,
> >  	value = readq(base + FME_ERROR);
> >  	mutex_unlock(&pdata->lock);
> >
> > -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> > +	return sprintf(buf, "0x%llx\n",
> > +		       (unsigned long long)set_error_revision(dev, value));
> >  }
> >
> >  static ssize_t fme_errors_store(struct device *dev,
> > --
> > 2.26.2

Wu, Hao May 30, 2022, 2:24 a.m. UTC | #3

> > > Subject: [PATCH v2] fpga: dfl: fme: adding reserved bits for revision
> > > of FME/Port error
> > >
> > > From: Tianfei zhang <[e-mail]>
> > >
> > > There are 2 different register layouts for FME/Port error registers.
> > > The new production like Intel PAC N6000 plans to deploy the new
> > > register format which it is more reasonable.
> > >
> >
> > Can you explain a little more why there would be different register layouts?
> 
> There is a historical problem for it, the older PAC card using the older register
> layout, but for now, we will use the newer register layout for OFS PAC card.

So OFS is using the same way to distinguish different register layout?

> 
> > There will only be two layouts or more?
> 
> Yes, we have two layouts now.

So it could be more in the future.

> 
> > Intel PAC N6000 "plans" to have new register format? Not finalized yet?
> 
> N6000 and all of PAC card in the future will use the new register layout for
> FME/Port error register.
> 
> > So anyone could define its own error register layout, is my understanding
> > correct?
> 
> Yes, we suggested that they are follow our OFS spec.

So it's still possible customers have their own register definitions.

> 
> >
> > > To distinguish the register layouts,we provide another sysfs node for
> > > revision info, but it is a bad method that using one sysfs node's
> > > value to determine the usage of other sysfs node.
> > >
> > > This patch introduces 4 reserved bits (Bit[59:56]) which are reserved
> > > by HW, dfl driver appends the FME/Port error revision info on those
> > > bits for attribution on the readout value.
> >
> > Will UUID be more reasonable than just 4bits value? If we consider this is a
> > FPGA, anyone could implement its own error register layout, then just reserve
> > 4bits will not be enough? I hope we don't need to modify the code and design
> > again...
> 
> The revision was came from DFH_REVISION field in DFH register which is 4-bits, I
> think is enough now.
> We suggested that the customers follow our register layout design.
> In the DFH v1 spec, we will consider use the UUID but this spec has not finalized
> now.

So OFS is still using DFHv0 not DFHv1?
I didn't get your point, why you think 4-bits field is enough? It's possible in the
future there could be more different register layouts, and customers could
have different register layouts, and even for OFS is still using the same way...

> 
> >
> > >
> > > Signed-off-by: Tianfei zhang <[e-mail]>
> > > ---
> > > v2:
> > >  - add documentation for this change.
> > >  - fix the reverse xmas tree declaration.
> > > ---
> > >  .../ABI/testing/sysfs-platform-dfl-fme        | 37 ++++++++++++-------
> > >  drivers/fpga/dfl-fme-error.c                  | 36 +++++++++++++++---
> > >  2 files changed, 54 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > > b/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > > index d6ab34e81b9b..b886568d6071 100644
> > > --- a/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > > +++ b/Documentation/ABI/testing/sysfs-platform-dfl-fme
> > > @@ -50,46 +50,57 @@ Date:		August 2019
> > >  KernelVersion:  5.4
> > >  Contact:	Wu Hao <[e-mail]>
> > >  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.
> > > +		The readout value has embedded 4 bits revision attribution
> > > +		in Bit[59:56] which reserved by hardware. Write this file to
> > > +		clear errors logged in pcie0_errors. Clean Bit[59:56] before
> > > +		write this file. 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 <[e-mail]>
> > >  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.
> > > +		The readout value has embedded 4 bits revision attribution
> > > +		in Bit[59:56] which reserved by hardware. Write this file to
> > > +		clear errors logged in pcie1_errors. Clean Bit[59:56] before
> > > +		write this file. 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 <[e-mail]>
> > > -Description:	Read-only. It returns non-fatal errors detected.
> > > +Description:	Read-only. It returns non-fatal errors detected. The
> readout
> > > +		value has embedded 4 bits revision attribution in Bit[59:56]
> > > +		which reserved by hardware.
> > >
> > >  What:		/sys/bus/platform/devices/dfl-
> > fme.0/errors/catfatal_errors
> > >  Date:		August 2019
> > >  KernelVersion:  5.4
> > >  Contact:	Wu Hao <[e-mail]>
> > >  Description:	Read-only. It returns catastrophic and fatal errors detected.
> > > +		The readout value has embedded 4 bits revision attribution in
> > > +		Bit[59:56] which reserved by hardware.
> > >
> > >  What:		/sys/bus/platform/devices/dfl-
> > fme.0/errors/inject_errors
> > >  Date:		August 2019
> > >  KernelVersion:  5.4
> > >  Contact:	Wu Hao <[e-mail]>
> > > -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.
> > > +Description:	Read-Write. Read this file to check errors injected. The
> readout
> > > +		value has embedded 4 bits revision attribution which reserved
> > > by
> > > +		hardware. Write this file to inject errors for testing purpose.
> > > +		Clean Bit[59:56] before write this file. 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 <[e-mail]>
> > > -Description:	Read-Write. Read this file to get errors detected on
> FME.
> > > -		Write this file to clear errors logged in fme_errors. Write
> > > +Description:	Read-Write. Read this file to get errors detected on
> FME. The
> > > +		readout value has embedded 4 bits revision attribution which
> > > +		reserved by hardware. Write this file to clear errors logged
> > > +		in fme_errors. Clean Bit[59:56] before write this file. Write
> > >  		fials with -EINVAL if input parsing fails or input error code
> > >  		doesn't match.
> > >
> > > diff --git a/drivers/fpga/dfl-fme-error.c
> > > b/drivers/fpga/dfl-fme-error.c index 51c2892ec06d..a440bc09938d 100644
> > > --- a/drivers/fpga/dfl-fme-error.c
> > > +++ b/drivers/fpga/dfl-fme-error.c
> > > @@ -39,6 +39,22 @@
> > >
> > >  #define ERROR_MASK		GENMASK_ULL(63, 0)
> > >
> > > +/* Bit[59:56] was reserved by software for error revision */ #define
> > > +ERROR_SW_REVISION_MASK GENMASK_ULL(59, 56)
> > > +
> > > +static u64 set_error_revision(struct device *dev, u64 value) {
> > > +	void __iomem *base;
> > > +	u64 revision;
> > > +	u64 dfh;
> > > +
> > > +	base = dfl_get_feature_ioaddr_by_id(dev,
> > > FME_FEATURE_ID_GLOBAL_ERR);
> > > +	dfh = readq(base);
> > > +	revision = FIELD_GET(DFH_REVISION, dfh);
> > > +
> > > +	return value | FIELD_PREP(ERROR_SW_REVISION_MASK, revision); }
> > > +
> > >  static ssize_t pcie0_errors_show(struct device *dev,
> > >  				 struct device_attribute *attr, char *buf)  { @@
> > -52,7 +68,8 @@
> > > static ssize_t pcie0_errors_show(struct device *dev,
> > >  	value = readq(base + PCIE0_ERROR);
> > >  	mutex_unlock(&pdata->lock);
> > >
> > > -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> > > +	return sprintf(buf, "0x%llx\n",
> > > +		       (unsigned long long)set_error_revision(dev, value));
> > >  }
> > >
> > >  static ssize_t pcie0_errors_store(struct device *dev, @@ -97,7 +114,8
> > > @@ static ssize_t pcie1_errors_show(struct device *dev,
> > >  	value = readq(base + PCIE1_ERROR);
> > >  	mutex_unlock(&pdata->lock);
> > >
> > > -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> > > +	return sprintf(buf, "0x%llx\n",
> > > +		       (unsigned long long)set_error_revision(dev, value));
> > >  }
> > >
> > >  static ssize_t pcie1_errors_store(struct device *dev, @@ -133,11
> > > +151,13 @@ static ssize_t nonfatal_errors_show(struct device *dev,
> > >  				    struct device_attribute *attr, char *buf)  {
> > >  	void __iomem *base;
> > > +	u64 value;
> > >
> > >  	base = dfl_get_feature_ioaddr_by_id(dev,
> > > FME_FEATURE_ID_GLOBAL_ERR);
> > > +	value = readq(base + RAS_NONFAT_ERROR);
> > >
> > >  	return sprintf(buf, "0x%llx\n",
> > > -		       (unsigned long long)readq(base + RAS_NONFAT_ERROR));
> > > +		       (unsigned long long)set_error_revision(dev, value));
> > >  }
> > >  static DEVICE_ATTR_RO(nonfatal_errors);
> > >
> > > @@ -145,11 +165,13 @@ static ssize_t catfatal_errors_show(struct device
> > *dev,
> > >  				    struct device_attribute *attr, char *buf)
> > >  {
> > >  	void __iomem *base;
> > > +	u64 value;
> > >
> > >  	base = dfl_get_feature_ioaddr_by_id(dev,
> > > FME_FEATURE_ID_GLOBAL_ERR);
> > > +	value = readq(base + RAS_CATFAT_ERROR);
> > >
> > >  	return sprintf(buf, "0x%llx\n",
> > > -		       (unsigned long long)readq(base + RAS_CATFAT_ERROR));
> > > +		       (unsigned long long)set_error_revision(dev, value));
> > >  }
> > >  static DEVICE_ATTR_RO(catfatal_errors);
> > >
> > > @@ -165,9 +187,10 @@ static ssize_t inject_errors_show(struct device *dev,
> > >  	mutex_lock(&pdata->lock);
> > >  	v = readq(base + RAS_ERROR_INJECT);
> > >  	mutex_unlock(&pdata->lock);
> > > +	v = FIELD_GET(INJECT_ERROR_MASK, v);
> > >
> > >  	return sprintf(buf, "0x%llx\n",
> > > -		       (unsigned long long)FIELD_GET(INJECT_ERROR_MASK, v));
> > > +		       (unsigned long long)set_error_revision(dev, v));
> > >  }
> > >
> > >  static ssize_t inject_errors_store(struct device *dev,
> > > @@ -211,7 +234,8 @@ static ssize_t fme_errors_show(struct device *dev,
> > >  	value = readq(base + FME_ERROR);
> > >  	mutex_unlock(&pdata->lock);
> > >
> > > -	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
> > > +	return sprintf(buf, "0x%llx\n",
> > > +		       (unsigned long long)set_error_revision(dev, value));
> > >  }
> > >
> > >  static ssize_t fme_errors_store(struct device *dev,
> > > --
> > > 2.26.2

Patch

diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-fme b/Documentation/ABI/testing/sysfs-platform-dfl-fme
index d6ab34e81b9b..b886568d6071 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-fme
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-fme
@@ -50,46 +50,57 @@  Date:		August 2019
 KernelVersion:  5.4
 Contact:	Wu Hao <[e-mail]>
 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.
+		The readout value has embedded 4 bits revision attribution
+		in Bit[59:56] which reserved by hardware. Write this file to
+		clear errors logged in pcie0_errors. Clean Bit[59:56] before
+		write this file. 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 <[e-mail]>
 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.
+		The readout value has embedded 4 bits revision attribution
+		in Bit[59:56] which reserved by hardware. Write this file to
+		clear errors logged in pcie1_errors. Clean Bit[59:56] before
+		write this file. 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 <[e-mail]>
-Description:	Read-only. It returns non-fatal errors detected.
+Description:	Read-only. It returns non-fatal errors detected. The readout
+		value has embedded 4 bits revision attribution in Bit[59:56]
+		which reserved by hardware.
 
 What:		/sys/bus/platform/devices/dfl-fme.0/errors/catfatal_errors
 Date:		August 2019
 KernelVersion:  5.4
 Contact:	Wu Hao <[e-mail]>
 Description:	Read-only. It returns catastrophic and fatal errors detected.
+		The readout value has embedded 4 bits revision attribution in
+		Bit[59:56] which reserved by hardware.
 
 What:		/sys/bus/platform/devices/dfl-fme.0/errors/inject_errors
 Date:		August 2019
 KernelVersion:  5.4
 Contact:	Wu Hao <[e-mail]>
-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.
+Description:	Read-Write. Read this file to check errors injected. The readout
+		value has embedded 4 bits revision attribution which reserved by
+		hardware. Write this file to inject errors for testing purpose.
+		Clean Bit[59:56] before write this file. 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 <[e-mail]>
-Description:	Read-Write. Read this file to get errors detected on FME.
-		Write this file to clear errors logged in fme_errors. Write
+Description:	Read-Write. Read this file to get errors detected on FME. The
+		readout value has embedded 4 bits revision attribution which
+		reserved by hardware. Write this file to clear errors logged
+		in fme_errors. Clean Bit[59:56] before write this file. Write
 		fials with -EINVAL if input parsing fails or input error code
 		doesn't match.
 
diff --git a/drivers/fpga/dfl-fme-error.c b/drivers/fpga/dfl-fme-error.c
index 51c2892ec06d..a440bc09938d 100644
--- a/drivers/fpga/dfl-fme-error.c
+++ b/drivers/fpga/dfl-fme-error.c
@@ -39,6 +39,22 @@ 
 
 #define ERROR_MASK		GENMASK_ULL(63, 0)
 
+/* Bit[59:56] was reserved by software for error revision */
+#define ERROR_SW_REVISION_MASK GENMASK_ULL(59, 56)
+
+static u64 set_error_revision(struct device *dev, u64 value)
+{
+	void __iomem *base;
+	u64 revision;
+	u64 dfh;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+	dfh = readq(base);
+	revision = FIELD_GET(DFH_REVISION, dfh);
+
+	return value | FIELD_PREP(ERROR_SW_REVISION_MASK, revision);
+}
+
 static ssize_t pcie0_errors_show(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
@@ -52,7 +68,8 @@  static ssize_t pcie0_errors_show(struct device *dev,
 	value = readq(base + PCIE0_ERROR);
 	mutex_unlock(&pdata->lock);
 
-	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)set_error_revision(dev, value));
 }
 
 static ssize_t pcie0_errors_store(struct device *dev,
@@ -97,7 +114,8 @@  static ssize_t pcie1_errors_show(struct device *dev,
 	value = readq(base + PCIE1_ERROR);
 	mutex_unlock(&pdata->lock);
 
-	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)set_error_revision(dev, value));
 }
 
 static ssize_t pcie1_errors_store(struct device *dev,
@@ -133,11 +151,13 @@  static ssize_t nonfatal_errors_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
 	void __iomem *base;
+	u64 value;
 
 	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+	value = readq(base + RAS_NONFAT_ERROR);
 
 	return sprintf(buf, "0x%llx\n",
-		       (unsigned long long)readq(base + RAS_NONFAT_ERROR));
+		       (unsigned long long)set_error_revision(dev, value));
 }
 static DEVICE_ATTR_RO(nonfatal_errors);
 
@@ -145,11 +165,13 @@  static ssize_t catfatal_errors_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
 	void __iomem *base;
+	u64 value;
 
 	base = dfl_get_feature_ioaddr_by_id(dev, FME_FEATURE_ID_GLOBAL_ERR);
+	value = readq(base + RAS_CATFAT_ERROR);
 
 	return sprintf(buf, "0x%llx\n",
-		       (unsigned long long)readq(base + RAS_CATFAT_ERROR));
+		       (unsigned long long)set_error_revision(dev, value));
 }
 static DEVICE_ATTR_RO(catfatal_errors);
 
@@ -165,9 +187,10 @@  static ssize_t inject_errors_show(struct device *dev,
 	mutex_lock(&pdata->lock);
 	v = readq(base + RAS_ERROR_INJECT);
 	mutex_unlock(&pdata->lock);
+	v = FIELD_GET(INJECT_ERROR_MASK, v);
 
 	return sprintf(buf, "0x%llx\n",
-		       (unsigned long long)FIELD_GET(INJECT_ERROR_MASK, v));
+		       (unsigned long long)set_error_revision(dev, v));
 }
 
 static ssize_t inject_errors_store(struct device *dev,
@@ -211,7 +234,8 @@  static ssize_t fme_errors_show(struct device *dev,
 	value = readq(base + FME_ERROR);
 	mutex_unlock(&pdata->lock);
 
-	return sprintf(buf, "0x%llx\n", (unsigned long long)value);
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)set_error_revision(dev, value));
 }
 
 static ssize_t fme_errors_store(struct device *dev,