Hi Jagan,
-----Original Message----- From: Jagan Teki [mailto:jagannadh.teki@gmail.com] Sent: Wednesday, May 09, 2018 4:18 PM To: Siva Durga Prasad Paladugu sivadur@xilinx.com Cc: Jagan Teki jagan@amarulasolutions.com; U-Boot-Denx <u- boot@lists.denx.de>; michal.simek@xilinx.com Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
On Wed, May 9, 2018 at 4:08 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu sivadur@xilinx.com wrote:
Hi Jagan,
-----Original Message----- From: Jagan Teki [mailto:jagannadh.teki@gmail.com] Sent: Wednesday, May 09, 2018 1:22 PM To: Siva Durga Prasad Paladugu sivadur@xilinx.com Cc: Jagan Teki jagan@amarulasolutions.com; U-Boot-Denx <u- boot@lists.denx.de>; michal.simek@xilinx.com Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
On Wed, May 9, 2018 at 1:20 PM, Siva Durga Prasad Paladugu sivadur@xilinx.com wrote:
Hi,
-----Original Message----- From: Jagan Teki [mailto:jagannadh.teki@gmail.com] Sent: Wednesday, May 09, 2018 1:12 PM To: Siva Durga Prasad Paladugu sivadur@xilinx.com Cc: Jagan Teki jagan@amarulasolutions.com; U-Boot-Denx <u- boot@lists.denx.de>; michal.simek@xilinx.com Subject: Re: [U-Boot] [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP qspi driver
On Wed, May 9, 2018 at 12:33 PM, Siva Durga Prasad Paladugu sivadur@xilinx.com wrote: > Hi Jagan, > >> -----Original Message----- >> From: Jagan Teki [mailto:jagan@amarulasolutions.com] >> Sent: Wednesday, May 09, 2018 12:01 PM >> To: Siva Durga Prasad Paladugu sivadur@xilinx.com >> Cc: U-Boot-Denx u-boot@lists.denx.de; >> michal.simek@xilinx.com >> Subject: Re: [PATCH v3 1/2] spi: zynqmp_gqspi: Add support for ZynqMP >> qspi driver >> >> On Tue, May 8, 2018 at 3:43 PM, Siva Durga Prasad Paladugu >> siva.durga.paladugu@xilinx.com wrote: >> > This patch adds qspi driver support for ZynqMP SoC. This >> > driver is responsible for communicating with qspi flash devices. >> > >> > Signed-off-by: Siva Durga Prasad Paladugu >> > siva.durga.paladugu@xilinx.com >> > --- >> > Changes for v3: >> > - Renamed all macros, functions, files and configs as per >> > comment >> > - Used wait_for_bit wherever required >> > - Removed unnecessary header inclusion >> > >> > Changes for v2: >> > - Rebased on top of latest master >> > - Moved macro definitions to .h file as per comment >> > - Fixed magic values with macros as per comment >> > --- >> > arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h | 154
++++++
>> > drivers/spi/Kconfig | 7 + >> > drivers/spi/Makefile | 1 + >> > drivers/spi/zynqmp_gqspi.c | 670 >> ++++++++++++++++++++++++ >> > 4 files changed, 832 insertions(+) create mode 100644 >> > arch/arm/include/asm/arch- >> zynqmp/zynqmp_gqspi.h >> > create mode 100644 drivers/spi/zynqmp_gqspi.c >> > >> > diff --git a/arch/arm/include/asm/arch-
zynqmp/zynqmp_gqspi.h
>> > b/arch/arm/include/asm/arch-zynqmp/zynqmp_gqspi.h >> >> already asked you to move this header code in driver .c file > > You might have missed my reply to your earlier comment on this. > These were moved to .h based on comment from Lukasz in v1. > I don’t have any issue in having them anywhere. Let me know > your
choice.
I'm trying to align Linux code, better to move like that and make sure to use similar macros.
>
[snip]
>> > +static void zynqmp_qspi_genfifo_cmd(struct
zynqmp_qspi_priv
*priv) { >> > + u8 command = 1; >> > + u32 gen_fifo_cmd; >> > + u32 bytecount = 0; >> > + >> > + while (priv->len) { >> > + gen_fifo_cmd = zynqmp_qspi_bus_select(priv); >> > + gen_fifo_cmd |= GQSPI_GFIFO_TX; >> > + >> > + if (command) { >> > + command = 0; >> > + last_cmd = *(u8 *)priv->tx_buf; >> > + } >> >> don't understand this code can you explain? command assigned
1
>> it will not updated anywhere? > > I want to store last command sent. As the first byte in loop > only contains command, it ensures it fills only for one time > and next time it may contain data to be sent along with command. > Command initialized to 1 while declaring it above(u8 command =
1).
Ok the first TX buf has command and reused in tx and rx fifo, how come to use use same in rx fifo? why is is last_cmd it is first_cmd?
This holds the command that was sent last to the device before we use in
tx/rx fill, hence used this name.
ie. what I wonder, usually the spi transfer start with cmd + data in that case it would be the first command, did gqspi work reverse way?
It's also same as others :-), the last_cmd holds the recent command that
was sent to the device.
Its just name. To avoid confusion, I can use other names like "cmd_sent or cmd_recent". Hope this is ok for you or suggest which one
would be appropriate.
Let's park the naming aside.
u8 command = 0;
while (priv->len) { gen_fifo_cmd = zynqmp_qspi_bus_select(priv); gen_fifo_cmd |= GQSPI_GFIFO_TX;
if (command) { command = 0; last_cmd = *(u8 *)priv->tx_buf; } ...... priv->len--;}
Why the last_cmd assignment is in loop? say priv->len has non-zero the command is 1 last_cmd assigned and command reassigned to 0 there is
no
way execute the if for next while isn't it?
The main issue with this driver, which I told/commented several times about using flash specific stuff.
+#define QUAD_OUT_READ_CMD 0x6B +#define QUAD_PAGE_PROGRAM_CMD 0x32 +#define DUAL_OUTPUT_FASTRD_CMD 0x3B
Yeah I know, that’s where our discussion stopped last time and would like to conclude using this series. https://patchwork.ozlabs.org/patch/829856/ We discussed this sometime back and I mentioned that this controller is not just targeted for spi-nor but even though none tested/used with devices other than spi-nor AFAIK.
Thanks, Siva
Jagan.