| Message ID | 20210420081153.17020-6-nava.manne@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | misc: Add afi config drivers support. | expand |
On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote: > This patch adds zynqmp afi config driver.This is useful for > the configuration of the PS-PL interface on Zynq US+ MPSoC > platform. Again, please spell out what those terms mean, as I have no idea :( > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > --- > drivers/misc/Kconfig | 11 ++++++ > drivers/misc/Makefile | 1 + > drivers/misc/zynqmp-afi.c | 83 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 95 insertions(+) > create mode 100644 drivers/misc/zynqmp-afi.c > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 877b43b3377d..d1ea1eeb3ac1 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -456,6 +456,17 @@ config ZYNQ_AFI > between PS and PL, the AXI port data path should be configured > with the proper Bus-width values > > +config ZYNQMP_AFI > + tristate "Xilinx ZYNQMP AFI support" > + help > + ZynqMP AFI driver support for writing to the AFI registers for > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > + PS to the programmable logic (PL) through the AXI port. This AXI > + port helps to establish the data path between the PS and PL. > + In-order to establish the proper communication path between > + PS and PL, the AXI port data path should be configured with > + the proper Bus-width values Please use tabs properly, you mix them above, checkpatch should have caught that. thanks, greg k-h
On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote: > This patch adds zynqmp afi config driver.This is useful for > the configuration of the PS-PL interface on Zynq US+ MPSoC > platform. > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > --- > drivers/misc/Kconfig | 11 ++++++ > drivers/misc/Makefile | 1 + > drivers/misc/zynqmp-afi.c | 83 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 95 insertions(+) > create mode 100644 drivers/misc/zynqmp-afi.c > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 877b43b3377d..d1ea1eeb3ac1 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -456,6 +456,17 @@ config ZYNQ_AFI > between PS and PL, the AXI port data path should be configured > with the proper Bus-width values > > +config ZYNQMP_AFI > + tristate "Xilinx ZYNQMP AFI support" > + help > + ZynqMP AFI driver support for writing to the AFI registers for > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > + PS to the programmable logic (PL) through the AXI port. This AXI > + port helps to establish the data path between the PS and PL. > + In-order to establish the proper communication path between > + PS and PL, the AXI port data path should be configured with > + the proper Bus-width values > + > source "drivers/misc/c2port/Kconfig" > source "drivers/misc/eeprom/Kconfig" > source "drivers/misc/cb710/Kconfig" > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index e9b03843100f..54bd0edc511e 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -57,3 +57,4 @@ obj-$(CONFIG_UACCE) += uacce/ > obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o > obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o > obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o > +obj-$(CONFIG_ZYNQMP_AFI) += zynqmp-afi.o > diff --git a/drivers/misc/zynqmp-afi.c b/drivers/misc/zynqmp-afi.c > new file mode 100644 > index 000000000000..a318652576d2 > --- /dev/null > +++ b/drivers/misc/zynqmp-afi.c > @@ -0,0 +1,83 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Xilinx FPGA AFI bridge. > + * Copyright (c) 2018-2021 Xilinx Inc. > + */ > + > +#include <linux/err.h> > +#include <linux/firmware/xlnx-zynqmp.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +/** > + * struct zynqmp_afi_fpga - AFI register description > + * @value: value to be written to the register > + * @regid: Register id for the register to be written > + */ > +struct zynqmp_afi_fpga { > + u32 value; > + u32 regid; > +}; > + > +static int zynqmp_afi_fpga_probe(struct platform_device *pdev) > +{ > + struct zynqmp_afi_fpga *zynqmp_afi_fpga; > + struct device_node *np = pdev->dev.of_node; > + int i, entries, ret; > + u32 reg, val; > + > + zynqmp_afi_fpga = devm_kzalloc(&pdev->dev, > + sizeof(*zynqmp_afi_fpga), GFP_KERNEL); > + if (!zynqmp_afi_fpga) > + return -ENOMEM; > + platform_set_drvdata(pdev, zynqmp_afi_fpga); > + > + entries = of_property_count_u32_elems(np, "config-afi"); > + if (!entries || (entries % 2)) { > + dev_err(&pdev->dev, "Invalid number of registers\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < entries / 2; i++) { > + ret = of_property_read_u32_index(np, "config-afi", i * 2, ®); > + if (ret) { > + dev_err(&pdev->dev, "failed to read register\n"); > + return -EINVAL; > + } > + ret = of_property_read_u32_index(np, "config-afi", i * 2 + 1, > + &val); > + if (ret) { > + dev_err(&pdev->dev, "failed to read value\n"); > + return -EINVAL; > + } > + ret = zynqmp_pm_afi(reg, val); > + if (ret < 0) { > + dev_err(&pdev->dev, "AFI register write error %d\n", > + ret); > + return ret; > + } > + } > + return 0; > +} Again, why does this have to be in the kernel? All it does is make a single call to the hardware based on some values read from the device tree. Can't you do this from userspace? thanks, greg k-h
Hi Greg, Please find my response inline. > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Tuesday, April 20, 2021 2:18 PM > To: Nava kishore Manne <navam@xilinx.com> > Cc: robh+dt@kernel.org; Michal Simek <michals@xilinx.com>; Derek Kiernan > <dkiernan@xilinx.com>; Dragan Cvetic <draganc@xilinx.com>; > arnd@arndb.de; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah > <JOLLYS@xilinx.com>; Tejas Patel <tejasp@xlnx.xilinx.com>; Amit Sunil > Dhamne <amitsuni@xilinx.com>; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > chinnikishore369@gmail.com; git <git@xilinx.com> > Subject: Re: [PATCH 5/5] misc: zynqmp: Add afi config driver > > On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote: > > This patch adds zynqmp afi config driver.This is useful for the > > configuration of the PS-PL interface on Zynq US+ MPSoC platform. > > Again, please spell out what those terms mean, as I have no idea :( > Will fix in v2 > > > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > > --- > > drivers/misc/Kconfig | 11 ++++++ > > drivers/misc/Makefile | 1 + > > drivers/misc/zynqmp-afi.c | 83 > > +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 95 insertions(+) > > create mode 100644 drivers/misc/zynqmp-afi.c > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index > > 877b43b3377d..d1ea1eeb3ac1 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -456,6 +456,17 @@ config ZYNQ_AFI > > between PS and PL, the AXI port data path should be configured > > with the proper Bus-width values > > > > +config ZYNQMP_AFI > > + tristate "Xilinx ZYNQMP AFI support" > > + help > > + ZynqMP AFI driver support for writing to the AFI registers for > > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > > + PS to the programmable logic (PL) through the AXI port. This AXI > > + port helps to establish the data path between the PS and PL. > > + In-order to establish the proper communication path between > > + PS and PL, the AXI port data path should be configured with > > + the proper Bus-width values > > Please use tabs properly, you mix them above, checkpatch should have > caught that. > Yes, Ideally check patch should report this issue but it's failed to report. Will fix this issue in v2. Regards, Navakishore.
Hi Greg, Please find my response inline. > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Tuesday, April 20, 2021 2:21 PM > To: Nava kishore Manne <navam@xilinx.com> > Cc: robh+dt@kernel.org; Michal Simek <michals@xilinx.com>; Derek Kiernan > <dkiernan@xilinx.com>; Dragan Cvetic <draganc@xilinx.com>; > arnd@arndb.de; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah > <JOLLYS@xilinx.com>; Tejas Patel <tejasp@xlnx.xilinx.com>; Amit Sunil > Dhamne <amitsuni@xilinx.com>; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > chinnikishore369@gmail.com; git <git@xilinx.com> > Subject: Re: [PATCH 5/5] misc: zynqmp: Add afi config driver > > On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote: > > This patch adds zynqmp afi config driver.This is useful for the > > configuration of the PS-PL interface on Zynq US+ MPSoC platform. > > > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > > --- > > drivers/misc/Kconfig | 11 ++++++ > > drivers/misc/Makefile | 1 + > > drivers/misc/zynqmp-afi.c | 83 > > +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 95 insertions(+) > > create mode 100644 drivers/misc/zynqmp-afi.c > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index > > 877b43b3377d..d1ea1eeb3ac1 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -456,6 +456,17 @@ config ZYNQ_AFI > > between PS and PL, the AXI port data path should be configured > > with the proper Bus-width values > > > > +config ZYNQMP_AFI > > + tristate "Xilinx ZYNQMP AFI support" > > + help > > + ZynqMP AFI driver support for writing to the AFI registers for > > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > > + PS to the programmable logic (PL) through the AXI port. This AXI > > + port helps to establish the data path between the PS and PL. > > + In-order to establish the proper communication path between > > + PS and PL, the AXI port data path should be configured with > > + the proper Bus-width values > > + > > source "drivers/misc/c2port/Kconfig" > > source "drivers/misc/eeprom/Kconfig" > > source "drivers/misc/cb710/Kconfig" > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index > > e9b03843100f..54bd0edc511e 100644 > > --- a/drivers/misc/Makefile > > +++ b/drivers/misc/Makefile > > @@ -57,3 +57,4 @@ obj-$(CONFIG_UACCE) += uacce/ > > obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o > > obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o > > obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o > > +obj-$(CONFIG_ZYNQMP_AFI) += zynqmp-afi.o > > diff --git a/drivers/misc/zynqmp-afi.c b/drivers/misc/zynqmp-afi.c new > > file mode 100644 index 000000000000..a318652576d2 > > --- /dev/null > > +++ b/drivers/misc/zynqmp-afi.c > > @@ -0,0 +1,83 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Xilinx FPGA AFI bridge. > > + * Copyright (c) 2018-2021 Xilinx Inc. > > + */ > > + > > +#include <linux/err.h> > > +#include <linux/firmware/xlnx-zynqmp.h> #include <linux/io.h> > > +#include <linux/module.h> #include <linux/of.h> #include > > +<linux/platform_device.h> #include <linux/slab.h> > > + > > +/** > > + * struct zynqmp_afi_fpga - AFI register description > > + * @value: value to be written to the register > > + * @regid: Register id for the register to be written > > + */ > > +struct zynqmp_afi_fpga { > > + u32 value; > > + u32 regid; > > +}; > > + > > +static int zynqmp_afi_fpga_probe(struct platform_device *pdev) > > +{ > > + struct zynqmp_afi_fpga *zynqmp_afi_fpga; > > + struct device_node *np = pdev->dev.of_node; > > + int i, entries, ret; > > + u32 reg, val; > > + > > + zynqmp_afi_fpga = devm_kzalloc(&pdev->dev, > > + sizeof(*zynqmp_afi_fpga), GFP_KERNEL); > > + if (!zynqmp_afi_fpga) > > + return -ENOMEM; > > + platform_set_drvdata(pdev, zynqmp_afi_fpga); > > + > > + entries = of_property_count_u32_elems(np, "config-afi"); > > + if (!entries || (entries % 2)) { > > + dev_err(&pdev->dev, "Invalid number of registers\n"); > > + return -EINVAL; > > + } > > + > > + for (i = 0; i < entries / 2; i++) { > > + ret = of_property_read_u32_index(np, "config-afi", i * 2, > ®); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to read register\n"); > > + return -EINVAL; > > + } > > + ret = of_property_read_u32_index(np, "config-afi", i * 2 + 1, > > + &val); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to read value\n"); > > + return -EINVAL; > > + } > > + ret = zynqmp_pm_afi(reg, val); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "AFI register write error %d\n", > > + ret); > > + return ret; > > + } > > + } > > + return 0; > > +} > > Again, why does this have to be in the kernel? All it does is make a > single call to the hardware based on some values read from the device > tree. Can't you do this from userspace? > For every PL design has its own PS-PL configuration. This driver will be used by the overlay framework for configuring the interface after programming the FPGA and before probing the drivers that are present in the PL. Regards, Navakishore.
On Tue, Apr 20, 2021 at 01:47:17PM +0000, Nava kishore Manne wrote: > Hi Greg, > > Please find my response inline. > > > -----Original Message----- > > From: Greg KH <gregkh@linuxfoundation.org> > > Sent: Tuesday, April 20, 2021 2:21 PM > > To: Nava kishore Manne <navam@xilinx.com> > > Cc: robh+dt@kernel.org; Michal Simek <michals@xilinx.com>; Derek Kiernan > > <dkiernan@xilinx.com>; Dragan Cvetic <draganc@xilinx.com>; > > arnd@arndb.de; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah > > <JOLLYS@xilinx.com>; Tejas Patel <tejasp@xlnx.xilinx.com>; Amit Sunil > > Dhamne <amitsuni@xilinx.com>; devicetree@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > > chinnikishore369@gmail.com; git <git@xilinx.com> > > Subject: Re: [PATCH 5/5] misc: zynqmp: Add afi config driver > > > > On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote: > > > This patch adds zynqmp afi config driver.This is useful for the > > > configuration of the PS-PL interface on Zynq US+ MPSoC platform. > > > > > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > > > --- > > > drivers/misc/Kconfig | 11 ++++++ > > > drivers/misc/Makefile | 1 + > > > drivers/misc/zynqmp-afi.c | 83 > > > +++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 95 insertions(+) > > > create mode 100644 drivers/misc/zynqmp-afi.c > > > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index > > > 877b43b3377d..d1ea1eeb3ac1 100644 > > > --- a/drivers/misc/Kconfig > > > +++ b/drivers/misc/Kconfig > > > @@ -456,6 +456,17 @@ config ZYNQ_AFI > > > between PS and PL, the AXI port data path should be configured > > > with the proper Bus-width values > > > > > > +config ZYNQMP_AFI > > > + tristate "Xilinx ZYNQMP AFI support" > > > + help > > > + ZynqMP AFI driver support for writing to the AFI registers for > > > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > > > + PS to the programmable logic (PL) through the AXI port. This AXI > > > + port helps to establish the data path between the PS and PL. > > > + In-order to establish the proper communication path between > > > + PS and PL, the AXI port data path should be configured with > > > + the proper Bus-width values > > > + > > > source "drivers/misc/c2port/Kconfig" > > > source "drivers/misc/eeprom/Kconfig" > > > source "drivers/misc/cb710/Kconfig" > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index > > > e9b03843100f..54bd0edc511e 100644 > > > --- a/drivers/misc/Makefile > > > +++ b/drivers/misc/Makefile > > > @@ -57,3 +57,4 @@ obj-$(CONFIG_UACCE) += uacce/ > > > obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o > > > obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o > > > obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o > > > +obj-$(CONFIG_ZYNQMP_AFI) += zynqmp-afi.o > > > diff --git a/drivers/misc/zynqmp-afi.c b/drivers/misc/zynqmp-afi.c new > > > file mode 100644 index 000000000000..a318652576d2 > > > --- /dev/null > > > +++ b/drivers/misc/zynqmp-afi.c > > > @@ -0,0 +1,83 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Xilinx FPGA AFI bridge. > > > + * Copyright (c) 2018-2021 Xilinx Inc. > > > + */ > > > + > > > +#include <linux/err.h> > > > +#include <linux/firmware/xlnx-zynqmp.h> #include <linux/io.h> > > > +#include <linux/module.h> #include <linux/of.h> #include > > > +<linux/platform_device.h> #include <linux/slab.h> > > > + > > > +/** > > > + * struct zynqmp_afi_fpga - AFI register description > > > + * @value: value to be written to the register > > > + * @regid: Register id for the register to be written > > > + */ > > > +struct zynqmp_afi_fpga { > > > + u32 value; > > > + u32 regid; > > > +}; > > > + > > > +static int zynqmp_afi_fpga_probe(struct platform_device *pdev) > > > +{ > > > + struct zynqmp_afi_fpga *zynqmp_afi_fpga; > > > + struct device_node *np = pdev->dev.of_node; > > > + int i, entries, ret; > > > + u32 reg, val; > > > + > > > + zynqmp_afi_fpga = devm_kzalloc(&pdev->dev, > > > + sizeof(*zynqmp_afi_fpga), GFP_KERNEL); > > > + if (!zynqmp_afi_fpga) > > > + return -ENOMEM; > > > + platform_set_drvdata(pdev, zynqmp_afi_fpga); > > > + > > > + entries = of_property_count_u32_elems(np, "config-afi"); > > > + if (!entries || (entries % 2)) { > > > + dev_err(&pdev->dev, "Invalid number of registers\n"); > > > + return -EINVAL; > > > + } > > > + > > > + for (i = 0; i < entries / 2; i++) { > > > + ret = of_property_read_u32_index(np, "config-afi", i * 2, > > ®); > > > + if (ret) { > > > + dev_err(&pdev->dev, "failed to read register\n"); > > > + return -EINVAL; > > > + } > > > + ret = of_property_read_u32_index(np, "config-afi", i * 2 + 1, > > > + &val); > > > + if (ret) { > > > + dev_err(&pdev->dev, "failed to read value\n"); > > > + return -EINVAL; > > > + } > > > + ret = zynqmp_pm_afi(reg, val); > > > + if (ret < 0) { > > > + dev_err(&pdev->dev, "AFI register write error %d\n", > > > + ret); > > > + return ret; > > > + } > > > + } > > > + return 0; > > > +} > > > > Again, why does this have to be in the kernel? All it does is make a > > single call to the hardware based on some values read from the device > > tree. Can't you do this from userspace? > > > For every PL design has its own PS-PL configuration. What is a "PL design", and what is a "PS-PL configuration"? :) > This driver will be used by the overlay framework for configuring the interface after programming the FPGA and before probing the drivers that are present in the PL. Again, I have no idea what this means at all. And again, why does this have to be done in the kernel? All you are doing is sending some random values read in DT down to a hardware device. What requires a kernel driver for this? thanks, greg k-h
On 4/20/21 1:11 AM, Nava kishore Manne wrote: > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 877b43b3377d..d1ea1eeb3ac1 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -456,6 +456,17 @@ config ZYNQ_AFI > between PS and PL, the AXI port data path should be configured > with the proper Bus-width values > > +config ZYNQMP_AFI > + tristate "Xilinx ZYNQMP AFI support" > + help > + ZynqMP AFI driver support for writing to the AFI registers for > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > + PS to the programmable logic (PL) through the AXI port. This AXI > + port helps to establish the data path between the PS and PL. > + In-order to establish the proper communication path between > + PS and PL, the AXI port data path should be configured with > + the proper Bus-width values > + > source "drivers/misc/c2port/Kconfig" > source "drivers/misc/eeprom/Kconfig" > source "drivers/misc/cb710/Kconfig" Hi, from my notabot: Please follow coding-style for Kconfig files: from Documentation/process/coding-style.rst, section 10): For all of the Kconfig* configuration files throughout the source tree, the indentation is somewhat different. Lines under a ``config`` definition are indented with one tab, while help text is indented an additional two spaces. thanks.
Hi Greg, Please find my response inline. > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Tuesday, April 20, 2021 8:58 PM > To: Nava kishore Manne <navam@xilinx.com> > Cc: robh+dt@kernel.org; Michal Simek <michals@xilinx.com>; Derek Kiernan > <dkiernan@xilinx.com>; Dragan Cvetic <draganc@xilinx.com>; > arnd@arndb.de; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah > <JOLLYS@xilinx.com>; Tejas Patel <tejasp@xlnx.xilinx.com>; Amit Sunil > Dhamne <amitsuni@xilinx.com>; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > chinnikishore369@gmail.com; git <git@xilinx.com> > Subject: Re: [PATCH 5/5] misc: zynqmp: Add afi config driver > > On Tue, Apr 20, 2021 at 01:47:17PM +0000, Nava kishore Manne wrote: > > Hi Greg, > > > > Please find my response inline. > > > > > -----Original Message----- > > > From: Greg KH <gregkh@linuxfoundation.org> > > > Sent: Tuesday, April 20, 2021 2:21 PM > > > To: Nava kishore Manne <navam@xilinx.com> > > > Cc: robh+dt@kernel.org; Michal Simek <michals@xilinx.com>; Derek > > > Kiernan <dkiernan@xilinx.com>; Dragan Cvetic <draganc@xilinx.com>; > > > arnd@arndb.de; Rajan Vaja <RAJANV@xilinx.com>; Jolly Shah > > > <JOLLYS@xilinx.com>; Tejas Patel <tejasp@xlnx.xilinx.com>; Amit > > > Sunil Dhamne <amitsuni@xilinx.com>; devicetree@vger.kernel.org; > > > linux-arm- kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > > > chinnikishore369@gmail.com; git <git@xilinx.com> > > > Subject: Re: [PATCH 5/5] misc: zynqmp: Add afi config driver > > > > > > On Tue, Apr 20, 2021 at 01:41:53PM +0530, Nava kishore Manne wrote: > > > > This patch adds zynqmp afi config driver.This is useful for the > > > > configuration of the PS-PL interface on Zynq US+ MPSoC platform. > > > > > > > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > > > > --- > > > > drivers/misc/Kconfig | 11 ++++++ > > > > drivers/misc/Makefile | 1 + > > > > drivers/misc/zynqmp-afi.c | 83 > > > > +++++++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 95 insertions(+) create mode 100644 > > > > drivers/misc/zynqmp-afi.c > > > > > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index > > > > 877b43b3377d..d1ea1eeb3ac1 100644 > > > > --- a/drivers/misc/Kconfig > > > > +++ b/drivers/misc/Kconfig > > > > @@ -456,6 +456,17 @@ config ZYNQ_AFI > > > > between PS and PL, the AXI port data path should be configured > > > > with the proper Bus-width values > > > > > > > > +config ZYNQMP_AFI > > > > + tristate "Xilinx ZYNQMP AFI support" > > > > + help > > > > + ZynqMP AFI driver support for writing to the AFI registers for > > > > + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the > > > > + PS to the programmable logic (PL) through the AXI port. This AXI > > > > + port helps to establish the data path between the PS and PL. > > > > + In-order to establish the proper communication path between > > > > + PS and PL, the AXI port data path should be configured with > > > > + the proper Bus-width values > > > > + > > > > source "drivers/misc/c2port/Kconfig" > > > > source "drivers/misc/eeprom/Kconfig" > > > > source "drivers/misc/cb710/Kconfig" > > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index > > > > e9b03843100f..54bd0edc511e 100644 > > > > --- a/drivers/misc/Makefile > > > > +++ b/drivers/misc/Makefile > > > > @@ -57,3 +57,4 @@ obj-$(CONFIG_UACCE) += uacce/ > > > > obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o > > > > obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o > > > > obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o > > > > +obj-$(CONFIG_ZYNQMP_AFI) += zynqmp-afi.o > > > > diff --git a/drivers/misc/zynqmp-afi.c b/drivers/misc/zynqmp-afi.c > > > > new file mode 100644 index 000000000000..a318652576d2 > > > > --- /dev/null > > > > +++ b/drivers/misc/zynqmp-afi.c > > > > @@ -0,0 +1,83 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * Xilinx FPGA AFI bridge. > > > > + * Copyright (c) 2018-2021 Xilinx Inc. > > > > + */ > > > > + > > > > +#include <linux/err.h> > > > > +#include <linux/firmware/xlnx-zynqmp.h> #include <linux/io.h> > > > > +#include <linux/module.h> #include <linux/of.h> #include > > > > +<linux/platform_device.h> #include <linux/slab.h> > > > > + > > > > +/** > > > > + * struct zynqmp_afi_fpga - AFI register description > > > > + * @value: value to be written to the register > > > > + * @regid: Register id for the register to be written */ struct > > > > +zynqmp_afi_fpga { > > > > + u32 value; > > > > + u32 regid; > > > > +}; > > > > + > > > > +static int zynqmp_afi_fpga_probe(struct platform_device *pdev) { > > > > + struct zynqmp_afi_fpga *zynqmp_afi_fpga; > > > > + struct device_node *np = pdev->dev.of_node; > > > > + int i, entries, ret; > > > > + u32 reg, val; > > > > + > > > > + zynqmp_afi_fpga = devm_kzalloc(&pdev->dev, > > > > + sizeof(*zynqmp_afi_fpga), GFP_KERNEL); > > > > + if (!zynqmp_afi_fpga) > > > > + return -ENOMEM; > > > > + platform_set_drvdata(pdev, zynqmp_afi_fpga); > > > > + > > > > + entries = of_property_count_u32_elems(np, "config-afi"); > > > > + if (!entries || (entries % 2)) { > > > > + dev_err(&pdev->dev, "Invalid number of registers\n"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + for (i = 0; i < entries / 2; i++) { > > > > + ret = of_property_read_u32_index(np, "config-afi", i * 2, > > > ®); > > > > + if (ret) { > > > > + dev_err(&pdev->dev, "failed to read register\n"); > > > > + return -EINVAL; > > > > + } > > > > + ret = of_property_read_u32_index(np, "config-afi", i * 2 + 1, > > > > + &val); > > > > + if (ret) { > > > > + dev_err(&pdev->dev, "failed to read value\n"); > > > > + return -EINVAL; > > > > + } > > > > + ret = zynqmp_pm_afi(reg, val); > > > > + if (ret < 0) { > > > > + dev_err(&pdev->dev, "AFI register write error > %d\n", > > > > + ret); > > > > + return ret; > > > > + } > > > > + } > > > > + return 0; > > > > +} > > > > > > Again, why does this have to be in the kernel? All it does is make > > > a single call to the hardware based on some values read from the > > > device tree. Can't you do this from userspace? > > > > > For every PL design has its own PS-PL configuration. > > What is a "PL design", and what is a "PS-PL configuration"? :) > > > This driver will be used by the overlay framework for configuring the > interface after programming the FPGA and before probing the drivers that > are present in the PL. > > Again, I have no idea what this means at all. > > And again, why does this have to be done in the kernel? All you are doing is > sending some random values read in DT down to a hardware > device. What requires a kernel driver for this? > The ZynqMP based processing system (PS) that contains ARM cores and Xilinx programmable logic (PL/FPGA) in a single device. The PS and PL can be tightly or loosely coupled using multiple interfaces and other signals. This enables the designer to effectively integrate user-created hardware accelerators and other functions in the PL logic that are accessible to the processors and can also access memory resources in the PS. https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf (Page No: 1085) To Program/Re-Program the PL at runtime in Linux we have a an FPGA Manger Framework and this frame work uses DT-Overlays to programming the FPGA and probing the relevant PL drivers. For more info please refer this link: https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/fpga/fpga-region.txt Every Zynq/ZynqMP PL(FPGA) design has its own PS-PL configuration. So after programming the FPGA and before probing the relevant PL drivers these PS-PL configurations should be set. Example DT-Overlay file to program the PL(FPGA) from Linux: /dts-v1/; /plugin/; / { fragment@0 { /* fragment 0 contains Bitstream info */ target = <&fpga_full>; overlay0: __overlay__ { #address-cells = <2>; #size-cells = <2>; firmware-name = "Base_Zynq_MPSoC_wrapper.bit.bin"; resets = <&zynqmp_reset 116>; }; }; fragment@1 { /* fragment 1 contains PS-PL configurations */ target = <&amba>; overlay1: __overlay__ { afi0: afi0 { compatible = "xlnx,afi-fpga"; config-afi = < 0 0>, <1 0>, <2 0>, <3 0>, <4 0>, <5 0>, <6 0>, <7 0>, <8 0>, <9 0>, <10 0>, <11 0>, <12 0>, <13 0>, <14 0xa00>, <15 0x000>; }; }; }; fragment@2 { /* Fragment 2 contains the relevant drivers for the IP's present in the FPGA design*/ target = <&amba>; overlay2: __overlay__ { axi_gpio_0: gpio@a0000000 { #gpio-cells = <2>; clock-names = "s_axi_aclk"; clocks = <&zynqmp_clk 71>; compatible = "xlnx,xps-gpio-1.00.a"; gpio-controller ; reg = <0x0 0xa0000000 0x0 0x1000>; }; }; } ; }; In-order to support the PL(FPGA) programming and to configure the interface between PS and PL using FPGA Manager. This Driver is needed in the kernel space. @Moritz Fischer: Can you please let us know your thoughts on this. Regards, Navakishore.
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 877b43b3377d..d1ea1eeb3ac1 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -456,6 +456,17 @@ config ZYNQ_AFI between PS and PL, the AXI port data path should be configured with the proper Bus-width values +config ZYNQMP_AFI + tristate "Xilinx ZYNQMP AFI support" + help + ZynqMP AFI driver support for writing to the AFI registers for + configuring PS_PL Bus-width. Xilinx Zynq US+ MPSoC connect the + PS to the programmable logic (PL) through the AXI port. This AXI + port helps to establish the data path between the PS and PL. + In-order to establish the proper communication path between + PS and PL, the AXI port data path should be configured with + the proper Bus-width values + source "drivers/misc/c2port/Kconfig" source "drivers/misc/eeprom/Kconfig" source "drivers/misc/cb710/Kconfig" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index e9b03843100f..54bd0edc511e 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -57,3 +57,4 @@ obj-$(CONFIG_UACCE) += uacce/ obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o obj-$(CONFIG_ZYNQ_AFI) += zynq-afi.o +obj-$(CONFIG_ZYNQMP_AFI) += zynqmp-afi.o diff --git a/drivers/misc/zynqmp-afi.c b/drivers/misc/zynqmp-afi.c new file mode 100644 index 000000000000..a318652576d2 --- /dev/null +++ b/drivers/misc/zynqmp-afi.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx FPGA AFI bridge. + * Copyright (c) 2018-2021 Xilinx Inc. + */ + +#include <linux/err.h> +#include <linux/firmware/xlnx-zynqmp.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +/** + * struct zynqmp_afi_fpga - AFI register description + * @value: value to be written to the register + * @regid: Register id for the register to be written + */ +struct zynqmp_afi_fpga { + u32 value; + u32 regid; +}; + +static int zynqmp_afi_fpga_probe(struct platform_device *pdev) +{ + struct zynqmp_afi_fpga *zynqmp_afi_fpga; + struct device_node *np = pdev->dev.of_node; + int i, entries, ret; + u32 reg, val; + + zynqmp_afi_fpga = devm_kzalloc(&pdev->dev, + sizeof(*zynqmp_afi_fpga), GFP_KERNEL); + if (!zynqmp_afi_fpga) + return -ENOMEM; + platform_set_drvdata(pdev, zynqmp_afi_fpga); + + entries = of_property_count_u32_elems(np, "config-afi"); + if (!entries || (entries % 2)) { + dev_err(&pdev->dev, "Invalid number of registers\n"); + return -EINVAL; + } + + for (i = 0; i < entries / 2; i++) { + ret = of_property_read_u32_index(np, "config-afi", i * 2, ®); + if (ret) { + dev_err(&pdev->dev, "failed to read register\n"); + return -EINVAL; + } + ret = of_property_read_u32_index(np, "config-afi", i * 2 + 1, + &val); + if (ret) { + dev_err(&pdev->dev, "failed to read value\n"); + return -EINVAL; + } + ret = zynqmp_pm_afi(reg, val); + if (ret < 0) { + dev_err(&pdev->dev, "AFI register write error %d\n", + ret); + return ret; + } + } + return 0; +} + +static const struct of_device_id zynqmp_afi_fpga_ids[] = { + { .compatible = "xlnx,zynqmp-afi-fpga" }, + { }, +}; +MODULE_DEVICE_TABLE(of, zynqmp_afi_fpga_ids); + +static struct platform_driver zynqmp_afi_fpga_driver = { + .driver = { + .name = "zynqmp-afi-fpga", + .of_match_table = zynqmp_afi_fpga_ids, + }, + .probe = zynqmp_afi_fpga_probe, +}; +module_platform_driver(zynqmp_afi_fpga_driver); + +MODULE_DESCRIPTION("ZYNQMP FPGA afi module"); +MODULE_AUTHOR("Nava kishore Manne <nava.manne@xilinx.com>"); +MODULE_LICENSE("GPL v2");
This patch adds zynqmp afi config driver.This is useful for the configuration of the PS-PL interface on Zynq US+ MPSoC platform. Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> --- drivers/misc/Kconfig | 11 ++++++ drivers/misc/Makefile | 1 + drivers/misc/zynqmp-afi.c | 83 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 drivers/misc/zynqmp-afi.c