diff mbox series

[v4,04/12] fpga: dfl: afu: add userclock sysfs interfaces.

Message ID 1564914022-3710-5-git-send-email-hao.wu@intel.com (mailing list archive)
State Superseded, archived
Headers show
Series FPGA DFL updates | expand

Commit Message

Wu, Hao Aug. 4, 2019, 10:20 a.m. UTC
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.
---
 Documentation/ABI/testing/sysfs-platform-dfl-port |  35 +++++++
 drivers/fpga/dfl-afu-main.c                       | 114 +++++++++++++++++++++-
 drivers/fpga/dfl.h                                |   9 ++
 3 files changed, 157 insertions(+), 1 deletion(-)

Comments

Greg Kroah-Hartman Aug. 5, 2019, 3:51 p.m. UTC | #1
On Sun, Aug 04, 2019 at 06:20:14PM +0800, Wu Hao wrote:
> 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.
> ---
>  Documentation/ABI/testing/sysfs-platform-dfl-port |  35 +++++++
>  drivers/fpga/dfl-afu-main.c                       | 114 +++++++++++++++++++++-
>  drivers/fpga/dfl.h                                |   9 ++
>  3 files changed, 157 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
> index 1ab3e6f..5663441 100644
> --- a/Documentation/ABI/testing/sysfs-platform-dfl-port
> +++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
> @@ -46,3 +46,38 @@ 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/revision
> +Date:		August 2019
> +KernelVersion:	5.4
> +Contact:	Wu Hao <hao.wu@intel.com>
> +Description:	Read-only. Read this file to get the revision of port header
> +		feature.

What does "revision" mean?

It feels like you are creating a different set of sysfs files depending
on the revision field.  Which is fine, sysfs is one-value-per-file and
userspace needs to handle if the file is present or not.  So why not
just rely on that and not have to mess with 'revision' at all?  What is
userspace going to do with that information?

