[RFC 0/1]Add user space interaction for FPGA programming

[RFC 0/1]Add user space interaction for FPGA programming

[Nava kishore Manne]

The existing FPGA manager subsystem didn't have any user space interface
(other than the status/state in sysfs) in Kernel. Basically, FPGAs are
semiconductor devices that can be reprogrammed for desired hardware
functionality.

FPGAs can be reprogrammed at runtime with different types of logic and IPs
as per user need and hence there is a need to use device tree overlays for
removing/updating/adding the devices at runtime for the IPs/controllers
that are present in FPGA. But we don't have any user interface in kernel
for updating the device tree at runtime.

Sometime back there was a series sent by Pantelis Antoniou
(https://lore.kernel.org/lkml/1414528565-10907-4-git-send-email-pantelis.antoniou@konsulko.com/).

This patch introduced a user interface configfs for Device Tree overlays,
a method of dynamically altering the kernel's live Device Tree. However,
this patch series was not accepted in mainline due to various concerns.
For more details refer to this link:
https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts#issues_and_what_needs_to_be_completed_--_Not_an_exhaustive_list

One of the major valid concerns that were raised with this configfs
interface was security. It provides a generic interface (Not specific
to the use cases) for modifying the live device tree.

In order to configure/program the FPGA devices, All the major vendors of
FPGA are using this configfs series as out-of-tree patch for configuring
the FPGAs and there was never an attempt to introduce a generic interface
to configure/program the FPGA in upstream and hence upstream kernel ended
up in not having proper support for FPGAs configure/program.
This series tries to address this gap of FPGA programmability by providing
a new sysfs interface to the user.

This newly added sysfs interface uses Device Tree Overlay (DTO) files to
configure/reprogram an FPGA while an operating system is running and it
restrict the overlay's subsystem usage only to FPGA regions in order to
mitigate the major security concern with configfs.This proposed solution
will not change the existing sequence when a DT overlay that
targets an FPGA Region is applied.
	- Disable appropriate FPGA bridges.
	- Program the FPGA using the FPGA manager.
	- Enable the FPGA bridges.
	- The Device Tree overlay is accepted into the live tree.
	- Child devices are populated.

When the overlay is removed, the child nodes will be removed, and the FPGA
Region will disable the bridges.

Usage:
To configure/reprogram an FPGA region:
echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load

To remove an FPGA region:
echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/remove

To get an FPGA region status:
cat /sys/class/fpga_region/<region>/device/status

Nava kishore Manne (1):
  of-fpga-region: Add sysfs interface support for FPGA configuration

 .../ABI/testing/sysfs-class-of-fpga-region    | 30 ++++++
 MAINTAINERS                                   |  1 +
 drivers/fpga/fpga-region.c                    |  4 +-
 drivers/fpga/of-fpga-region.c                 | 92 +++++++++++++++++++
 include/linux/fpga/fpga-region.h              | 15 +++
 5 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-of-fpga-region

-- 
2.34.1

[RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA configuration

[Nava kishore Manne]

Adds sysfs interface as part of the of-fpga-region. This newly added
sysfs interface uses Device Tree Overlay (DTO) files to configure/reprogram
an FPGA while an operating system is running.This solution will not change
the existing sequence When a DT overlay that targets an FPGA Region is
applied.
	- Disable appropriate FPGA bridges.
	- Program the FPGA using the FPGA manager.
	- Enable the FPGA bridges.
	- The Device Tree overlay is accepted into the live tree.
	- Child devices are populated.

When the overlay is removed, the child nodes will be removed, and the FPGA
Region will disable the bridges.

Usage:
To configure/reprogram an FPGA region:
echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load

To remove an FPGA region:
echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/remove

To get an FPGA region status:
cat /sys/class/fpga_region/<region>/device/status

Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
---
 .../ABI/testing/sysfs-class-of-fpga-region    | 30 ++++++
 MAINTAINERS                                   |  1 +
 drivers/fpga/fpga-region.c                    |  4 +-
 drivers/fpga/of-fpga-region.c                 | 92 +++++++++++++++++++
 include/linux/fpga/fpga-region.h              | 15 +++
 5 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-of-fpga-region

diff --git a/Documentation/ABI/testing/sysfs-class-of-fpga-region b/Documentation/ABI/testing/sysfs-class-of-fpga-region
new file mode 100644
index 000000000000..aeb4e3be4ff3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-of-fpga-region
@@ -0,0 +1,30 @@
+What:		/sys/class/fpga_region/<region>/device/load
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Nava kishore Manne <nava.kishore.manne@amd.com>
+Description:	(WO) Configure/Reprogram an FPGA region.
+		It uses Device Tree Overlay (DTO) files to configurer (or)
+		reprogram an FPGA. While an operating system is running.
+		The bitstream and the relevant DTO file has to be located
+		on the appropriate firmware path, typically, /lib/firmware.
+		For example, when user pass the option "echo fpga.dtbo"	the
+		file /lib/firmware/fpga.dtbo must be present.
+
+What:		/sys/class/fpga_region/<region>/device/remove
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Nava kishore Manne <nava.kishore.manne@amd.com>
+Description:	(WO) Revert the changes added by the load interface
+		It revert and free an overlay changeset added by the load
+		interface.
+
+What:		/sys/class/fpga_region/<region>/device/status
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Nava kishore Manne <nava.kishore.manne@amd.com>
+Description:	(RO) Status of the FPGA region
+		This file is used to check the status of the FPGA region.
+		This is a list of strings for the supported status.
+
+		* applied	= FPGA is programmed and operating
+		* unapplied	= Error while programing the FPGA
diff --git a/MAINTAINERS b/MAINTAINERS
index c0a3d9e93689..384f1d6f3af9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8795,6 +8795,7 @@ L:	linux-fpga@vger.kernel.org
 S:	Maintained
 Q:	http://patchwork.kernel.org/project/linux-fpga/list/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/fpga/linux-fpga.git
+F:	Documentation/ABI/testing/sysfs-class-of-fpga-region
 F:	Documentation/devicetree/bindings/fpga/
 F:	Documentation/driver-api/fpga/
 F:	Documentation/fpga/
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index 753cd142503e..0733db1347ea 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/of.h>
 
 static DEFINE_IDA(fpga_region_ida);
 static const struct class fpga_region_class;
@@ -192,6 +193,7 @@ struct fpga_region *
 __fpga_region_register_full(struct device *parent, const struct fpga_region_info *info,
 			    struct module *owner)
 {
+	struct device_node *np = parent->of_node;
 	struct fpga_region *region;
 	int id, ret = 0;
 
@@ -225,7 +227,7 @@ __fpga_region_register_full(struct device *parent, const struct fpga_region_info
 	region->dev.of_node = parent->of_node;
 	region->dev.id = id;
 
-	ret = dev_set_name(&region->dev, "region%d", id);
+	ret = dev_set_name(&region->dev, "%s", of_node_full_name(np));
 	if (ret)
 		goto err_remove;
 
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 8526a5a86f0c..edcc4c23a4b4 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -5,6 +5,7 @@
  *  Copyright (C) 2013-2016 Altera Corporation
  *  Copyright (C) 2017 Intel Corporation
  */
+#include <linux/firmware.h>
 #include <linux/fpga/fpga-bridge.h>
 #include <linux/fpga/fpga-mgr.h>
 #include <linux/fpga/fpga-region.h>
@@ -347,6 +348,7 @@ static int of_fpga_region_notify(struct notifier_block *nb,
 				 unsigned long action, void *arg)
 {
 	struct of_overlay_notify_data *nd = arg;
+	struct fpga_overlay_image_info *ovcs;
 	struct fpga_region *region;
 	int ret;
 
@@ -371,6 +373,10 @@ static int of_fpga_region_notify(struct notifier_block *nb,
 	if (!region)
 		return NOTIFY_OK;
 
+	ovcs = &region->ovcs;
+	if (!ovcs->fw)
+		return NOTIFY_STOP;
+
 	ret = 0;
 	switch (action) {
 	case OF_OVERLAY_PRE_APPLY:
@@ -394,6 +400,91 @@ static struct notifier_block fpga_region_of_nb = {
 	.notifier_call = of_fpga_region_notify,
 };
 
+static ssize_t load_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+	struct fpga_overlay_image_info *ovcs = &region->ovcs;
+	char *s;
+	int err;
+
+	/* if it's set do not allow changes */
+	if (ovcs->ovcs_id)
+		return -EPERM;
+
+	/* copy to path buffer (and make sure it's always zero terminated */
+	count = snprintf(ovcs->path, sizeof(ovcs->path) - 1, "%s", buf);
+	ovcs->path[sizeof(ovcs->path) - 1] = '\0';
+
+	/* strip trailing newlines */
+	s = ovcs->path + strlen(ovcs->path);
+	while (s > ovcs->path && *--s == '\n')
+		*s = '\0';
+
+	err = request_firmware(&ovcs->fw, ovcs->path, NULL);
+	if (err != 0)
+		goto out_err;
+
+	err = of_overlay_fdt_apply((void *)ovcs->fw->data, ovcs->fw->size,
+				   &ovcs->ovcs_id, NULL);
+	if (err < 0) {
+		pr_err("%s: Failed to create overlay (err=%d)\n",
+		       __func__, err);
+		release_firmware(ovcs->fw);
+		goto out_err;
+	}
+
+	return count;
+out_err:
+	ovcs->path[0] = '\0';
+	ovcs->ovcs_id = 0;
+	ovcs->fw = NULL;
+
+	return err;
+}
+
+static ssize_t remove_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+	struct fpga_overlay_image_info *ovcs = &region->ovcs;
+
+	if (!ovcs->ovcs_id)
+		return -EPERM;
+
+	of_overlay_remove(&ovcs->ovcs_id);
+	release_firmware(ovcs->fw);
+
+	ovcs->path[0] = '\0';
+	ovcs->ovcs_id = 0;
+	ovcs->fw = NULL;
+
+	return count;
+}
+
+static ssize_t status_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+	struct fpga_overlay_image_info *ovcs = &region->ovcs;
+
+	return sprintf(buf, "%s\n", ovcs->ovcs_id > 0 ?
+		       "applied" : "unapplied");
+}
+
+static DEVICE_ATTR_WO(load);
+static DEVICE_ATTR_WO(remove);
+static DEVICE_ATTR_RO(status);
+
+static struct attribute *of_fpga_region_attrs[] = {
+	&dev_attr_load.attr,
+	&dev_attr_remove.attr,
+	&dev_attr_status.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(of_fpga_region);
+
 static int of_fpga_region_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -440,6 +531,7 @@ static struct platform_driver of_fpga_region_driver = {
 	.driver = {
 		.name	= "of-fpga-region",
 		.of_match_table = of_match_ptr(fpga_region_of_match),
+		.dev_groups = of_fpga_region_groups,
 	},
 };
 
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 5fbc05fe70a6..36143301b49b 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -9,6 +9,19 @@
 
 struct fpga_region;
 
+/**
+ * struct fpga_overlay_image_info - information specific to an FPGA Overlay
+ * image.
+ * @fw: firmware of coeff table.
+ * @path: path of FPGA overlay image firmware file.
+ * @ovcs_id: overlay changeset id.
+ */
+struct fpga_overlay_image_info {
+	const struct firmware *fw;
+	char path[PATH_MAX];
+	int ovcs_id;
+};
+
 /**
  * struct fpga_region_info - collection of parameters an FPGA Region
  * @mgr: fpga region manager
@@ -37,6 +50,7 @@ struct fpga_region_info {
  * @info: FPGA image info
  * @compat_id: FPGA region id for compatibility check.
  * @ops_owner: module containing the get_bridges function
+ * @ovcs: FPGA overlay image info
  * @priv: private data
  * @get_bridges: optional function to get bridges to a list
  */
@@ -48,6 +62,7 @@ struct fpga_region {
 	struct fpga_image_info *info;
 	struct fpga_compat_id *compat_id;
 	struct module *ops_owner;
+	struct fpga_overlay_image_info ovcs;
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
 };
-- 
2.34.1

Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA configuration

[Xu Yilun]

On Fri, Jul 26, 2024 at 12:08:19PM +0530, Nava kishore Manne wrote:
> Adds sysfs interface as part of the of-fpga-region. This newly added
> sysfs interface uses Device Tree Overlay (DTO) files to configure/reprogram
> an FPGA while an operating system is running.This solution will not change
> the existing sequence When a DT overlay that targets an FPGA Region is
> applied.
> 	- Disable appropriate FPGA bridges.
> 	- Program the FPGA using the FPGA manager.
> 	- Enable the FPGA bridges.
> 	- The Device Tree overlay is accepted into the live tree.
> 	- Child devices are populated.
> 
> When the overlay is removed, the child nodes will be removed, and the FPGA
> Region will disable the bridges.
> 
> Usage:
> To configure/reprogram an FPGA region:
> echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load

IIRC, last time we are considering some generic interface for both OF &
non-OF FPGA region, but this is still OF specific.

Thanks,
Yilun

RE: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA configuration

[Manne, Nava kishore]

Hi Yilun,

> -----Original Message-----
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Monday, July 29, 2024 9:27 PM
> To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org; hao.wu@intel.com;
> yilun.xu@intel.com; trix@redhat.com; robh@kernel.org;
> saravanak@google.com; linux-fpga@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA
> configuration
> 
> On Fri, Jul 26, 2024 at 12:08:19PM +0530, Nava kishore Manne wrote:
> > Adds sysfs interface as part of the of-fpga-region. This newly added
> > sysfs interface uses Device Tree Overlay (DTO) files to
> > configure/reprogram an FPGA while an operating system is running.This
> > solution will not change the existing sequence When a DT overlay that
> > targets an FPGA Region is applied.
> > 	- Disable appropriate FPGA bridges.
> > 	- Program the FPGA using the FPGA manager.
> > 	- Enable the FPGA bridges.
> > 	- The Device Tree overlay is accepted into the live tree.
> > 	- Child devices are populated.
> >
> > When the overlay is removed, the child nodes will be removed, and the
> > FPGA Region will disable the bridges.
> >
> > Usage:
> > To configure/reprogram an FPGA region:
> > echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load
> 
> IIRC, last time we are considering some generic interface for both OF & non-
> OF FPGA region, but this is still OF specific.
> 
At AMD, we exclusively use OF for FPGA configuration/reconfiguration, utilizing overlay files as outlined in the fpga-region.txt documentation.
However, some devices, like dfl.c those relying solely on the FPGA region, do not use OF. 
For these non-OF devices, should we expect them to follow the fpga-region.txt guidelines for FPGA configuration/reconfiguration?
If so, it may be advantageous to develop a common interface for both OF and non-OF.
If not, it might be more appropriate to establish distinct interfaces to cater to their specific requirements.

Regards,
Navakishore.

Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA configuration

[Xu Yilun]

On Thu, Aug 01, 2024 at 04:25:42AM +0000, Manne, Nava kishore wrote:
> Hi Yilun,
> 
> > -----Original Message-----
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> > Sent: Monday, July 29, 2024 9:27 PM
> > To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> > Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org; hao.wu@intel.com;
> > yilun.xu@intel.com; trix@redhat.com; robh@kernel.org;
> > saravanak@google.com; linux-fpga@vger.kernel.org; linux-
> > kernel@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA
> > configuration
> > 
> > On Fri, Jul 26, 2024 at 12:08:19PM +0530, Nava kishore Manne wrote:
> > > Adds sysfs interface as part of the of-fpga-region. This newly added
> > > sysfs interface uses Device Tree Overlay (DTO) files to
> > > configure/reprogram an FPGA while an operating system is running.This
> > > solution will not change the existing sequence When a DT overlay that
> > > targets an FPGA Region is applied.
> > > 	- Disable appropriate FPGA bridges.
> > > 	- Program the FPGA using the FPGA manager.
> > > 	- Enable the FPGA bridges.
> > > 	- The Device Tree overlay is accepted into the live tree.
> > > 	- Child devices are populated.
> > >
> > > When the overlay is removed, the child nodes will be removed, and the
> > > FPGA Region will disable the bridges.
> > >
> > > Usage:
> > > To configure/reprogram an FPGA region:
> > > echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load
> > 
> > IIRC, last time we are considering some generic interface for both OF & non-
> > OF FPGA region, but this is still OF specific.
> > 
> At AMD, we exclusively use OF for FPGA configuration/reconfiguration, utilizing overlay files as outlined in the fpga-region.txt documentation.
> However, some devices, like dfl.c those relying solely on the FPGA region, do not use OF. 
> For these non-OF devices, should we expect them to follow the fpga-region.txt guidelines for FPGA configuration/reconfiguration?

I assume it is Documentation/devicetree/bindings/fpga/fpga-region.yaml.

No, Non-OF devices don't have to follow the DT binding.

> If so, it may be advantageous to develop a common interface for both OF and non-OF.
> If not, it might be more appropriate to establish distinct interfaces to cater to their specific requirements.

I think each vendor may have specific way for device enumeration, but
that doesn't mean we need distinct user interfaces. For all FPGA
devices, we should avoid the situation that the HW is changed but
system SW knows nothing. So the common needs are:

 - Find out and remove all devices within the fpga region before
   reprograming.
 - Re-enumerate devices in fpga region after reprograming.

I expect the fpga region class could generally enforce a flow for
the reprograming interface. And of-fpga-region could specifically
implement it using DT overlay.

Thanks,
Yilun

RE: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA configuration

[Manne, Nava kishore]

> -----Original Message-----
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Monday, August 5, 2024 11:51 PM
> To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org; hao.wu@intel.com;
> yilun.xu@intel.com; trix@redhat.com; robh@kernel.org; saravanak@google.com;
> linux-fpga@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA
> configuration
> 
> On Thu, Aug 01, 2024 at 04:25:42AM +0000, Manne, Nava kishore wrote:
> > Hi Yilun,
> >
> > > -----Original Message-----
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Monday, July 29, 2024 9:27 PM
> > > To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> > > Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org;
> > > hao.wu@intel.com; yilun.xu@intel.com; trix@redhat.com;
> > > robh@kernel.org; saravanak@google.com; linux-fpga@vger.kernel.org;
> > > linux- kernel@vger.kernel.org; devicetree@vger.kernel.org
> > > Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support
> > > for FPGA configuration
> > >
> > > On Fri, Jul 26, 2024 at 12:08:19PM +0530, Nava kishore Manne wrote:
> > > > Adds sysfs interface as part of the of-fpga-region. This newly
> > > > added sysfs interface uses Device Tree Overlay (DTO) files to
> > > > configure/reprogram an FPGA while an operating system is
> > > > running.This solution will not change the existing sequence When a
> > > > DT overlay that targets an FPGA Region is applied.
> > > > 	- Disable appropriate FPGA bridges.
> > > > 	- Program the FPGA using the FPGA manager.
> > > > 	- Enable the FPGA bridges.
> > > > 	- The Device Tree overlay is accepted into the live tree.
> > > > 	- Child devices are populated.
> > > >
> > > > When the overlay is removed, the child nodes will be removed, and
> > > > the FPGA Region will disable the bridges.
> > > >
> > > > Usage:
> > > > To configure/reprogram an FPGA region:
> > > > echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load
> > >
> > > IIRC, last time we are considering some generic interface for both
> > > OF & non- OF FPGA region, but this is still OF specific.
> > >
> > At AMD, we exclusively use OF for FPGA configuration/reconfiguration, utilizing
> overlay files as outlined in the fpga-region.txt documentation.
> > However, some devices, like dfl.c those relying solely on the FPGA region, do not
> use OF.
> > For these non-OF devices, should we expect them to follow the fpga-region.txt
> guidelines for FPGA configuration/reconfiguration?
> 
> I assume it is Documentation/devicetree/bindings/fpga/fpga-region.yaml.
> 
> No, Non-OF devices don't have to follow the DT binding.
> 
> > If so, it may be advantageous to develop a common interface for both OF and
> non-OF.
> > If not, it might be more appropriate to establish distinct interfaces to cater to their
> specific requirements.
> 
> I think each vendor may have specific way for device enumeration, but that doesn't
> mean we need distinct user interfaces. For all FPGA devices, we should avoid the
> situation that the HW is changed but system SW knows nothing. So the common
> needs are:
> 
>  - Find out and remove all devices within the fpga region before
>    reprograming.
>  - Re-enumerate devices in fpga region after reprograming.
> 
> I expect the fpga region class could generally enforce a flow for the reprograming
> interface. And of-fpga-region could specifically implement it using DT overlay.
> 

To address the vendor-specific nature(either of or non-of) of device enumeration
in FPGA regions, As you suggested, we can develop a common programming
interface that abstracts these vendor-specifc differences. This can be achieved
by integrating vendor-specific callbacks(ex: of and non-of) for device configuration,
enumeration and removal to fpga-region. 

I have outlined the top-level framework changes here:

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index b364a929425c..7d4b755dc8e0 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -213,6 +213,7 @@ fpga_region_register_full(struct device *parent, const struct fpga_region_info *
 	region->compat_id = info->compat_id;
 	region->priv = info->priv;
 	region->get_bridges = info->get_bridges;
+	region->region_ops = info->region_ops;
 
 	mutex_init(&region->mutex);
 	INIT_LIST_HEAD(&region->bridge_list);
@@ -257,17 +258,46 @@ EXPORT_SYMBOL_GPL(fpga_region_register_full);
  */
 struct fpga_region *
 fpga_region_register(struct device *parent, struct fpga_manager *mgr,
+		     struct fpga_region_ops *region_ops,
 		     int (*get_bridges)(struct fpga_region *))  {
 	struct fpga_region_info info = { 0 };
 
 	info.mgr = mgr;
 	info.get_bridges = get_bridges;
+	info.region_ops = region_ops;
 
 	return fpga_region_register_full(parent, &info);  }  EXPORT_SYMBOL_GPL(fpga_region_register);
 
+static int fpga_region_device_open(struct inode *inode, struct file *file) {
+	struct miscdevice *miscdev = file->private_data;
+	struct fpga_region *region = container_of(miscdev, struct fpga_region, 
+miscdev);
+
+	file->private_data = region;
+
+	return 0;
+}
+
+static long fpga_region_device_ioctl(struct file *file, unsigned int cmd, unsigned long arg) {
+	char __user *argp = (char __user *)arg;
+	struct fpga_region *region =  (struct fpga_region *)(file->private_data);
+	int err;
+
+	switch (cmd) {
+	case FPGA_REGION_IOCTL_LOAD:
+		err = region->region_ops->fpga_region_config_enumerate (region, argp);
+		break;
+	case FPGA_REGION_IOCTL_REMOVE:
+		err = region->region_ops->fpga_region_remove(region, argp);
+		break;
+	case FPGA_REGION_IOCTL_STATUS:
+		err = region->region_ops->fpga_region_status(region, argp);
+	default:
+		err = -ENOTTY;
+}
+
 /**
  * fpga_region_unregister - unregister an FPGA region
  * @region: FPGA region
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 9d4d32909340..725fdcbab3d8 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -9,6 +9,20 @@
 
 struct fpga_region;
 
+/**
+ * struct fpga_region_ops - ops for low level FPGA region ops for 
+device
+ * enumeration/removal
+ * @region_status: returns the FPGA region status
+ * @region_config_enumeration: Configure and enumerate the FPGA region.
+ * @region_remove: Remove all devices within the fpga region
+ * (which are added as part of the enumeration).
+ */
+struct fpga_region_ops {
+	int (*region_status)(struct fpga_region *bridge);
+	int (*region_config_enumeration)(struct fpga_region *region, void *args);
+	void (*region_remove)(struct fpga_region *region, void *args); };
+
 /**
  * struct fpga_region_info - collection of parameters an FPGA Region
  * @mgr: fpga region manager
@@ -26,6 +40,7 @@ struct fpga_region_info {
 	struct fpga_compat_id *compat_id;
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
+	struct fpga_region_ops *region_ops;
 };
 
 /**
@@ -48,6 +63,7 @@ struct fpga_region {
 	struct fpga_compat_id *compat_id;
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
+	struct fpga_region_ops *region_ops;
 };
 
 #define to_fpga_region(d) container_of(d, struct fpga_region, dev)

In this approach, we utilized an IOCTL-based user interface, but it doesn't have
to be confined to IOCTL. We can also use sysfs or configfs, or other appropriate
options as we finalized on it.

This call-backs approach works for both OF and non-OF devices. 
If this aligns with your expectations, I can do the necessary changes
for one vendor specific interface (of-fpga-region.c) devices and submit
the RFC patch shortly.


Regards,
Navakishore.

Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA configuration

[Xu Yilun]

On Tue, Sep 17, 2024 at 11:16:08AM +0000, Manne, Nava kishore wrote:
> > -----Original Message-----
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> > Sent: Monday, August 5, 2024 11:51 PM
> > To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> > Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org; hao.wu@intel.com;
> > yilun.xu@intel.com; trix@redhat.com; robh@kernel.org; saravanak@google.com;
> > linux-fpga@vger.kernel.org; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org
> > Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA
> > configuration
> > 
> > On Thu, Aug 01, 2024 at 04:25:42AM +0000, Manne, Nava kishore wrote:
> > > Hi Yilun,
> > >
> > > > -----Original Message-----
> > > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > > Sent: Monday, July 29, 2024 9:27 PM
> > > > To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> > > > Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org;
> > > > hao.wu@intel.com; yilun.xu@intel.com; trix@redhat.com;
> > > > robh@kernel.org; saravanak@google.com; linux-fpga@vger.kernel.org;
> > > > linux- kernel@vger.kernel.org; devicetree@vger.kernel.org
> > > > Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support
> > > > for FPGA configuration
> > > >
> > > > On Fri, Jul 26, 2024 at 12:08:19PM +0530, Nava kishore Manne wrote:
> > > > > Adds sysfs interface as part of the of-fpga-region. This newly
> > > > > added sysfs interface uses Device Tree Overlay (DTO) files to
> > > > > configure/reprogram an FPGA while an operating system is
> > > > > running.This solution will not change the existing sequence When a
> > > > > DT overlay that targets an FPGA Region is applied.
> > > > > 	- Disable appropriate FPGA bridges.
> > > > > 	- Program the FPGA using the FPGA manager.
> > > > > 	- Enable the FPGA bridges.
> > > > > 	- The Device Tree overlay is accepted into the live tree.
> > > > > 	- Child devices are populated.
> > > > >
> > > > > When the overlay is removed, the child nodes will be removed, and
> > > > > the FPGA Region will disable the bridges.
> > > > >
> > > > > Usage:
> > > > > To configure/reprogram an FPGA region:
> > > > > echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load
> > > >
> > > > IIRC, last time we are considering some generic interface for both
> > > > OF & non- OF FPGA region, but this is still OF specific.
> > > >
> > > At AMD, we exclusively use OF for FPGA configuration/reconfiguration, utilizing
> > overlay files as outlined in the fpga-region.txt documentation.
> > > However, some devices, like dfl.c those relying solely on the FPGA region, do not
> > use OF.
> > > For these non-OF devices, should we expect them to follow the fpga-region.txt
> > guidelines for FPGA configuration/reconfiguration?
> > 
> > I assume it is Documentation/devicetree/bindings/fpga/fpga-region.yaml.
> > 
> > No, Non-OF devices don't have to follow the DT binding.
> > 
> > > If so, it may be advantageous to develop a common interface for both OF and
> > non-OF.
> > > If not, it might be more appropriate to establish distinct interfaces to cater to their
> > specific requirements.
> > 
> > I think each vendor may have specific way for device enumeration, but that doesn't
> > mean we need distinct user interfaces. For all FPGA devices, we should avoid the
> > situation that the HW is changed but system SW knows nothing. So the common
> > needs are:
> > 
> >  - Find out and remove all devices within the fpga region before
> >    reprograming.
> >  - Re-enumerate devices in fpga region after reprograming.
> > 
> > I expect the fpga region class could generally enforce a flow for the reprograming
> > interface. And of-fpga-region could specifically implement it using DT overlay.
> > 
> 
> To address the vendor-specific nature(either of or non-of) of device enumeration
> in FPGA regions, As you suggested, we can develop a common programming
> interface that abstracts these vendor-specifc differences. This can be achieved
> by integrating vendor-specific callbacks(ex: of and non-of) for device configuration,
> enumeration and removal to fpga-region. 
> 
> I have outlined the top-level framework changes here:
> 
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index b364a929425c..7d4b755dc8e0 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -213,6 +213,7 @@ fpga_region_register_full(struct device *parent, const struct fpga_region_info *
>  	region->compat_id = info->compat_id;
>  	region->priv = info->priv;
>  	region->get_bridges = info->get_bridges;
> +	region->region_ops = info->region_ops;
>  
>  	mutex_init(&region->mutex);
>  	INIT_LIST_HEAD(&region->bridge_list);
> @@ -257,17 +258,46 @@ EXPORT_SYMBOL_GPL(fpga_region_register_full);
>   */
>  struct fpga_region *
>  fpga_region_register(struct device *parent, struct fpga_manager *mgr,
> +		     struct fpga_region_ops *region_ops,
>  		     int (*get_bridges)(struct fpga_region *))  {
>  	struct fpga_region_info info = { 0 };
>  
>  	info.mgr = mgr;
>  	info.get_bridges = get_bridges;
> +	info.region_ops = region_ops;
>  
>  	return fpga_region_register_full(parent, &info);  }  EXPORT_SYMBOL_GPL(fpga_region_register);
>  
> +static int fpga_region_device_open(struct inode *inode, struct file *file) {
> +	struct miscdevice *miscdev = file->private_data;
> +	struct fpga_region *region = container_of(miscdev, struct fpga_region, 
> +miscdev);
> +
> +	file->private_data = region;
> +
> +	return 0;
> +}
> +
> +static long fpga_region_device_ioctl(struct file *file, unsigned int cmd, unsigned long arg) {
> +	char __user *argp = (char __user *)arg;
> +	struct fpga_region *region =  (struct fpga_region *)(file->private_data);
> +	int err;
> +
> +	switch (cmd) {
> +	case FPGA_REGION_IOCTL_LOAD:
> +		err = region->region_ops->fpga_region_config_enumerate (region, argp);

Not sure "void *args" is a proposal or something yet to be decided.
I think we should try best not to give up parameter type and have a clear API
definition.

> +		break;
> +	case FPGA_REGION_IOCTL_REMOVE:
> +		err = region->region_ops->fpga_region_remove(region, argp);
> +		break;
> +	case FPGA_REGION_IOCTL_STATUS:
> +		err = region->region_ops->fpga_region_status(region, argp);
> +	default:
> +		err = -ENOTTY;
> +}
> +
>  /**
>   * fpga_region_unregister - unregister an FPGA region
>   * @region: FPGA region
> diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
> index 9d4d32909340..725fdcbab3d8 100644
> --- a/include/linux/fpga/fpga-region.h
> +++ b/include/linux/fpga/fpga-region.h
> @@ -9,6 +9,20 @@
>  
>  struct fpga_region;
>  
> +/**
> + * struct fpga_region_ops - ops for low level FPGA region ops for 
> +device
> + * enumeration/removal
> + * @region_status: returns the FPGA region status
> + * @region_config_enumeration: Configure and enumerate the FPGA region.

region config could be a common existing operation, fpga_region_program_fpga().
So maybe only enumeration is needed?

> + * @region_remove: Remove all devices within the fpga region
> + * (which are added as part of the enumeration).
> + */
> +struct fpga_region_ops {
> +	int (*region_status)(struct fpga_region *bridge);
> +	int (*region_config_enumeration)(struct fpga_region *region, void *args);
> +	void (*region_remove)(struct fpga_region *region, void *args); };
> +
>  /**
>   * struct fpga_region_info - collection of parameters an FPGA Region
>   * @mgr: fpga region manager
> @@ -26,6 +40,7 @@ struct fpga_region_info {
>  	struct fpga_compat_id *compat_id;
>  	void *priv;
>  	int (*get_bridges)(struct fpga_region *region);
> +	struct fpga_region_ops *region_ops;
>  };
>  
>  /**
> @@ -48,6 +63,7 @@ struct fpga_region {
>  	struct fpga_compat_id *compat_id;
>  	void *priv;
>  	int (*get_bridges)(struct fpga_region *region);
> +	struct fpga_region_ops *region_ops;
>  };
>  
>  #define to_fpga_region(d) container_of(d, struct fpga_region, dev)
> 
> In this approach, we utilized an IOCTL-based user interface, but it doesn't have
> to be confined to IOCTL. We can also use sysfs or configfs, or other appropriate
> options as we finalized on it.
> 
> This call-backs approach works for both OF and non-OF devices. 
> If this aligns with your expectations, I can do the necessary changes

There are still much to discuss, but yes this is a good start.

Thanks,
Yilun

> for one vendor specific interface (of-fpga-region.c) devices and submit
> the RFC patch shortly.
> 
> 
> Regards,
> Navakishore.

RE: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA configuration

[Manne, Nava kishore]

> -----Original Message-----
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, September 18, 2024 1:50 PM
> To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org; hao.wu@intel.com;
> yilun.xu@intel.com; trix@redhat.com; robh@kernel.org; saravanak@google.com;
> linux-fpga@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support for FPGA
> configuration
> 
> On Tue, Sep 17, 2024 at 11:16:08AM +0000, Manne, Nava kishore wrote:
> > > -----Original Message-----
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Monday, August 5, 2024 11:51 PM
> > > To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> > > Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org;
> > > hao.wu@intel.com; yilun.xu@intel.com; trix@redhat.com;
> > > robh@kernel.org; saravanak@google.com; linux-fpga@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> > > Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface support
> > > for FPGA configuration
> > >
> > > On Thu, Aug 01, 2024 at 04:25:42AM +0000, Manne, Nava kishore wrote:
> > > > Hi Yilun,
> > > >
> > > > > -----Original Message-----
> > > > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > > > Sent: Monday, July 29, 2024 9:27 PM
> > > > > To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> > > > > Cc: git (AMD-Xilinx) <git@amd.com>; mdf@kernel.org;
> > > > > hao.wu@intel.com; yilun.xu@intel.com; trix@redhat.com;
> > > > > robh@kernel.org; saravanak@google.com;
> > > > > linux-fpga@vger.kernel.org;
> > > > > linux- kernel@vger.kernel.org; devicetree@vger.kernel.org
> > > > > Subject: Re: [RFC 1/1] of-fpga-region: Add sysfs interface
> > > > > support for FPGA configuration
> > > > >
> > > > > On Fri, Jul 26, 2024 at 12:08:19PM +0530, Nava kishore Manne wrote:
> > > > > > Adds sysfs interface as part of the of-fpga-region. This newly
> > > > > > added sysfs interface uses Device Tree Overlay (DTO) files to
> > > > > > configure/reprogram an FPGA while an operating system is
> > > > > > running.This solution will not change the existing sequence
> > > > > > When a DT overlay that targets an FPGA Region is applied.
> > > > > > 	- Disable appropriate FPGA bridges.
> > > > > > 	- Program the FPGA using the FPGA manager.
> > > > > > 	- Enable the FPGA bridges.
> > > > > > 	- The Device Tree overlay is accepted into the live tree.
> > > > > > 	- Child devices are populated.
> > > > > >
> > > > > > When the overlay is removed, the child nodes will be removed,
> > > > > > and the FPGA Region will disable the bridges.
> > > > > >
> > > > > > Usage:
> > > > > > To configure/reprogram an FPGA region:
> > > > > > echo "fpga.dtbo" > /sys/class/fpga_region/<region>/device/load
> > > > >
> > > > > IIRC, last time we are considering some generic interface for
> > > > > both OF & non- OF FPGA region, but this is still OF specific.
> > > > >
> > > > At AMD, we exclusively use OF for FPGA
> > > > configuration/reconfiguration, utilizing
> > > overlay files as outlined in the fpga-region.txt documentation.
> > > > However, some devices, like dfl.c those relying solely on the FPGA
> > > > region, do not
> > > use OF.
> > > > For these non-OF devices, should we expect them to follow the
> > > > fpga-region.txt
> > > guidelines for FPGA configuration/reconfiguration?
> > >
> > > I assume it is Documentation/devicetree/bindings/fpga/fpga-region.yaml.
> > >
> > > No, Non-OF devices don't have to follow the DT binding.
> > >
> > > > If so, it may be advantageous to develop a common interface for
> > > > both OF and
> > > non-OF.
> > > > If not, it might be more appropriate to establish distinct
> > > > interfaces to cater to their
> > > specific requirements.
> > >
> > > I think each vendor may have specific way for device enumeration,
> > > but that doesn't mean we need distinct user interfaces. For all FPGA
> > > devices, we should avoid the situation that the HW is changed but
> > > system SW knows nothing. So the common needs are:
> > >
> > >  - Find out and remove all devices within the fpga region before
> > >    reprograming.
> > >  - Re-enumerate devices in fpga region after reprograming.
> > >
> > > I expect the fpga region class could generally enforce a flow for
> > > the reprograming interface. And of-fpga-region could specifically implement it
> using DT overlay.
> > >
> >
> > To address the vendor-specific nature(either of or non-of) of device
> > enumeration in FPGA regions, As you suggested, we can develop a common
> > programming interface that abstracts these vendor-specifc differences.
> > This can be achieved by integrating vendor-specific callbacks(ex: of
> > and non-of) for device configuration, enumeration and removal to fpga-region.
> >
> > I have outlined the top-level framework changes here:
> >
> > diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> > index b364a929425c..7d4b755dc8e0 100644
> > --- a/drivers/fpga/fpga-region.c
> > +++ b/drivers/fpga/fpga-region.c
> > @@ -213,6 +213,7 @@ fpga_region_register_full(struct device *parent, const
> struct fpga_region_info *
> >  	region->compat_id = info->compat_id;
> >  	region->priv = info->priv;
> >  	region->get_bridges = info->get_bridges;
> > +	region->region_ops = info->region_ops;
> >
> >  	mutex_init(&region->mutex);
> >  	INIT_LIST_HEAD(&region->bridge_list);
> > @@ -257,17 +258,46 @@ EXPORT_SYMBOL_GPL(fpga_region_register_full);
> >   */
> >  struct fpga_region *
> >  fpga_region_register(struct device *parent, struct fpga_manager *mgr,
> > +		     struct fpga_region_ops *region_ops,
> >  		     int (*get_bridges)(struct fpga_region *))  {
> >  	struct fpga_region_info info = { 0 };
> >
> >  	info.mgr = mgr;
> >  	info.get_bridges = get_bridges;
> > +	info.region_ops = region_ops;
> >
> >  	return fpga_region_register_full(parent, &info);  }
> > EXPORT_SYMBOL_GPL(fpga_region_register);
> >
> > +static int fpga_region_device_open(struct inode *inode, struct file *file) {
> > +	struct miscdevice *miscdev = file->private_data;
> > +	struct fpga_region *region = container_of(miscdev, struct
> > +fpga_region, miscdev);
> > +
> > +	file->private_data = region;
> > +
> > +	return 0;
> > +}
> > +
> > +static long fpga_region_device_ioctl(struct file *file, unsigned int cmd, unsigned
> long arg) {
> > +	char __user *argp = (char __user *)arg;
> > +	struct fpga_region *region =  (struct fpga_region *)(file->private_data);
> > +	int err;
> > +
> > +	switch (cmd) {
> > +	case FPGA_REGION_IOCTL_LOAD:
> > +		err = region->region_ops->fpga_region_config_enumerate (region,
> > +argp);
> 
> Not sure "void *args" is a proposal or something yet to be decided.
> I think we should try best not to give up parameter type and have a clear API
> definition.
> 

I agree fixing the parameter type is necessary, and I will work on
Implementing this change in the RFC. Following our discussion,
If any additional modifications or clarifications are required,
I will ensure to incorporate them in the later stages.

> > +		break;
> > +	case FPGA_REGION_IOCTL_REMOVE:
> > +		err = region->region_ops->fpga_region_remove(region, argp);
> > +		break;
> > +	case FPGA_REGION_IOCTL_STATUS:
> > +		err = region->region_ops->fpga_region_status(region, argp);
> > +	default:
> > +		err = -ENOTTY;
> > +}
> > +
> >  /**
> >   * fpga_region_unregister - unregister an FPGA region
> >   * @region: FPGA region
> > diff --git a/include/linux/fpga/fpga-region.h
> > b/include/linux/fpga/fpga-region.h
> > index 9d4d32909340..725fdcbab3d8 100644
> > --- a/include/linux/fpga/fpga-region.h
> > +++ b/include/linux/fpga/fpga-region.h
> > @@ -9,6 +9,20 @@
> >
> >  struct fpga_region;
> >
> > +/**
> > + * struct fpga_region_ops - ops for low level FPGA region ops for
> > +device
> > + * enumeration/removal
> > + * @region_status: returns the FPGA region status
> > + * @region_config_enumeration: Configure and enumerate the FPGA region.
> 
> region config could be a common existing operation, fpga_region_program_fpga().
> So maybe only enumeration is needed?
> 

I agree configuration and enumeration can be handled separately. However,
for some vendors, pre-configuration details such as port IDs(AFU) and
lower-level connected device information needs to be set before configuration.
To accommodate both vendor-specific pre and post configuration (enumerations),
I chose to use a single API for handling both FPGA configuration and enumeration.

> > + * @region_remove: Remove all devices within the fpga region
> > + * (which are added as part of the enumeration).
> > + */
> > +struct fpga_region_ops {
> > +	int (*region_status)(struct fpga_region *bridge);
> > +	int (*region_config_enumeration)(struct fpga_region *region, void *args);
> > +	void (*region_remove)(struct fpga_region *region, void *args); };
> > +
> >  /**
> >   * struct fpga_region_info - collection of parameters an FPGA Region
> >   * @mgr: fpga region manager
> > @@ -26,6 +40,7 @@ struct fpga_region_info {
> >  	struct fpga_compat_id *compat_id;
> >  	void *priv;
> >  	int (*get_bridges)(struct fpga_region *region);
> > +	struct fpga_region_ops *region_ops;
> >  };
> >
> >  /**
> > @@ -48,6 +63,7 @@ struct fpga_region {
> >  	struct fpga_compat_id *compat_id;
> >  	void *priv;
> >  	int (*get_bridges)(struct fpga_region *region);
> > +	struct fpga_region_ops *region_ops;
> >  };
> >
> >  #define to_fpga_region(d) container_of(d, struct fpga_region, dev)
> >
> > In this approach, we utilized an IOCTL-based user interface, but it
> > doesn't have to be confined to IOCTL. We can also use sysfs or
> > configfs, or other appropriate options as we finalized on it.
> >
> > This call-backs approach works for both OF and non-OF devices.
> > If this aligns with your expectations, I can do the necessary changes
> 
> There are still much to discuss, but yes this is a good start.
> 

Thanks for providing the quick confirmation will post RFC soon.

Regards,
Navakishore