Re: [PATCH v4 5/6] spi: zynqmp_gqspi: Add parallel memories support in GQSPI driver

2023-11-21 Thread Michal Simek




On 11/14/23 05:56, Venkatesh Yadav Abbarapu wrote:

Add support for parallel memories in zynqmp_gqspi.c driver. In case of
parallel memories STRIPE bit is set and sent to the qspi ip, which will
send data bits to both the flashes in parallel. However for few commands
we should not use stripe, instead send same data to both the flashes.
Those commands are exclueded by using zynqmp_qspi_update_stripe().

Also update copyright info for this file.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
  drivers/spi/zynqmp_gqspi.c | 140 -
  include/spi.h  |  12 
  2 files changed, 136 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c
index a323994fb2..7e90a989dd 100644
--- a/drivers/spi/zynqmp_gqspi.c
+++ b/drivers/spi/zynqmp_gqspi.c
@@ -1,6 +1,7 @@
  // SPDX-License-Identifier: GPL-2.0+
  /*
- * (C) Copyright 2018 Xilinx
+ * (C) Copyright 2013 - 2022, Xilinx, Inc.
+ * (C) Copyright 2023, Advanced Micro Devices, Inc.
   *
   * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller driver(master mode only)
   */
@@ -25,6 +26,8 @@
  #include 
  #include 
  #include 
+#include 
+#include "../mtd/spi/sf_internal.h"
  #include 
  
  #define GQSPI_GFIFO_STRT_MODE_MASK	BIT(29)

@@ -88,6 +91,9 @@
  #define SPI_XFER_ON_LOWER 1
  #define SPI_XFER_ON_UPPER 2
  
+#define GQSPI_SELECT_LOWER_CS  BIT(0)

+#define GQSPI_SELECT_UPPER_CS  BIT(1)
+
  #define GQSPI_DMA_ALIGN   0x4
  #define GQSPI_MAX_BAUD_RATE_VAL   7
  #define GQSPI_DFLT_BAUD_RATE_VAL  2
@@ -183,13 +189,14 @@ struct zynqmp_qspi_priv {
int bytes_to_transfer;
int bytes_to_receive;
const struct spi_mem_op *op;
+   unsigned int is_parallel;
+   unsigned int u_page;
+   unsigned int bus;
+   unsigned int stripe;
+   unsigned int flags;
+   u32 max_hz;
  };
  
-__weak int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 value)

-{
-   return 0;
-}
-
  static int zynqmp_qspi_of_to_plat(struct udevice *bus)
  {
struct zynqmp_qspi_plat *plat = dev_get_plat(bus);
@@ -234,9 +241,30 @@ static u32 zynqmp_qspi_bus_select(struct zynqmp_qspi_priv 
*priv)
  {
u32 gqspi_fifo_reg = 0;
  
-	gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |

-GQSPI_GFIFO_CS_LOWER;
-
+   if (priv->is_parallel) {
+   if (priv->bus == SPI_XFER_ON_BOTH)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_CS_UPPER |
+GQSPI_GFIFO_CS_LOWER;
+   else if (priv->bus == SPI_XFER_ON_LOWER)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_UPPER |
+GQSPI_GFIFO_CS_LOWER;
+   else if (priv->bus == SPI_XFER_ON_UPPER)
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_CS_LOWER |
+GQSPI_GFIFO_CS_UPPER;
+   else
+   debug("Wrong Bus selection:0x%x\n", priv->bus);
+   } else {
+   if (priv->u_page)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_UPPER;
+   else
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_LOWER;
+   }
return gqspi_fifo_reg;
  }
  
@@ -295,8 +323,14 @@ static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv *priv, int is_on)

