| Message ID | 20210420081153.17020-6-nava.manne@xilinx.com |
|---|---|
| State | New |
| Headers | show |
| Series | misc: Add afi config drivers support. | expand |
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