Received-SPF: pass (google.com: domain of linux-kernel+bounces-126088-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Sun, 31 Mar 2024 22:50:11 +0800
From: Xu Yilun <yilun.xu@linux.intel.com>
To: Charles Perry <charles.perry@savoirfairelinux.com>
Cc: mdf@kernel.org, avandiver@markem-imaje.com, bcody@markem-imaje.com,
	Wu Hao <hao.wu@intel.com>, Xu Yilun <yilun.xu@intel.com>,
	Tom Rix <trix@redhat.com>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Michal Simek <michal.simek@amd.com>, linux-fpga@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 0/4] fpga: xilinx-selectmap: add new driver
Message-ID: <Zgl4I6rykg7shI2K@yilunxu-OptiPlex-7050>
References: <20240321220447.3260065-1-charles.perry@savoirfairelinux.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240321220447.3260065-1-charles.perry@savoirfairelinux.com>

On Thu, Mar 21, 2024 at 06:04:32PM -0400, Charles Perry wrote:
> Hello,
> 
> This patchset adds a new driver for the 7 series FPGA's SelectMAP
> interface.
> 
> The SelectMAP interface shares a common GPIO protocol with the SPI
> interface which is already in the kernel (drivers/fpga/xilinx-spi.c).
> The approach proposed in this patchset is to refactor xilinx-spi.c into
> xilinx-core.c which would handle the common GPIO protocol. This is then
> used to build two drivers, the already existing xilinx-spi.c driver and
> a newly added xilinx-selectmap.c driver.
> 
> The SelectMAP driver proposed only supports 8 bit mode. This is because
> the 16 and 32 bits mode have limitations with regards to compressed
> bitstream support as well as introducing endianness considerations.
> 
> I'm testing xilinx-selectmap.c on a custom i.MX6 board connected to an
> Artix 7 FPGA. Flashing a 913K bitstream takes 0.44 seconds.
> 
> Changes since v5: (from Yilun review)
>  * xilinx-core.h: remove private fields kernel-doc
>  * xilinx-spi.c: rename conf into core in xilinx_spi_probe
>  * xilinx-core.c: introduce the new gpio names in patch 4/4
>  * xilinx-core.c: remove kernel-doc on xilinx_core_devm_gpiod_get()
>  * xilinx-selectmap.c:
>    * reorder includes in alphabetical order
>    * xilinx_selectmap_probe(): remove unused resource *r variable
>    * xilinx_selectmap_probe(): use a single gpio_desc* temporary
>    * xilinx_selectmap_probe(): declare variables in reverse xmas tree
> 
> Changes since v4: (from Yilun and Krzysztof review)
>  * xilinx-core: use sizeof() instead of hardcoded immediate
>  * xilinx-core: fix module compilation (EXPORT_SYMBOL_GPL, MODULE_LICENSE,
>    MODULE_AUTHOR, MODULE_DESCRIPTION)
>  * xilinx-core: add private/public qualifiers for struct xilinx_fpga_core
>  * xilinx-spi: remove struct xilinx_spi_conf. This struct isn't needed as
>    the struct spi_device* can be retrieved from the struct device*.
>  * dt-bindings: remove usage of "_b" and "-b" for the new driver. We
>    agreed that the spi and selectmap driver will use different bindings
>    which will be handled by the driver core and that the legacy names will
>    be used only for the spi compatible.
>  * xilinx-core: select between prog/init and prog_b/init-b
> 
> Changes since v3: (from Rob Herring review)
>  * Fix an error in the DT binding example compatible.
>  * Drop the renaming of "prog_b" to "prog" and "init-b" to "init".
>    Patches 2 and 3 are removed.
> 
> Changes since v2:
>  * Inserted patch 2 and 3 which rename "prog_b" and "init-b" into "prog"
>    and "init" for the SPI driver.
>  * From Krzysztof Kozlowski review's:
>    * Use more specific compatible names
>    * Remove other missing occurences of the slave word missed in v2.
>  * From Xu Yilun review's:
>    * Fix vertical whitespace in get_done_gpio().
>    * Combine write() and write_one_dummy_byte() together.
>    * Eliminate most of the xilinx_core_probe() arguments, the driver
>      needs to populate those directly into the xilinx_fpga_core struct.
>      Added some documentation to struct xilinx_fpga_core to clarify
>      this.
>    * Removed typedefs from xilinx-core.h.
>    * Moved null checks in xilinx_core_probe() to first patch.
>    * Move csi_b and rdwr_b out of xilinx_selectmap_conf as they are not
>      used out of the probe function.
> 
> Changes since v1: (from Krzysztof Kozlowski review's)
>   * Use more conventional names for gpio DT bindings
>   * fix example in DT bindings
>   * add mc-peripheral-props.yaml to DT bindings
>   * fix various formatting mistakes
>   * Remove all occurences of the "slave" word.
> 
> Charles Perry (4):
>   fpga: xilinx-spi: extract a common driver core
>   dt-bindings: fpga: xlnx,fpga-selectmap: add DT schema
>   fpga: xilinx-selectmap: add new driver
>   xilinx-core: add new gpio names for prog and init
> 
>  .../bindings/fpga/xlnx,fpga-selectmap.yaml    |  86 +++++++
>  drivers/fpga/Kconfig                          |  12 +
>  drivers/fpga/Makefile                         |   2 +
>  drivers/fpga/xilinx-core.c                    | 229 ++++++++++++++++++
>  drivers/fpga/xilinx-core.h                    |  27 +++
>  drivers/fpga/xilinx-selectmap.c               |  95 ++++++++
>  drivers/fpga/xilinx-spi.c                     | 224 ++---------------
>  7 files changed, 466 insertions(+), 209 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml
>  create mode 100644 drivers/fpga/xilinx-core.c
>  create mode 100644 drivers/fpga/xilinx-core.h
>  create mode 100644 drivers/fpga/xilinx-selectmap.c

Applied this series to for-next with a nit.

Thanks,
Yilun

> 
> --
> 2.43.0
>