Re: [PATCH V4 XRT Alveo 20/20] fpga: xrt: Kconfig and Makefile updates for XRT drivers

[Lizhi Hou <lizhi.hou@xilinx.com>]

Hi Tom,


On 04/06/2021 02:00 PM, Tom Rix wrote:
>
> On 3/23/21 10:29 PM, Lizhi Hou wrote:
>> Update fpga Kconfig/Makefile and add Kconfig/Makefile for new drivers.
>>
>> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
>> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
>> ---
>>   MAINTAINERS                        | 11 +++++++++++
>>   drivers/Makefile                   |  1 +
>>   drivers/fpga/Kconfig               |  2 ++
>>   drivers/fpga/Makefile              |  5 +++++
>>   drivers/fpga/xrt/Kconfig           |  8 ++++++++
>>   drivers/fpga/xrt/lib/Kconfig       | 17 +++++++++++++++++
>>   drivers/fpga/xrt/lib/Makefile      | 30 ++++++++++++++++++++++++++++++
>>   drivers/fpga/xrt/metadata/Kconfig  | 12 ++++++++++++
>>   drivers/fpga/xrt/metadata/Makefile | 16 ++++++++++++++++
>>   drivers/fpga/xrt/mgmt/Kconfig      | 15 +++++++++++++++
>>   drivers/fpga/xrt/mgmt/Makefile     | 19 +++++++++++++++++++
>>   11 files changed, 136 insertions(+)
>>   create mode 100644 drivers/fpga/xrt/Kconfig
>>   create mode 100644 drivers/fpga/xrt/lib/Kconfig
>>   create mode 100644 drivers/fpga/xrt/lib/Makefile
>>   create mode 100644 drivers/fpga/xrt/metadata/Kconfig
>>   create mode 100644 drivers/fpga/xrt/metadata/Makefile
>>   create mode 100644 drivers/fpga/xrt/mgmt/Kconfig
>>   create mode 100644 drivers/fpga/xrt/mgmt/Makefile
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index aa84121c5611..44ccc52987ac 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7009,6 +7009,17 @@ F:     Documentation/fpga/
>>   F:  drivers/fpga/
>>   F:  include/linux/fpga/
>>
>> +FPGA XRT DRIVERS
>> +M:   Lizhi Hou <lizhi.hou@xilinx.com>
>> +R:   Max Zhen <max.zhen@xilinx.com>
>> +R:   Sonal Santan <sonal.santan@xilinx.com>
>> +L:   linux-fpga@vger.kernel.org
>> +S:   Maintained
> Should this be 'Supported' ?
Sure.
>> +W:   https://github.com/Xilinx/XRT
>> +F:   Documentation/fpga/xrt.rst
>> +F:   drivers/fpga/xrt/
>> +F:   include/uapi/linux/xrt/
>> +
>>   FPU EMULATOR
>>   M:  Bill Metzenthen <billm@melbpc.org.au>
>>   S:  Maintained
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index 6fba7daba591..dbb3b727fc7a 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -179,6 +179,7 @@ obj-$(CONFIG_STM)         += hwtracing/stm/
>>   obj-$(CONFIG_ANDROID)               += android/
>>   obj-$(CONFIG_NVMEM)         += nvmem/
>>   obj-$(CONFIG_FPGA)          += fpga/
>> +obj-$(CONFIG_FPGA_XRT_METADATA) += fpga/
> CONFIG_FPGA_XRT_METADATA is only defined when CONFIG_FPGA is, so i don't
> think this line is needed.
CONFIG_FPGA could be 'm'.
And as we discussed before, CONFIG_FPGA_XRT_METADATA extending fdt_* can 
be only build in kernel ('y'). Maybe it can not rely on CONFIG_FPGA?
>>   obj-$(CONFIG_FSI)           += fsi/
>>   obj-$(CONFIG_TEE)           += tee/
>>   obj-$(CONFIG_MULTIPLEXER)   += mux/
>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>> index 5ff9438b7b46..01410ff000b9 100644
>> --- a/drivers/fpga/Kconfig
>> +++ b/drivers/fpga/Kconfig
>> @@ -227,4 +227,6 @@ config FPGA_MGR_ZYNQMP_FPGA
>>         to configure the programmable logic(PL) through PS
>>         on ZynqMP SoC.
>>
>> +source "drivers/fpga/xrt/Kconfig"
>> +
>>   endif # FPGA
> This is where it is defined..
>> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
>> index 18dc9885883a..4b887bf95cb3 100644
>> --- a/drivers/fpga/Makefile
>> +++ b/drivers/fpga/Makefile
>> @@ -48,3 +48,8 @@ obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000) += 
>> dfl-n3000-nios.o
>>
>>   # Drivers for FPGAs which implement DFL
>>   obj-$(CONFIG_FPGA_DFL_PCI)          += dfl-pci.o
>> +
>> +# XRT drivers for Alveo
>> +obj-$(CONFIG_FPGA_XRT_METADATA)              += xrt/metadata/
>> +obj-$(CONFIG_FPGA_XRT_LIB)           += xrt/lib/
>> +obj-$(CONFIG_FPGA_XRT_XMGMT)         += xrt/mgmt/
>> diff --git a/drivers/fpga/xrt/Kconfig b/drivers/fpga/xrt/Kconfig
>> new file mode 100644
>> index 000000000000..0e2c59589ddd
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/Kconfig
>> @@ -0,0 +1,8 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Xilinx Alveo FPGA device configuration
>> +#
>> +
>> +source "drivers/fpga/xrt/metadata/Kconfig"
>> +source "drivers/fpga/xrt/lib/Kconfig"
>> +source "drivers/fpga/xrt/mgmt/Kconfig"
>> diff --git a/drivers/fpga/xrt/lib/Kconfig b/drivers/fpga/xrt/lib/Kconfig
>> new file mode 100644
>> index 000000000000..935369fad570
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/lib/Kconfig
>> @@ -0,0 +1,17 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# XRT Alveo FPGA device configuration
>> +#
>> +
>> +config FPGA_XRT_LIB
>> +     tristate "XRT Alveo Driver Library"
>> +     depends on HWMON && PCI && HAS_IOMEM
>> +     select FPGA_XRT_METADATA
>> +     select REGMAP_MMIO
>> +     help
>> +       Select this option to enable Xilinx XRT Alveo driver library. 
>> This
>> +       library is core infrastructure of XRT Alveo FPGA drivers which
>> +       provides functions for working with device nodes, iteration and
>> +       lookup of platform devices, common interfaces for platform 
>> devices,
>> +       plumbing of function call and ioctls between platform devices 
>> and
>> +       parent partitions.
>> diff --git a/drivers/fpga/xrt/lib/Makefile 
>> b/drivers/fpga/xrt/lib/Makefile
>> new file mode 100644
>> index 000000000000..58563416efbf
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/lib/Makefile
>> @@ -0,0 +1,30 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Copyright (C) 2020-2021 Xilinx, Inc. All rights reserved.
>> +#
>> +# Authors: Sonal.Santan@xilinx.com
>> +#
>> +
>> +FULL_XRT_PATH=$(srctree)/$(src)/..
>> +FULL_DTC_PATH=$(srctree)/scripts/dtc/libfdt
>> +
>> +obj-$(CONFIG_FPGA_XRT_LIB) += xrt-lib.o
>> +
>> +xrt-lib-objs :=                      \
>> +     lib-drv.o               \
>> +     xroot.o                 \
>> +     xclbin.o                \
>> +     subdev.o                \
>> +     cdev.o                  \
>> +     group.o                 \
>> +     xleaf/vsec.o            \
>> +     xleaf/axigate.o         \
>> +     xleaf/devctl.o          \
>> +     xleaf/icap.o            \
>> +     xleaf/clock.o           \
>> +     xleaf/clkfreq.o         \
>> +     xleaf/ucs.o             \
>> +     xleaf/ddr_calibration.o
>> +
>> +ccflags-y := -I$(FULL_XRT_PATH)/include       \
>> +     -I$(FULL_DTC_PATH)
>> diff --git a/drivers/fpga/xrt/metadata/Kconfig 
>> b/drivers/fpga/xrt/metadata/Kconfig
>> new file mode 100644
>> index 000000000000..129adda47e94
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/metadata/Kconfig
>> @@ -0,0 +1,12 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# XRT Alveo FPGA device configuration
>> +#
>> +
>> +config FPGA_XRT_METADATA
>> +     bool "XRT Alveo Driver Metadata Parser"
>> +     select LIBFDT
>> +     help
>> +       This option provides helper functions to parse Xilinx Alveo FPGA
>> +       firmware metadata. The metadata is in device tree format and the
>> +       XRT driver uses it to discover the HW subsystems behind PCIe 
>> BAR.
>> diff --git a/drivers/fpga/xrt/metadata/Makefile 
>> b/drivers/fpga/xrt/metadata/Makefile
>> new file mode 100644
>> index 000000000000..14f65ef1595c
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/metadata/Makefile
>> @@ -0,0 +1,16 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Copyright (C) 2020-2021 Xilinx, Inc. All rights reserved.
>> +#
>> +# Authors: Sonal.Santan@xilinx.com
>> +#
>> +
>> +FULL_XRT_PATH=$(srctree)/$(src)/..
>> +FULL_DTC_PATH=$(srctree)/scripts/dtc/libfdt
>> +
>> +obj-$(CONFIG_FPGA_XRT_METADATA) += xrt-md.o
>> +
>> +xrt-md-objs := metadata.o
>> +
>> +ccflags-y := -I$(FULL_XRT_PATH)/include      \
>> +     -I$(FULL_DTC_PATH)
>> diff --git a/drivers/fpga/xrt/mgmt/Kconfig 
>> b/drivers/fpga/xrt/mgmt/Kconfig
>> new file mode 100644
>> index 000000000000..31e9e19fffb8
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/mgmt/Kconfig
>> @@ -0,0 +1,15 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Xilinx XRT FPGA device configuration
>> +#
>> +
>> +config FPGA_XRT_XMGMT
>> +     tristate "Xilinx Alveo Management Driver"
>> +     depends on FPGA_XRT_LIB
>> +     select FPGA_XRT_METADATA
>
> If the XRT driver depends on these other two configs and it does not
> make sense to build these two seperately, could you remove these configs
> and just use something like FPGA_XRT ?
This is similar reason with above. CONFIG_FPGA_XRT_METADATA can be only 
built in. And FPGA_XRT_LIB can be built as module.  They might not be 
built together.

