| Message ID | 20210604113332.1394-5-nava.manne@xilinx.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add Bitstream configuration support for Versal | expand |
On Fri, Jun 04, 2021 at 05:03:32PM +0530, Nava kishore Manne wrote: > Add support for Xilinx Versal FPGA manager. > > PDI source type can be DDR, OCM, QSPI flash etc.. > But driver allocates memory always from DDR, Since driver supports only > DDR source type. > > Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > Reviewed-by: Moritz Fischer <mdf@kernel.org> > --- > Changes for v2: > -Updated the Fpga Mgr registrations call's > to 5.11 > -Fixed some minor coding issues as suggested by > Moritz. > > Changes for v3: > -Rewritten the Versal fpga Kconfig contents. > > Changes for v4: > -Rebased the changes on linux-next. > No functional changes. > > Changes for v5: > -None. > > Changes for v6: > -None. > > Changes for v7: > -Updated driver to remove unwated priv struct dependency. > > drivers/fpga/Kconfig | 9 ++++ > drivers/fpga/Makefile | 1 + > drivers/fpga/versal-fpga.c | 96 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 106 insertions(+) > create mode 100644 drivers/fpga/versal-fpga.c > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index 33e15058d0dc..92c20b92357a 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -234,4 +234,13 @@ config FPGA_MGR_ZYNQMP_FPGA > to configure the programmable logic(PL) through PS > on ZynqMP SoC. > > +config FPGA_MGR_VERSAL_FPGA > + tristate "Xilinx Versal FPGA" > + depends on ARCH_ZYNQMP || COMPILE_TEST > + help > + Select this option to enable FPGA manager driver support for > + Xilinx Versal SoC. This driver uses the firmware interface to > + configure the programmable logic(PL). > + > + To compile this as a module, choose M here. > endif # FPGA > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index 18dc9885883a..0bff783d1b61 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -18,6 +18,7 @@ obj-$(CONFIG_FPGA_MGR_TS73XX) += ts73xx-fpga.o > obj-$(CONFIG_FPGA_MGR_XILINX_SPI) += xilinx-spi.o > obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o > obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o > +obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA) += versal-fpga.o > obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o > obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT) += altera-pr-ip-core-plat.o > > diff --git a/drivers/fpga/versal-fpga.c b/drivers/fpga/versal-fpga.c > new file mode 100644 > index 000000000000..1bd312a31b23 > --- /dev/null > +++ b/drivers/fpga/versal-fpga.c > @@ -0,0 +1,96 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019-2021 Xilinx, Inc. > + */ > + > +#include <linux/dma-mapping.h> > +#include <linux/fpga/fpga-mgr.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of_address.h> > +#include <linux/string.h> > +#include <linux/firmware/xlnx-zynqmp.h> > + > +static int versal_fpga_ops_write_init(struct fpga_manager *mgr, > + struct fpga_image_info *info, > + const char *buf, size_t size) > +{ > + return 0; Why have this if it does nothing? > +} > + > +static int versal_fpga_ops_write(struct fpga_manager *mgr, > + const char *buf, size_t size) > +{ > + dma_addr_t dma_addr = 0; > + char *kbuf; > + int ret; > + > + kbuf = dma_alloc_coherent(mgr->dev.parent, size, &dma_addr, GFP_KERNEL); > + if (!kbuf) > + return -ENOMEM; > + > + memcpy(kbuf, buf, size); > + ret = zynqmp_pm_load_pdi(PDI_SRC_DDR, dma_addr); > + dma_free_coherent(mgr->dev.parent, size, kbuf, dma_addr); > + > + return ret; > +} > + > +static int versal_fpga_ops_write_complete(struct fpga_manager *mgr, > + struct fpga_image_info *info) > +{ > + return 0; Same here, why have this at all? > +} > + > +static enum fpga_mgr_states versal_fpga_ops_state(struct fpga_manager *mgr) > +{ > + return FPGA_MGR_STATE_UNKNOWN; Shouln't that be the default state of the fpga manager if there is no state function callback? This driver should just need a write and probe function, and that's it, why make it more complex? thanks, greg k-h
On 6/4/21 4:48 AM, Greg KH wrote: > On Fri, Jun 04, 2021 at 05:03:32PM +0530, Nava kishore Manne wrote: >> Add support for Xilinx Versal FPGA manager. >> >> PDI source type can be DDR, OCM, QSPI flash etc.. >> But driver allocates memory always from DDR, Since driver supports only >> DDR source type. >> >> Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com> >> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> >> Reviewed-by: Moritz Fischer <mdf@kernel.org> >> --- >> Changes for v2: >> -Updated the Fpga Mgr registrations call's >> to 5.11 >> -Fixed some minor coding issues as suggested by >> Moritz. >> >> Changes for v3: >> -Rewritten the Versal fpga Kconfig contents. >> >> Changes for v4: >> -Rebased the changes on linux-next. >> No functional changes. >> >> Changes for v5: >> -None. >> >> Changes for v6: >> -None. >> >> Changes for v7: >> -Updated driver to remove unwated priv struct dependency. >> >> drivers/fpga/Kconfig | 9 ++++ >> drivers/fpga/Makefile | 1 + >> drivers/fpga/versal-fpga.c | 96 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 106 insertions(+) >> create mode 100644 drivers/fpga/versal-fpga.c >> >> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig >> index 33e15058d0dc..92c20b92357a 100644 >> --- a/drivers/fpga/Kconfig >> +++ b/drivers/fpga/Kconfig >> @@ -234,4 +234,13 @@ config FPGA_MGR_ZYNQMP_FPGA >> to configure the programmable logic(PL) through PS >> on ZynqMP SoC. >> >> +config FPGA_MGR_VERSAL_FPGA >> + tristate "Xilinx Versal FPGA" >> + depends on ARCH_ZYNQMP || COMPILE_TEST >> + help >> + Select this option to enable FPGA manager driver support for >> + Xilinx Versal SoC. This driver uses the firmware interface to >> + configure the programmable logic(PL). >> + >> + To compile this as a module, choose M here. >> endif # FPGA >> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile >> index 18dc9885883a..0bff783d1b61 100644 >> --- a/drivers/fpga/Makefile >> +++ b/drivers/fpga/Makefile >> @@ -18,6 +18,7 @@ obj-$(CONFIG_FPGA_MGR_TS73XX) += ts73xx-fpga.o >> obj-$(CONFIG_FPGA_MGR_XILINX_SPI) += xilinx-spi.o >> obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o >> obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o >> +obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA) += versal-fpga.o >> obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o >> obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT) += altera-pr-ip-core-plat.o >> >> diff --git a/drivers/fpga/versal-fpga.c b/drivers/fpga/versal-fpga.c >> new file mode 100644 >> index 000000000000..1bd312a31b23 >> --- /dev/null >> +++ b/drivers/fpga/versal-fpga.c >> @@ -0,0 +1,96 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2019-2021 Xilinx, Inc. >> + */ >> + >> +#include <linux/dma-mapping.h> >> +#include <linux/fpga/fpga-mgr.h> >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of_address.h> >> +#include <linux/string.h> >> +#include <linux/firmware/xlnx-zynqmp.h> >> + >> +static int versal_fpga_ops_write_init(struct fpga_manager *mgr, >> + struct fpga_image_info *info, >> + const char *buf, size_t size) >> +{ >> + return 0; > Why have this if it does nothing? > >> +} >> + >> +static int versal_fpga_ops_write(struct fpga_manager *mgr, >> + const char *buf, size_t size) >> +{ >> + dma_addr_t dma_addr = 0; >> + char *kbuf; >> + int ret; >> + >> + kbuf = dma_alloc_coherent(mgr->dev.parent, size, &dma_addr, GFP_KERNEL); >> + if (!kbuf) >> + return -ENOMEM; >> + >> + memcpy(kbuf, buf, size); >> + ret = zynqmp_pm_load_pdi(PDI_SRC_DDR, dma_addr); >> + dma_free_coherent(mgr->dev.parent, size, kbuf, dma_addr); >> + >> + return ret; >> +} >> + >> +static int versal_fpga_ops_write_complete(struct fpga_manager *mgr, >> + struct fpga_image_info *info) >> +{ >> + return 0; > Same here, why have this at all? > >> +} >> + >> +static enum fpga_mgr_states versal_fpga_ops_state(struct fpga_manager *mgr) >> +{ >> + return FPGA_MGR_STATE_UNKNOWN; > Shouln't that be the default state of the fpga manager if there is no > state function callback? > > This driver should just need a write and probe function, and that's it, > why make it more complex? These empty functions are needed by each board because of an aggressive check of ops early in the bringup of the fpga mgr. I did some deck chair shuffling here recently https://lore.kernel.org/linux-fpga/20210524162820.2221474-1-trix@redhat.com/ that shows location of check. A finer grained handling of the ops would be better. I'll add that to the next spin. Tom > thanks, > > greg k-h >
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 33e15058d0dc..92c20b92357a 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -234,4 +234,13 @@ config FPGA_MGR_ZYNQMP_FPGA to configure the programmable logic(PL) through PS on ZynqMP SoC. +config FPGA_MGR_VERSAL_FPGA + tristate "Xilinx Versal FPGA" + depends on ARCH_ZYNQMP || COMPILE_TEST + help + Select this option to enable FPGA manager driver support for + Xilinx Versal SoC. This driver uses the firmware interface to + configure the programmable logic(PL). + + To compile this as a module, choose M here. endif # FPGA diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile index 18dc9885883a..0bff783d1b61 100644 --- a/drivers/fpga/Makefile +++ b/drivers/fpga/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_FPGA_MGR_TS73XX) += ts73xx-fpga.o obj-$(CONFIG_FPGA_MGR_XILINX_SPI) += xilinx-spi.o obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o +obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA) += versal-fpga.o obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT) += altera-pr-ip-core-plat.o diff --git a/drivers/fpga/versal-fpga.c b/drivers/fpga/versal-fpga.c new file mode 100644 index 000000000000..1bd312a31b23 --- /dev/null +++ b/drivers/fpga/versal-fpga.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019-2021 Xilinx, Inc. + */ + +#include <linux/dma-mapping.h> +#include <linux/fpga/fpga-mgr.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/string.h> +#include <linux/firmware/xlnx-zynqmp.h> + +static int versal_fpga_ops_write_init(struct fpga_manager *mgr, + struct fpga_image_info *info, + const char *buf, size_t size) +{ + return 0; +} + +static int versal_fpga_ops_write(struct fpga_manager *mgr, + const char *buf, size_t size) +{ + dma_addr_t dma_addr = 0; + char *kbuf; + int ret; + + kbuf = dma_alloc_coherent(mgr->dev.parent, size, &dma_addr, GFP_KERNEL); + if (!kbuf) + return -ENOMEM; + + memcpy(kbuf, buf, size); + ret = zynqmp_pm_load_pdi(PDI_SRC_DDR, dma_addr); + dma_free_coherent(mgr->dev.parent, size, kbuf, dma_addr); + + return ret; +} + +static int versal_fpga_ops_write_complete(struct fpga_manager *mgr, + struct fpga_image_info *info) +{ + return 0; +} + +static enum fpga_mgr_states versal_fpga_ops_state(struct fpga_manager *mgr) +{ + return FPGA_MGR_STATE_UNKNOWN; +} + +static const struct fpga_manager_ops versal_fpga_ops = { + .state = versal_fpga_ops_state, + .write_init = versal_fpga_ops_write_init, + .write = versal_fpga_ops_write, + .write_complete = versal_fpga_ops_write_complete, +}; + +static int versal_fpga_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct fpga_manager *mgr; + int ret; + + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); + if (ret < 0) { + dev_err(dev, "no usable DMA configuration\n"); + return ret; + } + + mgr = devm_fpga_mgr_create(dev, "Xilinx Versal FPGA Manager", + &versal_fpga_ops, NULL); + if (!mgr) + return -ENOMEM; + + return devm_fpga_mgr_register(dev, mgr); +} + +static const struct of_device_id versal_fpga_of_match[] = { + { .compatible = "xlnx,versal-fpga", }, + {}, +}; +MODULE_DEVICE_TABLE(of, versal_fpga_of_match); + +static struct platform_driver versal_fpga_driver = { + .probe = versal_fpga_probe, + .driver = { + .name = "versal_fpga_manager", + .of_match_table = of_match_ptr(versal_fpga_of_match), + }, +}; +module_platform_driver(versal_fpga_driver); + +MODULE_AUTHOR("Nava kishore Manne <nava.manne@xilinx.com>"); +MODULE_AUTHOR("Appana Durga Kedareswara rao <appanad.durga.rao@xilinx.com>"); +MODULE_DESCRIPTION("Xilinx Versal FPGA Manager"); +MODULE_LICENSE("GPL");