gqspi_fifo_reg |= GQSPI_SPI_MODE_SPI |
  GQSPI_IMD_DATA_CS_ASSERT;
} else {
-   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
-   gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
+   if (priv->is_parallel)
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_LOW_BUS;
+   else if (priv->u_page)
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS;
+   else
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
+   gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;



Here you have bug. If it is the part of else there should be {} around.

If it is valid for all then indentation is not correct.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP/Versal ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal/Versal NET SoCs
TF-A maintainer - Xilinx ZynqMP/Versal/Versal NET SoCs


[PATCH v4 5/6] spi: zynqmp_gqspi: Add parallel memories support in GQSPI driver

2023-11-13 Thread Venkatesh Yadav Abbarapu
Add support for parallel memories in zynqmp_gqspi.c driver. In case of
parallel memories STRIPE bit is set and sent to the qspi ip, which will
send data bits to both the flashes in parallel. However for few commands
we should not use stripe, instead send same data to both the flashes.
Those commands are exclueded by using zynqmp_qspi_update_stripe().

Also update copyright info for this file.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
 drivers/spi/zynqmp_gqspi.c | 140 -
 include/spi.h  |  12 
 2 files changed, 136 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c
index a323994fb2..7e90a989dd 100644
--- a/drivers/spi/zynqmp_gqspi.c
+++ b/drivers/spi/zynqmp_gqspi.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * (C) Copyright 2018 Xilinx
+ * (C) Copyright 2013 - 2022, Xilinx, Inc.
+ * (C) Copyright 2023, Advanced Micro Devices, Inc.
  *
  * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller driver(master mode only)
  */
@@ -25,6 +26,8 @@
 #include 
 #include 
 #include 
+#include 
+#include "../mtd/spi/sf_internal.h"
 #include 
 
 #define GQSPI_GFIFO_STRT_MODE_MASK BIT(29)
@@ -88,6 +91,9 @@
 #define SPI_XFER_ON_LOWER  1
 #define SPI_XFER_ON_UPPER  2
 
+#define GQSPI_SELECT_LOWER_CS  BIT(0)
+#define GQSPI_SELECT_UPPER_CS  BIT(1)
+
 #define GQSPI_DMA_ALIGN0x4
 #define GQSPI_MAX_BAUD_RATE_VAL7
 #define GQSPI_DFLT_BAUD_RATE_VAL   2
@@ -183,13 +189,14 @@ struct zynqmp_qspi_priv {
int bytes_to_transfer;
int bytes_to_receive;
const struct spi_mem_op *op;
+   unsigned int is_parallel;
+   unsigned int u_page;
+   unsigned int bus;
+   unsigned int stripe;
+   unsigned int flags;
+   u32 max_hz;
 };
 
-__weak int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 
value)
-{
-   return 0;
-}
-
 static int zynqmp_qspi_of_to_plat(struct udevice *bus)
 {
struct zynqmp_qspi_plat *plat = dev_get_plat(bus);
@@ -234,9 +241,30 @@ static u32 zynqmp_qspi_bus_select(struct zynqmp_qspi_priv 
*priv)
 {
u32 gqspi_fifo_reg = 0;
 
-   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
-GQSPI_GFIFO_CS_LOWER;
-
+   if (priv->is_parallel) {
+   if (priv->bus == SPI_XFER_ON_BOTH)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_CS_UPPER |
+GQSPI_GFIFO_CS_LOWER;
+   else if (priv->bus == SPI_XFER_ON_LOWER)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_UPPER |
+GQSPI_GFIFO_CS_LOWER;
+   else if (priv->bus == SPI_XFER_ON_UPPER)
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_CS_LOWER |
+GQSPI_GFIFO_CS_UPPER;
+   else
+   debug("Wrong Bus selection:0x%x\n", priv->bus);
+   } else {
+   if (priv->u_page)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_UPPER;
+   else
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_LOWER;
+   }
return gqspi_fifo_reg;
 }
 
@@ -295,8 +323,14 @@ static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv 
*priv, int is_on)
gqspi_fifo_reg |= GQSPI_SPI_MODE_SPI |
  GQSPI_IMD_DATA_CS_ASSERT;
} else {
-   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
-   gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
+   if (priv->is_parallel)
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_LOW_BUS;
+   else if (priv->u_page)
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS;
+   else
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
+   gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
}
 
zynqmp_qspi_fill_gen_fifo(priv, gqspi_fifo_reg);
@@ -366,12 +400,13 @@ static int zynqmp_qspi_set_speed(struct udevice *bus, 
uint speed)
 
log_debug("%s, Speed: %d, Max: %d\n", __func__, speed, plat->frequency);
 
-   if (speed > plat->frequency)
-   speed = plat->frequency;
+   /*
+* If speed == 0 or speed > max freq, then set speed to highest
+*/
+   if (!speed || speed > priv->max_hz)
+   speed = priv->max_hz;
 
if (plat->speed_hz != speed) {
-   /* Set the clock frequency */
-