> +
> +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 12175bb..407c97d 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -142,6 +142,17 @@ static int port_get_id(struct platform_device *pdev)
>  static DEVICE_ATTR_RO(id);
>  
>  static ssize_t
> +revision_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	void __iomem *base;
> +
> +	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> +
> +	return sprintf(buf, "%x\n", dfl_feature_revision(base));
> +}
> +static DEVICE_ATTR_RO(revision);
> +
> +static ssize_t
>  ltr_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> @@ -276,6 +287,7 @@ static int port_get_id(struct platform_device *pdev)
>  
>  static struct attribute *port_hdr_attrs[] = {
>  	&dev_attr_id.attr,
> +	&dev_attr_revision.attr,
>  	&dev_attr_ltr.attr,
>  	&dev_attr_ap1_event.attr,
>  	&dev_attr_ap2_event.attr,
> @@ -284,14 +296,113 @@ static int port_get_id(struct platform_device *pdev)
>  };
>  ATTRIBUTE_GROUPS(port_hdr);
>  
> +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)
> +{
> +	u64 userclk_freqsts;
> +	void __iomem *base;
> +
> +	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> +
> +	userclk_freqsts = readq(base + PORT_HDR_USRCLK_STS0);
> +
> +	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)
> +{
> +	u64 userclk_freqcntrsts;
> +	void __iomem *base;
> +
> +	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> +
> +	userclk_freqcntrsts = readq(base + PORT_HDR_USRCLK_STS1);
> +
> +	return sprintf(buf, "0x%llx\n",
> +		       (unsigned long long)userclk_freqcntrsts);
> +}
> +static DEVICE_ATTR_RO(userclk_freqcntrsts);
> +
> +static struct attribute *port_hdr_userclk_attrs[] = {
> +	&dev_attr_userclk_freqcmd.attr,
> +	&dev_attr_userclk_freqcntrcmd.attr,
> +	&dev_attr_userclk_freqsts.attr,
> +	&dev_attr_userclk_freqcntrsts.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(port_hdr_userclk);
> +
>  static int port_hdr_init(struct platform_device *pdev,
>  			 struct dfl_feature *feature)
>  {
> +	int ret;
> +
>  	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
>  
>  	port_reset(pdev);
>  
> -	return device_add_groups(&pdev->dev, port_hdr_groups);
> +	ret = device_add_groups(&pdev->dev, port_hdr_groups);

This all needs to be reworked based on the ability for devices to
properly add groups when they are bound on probe (the core does it for
you, no need for the driver to do it.)  But until then, you should at
least consider:

> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * if revision > 0, the userclock will be moved from port hdr register
> +	 * region to a separated private feature.
> +	 */
> +	if (dfl_feature_revision(feature->ioaddr) > 0)
> +		return 0;
> +
> +	ret = device_add_groups(&pdev->dev, port_hdr_userclk_groups);
> +	if (ret)
> +		device_remove_groups(&pdev->dev, port_hdr_groups);

struct attribute_group has is_visible() as a callback to have the core
show or not show, individual attributes when they are created.  So no
need for a second group of attributes and you needing to add/remove
them, just add them all and let the callback handle the "is visible"
logic.  Makes cleanup _so_ much easier (i.e. you don't have to do it.)

thanks,

greg k-h
Wu, Hao Aug. 7, 2019, 2:18 a.m. UTC | #2
On Mon, Aug 05, 2019 at 05:51:13PM +0200, Greg KH wrote:
> On Sun, Aug 04, 2019 at 06:20:14PM +0800, Wu Hao wrote:
> > 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.
> > ---
> >  Documentation/ABI/testing/sysfs-platform-dfl-port |  35 +++++++
> >  drivers/fpga/dfl-afu-main.c                       | 114 +++++++++++++++++++++-
> >  drivers/fpga/dfl.h                                |   9 ++
> >  3 files changed, 157 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
> > index 1ab3e6f..5663441 100644
> > --- a/Documentation/ABI/testing/sysfs-platform-dfl-port
> > +++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
> > @@ -46,3 +46,38 @@ 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/revision
> > +Date:		August 2019
> > +KernelVersion:	5.4
> > +Contact:	Wu Hao <hao.wu@intel.com>
> > +Description:	Read-only. Read this file to get the revision of port header
> > +		feature.
> 
> What does "revision" mean?
> 
> It feels like you are creating a different set of sysfs files depending
> on the revision field.  Which is fine, sysfs is one-value-per-file and
> userspace needs to handle if the file is present or not.  So why not
> just rely on that and not have to mess with 'revision' at all?  What is
> userspace going to do with that information?

Hi Greg

Thanks for the review and comments,

Yes, different revision of private feature may have different hardware
features, so driver will expose different set of sysfs entries. revision
here is used to help userspace to distinguish them. I think it makes
sense to just rely on if sysfs entry exists or not, manage revision in
userspace code may be quit difficult. Plan to remove this entry in the
next version.

> 
> > +
> > +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 12175bb..407c97d 100644
> > --- a/drivers/fpga/dfl-afu-main.c
> > +++ b/drivers/fpga/dfl-afu-main.c
> > @@ -142,6 +142,17 @@ static int port_get_id(struct platform_device *pdev)
> >  static DEVICE_ATTR_RO(id);
> >  
> >  static ssize_t
> > +revision_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +	void __iomem *base;
> > +
> > +	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> > +
> > +	return sprintf(buf, "%x\n", dfl_feature_revision(base));
> > +}
> > +static DEVICE_ATTR_RO(revision);
> > +
> > +static ssize_t
> >  ltr_show(struct device *dev, struct device_attribute *attr, char *buf)
> >  {
> >  	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> > @@ -276,6 +287,7 @@ static int port_get_id(struct platform_device *pdev)
> >  
> >  static struct attribute *port_hdr_attrs[] = {
> >  	&dev_attr_id.attr,
> > +	&dev_attr_revision.attr,
> >  	&dev_attr_ltr.attr,
> >  	&dev_attr_ap1_event.attr,
> >  	&dev_attr_ap2_event.attr,
> > @@ -284,14 +296,113 @@ static int port_get_id(struct platform_device *pdev)
> >  };
> >  ATTRIBUTE_GROUPS(port_hdr);
> >  
> > +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)
> > +{
> > +	u64 userclk_freqsts;
> > +	void __iomem *base;
> > +
> > +	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> > +
> > +	userclk_freqsts = readq(base + PORT_HDR_USRCLK_STS0);
> > +
> > +	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)
> > +{
> > +	u64 userclk_freqcntrsts;
> > +	void __iomem *base;
> > +
> > +	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> > +
> > +	userclk_freqcntrsts = readq(base + PORT_HDR_USRCLK_STS1);
> > +
> > +	return sprintf(buf, "0x%llx\n",
> > +		       (unsigned long long)userclk_freqcntrsts);
> > +}
> > +static DEVICE_ATTR_RO(userclk_freqcntrsts);
> > +
> > +static struct attribute *port_hdr_userclk_attrs[] = {
> > +	&dev_attr_userclk_freqcmd.attr,
> > +	&dev_attr_userclk_freqcntrcmd.attr,
> > +	&dev_attr_userclk_freqsts.attr,
> > +	&dev_attr_userclk_freqcntrsts.attr,
> > +	NULL,
> > +};
> > +ATTRIBUTE_GROUPS(port_hdr_userclk);
> > +
> >  static int port_hdr_init(struct platform_device *pdev,
> >  			 struct dfl_feature *feature)
> >  {
> > +	int ret;
> > +
> >  	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
> >  
> >  	port_reset(pdev);
> >  
> > -	return device_add_groups(&pdev->dev, port_hdr_groups);
> > +	ret = device_add_groups(&pdev->dev, port_hdr_groups);
> 
> This all needs to be reworked based on the ability for devices to
> properly add groups when they are bound on probe (the core does it for
> you, no need for the driver to do it.)  But until then, you should at
> least consider:
> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * if revision > 0, the userclock will be moved from port hdr register
> > +	 * region to a separated private feature.
> > +	 */
> > +	if (dfl_feature_revision(feature->ioaddr) > 0)
> > +		return 0;
> > +
> > +	ret = device_add_groups(&pdev->dev, port_hdr_userclk_groups);
> > +	if (ret)
> > +		device_remove_groups(&pdev->dev, port_hdr_groups);
> 
> struct attribute_group has is_visible() as a callback to have the core
> show or not show, individual attributes when they are created.  So no
> need for a second group of attributes and you needing to add/remove
> them, just add them all and let the callback handle the "is visible"
> logic.  Makes cleanup _so_ much easier (i.e. you don't have to do it.)

Sure, will use is_visible() here instead in the next version, it does
make thing more clear. Thanks a lot of the comments.

Hao

> 
> thanks,
> 
> greg k-h
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
index 1ab3e6f..5663441 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-port
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
@@ -46,3 +46,38 @@  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/revision
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. Read this file to get the revision of port header
+		feature.
+
+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 12175bb..407c97d 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -142,6 +142,17 @@  static int port_get_id(struct platform_device *pdev)
 static DEVICE_ATTR_RO(id);
 
 static ssize_t
+revision_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	return sprintf(buf, "%x\n", dfl_feature_revision(base));
+}
+static DEVICE_ATTR_RO(revision);
+
+static ssize_t
 ltr_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
@@ -276,6 +287,7 @@  static int port_get_id(struct platform_device *pdev)
 
 static struct attribute *port_hdr_attrs[] = {
 	&dev_attr_id.attr,
+	&dev_attr_revision.attr,
 	&dev_attr_ltr.attr,
 	&dev_attr_ap1_event.attr,
 	&dev_attr_ap2_event.attr,
@@ -284,14 +296,113 @@  static int port_get_id(struct platform_device *pdev)
 };
 ATTRIBUTE_GROUPS(port_hdr);
 
+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)
+{
+	u64 userclk_freqsts;
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	userclk_freqsts = readq(base + PORT_HDR_USRCLK_STS0);
+
+	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)
+{
+	u64 userclk_freqcntrsts;
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+
+	userclk_freqcntrsts = readq(base + PORT_HDR_USRCLK_STS1);
+
+	return sprintf(buf, "0x%llx\n",
+		       (unsigned long long)userclk_freqcntrsts);
+}
+static DEVICE_ATTR_RO(userclk_freqcntrsts);
+
+static struct attribute *port_hdr_userclk_attrs[] = {
+	&dev_attr_userclk_freqcmd.attr,
+	&dev_attr_userclk_freqcntrcmd.attr,
+	&dev_attr_userclk_freqsts.attr,
+	&dev_attr_userclk_freqcntrsts.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(port_hdr_userclk);
+
 static int port_hdr_init(struct platform_device *pdev,
 			 struct dfl_feature *feature)
 {
+	int ret;
+
 	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
 
 	port_reset(pdev);
 
-	return device_add_groups(&pdev->dev, port_hdr_groups);
+	ret = device_add_groups(&pdev->dev, port_hdr_groups);
+	if (ret)
+		return ret;
+
+	/*
+	 * if revision > 0, the userclock will be moved from port hdr register
+	 * region to a separated private feature.
+	 */
+	if (dfl_feature_revision(feature->ioaddr) > 0)
+		return 0;
+
+	ret = device_add_groups(&pdev->dev, port_hdr_userclk_groups);
+	if (ret)
+		device_remove_groups(&pdev->dev, port_hdr_groups);
+
+	return ret;
 }
 
 static void port_hdr_uinit(struct platform_device *pdev,
@@ -299,6 +410,7 @@  static void port_hdr_uinit(struct platform_device *pdev,
 {
 	dev_dbg(&pdev->dev, "PORT HDR UInit.\n");
 
+	device_remove_groups(&pdev->dev, port_hdr_userclk_groups);
 	device_remove_groups(&pdev->dev, port_hdr_groups);
 }
 
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 6625d73..c65ab29 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 */
@@ -346,6 +350,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
  *