Thanks,
Lizhi
>
> Tom
>
>> +     select FPGA_BRIDGE
>> +     select FPGA_REGION
>> +     help
>> +       Select this option to enable XRT PCIe driver for Xilinx Alveo 
>> FPGA.
>> +       This driver provides interfaces for userspace application to 
>> access
>> +       Alveo FPGA device.
>> diff --git a/drivers/fpga/xrt/mgmt/Makefile 
>> b/drivers/fpga/xrt/mgmt/Makefile
>> new file mode 100644
>> index 000000000000..acabd811f3fd
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/mgmt/Makefile
>> @@ -0,0 +1,19 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Copyright (C) 2020-2021 Xilinx, Inc. All rights reserved.
>> +#
>> +# Authors: Sonal.Santan@xilinx.com
>> +#
>> +
>> +FULL_XRT_PATH=$(srctree)/$(src)/..
>> +FULL_DTC_PATH=$(srctree)/scripts/dtc/libfdt
>> +
>> +obj-$(CONFIG_FPGA_XRT_XMGMT) += xrt-mgmt.o
>> +
>> +xrt-mgmt-objs := root.o              \
>> +        main.o               \
>> +        fmgr-drv.o           \
>> +        main-region.o
>> +
>> +ccflags-y := -I$(FULL_XRT_PATH)/include              \
>> +     -I$(FULL_DTC_PATH)
>