[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1609084823.2225067.1712066232087.JavaMail.zimbra@savoirfairelinux.com>
Date: Tue, 2 Apr 2024 09:57:12 -0400 (EDT)
From: Charles Perry <charles.perry@...oirfairelinux.com>
To: Xu Yilun <yilun.xu@...ux.intel.com>
Cc: mdf <mdf@...nel.org>, Allen VANDIVER <avandiver@...kem-imaje.com>,
Brian CODY <bcody@...kem-imaje.com>, hao wu <hao.wu@...el.com>,
yilun xu <yilun.xu@...el.com>, Tom Rix <trix@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
krzysztof kozlowski+dt <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Michal Simek <michal.simek@....com>,
linux-fpga <linux-fpga@...r.kernel.org>,
devicetree <devicetree@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v6 0/4] fpga: xilinx-selectmap: add new driver
On Mar 31, 2024, at 10:50 AM, Xu Yilun yilun.xu@...ux.intel.com wrote:
> 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
>
Thanks again for all the good reviews Yilun.
Regards,
Charles
>>
>> --
>> 2.43.0
Powered by blists - more mailing lists