Linux-FPGA Archive mirror
 help / color / mirror / Atom feed
From: Charles Perry <charles.perry@savoirfairelinux.com>
To: Xu Yilun <yilun.xu@linux.intel.com>
Cc: mdf <mdf@kernel.org>, Allen VANDIVER <avandiver@markem-imaje.com>,
	 Brian CODY <bcody@markem-imaje.com>, hao wu <hao.wu@intel.com>,
	 yilun xu <yilun.xu@intel.com>, Tom Rix <trix@redhat.com>,
	 Rob Herring <robh+dt@kernel.org>,
	 krzysztof kozlowski+dt <krzysztof.kozlowski+dt@linaro.org>,
	 Conor Dooley <conor+dt@kernel.org>,
	 Michal Simek <michal.simek@amd.com>,
	 linux-fpga <linux-fpga@vger.kernel.org>,
	 devicetree <devicetree@vger.kernel.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>,
	 linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v6 0/4] fpga: xilinx-selectmap: add new driver
Date: Tue, 2 Apr 2024 09:57:12 -0400 (EDT)	[thread overview]
Message-ID: <1609084823.2225067.1712066232087.JavaMail.zimbra@savoirfairelinux.com> (raw)
In-Reply-To: <Zgl4I6rykg7shI2K@yilunxu-OptiPlex-7050>


On Mar 31, 2024, at 10:50 AM, Xu Yilun yilun.xu@linux.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

      reply	other threads:[~2024-04-02 14:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-21 22:04 [PATCH v6 0/4] fpga: xilinx-selectmap: add new driver Charles Perry
2024-03-21 22:04 ` [PATCH v6 1/4] fpga: xilinx-spi: extract a common driver core Charles Perry
2024-03-21 22:04 ` [PATCH v6 2/4] dt-bindings: fpga: xlnx,fpga-selectmap: add DT schema Charles Perry
2024-03-21 22:04 ` [PATCH v6 3/4] fpga: xilinx-selectmap: add new driver Charles Perry
2024-03-31 14:34   ` Xu Yilun
2024-03-21 22:04 ` [PATCH v6 4/4] fpga: xilinx-core: add new gpio names for prog and init Charles Perry
2024-03-31 14:50 ` [PATCH v6 0/4] fpga: xilinx-selectmap: add new driver Xu Yilun
2024-04-02 13:57   ` Charles Perry [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1609084823.2225067.1712066232087.JavaMail.zimbra@savoirfairelinux.com \
    --to=charles.perry@savoirfairelinux.com \
    --cc=avandiver@markem-imaje.com \
    --cc=bcody@markem-imaje.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hao.wu@intel.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fpga@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mdf@kernel.org \
    --cc=michal.simek@amd.com \
    --cc=robh+dt@kernel.org \
    --cc=trix@redhat.com \
    --cc=yilun.xu@intel.com \
    --cc=yilun.xu@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).