On Wed, May 9, 2018 at 4:08 PM, Jagan Teki <jagannadh.t...@gmail.com> wrote: > On Wed, May 9, 2018 at 1:31 PM, Siva Durga Prasad Paladugu > <siva...@xilinx.com> wrote: >> Hi Jagan, >> >>> -----Original Message----- >>> From: Jagan Teki [mailto:jagannadh.t...@gmail.com] >>> Sent: Wednesday, May 09, 2018 1:22 PM >>> To: Siva Durga Prasad Paladugu <siva...@xilinx.com> >>> Cc: Jagan Teki <ja...@amarulasolutions.com>; U-Boot-Denx <u- >>> b...@lists.denx.de>; michal.si...@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 >>> <siva...@xilinx.com> wrote: >>> > >>> > Hi, >>> > >>> >> -----Original Message----- >>> >> From: Jagan Teki [mailto:jagannadh.t...@gmail.com] >>> >> Sent: Wednesday, May 09, 2018 1:12 PM >>> >> To: Siva Durga Prasad Paladugu <siva...@xilinx.com> >>> >> Cc: Jagan Teki <ja...@amarulasolutions.com>; U-Boot-Denx <u- >>> >> b...@lists.denx.de>; michal.si...@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 >>> >> <siva...@xilinx.com> wrote: >>> >> > Hi Jagan, >>> >> > >>> >> >> -----Original Message----- >>> >> >> From: Jagan Teki [mailto:ja...@amarulasolutions.com] >>> >> >> Sent: Wednesday, May 09, 2018 12:01 PM >>> >> >> To: Siva Durga Prasad Paladugu <siva...@xilinx.com> >>> >> >> Cc: U-Boot-Denx <u-boot@lists.denx.de>; michal.si...@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.palad...@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.palad...@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 + Jagan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot