Hi Greg,
Please find my response inline.
> -----Original Message-----
> From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, April 20, 2021 2:17 PM
> To: Nava kishore Manne <navam@xxxxxxxxxx>
> Cc: robh+dt@xxxxxxxxxx; Michal Simek <michals@xxxxxxxxxx>; Derek Kiernan
> <dkiernan@xxxxxxxxxx>; Dragan Cvetic <draganc@xxxxxxxxxx>;
> arnd@xxxxxxxx; Rajan Vaja <RAJANV@xxxxxxxxxx>; Jolly Shah
> <JOLLYS@xxxxxxxxxx>; Tejas Patel <tejasp@xxxxxxxxxxxxxxx>; Amit Sunil
> Dhamne <amitsuni@xxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> chinnikishore369@xxxxxxxxx; git <git@xxxxxxxxxx>
> Subject: Re: [PATCH 2/5] misc: zynq: Add afi config driver
>
> On Tue, Apr 20, 2021 at 01:41:50PM +0530, Nava kishore Manne wrote:
> > This patch adds zynq afi config driver. This is useful for the
> > configuration of the PS-PL interface on zynq platform.
>
> What is "PS-PL"? Can you describe it better please?
>
PS-PL interface is nothing but the interface between processing system(PS) that contains arm cores and Programmable Logic(PL) i.e FPGA.
Will update the description in v2.
> >
> > Signed-off-by: Nava kishore Manne <nava.manne@xxxxxxxxxx>
> > ---
> > drivers/misc/Kconfig | 11 ++++++
> > drivers/misc/Makefile | 1 +
> > drivers/misc/zynq-afi.c | 81
> > +++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 93 insertions(+)
> > create mode 100644 drivers/misc/zynq-afi.c
> >
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index
> > f532c59bb59b..877b43b3377d 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -445,6 +445,17 @@ config HISI_HIKEY_USB
> > switching between the dual-role USB-C port and the USB-A host
> ports
> > using only one USB controller.
> >
> > +config ZYNQ_AFI
> > + tristate "Xilinx ZYNQ AFI support"
> > + help
> > + Zynq AFI driver support for writing to the AFI registers
> > + for configuring PS_PL Bus-width. Xilinx Zynq SoC 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
> > 99b6f15a3c70..e9b03843100f 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -56,3 +56,4 @@ obj-$(CONFIG_HABANA_AI) +=
> habanalabs/
> > 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
> > diff --git a/drivers/misc/zynq-afi.c b/drivers/misc/zynq-afi.c new
> > file mode 100644 index 000000000000..04317d1bdb98
> > --- /dev/null
> > +++ b/drivers/misc/zynq-afi.c
> > @@ -0,0 +1,81 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Xilinx ZYNQ AFI driver.
> > + * Copyright (c) 2018-2021 Xilinx Inc.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +
> > +/* Registers and special values for doing register-based operations */
> > +#define AFI_RDCHAN_CTRL_OFFSET 0x00
> > +#define AFI_WRCHAN_CTRL_OFFSET 0x14
> > +
> > +#define AFI_BUSWIDTH_MASK 0x01
> > +
> > +/**
> > + * struct afi_fpga - AFI register description
> > + * @membase: pointer to register struct
> > + * @afi_width: AFI bus width to be written
> > + */
> > +struct zynq_afi_fpga {
> > + void __iomem *membase;
> > + u32 afi_width;
> > +};
> > +
> > +static int zynq_afi_fpga_probe(struct platform_device *pdev) {
> > + struct zynq_afi_fpga *afi_fpga;
> > + struct resource *res;
> > + u32 reg_val;
> > + u32 val;
> > +
> > + afi_fpga = devm_kzalloc(&pdev->dev, sizeof(*afi_fpga),
> GFP_KERNEL);
> > + if (!afi_fpga)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + afi_fpga->membase = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(afi_fpga->membase))
> > + return PTR_ERR(afi_fpga->membase);
> > +
> > + val = device_property_read_u32(&pdev->dev, "xlnx,afi-width",
> > + &afi_fpga->afi_width);
> > + if (val) {
> > + dev_err(&pdev->dev, "failed to get the afi bus width\n");
> > + return -EINVAL;
> > + }
> > +
> > + reg_val = readl(afi_fpga->membase + AFI_RDCHAN_CTRL_OFFSET);
> > + reg_val &= ~AFI_BUSWIDTH_MASK;
> > + writel(reg_val | afi_fpga->afi_width,
> > + afi_fpga->membase + AFI_RDCHAN_CTRL_OFFSET);
> > + reg_val = readl(afi_fpga->membase + AFI_WRCHAN_CTRL_OFFSET);
> > + reg_val &= ~AFI_BUSWIDTH_MASK;
> > + writel(reg_val | afi_fpga->afi_width,
> > + afi_fpga->membase + AFI_WRCHAN_CTRL_OFFSET);
> > +
> > + return 0;
> > +}
>
> I do not understand, why is this driver needed at all? Why can't you do the
> above from userspace?
>
> All this does is write some values to the hardware at probe time, who needs
> this?
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.
