[U-Boot] [UBOOT][PATCHv4 0/6] Add TI qspi controller with memory mapped support

This patch series add support for TI qspi controller and in the process also add support for quad read and memory mapped read in mtd spi framework.
Testing details: Did a boot from qspi mode on DRA7xx. Testing details present in the last patch of the series.
Currently, TI qpsi controller supports only 16MB access. Access for higher MB area will be added later.
v3->v4: 1. Remove quad support for now 2. Rebase to master-probe barnach, where qspi framework has changed. 3. Adapt my qspi driver according to the new format suggested by Jagan..
Patches are available at: git://gitorious.org/u-boot-shared/u-boot-qspi.git qspi_v4
Matt Porter (3): omap5: add qspi support spi: add TI QSPI driver dra7xx_evm: add SPL API, QSPI, and serial flash support
Sourav Poddar (3): armv7: hw_data: change clock divider setting. driver: mtd: spi: Add memory mapped read support README: qspi usecase and testing documentation.
arch/arm/cpu/armv7/omap5/hw_data.c | 10 +- arch/arm/cpu/armv7/omap5/prcm-regs.c | 1 + arch/arm/include/asm/arch-omap5/omap.h | 3 + arch/arm/include/asm/arch-omap5/spl.h | 1 + arch/arm/include/asm/omap_common.h | 1 + board/ti/dra7xx/mux_data.h | 10 + doc/README.ti_qspi_dra_test | 38 ++++ doc/README.ti_qspi_flash | 47 +++++ drivers/mtd/spi/sf_ops.c | 2 + drivers/mtd/spi/sf_probe.c | 1 + drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328 ++++++++++++++++++++++++++++++++ include/configs/dra7xx_evm.h | 19 ++ include/spi.h | 3 + 14 files changed, 464 insertions(+), 1 deletions(-) create mode 100644 doc/README.ti_qspi_dra_test create mode 100644 doc/README.ti_qspi_flash create mode 100644 drivers/spi/ti_qspi.c

From: Matt Porter matt.porter@linaro.org
Add QSPI definitions and clock configuration support.
Signed-off-by: Matt Porter matt.porter@linaro.org Signed-off-by: Sourav Poddar sourav.poddar@ti.com --- arch/arm/cpu/armv7/omap5/hw_data.c | 8 ++++++++ arch/arm/cpu/armv7/omap5/prcm-regs.c | 1 + arch/arm/include/asm/arch-omap5/omap.h | 3 +++ arch/arm/include/asm/arch-omap5/spl.h | 1 + arch/arm/include/asm/omap_common.h | 1 + 5 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c index fbbc486..c00bfb8 100644 --- a/arch/arm/cpu/armv7/omap5/hw_data.c +++ b/arch/arm/cpu/armv7/omap5/hw_data.c @@ -426,6 +426,10 @@ void enable_basic_clocks(void) #ifdef CONFIG_DRIVER_TI_CPSW (*prcm)->cm_gmac_gmac_clkctrl, #endif + +#ifdef CONFIG_TI_QSPI + (*prcm)->cm_l4per_qspi_clkctrl, +#endif 0 };
@@ -454,6 +458,10 @@ void enable_basic_clocks(void) clk_modules_explicit_en_essential, 1);
+#ifdef CONFIG_TI_QSPI + setbits_le32((*prcm)->cm_l4per_qspi_clkctrl, (1<<24)); +#endif + /* Enable SCRM OPT clocks for PER and CORE dpll */ setbits_le32((*prcm)->cm_wkupaon_scrm_clkctrl, OPTFCLKEN_SCRM_PER_MASK); diff --git a/arch/arm/cpu/armv7/omap5/prcm-regs.c b/arch/arm/cpu/armv7/omap5/prcm-regs.c index 579818d..0b1bb46 100644 --- a/arch/arm/cpu/armv7/omap5/prcm-regs.c +++ b/arch/arm/cpu/armv7/omap5/prcm-regs.c @@ -933,6 +933,7 @@ struct prcm_regs const dra7xx_prcm = { .cm_l4per_gpio8_clkctrl = 0x4a009818, .cm_l4per_mmcsd3_clkctrl = 0x4a009820, .cm_l4per_mmcsd4_clkctrl = 0x4a009828, + .cm_l4per_qspi_clkctrl = 0x4a009838, .cm_l4per_uart1_clkctrl = 0x4a009840, .cm_l4per_uart2_clkctrl = 0x4a009848, .cm_l4per_uart3_clkctrl = 0x4a009850, diff --git a/arch/arm/include/asm/arch-omap5/omap.h b/arch/arm/include/asm/arch-omap5/omap.h index e9a51d3..414d37a 100644 --- a/arch/arm/include/asm/arch-omap5/omap.h +++ b/arch/arm/include/asm/arch-omap5/omap.h @@ -61,6 +61,9 @@ /* GPMC */ #define OMAP54XX_GPMC_BASE 0x50000000
+/* QSPI */ +#define QSPI_BASE 0x4B300000 + /* * Hardware Register Details */ diff --git a/arch/arm/include/asm/arch-omap5/spl.h b/arch/arm/include/asm/arch-omap5/spl.h index fe8b0c0..57f0de5 100644 --- a/arch/arm/include/asm/arch-omap5/spl.h +++ b/arch/arm/include/asm/arch-omap5/spl.h @@ -15,6 +15,7 @@ #define BOOT_DEVICE_MMC1 5 #define BOOT_DEVICE_MMC2 6 #define BOOT_DEVICE_MMC2_2 7 +#define BOOT_DEVICE_SPI 10
#define MMC_BOOT_DEVICES_START BOOT_DEVICE_MMC1 #define MMC_BOOT_DEVICES_END BOOT_DEVICE_MMC2_2 diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h index 5e2f027..f865c14 100644 --- a/arch/arm/include/asm/omap_common.h +++ b/arch/arm/include/asm/omap_common.h @@ -266,6 +266,7 @@ struct prcm_regs { u32 cm_l4per_mmcsd4_clkctrl; u32 cm_l4per_msprohg_clkctrl; u32 cm_l4per_slimbus2_clkctrl; + u32 cm_l4per_qspi_clkctrl; u32 cm_l4per_uart1_clkctrl; u32 cm_l4per_uart2_clkctrl; u32 cm_l4per_uart3_clkctrl;

Clock requirement for qspi clk is 192 Mhz. According to the below formulae,
f dpll = f ref * 2 * m /(n + 1) clockoutx2_Hmn = f dpll / (hmn+ 1)
fref = 20 Mhz, m = 96, n = 4 gives f dpll = 768 Mhz For clockoutx2_Hmn to be 768, hmn + 1 should be 4.
Signed-off-by: Sourav Poddar sourav.poddar@ti.com --- arch/arm/cpu/armv7/omap5/hw_data.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c index c00bfb8..a1b249e 100644 --- a/arch/arm/cpu/armv7/omap5/hw_data.c +++ b/arch/arm/cpu/armv7/omap5/hw_data.c @@ -170,7 +170,7 @@ static const struct dpll_params per_dpll_params_768mhz_es2[NUM_SYS_CLKS] = {
static const struct dpll_params per_dpll_params_768mhz_dra7xx[NUM_SYS_CLKS] = { {32, 0, 4, 1, 3, 4, 10, 2, -1, -1, -1, -1}, /* 12 MHz */ - {96, 4, 4, 1, 3, 4, 10, 2, -1, -1, -1, -1}, /* 20 MHz */ + {96, 4, 4, 1, 3, 4, 4, 2, -1, -1, -1, -1}, /* 20 MHz */ {160, 6, 4, 1, 3, 4, 10, 2, -1, -1, -1, -1}, /* 16.8 MHz */ {20, 0, 4, 1, 3, 4, 10, 2, -1, -1, -1, -1}, /* 19.2 MHz */ {192, 12, 4, 1, 3, 4, 10, 2, -1, -1, -1, -1}, /* 26 MHz */

Qspi controller can have a memory mapped port which can be used for data read. Added support to enable memory mapped port read.
This patch enables the following: - It enables exchange of memory map address between mtd and qspi through the introduction of "memory_map" flag. - Add support to communicate to the driver that memory mapped transfer is to be started through introduction of new flags like "SPI_XFER_MEM_MAP" and "SPI_XFER_MEM_MAP_END".
This will enable the spi controller to do memory mapped configurations if required.
Signed-off-by: Sourav Poddar sourav.poddar@ti.com --- drivers/mtd/spi/sf_ops.c | 2 ++ drivers/mtd/spi/sf_probe.c | 1 + include/spi.h | 3 +++ 3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index c009af5..bee4128 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
/* Handle memory-mapped SPI */ if (flash->memory_map) { + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); memcpy(data, flash->memory_map + offset, len); + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); return 0; }
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index 1525636..6aa7086 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -203,6 +203,7 @@ struct spi_flash *spi_flash_validate_params(struct spi_slave *spi, u8 *idcode) flash->page_size = (ext_jedec == 0x4d00) ? 512 : 256; flash->sector_size = params->sector_size; flash->size = flash->sector_size * params->nr_sectors; + flash->memory_map = spi->memory_map;
/* Compute erase sector and command */ if (params->flags & SECT_4K) { diff --git a/include/spi.h b/include/spi.h index c44ebe8..d5c4e08 100644 --- a/include/spi.h +++ b/include/spi.h @@ -27,6 +27,8 @@ /* SPI transfer flags */ #define SPI_XFER_BEGIN 0x01 /* Assert CS before transfer */ #define SPI_XFER_END 0x02 /* Deassert CS after transfer */ +#define SPI_XFER_MEM_MAP 0x08 /* Memory Mapped start */ +#define SPI_XFER_MEM_MAP_END 0x10 /* Memory Mapped End */
/* Header byte that marks the start of the message */ #define SPI_PREAMBLE_END_BYTE 0xec @@ -46,6 +48,7 @@ struct spi_slave { unsigned int bus; unsigned int cs; unsigned int max_write_size; + void *memory_map; };
/**

Please use the commit msg head as "sf: .."
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar sourav.poddar@ti.com wrote:
Qspi controller can have a memory mapped port which can be used for data read. Added support to enable memory mapped port read.
This patch enables the following:
- It enables exchange of memory map address between mtd and qspi
through the introduction of "memory_map" flag.
- Add support to communicate to the driver that memory mapped
transfer is to be started through introduction of new flags like "SPI_XFER_MEM_MAP" and "SPI_XFER_MEM_MAP_END".
This will enable the spi controller to do memory mapped configurations if required.
Signed-off-by: Sourav Poddar sourav.poddar@ti.com
drivers/mtd/spi/sf_ops.c | 2 ++ drivers/mtd/spi/sf_probe.c | 1 + include/spi.h | 3 +++ 3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index c009af5..bee4128 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
/* Handle memory-mapped SPI */ if (flash->memory_map) {
spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); memcpy(data, flash->memory_map + offset, len);
spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP);
Is it correct, can you check it once. where is SPI_XFER_MEM_MAP_END used?
Looks like you have used mem-map for only reads is it? if so where is SPI_XFER_BEGIN is using? Please use _MMAP instead of _MEM_MAP for simple naming convention.

On Saturday 05 October 2013 01:36 AM, Jagan Teki wrote:
Please use the commit msg head as "sf: .."
Ok.
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
Qspi controller can have a memory mapped port which can be used for data read. Added support to enable memory mapped port read.
This patch enables the following:
- It enables exchange of memory map address between mtd and qspi
through the introduction of "memory_map" flag.
- Add support to communicate to the driver that memory mapped transfer is to be started through introduction of new flags like
"SPI_XFER_MEM_MAP" and "SPI_XFER_MEM_MAP_END".
This will enable the spi controller to do memory mapped configurations if required.
Signed-off-by: Sourav Poddarsourav.poddar@ti.com
drivers/mtd/spi/sf_ops.c | 2 ++ drivers/mtd/spi/sf_probe.c | 1 + include/spi.h | 3 +++ 3 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index c009af5..bee4128 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
/* Handle memory-mapped SPI */ if (flash->memory_map) {
spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); memcpy(data, flash->memory_map + offset, len);
spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP);
Is it correct, can you check it once. where is SPI_XFER_MEM_MAP_END used?
It will be used in the driver. check 4/6 patch of this series.
Looks like you have used mem-map for only reads is it? if so where is SPI_XFER_BEGIN is using?
Yes, only memory mapped read is supported.
Ideally, we dont need BEGIN flag for memory mapped cases. I have explained a bit more on your similar comment on patch 4/6.
Please use _MMAP instead of _MEM_MAP for simple naming convention.
OK.

On Fri, Oct 04, 2013 at 20:21 +0530, Sourav Poddar wrote:
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index c009af5..bee4128 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
/* Handle memory-mapped SPI */ if (flash->memory_map) {
memcpy(data, flash->memory_map + offset, len);spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP);
return 0; }spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP);
Feedback has been sent before, but I'm afraid the motivation wasn't received appropriately.
Shouldn't the memcpy() call be surrounded by _MAP and _MAP_END (please note the _END in the second spi_xfer() invocation)? The current patch doesn't "close" the transfer, which appears to pass tests but isn't correct.
virtually yours Gerhard Sittig

On Sunday 06 October 2013 03:03 PM, Gerhard Sittig wrote:
On Fri, Oct 04, 2013 at 20:21 +0530, Sourav Poddar wrote:
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index c009af5..bee4128 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
/* Handle memory-mapped SPI */ if (flash->memory_map) {
memcpy(data, flash->memory_map + offset, len);spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP);
return 0; }spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP);
Feedback has been sent before, but I'm afraid the motivation wasn't received appropriately.
Sorry, If I missed any mails.
Shouldn't the memcpy() call be surrounded by _MAP and _MAP_END (please note the _END in the second spi_xfer() invocation)? The current patch doesn't "close" the transfer, which appears to pass tests but isn't correct.
Yes, you are correct. Second xfer should be with a END flag. I will add it in my next version, thanks for pointing out.
virtually yours Gerhard Sittig

From: Matt Porter matt.porter@linaro.org
Adds a SPI master driver for the TI QSPI peripheral.
Signed-off-by: Matt Porter matt.porter@linaro.org Signed-off-by: Sourav Poddar sourav.poddar@ti.com [Added quad read support and memory mapped support). --- drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 0 deletions(-) create mode 100644 drivers/spi/ti_qspi.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 91d24ce..e5941b0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c new file mode 100644 index 0000000..d8a03a8 --- /dev/null +++ b/drivers/spi/ti_qspi.c @@ -0,0 +1,328 @@ +/* + * TI QSPI driver + * + * Copyright (C) 2013, Texas Instruments, Incorporated + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <asm/io.h> +#include <asm/arch/omap.h> +#include <malloc.h> +#include <spi.h> + +struct qspi_regs { +u32 pid; +u32 pad0[3]; +u32 sysconfig; +u32 pad1[3]; +u32 intr_status_raw_set; +u32 intr_status_enabled_clear; +u32 intr_enable_set; +u32 intr_enable_clear; +u32 intc_eoi; +u32 pad2[3]; +u32 spi_clock_cntrl; +u32 spi_dc; +u32 spi_cmd; +u32 spi_status; +u32 spi_data; +u32 spi_setup0; +u32 spi_setup1; +u32 spi_setup2; +u32 spi_setup3; +u32 spi_switch; +u32 spi_data1; +u32 spi_data2; +u32 spi_data3; +}; + +struct qspi_slave { + struct spi_slave slave; + struct qspi_regs *base; + unsigned int mode; + u32 cmd; + u32 dc; +}; + +#define QSPI_TIMEOUT 2000000 + +#define QSPI_FCLK 192000000 + +/* Clock Control */ +#define QSPI_CLK_EN (1 << 31) +#define QSPI_CLK_DIV_MAX 0xffff + +/* Command */ +#define QSPI_EN_CS(n) (n << 28) +#define QSPI_WLEN(n) ((n-1) << 19) +#define QSPI_3_PIN (1 << 18) +#define QSPI_RD_SNGL (1 << 16) +#define QSPI_WR_SNGL (2 << 16) +#define QSPI_INVAL (4 << 16) +#define QSPI_RD_QUAD (7 << 16) + +/* Device Control */ +#define QSPI_DD(m, n) (m << (3 + n*8)) +#define QSPI_CKPHA(n) (1 << (2 + n*8)) +#define QSPI_CSPOL(n) (1 << (1 + n*8)) +#define QSPI_CKPOL(n) (1 << (n*8)) + +/* Status */ +#define QSPI_WC (1 << 1) +#define QSPI_BUSY (1 << 0) +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) +#define QSPI_XFER_DONE QSPI_WC + +#define MM_SWITCH 0x01 +#define MEM_CS 0x100 +#define MEM_CS_UNSELECT 0xfffff0ff +#define MMAP_START_ADDR 0x5c000000 +#define CORE_CTRL_IO 0x4a002558 + +#define QSPI_CMD_READ (0x3 << 0) +#define QSPI_CMD_READ_QUAD (0x6b << 0) +#define QSPI_CMD_READ_FAST (0x0b << 0) +#define QSPI_SETUP0_NUM_A_BYTES (0x2 << 8) +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0 << 10) +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1 << 10) +#define QSPI_SETUP0_READ_NORMAL (0x0 << 12) +#define QSPI_SETUP0_READ_QUAD (0x3 << 12) +#define QSPI_CMD_WRITE (0x2 << 16) +#define QSPI_NUM_DUMMY_BITS (0x0 << 24) + +static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave) +{ + return container_of(slave, struct qspi_slave, slave); +} +static inline struct qspi_regs *get_qspi_bus(int dev) +{ + if (!dev) + return (struct qspi_regs *)QSPI_BASE; + else + return NULL; +} + +int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{ + return 1; +} + +void spi_cs_activate(struct spi_slave *slave) +{ + /* CS handled in xfer */ + return; +} + +void spi_cs_deactivate(struct spi_slave *slave) +{ + /* CS handled in xfer */ + return; +} + +void spi_init(void) +{ + /* nothing to do */ +} + +void spi_set_up_spi_register(struct qspi_slave *qslave) +{ + u32 memval = 0; + struct spi_slave *slave = &qslave->slave; + + slave->memory_map = (void *)MMAP_START_ADDR; + + memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES | + QSPI_SETUP0_NUM_D_BYTES_NO_BITS | QSPI_SETUP0_READ_NORMAL | + QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS); + + writel(memval, &qslave->base->spi_setup0); +} + +void spi_set_speed(struct spi_slave *slave, uint hz) +{ + struct qspi_slave *qslave = to_qspi_slave(slave); + + uint clk_div; + + if (!hz) + clk_div = 0; + else + clk_div = (QSPI_FCLK / hz) - 1; + + debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div); + + /* disable SCLK */ + writel(readl(&qslave->base->spi_clock_cntrl) & ~QSPI_CLK_EN, + &qslave->base->spi_clock_cntrl); + + if (clk_div < 0) { + debug("%s: clock divider < 0, using /1 divider\n", __func__); + clk_div = 0; + } + + if (clk_div > QSPI_CLK_DIV_MAX) { + debug("%s: clock divider >%d , using /%d divider\n", + __func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX + 1); + clk_div = QSPI_CLK_DIV_MAX; + } + + /* enable SCLK */ + writel(QSPI_CLK_EN | clk_div, &qslave->base->spi_clock_cntrl); + debug("%s: spi_clock_cntrl %08x\n", __func__, + readl(&qslave->base->spi_clock_cntrl)); +} + +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, + unsigned int max_hz, unsigned int mode) +{ + struct qspi_slave *qslave; + + qslave = spi_alloc_slave(struct qspi_slave, bus, cs); + if (!qslave) + return NULL; + + qslave->base = get_qspi_bus(bus); + if (qslave->base) + debug("No Qspi device found\n"); + spi_set_speed(&qslave->slave, max_hz); + qslave->mode = mode; + +#ifdef CONFIG_MMAP + spi_set_up_spi_register(qslave); +#endif + + debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode); + + return &qslave->slave; +} + +void spi_free_slave(struct spi_slave *slave) +{ + struct qspi_slave *qslave = to_qspi_slave(slave); + free(qslave); +} + +int spi_claim_bus(struct spi_slave *slave) +{ + struct qspi_slave *qslave = to_qspi_slave(slave); + + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs); + + writel(0, &qslave->base->spi_dc); + writel(0, &qslave->base->spi_cmd); + writel(0, &qslave->base->spi_data); + + return 0; +} + +void spi_release_bus(struct spi_slave *slave) +{ + struct qspi_slave *qslave = to_qspi_slave(slave); + + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs); + + writel(0, &qslave->base->spi_dc); + writel(0, &qslave->base->spi_cmd); + writel(0, &qslave->base->spi_data); +} + +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, + void *din, unsigned long flags) +{ + struct qspi_slave *qslave = to_qspi_slave(slave); + uint words = bitlen >> 3; /* fixed 8-bit word length */ + const uchar *txp = dout; + uchar *rxp = din; + uint status; + int timeout, val; + + debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", __func__, + slave->bus, slave->cs, bitlen, words, flags); + + qslave->dc = 0; + if (qslave->mode & SPI_CPHA) + qslave->dc |= QSPI_CKPHA(slave->cs); + if (qslave->mode & SPI_CPOL) + qslave->dc |= QSPI_CKPOL(slave->cs); + if (qslave->mode & SPI_CS_HIGH) + qslave->dc |= QSPI_CSPOL(slave->cs); + + writel(qslave->dc, &qslave->base->spi_dc); + + if (flags & SPI_XFER_MEM_MAP) { + writel(MM_SWITCH, &qslave->base->spi_switch); + val = readl(CORE_CTRL_IO); + val |= MEM_CS; + writel(val, CORE_CTRL_IO); + return 0; + } else if (flags & SPI_XFER_MEM_MAP_END) { + writel(~MM_SWITCH, &qslave->base->spi_switch); + val = readl(CORE_CTRL_IO); + val &= MEM_CS_UNSELECT; + writel(val, CORE_CTRL_IO); + return 0; + } + + if (bitlen == 0) + goto out; + + if (bitlen % 8) { + flags |= SPI_XFER_END; + goto out; + } + + /* setup command reg */ + qslave->cmd = 0; + qslave->cmd |= QSPI_WLEN(8); + qslave->cmd |= QSPI_EN_CS(slave->cs); + if (flags & SPI_3WIRE) + qslave->cmd |= QSPI_3_PIN; + qslave->cmd |= 0xfff; + + while (words--) { + if (txp) { + debug("tx cmd %08x dc %08x data %02x\n", + qslave->cmd | QSPI_WR_SNGL, qslave->dc, *txp); + writel(*txp++, &qslave->base->spi_data); + writel(qslave->cmd | QSPI_WR_SNGL, + &qslave->base->spi_cmd); + status = readl(&qslave->base->spi_status); + timeout = QSPI_TIMEOUT; + while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) { + if (--timeout < 0) { + printf("QSPI tx timed out\n"); + return -1; + } + status = readl(&qslave->base->spi_status); + } + debug("tx done, status %08x\n", status); + } + if (rxp) { + qslave->cmd |= QSPI_RD_SNGL; + debug("rx cmd %08x dc %08x\n", + qslave->cmd, qslave->dc); + writel(qslave->cmd, &qslave->base->spi_cmd); + status = readl(&qslave->base->spi_status); + timeout = QSPI_TIMEOUT; + while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) { + if (--timeout < 0) { + printf("QSPI rx timed out\n"); + return -1; + } + status = readl(&qslave->base->spi_status); + } + *rxp++ = readl(&qslave->base->spi_data); + debug("rx done, status %08x, read %02x\n", + status, *(rxp-1)); + } + } + +out: + /* Terminate frame */ + if (flags & SPI_XFER_END) + writel(qslave->cmd | QSPI_INVAL, &qslave->base->spi_cmd); + + return 0; +}

On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar sourav.poddar@ti.com wrote:
From: Matt Porter matt.porter@linaro.org
Adds a SPI master driver for the TI QSPI peripheral.
Signed-off-by: Matt Porter matt.porter@linaro.org Signed-off-by: Sourav Poddar sourav.poddar@ti.com [Added quad read support and memory mapped support).
What is this comment, any specific?
You missed change log for all patches, i think you have summarized in 0/6. I feel it's better write it on individual patches.
drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 0 deletions(-) create mode 100644 drivers/spi/ti_qspi.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 91d24ce..e5941b0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c new file mode 100644 index 0000000..d8a03a8 --- /dev/null +++ b/drivers/spi/ti_qspi.c @@ -0,0 +1,328 @@ +/*
- TI QSPI driver
- Copyright (C) 2013, Texas Instruments, Incorporated
- SPDX-License-Identifier: GPL-2.0+
Got below format after apply this patch - please check *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+
- */
+#include <common.h> +#include <asm/io.h> +#include <asm/arch/omap.h> +#include <malloc.h> +#include <spi.h>
+struct qspi_regs { +u32 pid; +u32 pad0[3]; +u32 sysconfig; +u32 pad1[3]; +u32 intr_status_raw_set; +u32 intr_status_enabled_clear; +u32 intr_enable_set; +u32 intr_enable_clear; +u32 intc_eoi; +u32 pad2[3]; +u32 spi_clock_cntrl; +u32 spi_dc; +u32 spi_cmd; +u32 spi_status; +u32 spi_data; +u32 spi_setup0; +u32 spi_setup1; +u32 spi_setup2; +u32 spi_setup3; +u32 spi_switch; +u32 spi_data1; +u32 spi_data2; +u32 spi_data3;
Please add tab space.
+};
+struct qspi_slave {
struct spi_slave slave;
struct qspi_regs *base;
unsigned int mode;
u32 cmd;
u32 dc;
+};
-- TAG+
+#define QSPI_TIMEOUT 2000000
+#define QSPI_FCLK 192000000
+/* Clock Control */ +#define QSPI_CLK_EN (1 << 31) +#define QSPI_CLK_DIV_MAX 0xffff
+/* Command */ +#define QSPI_EN_CS(n) (n << 28) +#define QSPI_WLEN(n) ((n-1) << 19) +#define QSPI_3_PIN (1 << 18) +#define QSPI_RD_SNGL (1 << 16) +#define QSPI_WR_SNGL (2 << 16) +#define QSPI_INVAL (4 << 16) +#define QSPI_RD_QUAD (7 << 16)
+/* Device Control */ +#define QSPI_DD(m, n) (m << (3 + n*8)) +#define QSPI_CKPHA(n) (1 << (2 + n*8)) +#define QSPI_CSPOL(n) (1 << (1 + n*8)) +#define QSPI_CKPOL(n) (1 << (n*8))
+/* Status */ +#define QSPI_WC (1 << 1) +#define QSPI_BUSY (1 << 0) +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) +#define QSPI_XFER_DONE QSPI_WC
+#define MM_SWITCH 0x01 +#define MEM_CS 0x100 +#define MEM_CS_UNSELECT 0xfffff0ff +#define MMAP_START_ADDR 0x5c000000 +#define CORE_CTRL_IO 0x4a002558
+#define QSPI_CMD_READ (0x3 << 0) +#define QSPI_CMD_READ_QUAD (0x6b << 0) +#define QSPI_CMD_READ_FAST (0x0b << 0) +#define QSPI_SETUP0_NUM_A_BYTES (0x2 << 8) +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0 << 10) +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1 << 10) +#define QSPI_SETUP0_READ_NORMAL (0x0 << 12) +#define QSPI_SETUP0_READ_QUAD (0x3 << 12) +#define QSPI_CMD_WRITE (0x2 << 16) +#define QSPI_NUM_DUMMY_BITS (0x0 << 24)
--TAG-
TAG+ ... TAG- please move these macro definitions in below headers
+static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave) +{
return container_of(slave, struct qspi_slave, slave);
+} +static inline struct qspi_regs *get_qspi_bus(int dev) +{
if (!dev)
return (struct qspi_regs *)QSPI_BASE;
else
return NULL;
+}
Is this function really required, how many bus controller you have?
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
return 1;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_init(void) +{
/* nothing to do */
+}
+void spi_set_up_spi_register(struct qspi_slave *qslave) +{
u32 memval = 0;
struct spi_slave *slave = &qslave->slave;
slave->memory_map = (void *)MMAP_START_ADDR;
memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
QSPI_SETUP0_NUM_D_BYTES_NO_BITS | QSPI_SETUP0_READ_NORMAL |
QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS);
writel(memval, &qslave->base->spi_setup0);
+}
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint clk_div;
if (!hz)
clk_div = 0;
else
clk_div = (QSPI_FCLK / hz) - 1;
debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div);
/* disable SCLK */
writel(readl(&qslave->base->spi_clock_cntrl) & ~QSPI_CLK_EN,
&qslave->base->spi_clock_cntrl);
if (clk_div < 0) {
debug("%s: clock divider < 0, using /1 divider\n", __func__);
clk_div = 0;
}
if (clk_div > QSPI_CLK_DIV_MAX) {
debug("%s: clock divider >%d , using /%d divider\n",
__func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX + 1);
clk_div = QSPI_CLK_DIV_MAX;
}
/* enable SCLK */
writel(QSPI_CLK_EN | clk_div, &qslave->base->spi_clock_cntrl);
debug("%s: spi_clock_cntrl %08x\n", __func__,
readl(&qslave->base->spi_clock_cntrl));
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int mode)
+{
struct qspi_slave *qslave;
qslave = spi_alloc_slave(struct qspi_slave, bus, cs);
if (!qslave)
return NULL;
qslave->base = get_qspi_bus(bus);
if (qslave->base)
debug("No Qspi device found\n");
spi_set_speed(&qslave->slave, max_hz);
qslave->mode = mode;
+#ifdef CONFIG_MMAP
spi_set_up_spi_register(qslave);
+#endif
debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode);
return &qslave->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
free(qslave);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0, &qslave->base->spi_dc);
writel(0, &qslave->base->spi_cmd);
writel(0, &qslave->base->spi_data);
return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0, &qslave->base->spi_dc);
writel(0, &qslave->base->spi_cmd);
writel(0, &qslave->base->spi_data);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
void *din, unsigned long flags)
+{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint words = bitlen >> 3; /* fixed 8-bit word length */
const uchar *txp = dout;
uchar *rxp = din;
uint status;
int timeout, val;
debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", __func__,
slave->bus, slave->cs, bitlen, words, flags);
qslave->dc = 0;
if (qslave->mode & SPI_CPHA)
qslave->dc |= QSPI_CKPHA(slave->cs);
if (qslave->mode & SPI_CPOL)
qslave->dc |= QSPI_CKPOL(slave->cs);
if (qslave->mode & SPI_CS_HIGH)
qslave->dc |= QSPI_CSPOL(slave->cs);
writel(qslave->dc, &qslave->base->spi_dc);
Adjust this code in spi_claim_bus()
if (flags & SPI_XFER_MEM_MAP) {
writel(MM_SWITCH, &qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val |= MEM_CS;
writel(val, CORE_CTRL_IO);
return 0;
} else if (flags & SPI_XFER_MEM_MAP_END) {
writel(~MM_SWITCH, &qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val &= MEM_CS_UNSELECT;
writel(val, CORE_CTRL_IO);
return 0;
}
What is this your are returning from here directly for memory_map? is it?
--TAG+
if (bitlen == 0)
goto out;
if (bitlen % 8) {
flags |= SPI_XFER_END;
goto out;
}
--TAG-
TAG+ .. TAG- please move this code on start of the xfer..ie below debug();
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags & SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
while (words--) {
if (txp) {
debug("tx cmd %08x dc %08x data %02x\n",
qslave->cmd | QSPI_WR_SNGL, qslave->dc, *txp);
writel(*txp++, &qslave->base->spi_data);
writel(qslave->cmd | QSPI_WR_SNGL,
&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) {
if (--timeout < 0) {
printf("QSPI tx timed out\n");
return -1;
}
status = readl(&qslave->base->spi_status);
}
debug("tx done, status %08x\n", status);
}
if (rxp) {
qslave->cmd |= QSPI_RD_SNGL;
debug("rx cmd %08x dc %08x\n",
qslave->cmd, qslave->dc);
writel(qslave->cmd, &qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) {
if (--timeout < 0) {
printf("QSPI rx timed out\n");
return -1;
}
status = readl(&qslave->base->spi_status);
}
*rxp++ = readl(&qslave->base->spi_data);
debug("rx done, status %08x, read %02x\n",
status, *(rxp-1));
}
}
+out:
/* Terminate frame */
if (flags & SPI_XFER_END)
writel(qslave->cmd | QSPI_INVAL, &qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()
I request you please follow the code structure in below thread. I feel better if you use same prints that used in below example driver. http://patchwork.ozlabs.org/patch/265683/

On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote:
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
From: Matt Portermatt.porter@linaro.org
Adds a SPI master driver for the TI QSPI peripheral.
Signed-off-by: Matt Portermatt.porter@linaro.org Signed-off-by: Sourav Poddarsourav.poddar@ti.com [Added quad read support and memory mapped support).
What is this comment, any specific?
This simply tell the portion which i did in the patch.
You missed change log for all patches, i think you have summarized in 0/6. I feel it's better write it on individual patches.
Ok.
drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 0 deletions(-) create mode 100644 drivers/spi/ti_qspi.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 91d24ce..e5941b0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c new file mode 100644 index 0000000..d8a03a8 --- /dev/null +++ b/drivers/spi/ti_qspi.c @@ -0,0 +1,328 @@ +/*
- TI QSPI driver
- Copyright (C) 2013, Texas Instruments, Incorporated
- SPDX-License-Identifier: GPL-2.0+
Got below format after apply this patch - please check *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+
ahh..I copied it from a patch on some list. May be something went wrong, I will check.
- */
+#include<common.h> +#include<asm/io.h> +#include<asm/arch/omap.h> +#include<malloc.h> +#include<spi.h>
+struct qspi_regs { +u32 pid; +u32 pad0[3]; +u32 sysconfig; +u32 pad1[3]; +u32 intr_status_raw_set; +u32 intr_status_enabled_clear; +u32 intr_enable_set; +u32 intr_enable_clear; +u32 intc_eoi; +u32 pad2[3]; +u32 spi_clock_cntrl; +u32 spi_dc; +u32 spi_cmd; +u32 spi_status; +u32 spi_data; +u32 spi_setup0; +u32 spi_setup1; +u32 spi_setup2; +u32 spi_setup3; +u32 spi_switch; +u32 spi_data1; +u32 spi_data2; +u32 spi_data3;
Please add tab space.
ok
+};
+struct qspi_slave {
struct spi_slave slave;
struct qspi_regs *base;
unsigned int mode;
u32 cmd;
u32 dc;
+};
-- TAG+
+#define QSPI_TIMEOUT 2000000
+#define QSPI_FCLK 192000000
+/* Clock Control */ +#define QSPI_CLK_EN (1<< 31) +#define QSPI_CLK_DIV_MAX 0xffff
+/* Command */ +#define QSPI_EN_CS(n) (n<< 28) +#define QSPI_WLEN(n) ((n-1)<< 19) +#define QSPI_3_PIN (1<< 18) +#define QSPI_RD_SNGL (1<< 16) +#define QSPI_WR_SNGL (2<< 16) +#define QSPI_INVAL (4<< 16) +#define QSPI_RD_QUAD (7<< 16)
+/* Device Control */ +#define QSPI_DD(m, n) (m<< (3 + n*8)) +#define QSPI_CKPHA(n) (1<< (2 + n*8)) +#define QSPI_CSPOL(n) (1<< (1 + n*8)) +#define QSPI_CKPOL(n) (1<< (n*8))
+/* Status */ +#define QSPI_WC (1<< 1) +#define QSPI_BUSY (1<< 0) +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) +#define QSPI_XFER_DONE QSPI_WC
+#define MM_SWITCH 0x01 +#define MEM_CS 0x100 +#define MEM_CS_UNSELECT 0xfffff0ff +#define MMAP_START_ADDR 0x5c000000 +#define CORE_CTRL_IO 0x4a002558
+#define QSPI_CMD_READ (0x3<< 0) +#define QSPI_CMD_READ_QUAD (0x6b<< 0) +#define QSPI_CMD_READ_FAST (0x0b<< 0) +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) +#define QSPI_CMD_WRITE (0x2<< 16) +#define QSPI_NUM_DUMMY_BITS (0x0<< 24)
--TAG-
TAG+ ... TAG- please move these macro definitions in below headers
Ok.
+static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave) +{
return container_of(slave, struct qspi_slave, slave);
+} +static inline struct qspi_regs *get_qspi_bus(int dev) +{
if (!dev)
return (struct qspi_regs *)QSPI_BASE;
else
return NULL;
+}
Is this function really required, how many bus controller you have?
Actually one.
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
return 1;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_init(void) +{
/* nothing to do */
+}
+void spi_set_up_spi_register(struct qspi_slave *qslave) +{
u32 memval = 0;
struct spi_slave *slave =&qslave->slave;
slave->memory_map = (void *)MMAP_START_ADDR;
memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
QSPI_SETUP0_NUM_D_BYTES_NO_BITS | QSPI_SETUP0_READ_NORMAL |
QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS);
writel(memval,&qslave->base->spi_setup0);
+}
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint clk_div;
if (!hz)
clk_div = 0;
else
clk_div = (QSPI_FCLK / hz) - 1;
debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div);
/* disable SCLK */
writel(readl(&qslave->base->spi_clock_cntrl)& ~QSPI_CLK_EN,
+&qslave->base->spi_clock_cntrl);
if (clk_div< 0) {
debug("%s: clock divider< 0, using /1 divider\n", __func__);
clk_div = 0;
}
if (clk_div> QSPI_CLK_DIV_MAX) {
debug("%s: clock divider>%d , using /%d divider\n",
__func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX + 1);
clk_div = QSPI_CLK_DIV_MAX;
}
/* enable SCLK */
writel(QSPI_CLK_EN | clk_div,&qslave->base->spi_clock_cntrl);
debug("%s: spi_clock_cntrl %08x\n", __func__,
readl(&qslave->base->spi_clock_cntrl));
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int mode)
+{
struct qspi_slave *qslave;
qslave = spi_alloc_slave(struct qspi_slave, bus, cs);
if (!qslave)
return NULL;
qslave->base = get_qspi_bus(bus);
if (qslave->base)
debug("No Qspi device found\n");
spi_set_speed(&qslave->slave, max_hz);
qslave->mode = mode;
+#ifdef CONFIG_MMAP
spi_set_up_spi_register(qslave);
+#endif
debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode);
return&qslave->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
free(qslave);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
void *din, unsigned long flags)
+{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint words = bitlen>> 3; /* fixed 8-bit word length */
const uchar *txp = dout;
uchar *rxp = din;
uint status;
int timeout, val;
debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", __func__,
slave->bus, slave->cs, bitlen, words, flags);
qslave->dc = 0;
if (qslave->mode& SPI_CPHA)
qslave->dc |= QSPI_CKPHA(slave->cs);
if (qslave->mode& SPI_CPOL)
qslave->dc |= QSPI_CKPOL(slave->cs);
if (qslave->mode& SPI_CS_HIGH)
qslave->dc |= QSPI_CSPOL(slave->cs);
writel(qslave->dc,&qslave->base->spi_dc);
Adjust this code in spi_claim_bus()
Ok.
if (flags& SPI_XFER_MEM_MAP) {
writel(MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val |= MEM_CS;
writel(val, CORE_CTRL_IO);
return 0;
} else if (flags& SPI_XFER_MEM_MAP_END) {
writel(~MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val&= MEM_CS_UNSELECT;
writel(val, CORE_CTRL_IO);
return 0;
}
What is this your are returning from here directly for memory_map? is it?
Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000).
--TAG+
if (bitlen == 0)
goto out;
if (bitlen % 8) {
flags |= SPI_XFER_END;
goto out;
}
--TAG-
TAG+ .. TAG- please move this code on start of the xfer..ie below debug();
I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out.
So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ?
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
while (words--) {
if (txp) {
debug("tx cmd %08x dc %08x data %02x\n",
qslave->cmd | QSPI_WR_SNGL, qslave->dc, *txp);
writel(*txp++,&qslave->base->spi_data);
writel(qslave->cmd | QSPI_WR_SNGL,
+&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) != QSPI_XFER_DONE) {
if (--timeout< 0) {
printf("QSPI tx timed out\n");
return -1;
}
status = readl(&qslave->base->spi_status);
}
debug("tx done, status %08x\n", status);
}
if (rxp) {
qslave->cmd |= QSPI_RD_SNGL;
debug("rx cmd %08x dc %08x\n",
qslave->cmd, qslave->dc);
writel(qslave->cmd,&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) != QSPI_XFER_DONE) {
if (--timeout< 0) {
printf("QSPI rx timed out\n");
return -1;
}
status = readl(&qslave->base->spi_status);
}
*rxp++ = readl(&qslave->base->spi_data);
debug("rx done, status %08x, read %02x\n",
status, *(rxp-1));
}
}
+out:
/* Terminate frame */
if (flags& SPI_XFER_END)
writel(qslave->cmd | QSPI_INVAL,&qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()
I request you please follow the code structure in below thread. I feel better if you use same prints that used in below example driver. http://patchwork.ozlabs.org/patch/265683/

On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddar sourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote:
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
From: Matt Portermatt.porter@linaro.org
Adds a SPI master driver for the TI QSPI peripheral.
Signed-off-by: Matt Portermatt.porter@linaro.org Signed-off-by: Sourav Poddarsourav.poddar@ti.com [Added quad read support and memory mapped support).
What is this comment, any specific?
This simply tell the portion which i did in the patch.
May be not required, bcz it will come after i apply below s-o-b
You missed change log for all patches, i think you have summarized in 0/6. I feel it's better write it on individual patches.
Ok.
drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 0 deletions(-) create mode 100644 drivers/spi/ti_qspi.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 91d24ce..e5941b0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c new file mode 100644 index 0000000..d8a03a8 --- /dev/null +++ b/drivers/spi/ti_qspi.c @@ -0,0 +1,328 @@ +/*
- TI QSPI driver
- Copyright (C) 2013, Texas Instruments, Incorporated
- SPDX-License-Identifier: GPL-2.0+
Got below format after apply this patch - please check *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+
ahh..I copied it from a patch on some list. May be something went wrong, I will check.
- */
+#include<common.h> +#include<asm/io.h> +#include<asm/arch/omap.h> +#include<malloc.h> +#include<spi.h>
+struct qspi_regs { +u32 pid; +u32 pad0[3]; +u32 sysconfig; +u32 pad1[3]; +u32 intr_status_raw_set; +u32 intr_status_enabled_clear; +u32 intr_enable_set; +u32 intr_enable_clear; +u32 intc_eoi; +u32 pad2[3]; +u32 spi_clock_cntrl; +u32 spi_dc; +u32 spi_cmd; +u32 spi_status; +u32 spi_data; +u32 spi_setup0; +u32 spi_setup1; +u32 spi_setup2; +u32 spi_setup3; +u32 spi_switch; +u32 spi_data1; +u32 spi_data2; +u32 spi_data3;
Please add tab space.
ok
+};
+struct qspi_slave {
struct spi_slave slave;
struct qspi_regs *base;
unsigned int mode;
u32 cmd;
u32 dc;
+};
-- TAG+
+#define QSPI_TIMEOUT 2000000
+#define QSPI_FCLK 192000000
+/* Clock Control */ +#define QSPI_CLK_EN (1<< 31) +#define QSPI_CLK_DIV_MAX 0xffff
+/* Command */ +#define QSPI_EN_CS(n) (n<< 28) +#define QSPI_WLEN(n) ((n-1)<< 19) +#define QSPI_3_PIN (1<< 18) +#define QSPI_RD_SNGL (1<< 16) +#define QSPI_WR_SNGL (2<< 16) +#define QSPI_INVAL (4<< 16) +#define QSPI_RD_QUAD (7<< 16)
+/* Device Control */ +#define QSPI_DD(m, n) (m<< (3 + n*8)) +#define QSPI_CKPHA(n) (1<< (2 + n*8)) +#define QSPI_CSPOL(n) (1<< (1 + n*8)) +#define QSPI_CKPOL(n) (1<< (n*8))
+/* Status */ +#define QSPI_WC (1<< 1) +#define QSPI_BUSY (1<< 0) +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) +#define QSPI_XFER_DONE QSPI_WC
+#define MM_SWITCH 0x01 +#define MEM_CS 0x100 +#define MEM_CS_UNSELECT 0xfffff0ff +#define MMAP_START_ADDR 0x5c000000 +#define CORE_CTRL_IO 0x4a002558
+#define QSPI_CMD_READ (0x3<< 0) +#define QSPI_CMD_READ_QUAD (0x6b<< 0) +#define QSPI_CMD_READ_FAST (0x0b<< 0) +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) +#define QSPI_CMD_WRITE (0x2<< 16) +#define QSPI_NUM_DUMMY_BITS (0x0<< 24)
--TAG-
TAG+ ... TAG- please move these macro definitions in below headers
Ok.
+static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave) +{
return container_of(slave, struct qspi_slave, slave);
+} +static inline struct qspi_regs *get_qspi_bus(int dev) +{
if (!dev)
return (struct qspi_regs *)QSPI_BASE;
else
return NULL;
+}
Is this function really required, how many bus controller you have?
Actually one.
Ok, Please remove this function and assign directly.
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
return 1;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_init(void) +{
/* nothing to do */
+}
+void spi_set_up_spi_register(struct qspi_slave *qslave) +{
u32 memval = 0;
struct spi_slave *slave =&qslave->slave;
slave->memory_map = (void *)MMAP_START_ADDR;
memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
QSPI_SETUP0_NUM_D_BYTES_NO_BITS | QSPI_SETUP0_READ_NORMAL
|
QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS);
writel(memval,&qslave->base->spi_setup0);
+}
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint clk_div;
if (!hz)
clk_div = 0;
else
clk_div = (QSPI_FCLK / hz) - 1;
debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div);
/* disable SCLK */
writel(readl(&qslave->base->spi_clock_cntrl)& ~QSPI_CLK_EN,
+&qslave->base->spi_clock_cntrl);
if (clk_div< 0) {
debug("%s: clock divider< 0, using /1 divider\n",
__func__);
clk_div = 0;
}
if (clk_div> QSPI_CLK_DIV_MAX) {
debug("%s: clock divider>%d , using /%d divider\n",
__func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX +
1);
clk_div = QSPI_CLK_DIV_MAX;
}
/* enable SCLK */
writel(QSPI_CLK_EN | clk_div,&qslave->base->spi_clock_cntrl);
debug("%s: spi_clock_cntrl %08x\n", __func__,
readl(&qslave->base->spi_clock_cntrl));
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int mode)
+{
struct qspi_slave *qslave;
qslave = spi_alloc_slave(struct qspi_slave, bus, cs);
if (!qslave)
return NULL;
qslave->base = get_qspi_bus(bus);
if (qslave->base)
debug("No Qspi device found\n");
spi_set_speed(&qslave->slave, max_hz);
qslave->mode = mode;
+#ifdef CONFIG_MMAP
spi_set_up_spi_register(qslave);
+#endif
debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode);
return&qslave->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
free(qslave);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
void *din, unsigned long flags)
+{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint words = bitlen>> 3; /* fixed 8-bit word length */
const uchar *txp = dout;
uchar *rxp = din;
uint status;
int timeout, val;
debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n",
__func__,
slave->bus, slave->cs, bitlen, words, flags);
qslave->dc = 0;
if (qslave->mode& SPI_CPHA)
qslave->dc |= QSPI_CKPHA(slave->cs);
if (qslave->mode& SPI_CPOL)
qslave->dc |= QSPI_CKPOL(slave->cs);
if (qslave->mode& SPI_CS_HIGH)
qslave->dc |= QSPI_CSPOL(slave->cs);
writel(qslave->dc,&qslave->base->spi_dc);
Adjust this code in spi_claim_bus()
Ok.
if (flags& SPI_XFER_MEM_MAP) {
writel(MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val |= MEM_CS;
writel(val, CORE_CTRL_IO);
return 0;
} else if (flags& SPI_XFER_MEM_MAP_END) {
writel(~MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val&= MEM_CS_UNSELECT;
writel(val, CORE_CTRL_IO);
return 0;
}
What is this your are returning from here directly for memory_map? is it?
Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000).
OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code.
Where is SPI_XFER_BEGIN are you not using this?
--TAG+
if (bitlen == 0)
goto out;
if (bitlen % 8) {
flags |= SPI_XFER_END;
goto out;
}
--TAG-
TAG+ .. TAG- please move this code on start of the xfer..ie below debug();
I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out.
So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ?
Sound good, please keep here.
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
while (words--) {
if (txp) {
debug("tx cmd %08x dc %08x data %02x\n",
qslave->cmd | QSPI_WR_SNGL, qslave->dc,
*txp);
writel(*txp++,&qslave->base->spi_data);
writel(qslave->cmd | QSPI_WR_SNGL,
+&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) != QSPI_XFER_DONE)
{
if (--timeout< 0) {
printf("QSPI tx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
debug("tx done, status %08x\n", status);
}
if (rxp) {
qslave->cmd |= QSPI_RD_SNGL;
debug("rx cmd %08x dc %08x\n",
qslave->cmd, qslave->dc);
writel(qslave->cmd,&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) != QSPI_XFER_DONE)
{
if (--timeout< 0) {
printf("QSPI rx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
*rxp++ = readl(&qslave->base->spi_data);
debug("rx done, status %08x, read %02x\n",
status, *(rxp-1));
}
}
+out:
/* Terminate frame */
if (flags& SPI_XFER_END)
writel(qslave->cmd | QSPI_INVAL,&qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()
I request you please follow the code structure in below thread. I feel better if you use same prints that used in below example driver. http://patchwork.ozlabs.org/patch/265683/

On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote:
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
From: Matt Portermatt.porter@linaro.org
Adds a SPI master driver for the TI QSPI peripheral.
Signed-off-by: Matt Portermatt.porter@linaro.org Signed-off-by: Sourav Poddarsourav.poddar@ti.com [Added quad read support and memory mapped support).
What is this comment, any specific?
This simply tell the portion which i did in the patch.
May be not required, bcz it will come after i apply below s-o-b
You missed change log for all patches, i think you have summarized in 0/6. I feel it's better write it on individual patches.
Ok.
drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 0 deletions(-) create mode 100644 drivers/spi/ti_qspi.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 91d24ce..e5941b0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c new file mode 100644 index 0000000..d8a03a8 --- /dev/null +++ b/drivers/spi/ti_qspi.c @@ -0,0 +1,328 @@ +/*
- TI QSPI driver
- Copyright (C) 2013, Texas Instruments, Incorporated
- SPDX-License-Identifier: GPL-2.0+
Got below format after apply this patch - please check *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+
ahh..I copied it from a patch on some list. May be something went wrong, I will check.
- */
+#include<common.h> +#include<asm/io.h> +#include<asm/arch/omap.h> +#include<malloc.h> +#include<spi.h>
+struct qspi_regs { +u32 pid; +u32 pad0[3]; +u32 sysconfig; +u32 pad1[3]; +u32 intr_status_raw_set; +u32 intr_status_enabled_clear; +u32 intr_enable_set; +u32 intr_enable_clear; +u32 intc_eoi; +u32 pad2[3]; +u32 spi_clock_cntrl; +u32 spi_dc; +u32 spi_cmd; +u32 spi_status; +u32 spi_data; +u32 spi_setup0; +u32 spi_setup1; +u32 spi_setup2; +u32 spi_setup3; +u32 spi_switch; +u32 spi_data1; +u32 spi_data2; +u32 spi_data3;
Please add tab space.
ok
+};
+struct qspi_slave {
struct spi_slave slave;
struct qspi_regs *base;
unsigned int mode;
u32 cmd;
u32 dc;
+};
-- TAG+
+#define QSPI_TIMEOUT 2000000
+#define QSPI_FCLK 192000000
+/* Clock Control */ +#define QSPI_CLK_EN (1<< 31) +#define QSPI_CLK_DIV_MAX 0xffff
+/* Command */ +#define QSPI_EN_CS(n) (n<< 28) +#define QSPI_WLEN(n) ((n-1)<< 19) +#define QSPI_3_PIN (1<< 18) +#define QSPI_RD_SNGL (1<< 16) +#define QSPI_WR_SNGL (2<< 16) +#define QSPI_INVAL (4<< 16) +#define QSPI_RD_QUAD (7<< 16)
+/* Device Control */ +#define QSPI_DD(m, n) (m<< (3 + n*8)) +#define QSPI_CKPHA(n) (1<< (2 + n*8)) +#define QSPI_CSPOL(n) (1<< (1 + n*8)) +#define QSPI_CKPOL(n) (1<< (n*8))
+/* Status */ +#define QSPI_WC (1<< 1) +#define QSPI_BUSY (1<< 0) +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) +#define QSPI_XFER_DONE QSPI_WC
+#define MM_SWITCH 0x01 +#define MEM_CS 0x100 +#define MEM_CS_UNSELECT 0xfffff0ff +#define MMAP_START_ADDR 0x5c000000 +#define CORE_CTRL_IO 0x4a002558
+#define QSPI_CMD_READ (0x3<< 0) +#define QSPI_CMD_READ_QUAD (0x6b<< 0) +#define QSPI_CMD_READ_FAST (0x0b<< 0) +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) +#define QSPI_CMD_WRITE (0x2<< 16) +#define QSPI_NUM_DUMMY_BITS (0x0<< 24)
--TAG-
TAG+ ... TAG- please move these macro definitions in below headers
Ok.
+static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave) +{
return container_of(slave, struct qspi_slave, slave);
+} +static inline struct qspi_regs *get_qspi_bus(int dev) +{
if (!dev)
return (struct qspi_regs *)QSPI_BASE;
else
return NULL;
+}
Is this function really required, how many bus controller you have?
Actually one.
Ok, Please remove this function and assign directly.
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
return 1;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_init(void) +{
/* nothing to do */
+}
+void spi_set_up_spi_register(struct qspi_slave *qslave) +{
u32 memval = 0;
struct spi_slave *slave =&qslave->slave;
slave->memory_map = (void *)MMAP_START_ADDR;
memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
QSPI_SETUP0_NUM_D_BYTES_NO_BITS | QSPI_SETUP0_READ_NORMAL
|
QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS);
writel(memval,&qslave->base->spi_setup0);
+}
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint clk_div;
if (!hz)
clk_div = 0;
else
clk_div = (QSPI_FCLK / hz) - 1;
debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div);
/* disable SCLK */
writel(readl(&qslave->base->spi_clock_cntrl)& ~QSPI_CLK_EN,
+&qslave->base->spi_clock_cntrl);
if (clk_div< 0) {
debug("%s: clock divider< 0, using /1 divider\n",
__func__);
clk_div = 0;
}
if (clk_div> QSPI_CLK_DIV_MAX) {
debug("%s: clock divider>%d , using /%d divider\n",
__func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX +
1);
clk_div = QSPI_CLK_DIV_MAX;
}
/* enable SCLK */
writel(QSPI_CLK_EN | clk_div,&qslave->base->spi_clock_cntrl);
debug("%s: spi_clock_cntrl %08x\n", __func__,
readl(&qslave->base->spi_clock_cntrl));
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int mode)
+{
struct qspi_slave *qslave;
qslave = spi_alloc_slave(struct qspi_slave, bus, cs);
if (!qslave)
return NULL;
qslave->base = get_qspi_bus(bus);
if (qslave->base)
debug("No Qspi device found\n");
spi_set_speed(&qslave->slave, max_hz);
qslave->mode = mode;
+#ifdef CONFIG_MMAP
spi_set_up_spi_register(qslave);
+#endif
debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode);
return&qslave->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
free(qslave);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
void *din, unsigned long flags)
+{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint words = bitlen>> 3; /* fixed 8-bit word length */
const uchar *txp = dout;
uchar *rxp = din;
uint status;
int timeout, val;
debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n",
__func__,
slave->bus, slave->cs, bitlen, words, flags);
qslave->dc = 0;
if (qslave->mode& SPI_CPHA)
qslave->dc |= QSPI_CKPHA(slave->cs);
if (qslave->mode& SPI_CPOL)
qslave->dc |= QSPI_CKPOL(slave->cs);
if (qslave->mode& SPI_CS_HIGH)
qslave->dc |= QSPI_CSPOL(slave->cs);
writel(qslave->dc,&qslave->base->spi_dc);
Adjust this code in spi_claim_bus()
Ok.
if (flags& SPI_XFER_MEM_MAP) {
writel(MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val |= MEM_CS;
writel(val, CORE_CTRL_IO);
return 0;
} else if (flags& SPI_XFER_MEM_MAP_END) {
writel(~MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val&= MEM_CS_UNSELECT;
writel(val, CORE_CTRL_IO);
return 0;
}
What is this your are returning from here directly for memory_map? is it?
Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000).
OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code.
I dont think so it make much sense to put it ib cs_activate apis, this portion does not really justify that. This code is more SOC specific control module parameters which allows you to switch between the configuaration port and memory mapped port.
Where is SPI_XFER_BEGIN are you not using this?
We dont need SPI_XFER_BEGIN here, for memory mapped we just need MEM_MAP flag, do the required memory mapped read and end the transfer. This is what differentiate a SPI based read and a memory mapped read.
--TAG+
if (bitlen == 0)
goto out;
if (bitlen % 8) {
flags |= SPI_XFER_END;
goto out;
}
--TAG-
TAG+ .. TAG- please move this code on start of the xfer..ie below debug();
I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out.
So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ?
Sound good, please keep here.
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
while (words--) {
if (txp) {
debug("tx cmd %08x dc %08x data %02x\n",
qslave->cmd | QSPI_WR_SNGL, qslave->dc,
*txp);
writel(*txp++,&qslave->base->spi_data);
writel(qslave->cmd | QSPI_WR_SNGL,
+&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) != QSPI_XFER_DONE)
{
if (--timeout< 0) {
printf("QSPI tx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
debug("tx done, status %08x\n", status);
}
if (rxp) {
qslave->cmd |= QSPI_RD_SNGL;
debug("rx cmd %08x dc %08x\n",
qslave->cmd, qslave->dc);
writel(qslave->cmd,&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) != QSPI_XFER_DONE)
{
if (--timeout< 0) {
printf("QSPI rx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
*rxp++ = readl(&qslave->base->spi_data);
debug("rx done, status %08x, read %02x\n",
status, *(rxp-1));
}
}
+out:
/* Terminate frame */
if (flags& SPI_XFER_END)
writel(qslave->cmd | QSPI_INVAL,&qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()
I request you please follow the code structure in below thread. I feel better if you use same prints that used in below example driver. http://patchwork.ozlabs.org/patch/265683/

On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddar sourav.poddar@ti.com wrote:
On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote:
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
From: Matt Portermatt.porter@linaro.org
Adds a SPI master driver for the TI QSPI peripheral.
Signed-off-by: Matt Portermatt.porter@linaro.org Signed-off-by: Sourav Poddarsourav.poddar@ti.com [Added quad read support and memory mapped support).
What is this comment, any specific?
This simply tell the portion which i did in the patch.
May be not required, bcz it will come after i apply below s-o-b
You missed change log for all patches, i think you have summarized in 0/6. I feel it's better write it on individual patches.
Ok.
drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 0 deletions(-) create mode 100644 drivers/spi/ti_qspi.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 91d24ce..e5941b0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c new file mode 100644 index 0000000..d8a03a8 --- /dev/null +++ b/drivers/spi/ti_qspi.c @@ -0,0 +1,328 @@ +/*
- TI QSPI driver
- Copyright (C) 2013, Texas Instruments, Incorporated
- SPDX-License-Identifier: GPL-2.0+
Got below format after apply this patch - please check *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+
ahh..I copied it from a patch on some list. May be something went wrong, I will check.
- */
+#include<common.h> +#include<asm/io.h> +#include<asm/arch/omap.h> +#include<malloc.h> +#include<spi.h>
+struct qspi_regs { +u32 pid; +u32 pad0[3]; +u32 sysconfig; +u32 pad1[3]; +u32 intr_status_raw_set; +u32 intr_status_enabled_clear; +u32 intr_enable_set; +u32 intr_enable_clear; +u32 intc_eoi; +u32 pad2[3]; +u32 spi_clock_cntrl; +u32 spi_dc; +u32 spi_cmd; +u32 spi_status; +u32 spi_data; +u32 spi_setup0; +u32 spi_setup1; +u32 spi_setup2; +u32 spi_setup3; +u32 spi_switch; +u32 spi_data1; +u32 spi_data2; +u32 spi_data3;
Please add tab space.
ok
+};
+struct qspi_slave {
struct spi_slave slave;
struct qspi_regs *base;
unsigned int mode;
u32 cmd;
u32 dc;
+};
-- TAG+
+#define QSPI_TIMEOUT 2000000
+#define QSPI_FCLK 192000000
+/* Clock Control */ +#define QSPI_CLK_EN (1<< 31) +#define QSPI_CLK_DIV_MAX 0xffff
+/* Command */ +#define QSPI_EN_CS(n) (n<< 28) +#define QSPI_WLEN(n) ((n-1)<< 19) +#define QSPI_3_PIN (1<< 18) +#define QSPI_RD_SNGL (1<< 16) +#define QSPI_WR_SNGL (2<< 16) +#define QSPI_INVAL (4<< 16) +#define QSPI_RD_QUAD (7<< 16)
+/* Device Control */ +#define QSPI_DD(m, n) (m<< (3 + n*8)) +#define QSPI_CKPHA(n) (1<< (2 + n*8)) +#define QSPI_CSPOL(n) (1<< (1 + n*8)) +#define QSPI_CKPOL(n) (1<< (n*8))
+/* Status */ +#define QSPI_WC (1<< 1) +#define QSPI_BUSY (1<< 0) +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) +#define QSPI_XFER_DONE QSPI_WC
+#define MM_SWITCH 0x01 +#define MEM_CS 0x100 +#define MEM_CS_UNSELECT 0xfffff0ff +#define MMAP_START_ADDR 0x5c000000 +#define CORE_CTRL_IO 0x4a002558
+#define QSPI_CMD_READ (0x3<< 0) +#define QSPI_CMD_READ_QUAD (0x6b<< 0) +#define QSPI_CMD_READ_FAST (0x0b<< 0) +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) +#define QSPI_CMD_WRITE (0x2<< 16) +#define QSPI_NUM_DUMMY_BITS (0x0<< 24)
--TAG-
TAG+ ... TAG- please move these macro definitions in below headers
Ok.
+static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave) +{
return container_of(slave, struct qspi_slave, slave);
+} +static inline struct qspi_regs *get_qspi_bus(int dev) +{
if (!dev)
return (struct qspi_regs *)QSPI_BASE;
else
return NULL;
+}
Is this function really required, how many bus controller you have?
Actually one.
Ok, Please remove this function and assign directly.
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
return 1;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_init(void) +{
/* nothing to do */
+}
+void spi_set_up_spi_register(struct qspi_slave *qslave) +{
u32 memval = 0;
struct spi_slave *slave =&qslave->slave;
slave->memory_map = (void *)MMAP_START_ADDR;
memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
QSPI_SETUP0_NUM_D_BYTES_NO_BITS |
QSPI_SETUP0_READ_NORMAL |
QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS);
writel(memval,&qslave->base->spi_setup0);
+}
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint clk_div;
if (!hz)
clk_div = 0;
else
clk_div = (QSPI_FCLK / hz) - 1;
debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div);
/* disable SCLK */
writel(readl(&qslave->base->spi_clock_cntrl)& ~QSPI_CLK_EN,
+&qslave->base->spi_clock_cntrl);
if (clk_div< 0) {
debug("%s: clock divider< 0, using /1 divider\n",
__func__);
clk_div = 0;
}
if (clk_div> QSPI_CLK_DIV_MAX) {
debug("%s: clock divider>%d , using /%d divider\n",
__func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX +
1);
clk_div = QSPI_CLK_DIV_MAX;
}
/* enable SCLK */
writel(QSPI_CLK_EN | clk_div,&qslave->base->spi_clock_cntrl);
debug("%s: spi_clock_cntrl %08x\n", __func__,
readl(&qslave->base->spi_clock_cntrl));
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int
mode) +{
struct qspi_slave *qslave;
qslave = spi_alloc_slave(struct qspi_slave, bus, cs);
if (!qslave)
return NULL;
qslave->base = get_qspi_bus(bus);
if (qslave->base)
debug("No Qspi device found\n");
spi_set_speed(&qslave->slave, max_hz);
qslave->mode = mode;
+#ifdef CONFIG_MMAP
spi_set_up_spi_register(qslave);
+#endif
debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode);
return&qslave->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
free(qslave);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
void *din, unsigned long flags)
+{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint words = bitlen>> 3; /* fixed 8-bit word length */
const uchar *txp = dout;
uchar *rxp = din;
uint status;
int timeout, val;
debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n",
__func__,
slave->bus, slave->cs, bitlen, words, flags);
qslave->dc = 0;
if (qslave->mode& SPI_CPHA)
qslave->dc |= QSPI_CKPHA(slave->cs);
if (qslave->mode& SPI_CPOL)
qslave->dc |= QSPI_CKPOL(slave->cs);
if (qslave->mode& SPI_CS_HIGH)
qslave->dc |= QSPI_CSPOL(slave->cs);
writel(qslave->dc,&qslave->base->spi_dc);
Adjust this code in spi_claim_bus()
Ok.
if (flags& SPI_XFER_MEM_MAP) {
writel(MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val |= MEM_CS;
writel(val, CORE_CTRL_IO);
return 0;
} else if (flags& SPI_XFER_MEM_MAP_END) {
writel(~MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val&= MEM_CS_UNSELECT;
writel(val, CORE_CTRL_IO);
return 0;
}
What is this your are returning from here directly for memory_map? is it?
Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000).
OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code.
I dont think so it make much sense to put it ib cs_activate apis, this portion does not really justify that. This code is more SOC specific control module parameters which allows you to switch between the configuaration port and memory mapped port.
Where is SPI_XFER_BEGIN are you not using this?
We dont need SPI_XFER_BEGIN here, for memory mapped we just need MEM_MAP flag, do the required memory mapped read and end the transfer. This is what differentiate a SPI based read and a memory mapped read.
OK. Understand, let me clear once are you trying to use erase/write from spi_flash or not? The reason for asking if yes, we need to use BEGIN on driver and also these is one more controller already exists on spi where the controller uses read for mmap and erase/write is common - are your driver looks same as this or different?
--TAG+
if (bitlen == 0)
goto out;
if (bitlen % 8) {
flags |= SPI_XFER_END;
goto out;
}
--TAG-
TAG+ .. TAG- please move this code on start of the xfer..ie below debug();
I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out.
So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ?
Sound good, please keep here.
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
while (words--) {
if (txp) {
debug("tx cmd %08x dc %08x data %02x\n",
qslave->cmd | QSPI_WR_SNGL, qslave->dc,
*txp);
writel(*txp++,&qslave->base->spi_data);
writel(qslave->cmd | QSPI_WR_SNGL,
+&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) !=
QSPI_XFER_DONE) {
if (--timeout< 0) {
printf("QSPI tx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
debug("tx done, status %08x\n", status);
}
if (rxp) {
qslave->cmd |= QSPI_RD_SNGL;
debug("rx cmd %08x dc %08x\n",
qslave->cmd, qslave->dc);
writel(qslave->cmd,&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) !=
QSPI_XFER_DONE) {
if (--timeout< 0) {
printf("QSPI rx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
*rxp++ = readl(&qslave->base->spi_data);
debug("rx done, status %08x, read %02x\n",
status, *(rxp-1));
}
}
+out:
/* Terminate frame */
if (flags& SPI_XFER_END)
writel(qslave->cmd |
QSPI_INVAL,&qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()
I request you please follow the code structure in below thread. I feel better if you use same prints that used in below example driver. http://patchwork.ozlabs.org/patch/265683/

On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote:
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
From: Matt Portermatt.porter@linaro.org
Adds a SPI master driver for the TI QSPI peripheral.
Signed-off-by: Matt Portermatt.porter@linaro.org Signed-off-by: Sourav Poddarsourav.poddar@ti.com [Added quad read support and memory mapped support).
What is this comment, any specific?
This simply tell the portion which i did in the patch.
May be not required, bcz it will come after i apply below s-o-b
You missed change log for all patches, i think you have summarized in 0/6. I feel it's better write it on individual patches.
Ok.
drivers/spi/Makefile | 1 + drivers/spi/ti_qspi.c | 328
+++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+), 0 deletions(-) create mode 100644 drivers/spi/ti_qspi.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 91d24ce..e5941b0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c new file mode 100644 index 0000000..d8a03a8 --- /dev/null +++ b/drivers/spi/ti_qspi.c @@ -0,0 +1,328 @@ +/*
- TI QSPI driver
- Copyright (C) 2013, Texas Instruments, Incorporated
- SPDX-License-Identifier: GPL-2.0+
Got below format after apply this patch - please check *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+
ahh..I copied it from a patch on some list. May be something went wrong, I will check.
- */
+#include<common.h> +#include<asm/io.h> +#include<asm/arch/omap.h> +#include<malloc.h> +#include<spi.h>
+struct qspi_regs { +u32 pid; +u32 pad0[3]; +u32 sysconfig; +u32 pad1[3]; +u32 intr_status_raw_set; +u32 intr_status_enabled_clear; +u32 intr_enable_set; +u32 intr_enable_clear; +u32 intc_eoi; +u32 pad2[3]; +u32 spi_clock_cntrl; +u32 spi_dc; +u32 spi_cmd; +u32 spi_status; +u32 spi_data; +u32 spi_setup0; +u32 spi_setup1; +u32 spi_setup2; +u32 spi_setup3; +u32 spi_switch; +u32 spi_data1; +u32 spi_data2; +u32 spi_data3;
Please add tab space.
ok
+};
+struct qspi_slave {
struct spi_slave slave;
struct qspi_regs *base;
unsigned int mode;
u32 cmd;
u32 dc;
+};
-- TAG+
+#define QSPI_TIMEOUT 2000000
+#define QSPI_FCLK 192000000
+/* Clock Control */ +#define QSPI_CLK_EN (1<< 31) +#define QSPI_CLK_DIV_MAX 0xffff
+/* Command */ +#define QSPI_EN_CS(n) (n<< 28) +#define QSPI_WLEN(n) ((n-1)<< 19) +#define QSPI_3_PIN (1<< 18) +#define QSPI_RD_SNGL (1<< 16) +#define QSPI_WR_SNGL (2<< 16) +#define QSPI_INVAL (4<< 16) +#define QSPI_RD_QUAD (7<< 16)
+/* Device Control */ +#define QSPI_DD(m, n) (m<< (3 + n*8)) +#define QSPI_CKPHA(n) (1<< (2 + n*8)) +#define QSPI_CSPOL(n) (1<< (1 + n*8)) +#define QSPI_CKPOL(n) (1<< (n*8))
+/* Status */ +#define QSPI_WC (1<< 1) +#define QSPI_BUSY (1<< 0) +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) +#define QSPI_XFER_DONE QSPI_WC
+#define MM_SWITCH 0x01 +#define MEM_CS 0x100 +#define MEM_CS_UNSELECT 0xfffff0ff +#define MMAP_START_ADDR 0x5c000000 +#define CORE_CTRL_IO 0x4a002558
+#define QSPI_CMD_READ (0x3<< 0) +#define QSPI_CMD_READ_QUAD (0x6b<< 0) +#define QSPI_CMD_READ_FAST (0x0b<< 0) +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) +#define QSPI_CMD_WRITE (0x2<< 16) +#define QSPI_NUM_DUMMY_BITS (0x0<< 24)
--TAG-
TAG+ ... TAG- please move these macro definitions in below headers
Ok.
+static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave) +{
return container_of(slave, struct qspi_slave, slave);
+} +static inline struct qspi_regs *get_qspi_bus(int dev) +{
if (!dev)
return (struct qspi_regs *)QSPI_BASE;
else
return NULL;
+}
Is this function really required, how many bus controller you have?
Actually one.
Ok, Please remove this function and assign directly.
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
return 1;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
/* CS handled in xfer */
return;
+}
+void spi_init(void) +{
/* nothing to do */
+}
+void spi_set_up_spi_register(struct qspi_slave *qslave) +{
u32 memval = 0;
struct spi_slave *slave =&qslave->slave;
slave->memory_map = (void *)MMAP_START_ADDR;
memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
QSPI_SETUP0_NUM_D_BYTES_NO_BITS |
QSPI_SETUP0_READ_NORMAL |
QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS);
writel(memval,&qslave->base->spi_setup0);
+}
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint clk_div;
if (!hz)
clk_div = 0;
else
clk_div = (QSPI_FCLK / hz) - 1;
debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div);
/* disable SCLK */
writel(readl(&qslave->base->spi_clock_cntrl)& ~QSPI_CLK_EN,
+&qslave->base->spi_clock_cntrl);
if (clk_div< 0) {
debug("%s: clock divider< 0, using /1 divider\n",
__func__);
clk_div = 0;
}
if (clk_div> QSPI_CLK_DIV_MAX) {
debug("%s: clock divider>%d , using /%d divider\n",
__func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX +
1);
clk_div = QSPI_CLK_DIV_MAX;
}
/* enable SCLK */
writel(QSPI_CLK_EN | clk_div,&qslave->base->spi_clock_cntrl);
debug("%s: spi_clock_cntrl %08x\n", __func__,
readl(&qslave->base->spi_clock_cntrl));
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int
mode) +{
struct qspi_slave *qslave;
qslave = spi_alloc_slave(struct qspi_slave, bus, cs);
if (!qslave)
return NULL;
qslave->base = get_qspi_bus(bus);
if (qslave->base)
debug("No Qspi device found\n");
spi_set_speed(&qslave->slave, max_hz);
qslave->mode = mode;
+#ifdef CONFIG_MMAP
spi_set_up_spi_register(qslave);
+#endif
debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode);
return&qslave->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
free(qslave);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
struct qspi_slave *qslave = to_qspi_slave(slave);
debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
writel(0,&qslave->base->spi_dc);
writel(0,&qslave->base->spi_cmd);
writel(0,&qslave->base->spi_data);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
void *din, unsigned long flags)
+{
struct qspi_slave *qslave = to_qspi_slave(slave);
uint words = bitlen>> 3; /* fixed 8-bit word length */
const uchar *txp = dout;
uchar *rxp = din;
uint status;
int timeout, val;
debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n",
__func__,
slave->bus, slave->cs, bitlen, words, flags);
qslave->dc = 0;
if (qslave->mode& SPI_CPHA)
qslave->dc |= QSPI_CKPHA(slave->cs);
if (qslave->mode& SPI_CPOL)
qslave->dc |= QSPI_CKPOL(slave->cs);
if (qslave->mode& SPI_CS_HIGH)
qslave->dc |= QSPI_CSPOL(slave->cs);
writel(qslave->dc,&qslave->base->spi_dc);
Adjust this code in spi_claim_bus()
Ok.
if (flags& SPI_XFER_MEM_MAP) {
writel(MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val |= MEM_CS;
writel(val, CORE_CTRL_IO);
return 0;
} else if (flags& SPI_XFER_MEM_MAP_END) {
writel(~MM_SWITCH,&qslave->base->spi_switch);
val = readl(CORE_CTRL_IO);
val&= MEM_CS_UNSELECT;
writel(val, CORE_CTRL_IO);
return 0;
}
What is this your are returning from here directly for memory_map? is it?
Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000).
OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code.
I dont think so it make much sense to put it ib cs_activate apis, this portion does not really justify that. This code is more SOC specific control module parameters which allows you to switch between the configuaration port and memory mapped port.
Where is SPI_XFER_BEGIN are you not using this?
We dont need SPI_XFER_BEGIN here, for memory mapped we just need MEM_MAP flag, do the required memory mapped read and end the transfer. This is what differentiate a SPI based read and a memory mapped read.
OK. Understand, let me clear once are you trying to use erase/write from spi_flash or not? The reason for asking if yes, we need to use BEGIN on driver and also these is one more controller already exists on spi where the controller uses read for mmap and erase/write is common - are your driver looks same as this or different?
Yes, erase and write commands are also done. Generally, BEGIN flag is used sp that cs_activate can be called. But, in my case cs is activated in a different way, its enabled by writing in a command register. So, there is no need for a BEGIN.
--TAG+
if (bitlen == 0)
goto out;
if (bitlen % 8) {
flags |= SPI_XFER_END;
goto out;
}
--TAG-
TAG+ .. TAG- please move this code on start of the xfer..ie below debug();
I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out.
So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ?
Sound good, please keep here.
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
while (words--) {
if (txp) {
debug("tx cmd %08x dc %08x data %02x\n",
qslave->cmd | QSPI_WR_SNGL, qslave->dc,
*txp);
writel(*txp++,&qslave->base->spi_data);
writel(qslave->cmd | QSPI_WR_SNGL,
+&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) !=
QSPI_XFER_DONE) {
if (--timeout< 0) {
printf("QSPI tx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
debug("tx done, status %08x\n", status);
}
if (rxp) {
qslave->cmd |= QSPI_RD_SNGL;
debug("rx cmd %08x dc %08x\n",
qslave->cmd, qslave->dc);
writel(qslave->cmd,&qslave->base->spi_cmd);
status = readl(&qslave->base->spi_status);
timeout = QSPI_TIMEOUT;
while ((status& QSPI_WC_BUSY) !=
QSPI_XFER_DONE) {
if (--timeout< 0) {
printf("QSPI rx timed out\n");
return -1;
}
status =
readl(&qslave->base->spi_status);
}
*rxp++ = readl(&qslave->base->spi_data);
debug("rx done, status %08x, read %02x\n",
status, *(rxp-1));
}
}
+out:
/* Terminate frame */
if (flags& SPI_XFER_END)
writel(qslave->cmd |
QSPI_INVAL,&qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()
I request you please follow the code structure in below thread. I feel better if you use same prints that used in below example driver. http://patchwork.ozlabs.org/patch/265683/

On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddar sourav.poddar@ti.com wrote:
On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote:
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote: > > From: Matt Portermatt.porter@linaro.org > > Adds a SPI master driver for the TI QSPI peripheral. > > Signed-off-by: Matt Portermatt.porter@linaro.org > Signed-off-by: Sourav Poddarsourav.poddar@ti.com > [Added quad read support and memory mapped support).
What is this comment, any specific?
This simply tell the portion which i did in the patch.
May be not required, bcz it will come after i apply below s-o-b
> ---
You missed change log for all patches, i think you have summarized in 0/6. I feel it's better write it on individual patches.
Ok.
> drivers/spi/Makefile | 1 + > drivers/spi/ti_qspi.c | 328 > +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 329 insertions(+), 0 deletions(-) > create mode 100644 drivers/spi/ti_qspi.c > > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile > index 91d24ce..e5941b0 100644 > --- a/drivers/spi/Makefile > +++ b/drivers/spi/Makefile > @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o > COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o > COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o > COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o > +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o > COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o > COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o > > diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c > new file mode 100644 > index 0000000..d8a03a8 > --- /dev/null > +++ b/drivers/spi/ti_qspi.c > @@ -0,0 +1,328 @@ > +/* > + * TI QSPI driver > + * > + * Copyright (C) 2013, Texas Instruments, Incorporated > + * > + * SPDX-License-Identifier: GPL-2.0+
Got below format after apply this patch - please check *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+
ahh..I copied it from a patch on some list. May be something went wrong, I will check.
> + */ > + > +#include<common.h> > +#include<asm/io.h> > +#include<asm/arch/omap.h> > +#include<malloc.h> > +#include<spi.h> > + > +struct qspi_regs { > +u32 pid; > +u32 pad0[3]; > +u32 sysconfig; > +u32 pad1[3]; > +u32 intr_status_raw_set; > +u32 intr_status_enabled_clear; > +u32 intr_enable_set; > +u32 intr_enable_clear; > +u32 intc_eoi; > +u32 pad2[3]; > +u32 spi_clock_cntrl; > +u32 spi_dc; > +u32 spi_cmd; > +u32 spi_status; > +u32 spi_data; > +u32 spi_setup0; > +u32 spi_setup1; > +u32 spi_setup2; > +u32 spi_setup3; > +u32 spi_switch; > +u32 spi_data1; > +u32 spi_data2; > +u32 spi_data3;
Please add tab space.
ok
> +}; > + > +struct qspi_slave { > + struct spi_slave slave; > + struct qspi_regs *base; > + unsigned int mode; > + u32 cmd; > + u32 dc; > +}; > +
-- TAG+ > > +#define QSPI_TIMEOUT 2000000 > + > +#define QSPI_FCLK 192000000 > + > +/* Clock Control */ > +#define QSPI_CLK_EN (1<< 31) > +#define QSPI_CLK_DIV_MAX 0xffff > + > +/* Command */ > +#define QSPI_EN_CS(n) (n<< 28) > +#define QSPI_WLEN(n) ((n-1)<< 19) > +#define QSPI_3_PIN (1<< 18) > +#define QSPI_RD_SNGL (1<< 16) > +#define QSPI_WR_SNGL (2<< 16) > +#define QSPI_INVAL (4<< 16) > +#define QSPI_RD_QUAD (7<< 16) > + > +/* Device Control */ > +#define QSPI_DD(m, n) (m<< (3 + n*8)) > +#define QSPI_CKPHA(n) (1<< (2 + n*8)) > +#define QSPI_CSPOL(n) (1<< (1 + n*8)) > +#define QSPI_CKPOL(n) (1<< (n*8)) > + > +/* Status */ > +#define QSPI_WC (1<< 1) > +#define QSPI_BUSY (1<< 0) > +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) > +#define QSPI_XFER_DONE QSPI_WC > + > +#define MM_SWITCH 0x01 > +#define MEM_CS 0x100 > +#define MEM_CS_UNSELECT 0xfffff0ff > +#define MMAP_START_ADDR 0x5c000000 > +#define CORE_CTRL_IO 0x4a002558 > + > +#define QSPI_CMD_READ (0x3<< 0) > +#define QSPI_CMD_READ_QUAD (0x6b<< 0) > +#define QSPI_CMD_READ_FAST (0x0b<< 0) > +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) > +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) > +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) > +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) > +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) > +#define QSPI_CMD_WRITE (0x2<< 16) > +#define QSPI_NUM_DUMMY_BITS (0x0<< 24)
--TAG-
TAG+ ... TAG- please move these macro definitions in below headers
Ok.
> + > +static inline struct qspi_slave *to_qspi_slave(struct spi_slave > *slave) > +{ > + return container_of(slave, struct qspi_slave, slave); > +} > +static inline struct qspi_regs *get_qspi_bus(int dev) > +{ > + if (!dev) > + return (struct qspi_regs *)QSPI_BASE; > + else > + return NULL; > +}
Is this function really required, how many bus controller you have?
Actually one.
Ok, Please remove this function and assign directly.
> + > +int spi_cs_is_valid(unsigned int bus, unsigned int cs) > +{ > + return 1; > +} > + > +void spi_cs_activate(struct spi_slave *slave) > +{ > + /* CS handled in xfer */ > + return; > +} > + > +void spi_cs_deactivate(struct spi_slave *slave) > +{ > + /* CS handled in xfer */ > + return; > +} > + > +void spi_init(void) > +{ > + /* nothing to do */ > +} > + > +void spi_set_up_spi_register(struct qspi_slave *qslave) > +{ > + u32 memval = 0; > + struct spi_slave *slave =&qslave->slave; > > + > + slave->memory_map = (void *)MMAP_START_ADDR; > + > + memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES | > + QSPI_SETUP0_NUM_D_BYTES_NO_BITS | > QSPI_SETUP0_READ_NORMAL > | > + QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS); > + > + writel(memval,&qslave->base->spi_setup0); > > +} > + > +void spi_set_speed(struct spi_slave *slave, uint hz) > +{ > + struct qspi_slave *qslave = to_qspi_slave(slave); > + > + uint clk_div; > + > + if (!hz) > + clk_div = 0; > + else > + clk_div = (QSPI_FCLK / hz) - 1; > + > + debug("%s: hz: %d, clock divider %d\n", __func__, hz, > clk_div); > + > + /* disable SCLK */ > + writel(readl(&qslave->base->spi_clock_cntrl)& > ~QSPI_CLK_EN, > +&qslave->base->spi_clock_cntrl); > > + > + if (clk_div< 0) { > + debug("%s: clock divider< 0, using /1 divider\n", > __func__); > + clk_div = 0; > + } > + > + if (clk_div> QSPI_CLK_DIV_MAX) { > + debug("%s: clock divider>%d , using /%d divider\n", > + __func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX > + > 1); > + clk_div = QSPI_CLK_DIV_MAX; > + } > + > + /* enable SCLK */ > + writel(QSPI_CLK_EN | clk_div,&qslave->base->spi_clock_cntrl); > > + debug("%s: spi_clock_cntrl %08x\n", __func__, > + readl(&qslave->base->spi_clock_cntrl)); > +} > + > +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > + unsigned int max_hz, unsigned int > mode) > +{ > + struct qspi_slave *qslave; > + > + qslave = spi_alloc_slave(struct qspi_slave, bus, cs); > + if (!qslave) > + return NULL; > + > + qslave->base = get_qspi_bus(bus); > + if (qslave->base) > + debug("No Qspi device found\n"); > + spi_set_speed(&qslave->slave, max_hz); > + qslave->mode = mode; > + > +#ifdef CONFIG_MMAP > + spi_set_up_spi_register(qslave); > +#endif > + > + debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode); > + > + return&qslave->slave; > > +} > + > +void spi_free_slave(struct spi_slave *slave) > +{ > + struct qspi_slave *qslave = to_qspi_slave(slave); > + free(qslave); > +} > + > +int spi_claim_bus(struct spi_slave *slave) > +{ > + struct qspi_slave *qslave = to_qspi_slave(slave); > + > + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs); > + > + writel(0,&qslave->base->spi_dc); > + writel(0,&qslave->base->spi_cmd); > + writel(0,&qslave->base->spi_data); > > + > + return 0; > +} > + > +void spi_release_bus(struct spi_slave *slave) > +{ > + struct qspi_slave *qslave = to_qspi_slave(slave); > + > + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs); > + > + writel(0,&qslave->base->spi_dc); > + writel(0,&qslave->base->spi_cmd); > + writel(0,&qslave->base->spi_data); > > +} > + > +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const > void > *dout, > + void *din, unsigned long flags) > +{ > + struct qspi_slave *qslave = to_qspi_slave(slave); > + uint words = bitlen>> 3; /* fixed 8-bit word length */ > + const uchar *txp = dout; > + uchar *rxp = din; > + uint status; > + int timeout, val; > + > + debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", > __func__, > + slave->bus, slave->cs, bitlen, words, flags); > + > + qslave->dc = 0; > + if (qslave->mode& SPI_CPHA) > > + qslave->dc |= QSPI_CKPHA(slave->cs); > + if (qslave->mode& SPI_CPOL) > > + qslave->dc |= QSPI_CKPOL(slave->cs); > + if (qslave->mode& SPI_CS_HIGH) > > + qslave->dc |= QSPI_CSPOL(slave->cs); > + > + writel(qslave->dc,&qslave->base->spi_dc);
Adjust this code in spi_claim_bus()
Ok.
> > + > + if (flags& SPI_XFER_MEM_MAP) { > + writel(MM_SWITCH,&qslave->base->spi_switch); > > + val = readl(CORE_CTRL_IO); > + val |= MEM_CS; > + writel(val, CORE_CTRL_IO); > + return 0; > + } else if (flags& SPI_XFER_MEM_MAP_END) { > + writel(~MM_SWITCH,&qslave->base->spi_switch); > + val = readl(CORE_CTRL_IO); > + val&= MEM_CS_UNSELECT; > > + writel(val, CORE_CTRL_IO); > + return 0; > + }
What is this your are returning from here directly for memory_map? is it?
Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000).
OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code.
I dont think so it make much sense to put it ib cs_activate apis, this portion does not really justify that. This code is more SOC specific control module parameters which allows you to switch between the configuaration port and memory mapped port.
Where is SPI_XFER_BEGIN are you not using this?
We dont need SPI_XFER_BEGIN here, for memory mapped we just need MEM_MAP flag, do the required memory mapped read and end the transfer. This is what differentiate a SPI based read and a memory mapped read.
OK. Understand, let me clear once are you trying to use erase/write from spi_flash or not? The reason for asking if yes, we need to use BEGIN on driver and also these is one more controller already exists on spi where the controller uses read for mmap and erase/write is common - are your driver looks same as this or different?
Yes, erase and write commands are also done. Generally, BEGIN flag is used sp that cs_activate can be called. But, in my case cs is activated in a different way, its enabled by writing in a command register. So, there is no need for a BEGIN.
OK, please do all modifications as we discussed so-far. and send the next level- so we can discuss more.
Please follow the driver code model- what I referred in previous mail.
> +
--TAG+ > > + if (bitlen == 0) > + goto out; > + > + if (bitlen % 8) { > + flags |= SPI_XFER_END; > + goto out; > + }
--TAG-
TAG+ .. TAG- please move this code on start of the xfer..ie below debug();
I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out.
So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ?
Sound good, please keep here.
> + > + /* setup command reg */ > + qslave->cmd = 0; > + qslave->cmd |= QSPI_WLEN(8); > + qslave->cmd |= QSPI_EN_CS(slave->cs); > + if (flags& SPI_3WIRE) > > + qslave->cmd |= QSPI_3_PIN; > + qslave->cmd |= 0xfff; > + > + while (words--) { > + if (txp) { > + debug("tx cmd %08x dc %08x data %02x\n", > + qslave->cmd | QSPI_WR_SNGL, qslave->dc, > *txp); > + writel(*txp++,&qslave->base->spi_data); > + writel(qslave->cmd | QSPI_WR_SNGL, > +&qslave->base->spi_cmd); > > + status = readl(&qslave->base->spi_status); > + timeout = QSPI_TIMEOUT; > + while ((status& QSPI_WC_BUSY) != > QSPI_XFER_DONE) > { > > + if (--timeout< 0) { > + printf("QSPI tx timed > out\n"); > + return -1; > + } > + status = > readl(&qslave->base->spi_status); > + } > + debug("tx done, status %08x\n", status); > + } > + if (rxp) { > + qslave->cmd |= QSPI_RD_SNGL; > + debug("rx cmd %08x dc %08x\n", > + qslave->cmd, qslave->dc); > + writel(qslave->cmd,&qslave->base->spi_cmd); > > + status = readl(&qslave->base->spi_status); > + timeout = QSPI_TIMEOUT; > + while ((status& QSPI_WC_BUSY) != > QSPI_XFER_DONE) > { > > + if (--timeout< 0) { > + printf("QSPI rx timed > out\n"); > + return -1; > + } > + status = > readl(&qslave->base->spi_status); > + } > + *rxp++ = readl(&qslave->base->spi_data); > + debug("rx done, status %08x, read %02x\n", > + status, *(rxp-1)); > + } > + } > + > +out: > + /* Terminate frame */ > + if (flags& SPI_XFER_END) > + writel(qslave->cmd | > QSPI_INVAL,&qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()
I request you please follow the code structure in below thread. I feel better if you use same prints that used in below example driver. http://patchwork.ozlabs.org/patch/265683/

On Saturday 05 October 2013 05:10 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote: > On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com > wrote: >> From: Matt Portermatt.porter@linaro.org >> >> Adds a SPI master driver for the TI QSPI peripheral. >> >> Signed-off-by: Matt Portermatt.porter@linaro.org >> Signed-off-by: Sourav Poddarsourav.poddar@ti.com >> [Added quad read support and memory mapped support). > What is this comment, any specific? This simply tell the portion which i did in the patch.
May be not required, bcz it will come after i apply below s-o-b
>> --- > You missed change log for all patches, i think you have summarized in > 0/6. > I feel it's better write it on individual patches. > Ok.
>> drivers/spi/Makefile | 1 + >> drivers/spi/ti_qspi.c | 328 >> +++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 329 insertions(+), 0 deletions(-) >> create mode 100644 drivers/spi/ti_qspi.c >> >> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile >> index 91d24ce..e5941b0 100644 >> --- a/drivers/spi/Makefile >> +++ b/drivers/spi/Makefile >> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o >> COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o >> COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o >> COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o >> +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o >> COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o >> COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o >> >> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >> new file mode 100644 >> index 0000000..d8a03a8 >> --- /dev/null >> +++ b/drivers/spi/ti_qspi.c >> @@ -0,0 +1,328 @@ >> +/* >> + * TI QSPI driver >> + * >> + * Copyright (C) 2013, Texas Instruments, Incorporated >> + * >> + * SPDX-License-Identifier: GPL-2.0+ > Got below format after apply this patch - please check > *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+ > ahh..I copied it from a patch on some list. May be something went wrong, I will check.
>> + */ >> + >> +#include<common.h> >> +#include<asm/io.h> >> +#include<asm/arch/omap.h> >> +#include<malloc.h> >> +#include<spi.h> >> + >> +struct qspi_regs { >> +u32 pid; >> +u32 pad0[3]; >> +u32 sysconfig; >> +u32 pad1[3]; >> +u32 intr_status_raw_set; >> +u32 intr_status_enabled_clear; >> +u32 intr_enable_set; >> +u32 intr_enable_clear; >> +u32 intc_eoi; >> +u32 pad2[3]; >> +u32 spi_clock_cntrl; >> +u32 spi_dc; >> +u32 spi_cmd; >> +u32 spi_status; >> +u32 spi_data; >> +u32 spi_setup0; >> +u32 spi_setup1; >> +u32 spi_setup2; >> +u32 spi_setup3; >> +u32 spi_switch; >> +u32 spi_data1; >> +u32 spi_data2; >> +u32 spi_data3; > Please add tab space. > ok
>> +}; >> + >> +struct qspi_slave { >> + struct spi_slave slave; >> + struct qspi_regs *base; >> + unsigned int mode; >> + u32 cmd; >> + u32 dc; >> +}; >> + > -- TAG+ >> +#define QSPI_TIMEOUT 2000000 >> + >> +#define QSPI_FCLK 192000000 >> + >> +/* Clock Control */ >> +#define QSPI_CLK_EN (1<< 31) >> +#define QSPI_CLK_DIV_MAX 0xffff >> + >> +/* Command */ >> +#define QSPI_EN_CS(n) (n<< 28) >> +#define QSPI_WLEN(n) ((n-1)<< 19) >> +#define QSPI_3_PIN (1<< 18) >> +#define QSPI_RD_SNGL (1<< 16) >> +#define QSPI_WR_SNGL (2<< 16) >> +#define QSPI_INVAL (4<< 16) >> +#define QSPI_RD_QUAD (7<< 16) >> + >> +/* Device Control */ >> +#define QSPI_DD(m, n) (m<< (3 + n*8)) >> +#define QSPI_CKPHA(n) (1<< (2 + n*8)) >> +#define QSPI_CSPOL(n) (1<< (1 + n*8)) >> +#define QSPI_CKPOL(n) (1<< (n*8)) >> + >> +/* Status */ >> +#define QSPI_WC (1<< 1) >> +#define QSPI_BUSY (1<< 0) >> +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) >> +#define QSPI_XFER_DONE QSPI_WC >> + >> +#define MM_SWITCH 0x01 >> +#define MEM_CS 0x100 >> +#define MEM_CS_UNSELECT 0xfffff0ff >> +#define MMAP_START_ADDR 0x5c000000 >> +#define CORE_CTRL_IO 0x4a002558 >> + >> +#define QSPI_CMD_READ (0x3<< 0) >> +#define QSPI_CMD_READ_QUAD (0x6b<< 0) >> +#define QSPI_CMD_READ_FAST (0x0b<< 0) >> +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) >> +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) >> +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) >> +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) >> +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) >> +#define QSPI_CMD_WRITE (0x2<< 16) >> +#define QSPI_NUM_DUMMY_BITS (0x0<< 24) > --TAG- > > TAG+ ... TAG- please move these macro definitions in below headers Ok.
>> + >> +static inline struct qspi_slave *to_qspi_slave(struct spi_slave >> *slave) >> +{ >> + return container_of(slave, struct qspi_slave, slave); >> +} >> +static inline struct qspi_regs *get_qspi_bus(int dev) >> +{ >> + if (!dev) >> + return (struct qspi_regs *)QSPI_BASE; >> + else >> + return NULL; >> +} > Is this function really required, how many bus controller you have? Actually one.
Ok, Please remove this function and assign directly.
>> + >> +int spi_cs_is_valid(unsigned int bus, unsigned int cs) >> +{ >> + return 1; >> +} >> + >> +void spi_cs_activate(struct spi_slave *slave) >> +{ >> + /* CS handled in xfer */ >> + return; >> +} >> + >> +void spi_cs_deactivate(struct spi_slave *slave) >> +{ >> + /* CS handled in xfer */ >> + return; >> +} >> + >> +void spi_init(void) >> +{ >> + /* nothing to do */ >> +} >> + >> +void spi_set_up_spi_register(struct qspi_slave *qslave) >> +{ >> + u32 memval = 0; >> + struct spi_slave *slave =&qslave->slave; >> >> + >> + slave->memory_map = (void *)MMAP_START_ADDR; >> + >> + memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES | >> + QSPI_SETUP0_NUM_D_BYTES_NO_BITS | >> QSPI_SETUP0_READ_NORMAL >> | >> + QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS); >> + >> + writel(memval,&qslave->base->spi_setup0); >> >> +} >> + >> +void spi_set_speed(struct spi_slave *slave, uint hz) >> +{ >> + struct qspi_slave *qslave = to_qspi_slave(slave); >> + >> + uint clk_div; >> + >> + if (!hz) >> + clk_div = 0; >> + else >> + clk_div = (QSPI_FCLK / hz) - 1; >> + >> + debug("%s: hz: %d, clock divider %d\n", __func__, hz, >> clk_div); >> + >> + /* disable SCLK */ >> + writel(readl(&qslave->base->spi_clock_cntrl)& >> ~QSPI_CLK_EN, >> +&qslave->base->spi_clock_cntrl); >> >> + >> + if (clk_div< 0) { >> + debug("%s: clock divider< 0, using /1 divider\n", >> __func__); >> + clk_div = 0; >> + } >> + >> + if (clk_div> QSPI_CLK_DIV_MAX) { >> + debug("%s: clock divider>%d , using /%d divider\n", >> + __func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX >> + >> 1); >> + clk_div = QSPI_CLK_DIV_MAX; >> + } >> + >> + /* enable SCLK */ >> + writel(QSPI_CLK_EN | clk_div,&qslave->base->spi_clock_cntrl); >> >> + debug("%s: spi_clock_cntrl %08x\n", __func__, >> + readl(&qslave->base->spi_clock_cntrl)); >> +} >> + >> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, >> + unsigned int max_hz, unsigned int >> mode) >> +{ >> + struct qspi_slave *qslave; >> + >> + qslave = spi_alloc_slave(struct qspi_slave, bus, cs); >> + if (!qslave) >> + return NULL; >> + >> + qslave->base = get_qspi_bus(bus); >> + if (qslave->base) >> + debug("No Qspi device found\n"); >> + spi_set_speed(&qslave->slave, max_hz); >> + qslave->mode = mode; >> + >> +#ifdef CONFIG_MMAP >> + spi_set_up_spi_register(qslave); >> +#endif >> + >> + debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode); >> + >> + return&qslave->slave; >> >> +} >> + >> +void spi_free_slave(struct spi_slave *slave) >> +{ >> + struct qspi_slave *qslave = to_qspi_slave(slave); >> + free(qslave); >> +} >> + >> +int spi_claim_bus(struct spi_slave *slave) >> +{ >> + struct qspi_slave *qslave = to_qspi_slave(slave); >> + >> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs); >> + >> + writel(0,&qslave->base->spi_dc); >> + writel(0,&qslave->base->spi_cmd); >> + writel(0,&qslave->base->spi_data); >> >> + >> + return 0; >> +} >> + >> +void spi_release_bus(struct spi_slave *slave) >> +{ >> + struct qspi_slave *qslave = to_qspi_slave(slave); >> + >> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs); >> + >> + writel(0,&qslave->base->spi_dc); >> + writel(0,&qslave->base->spi_cmd); >> + writel(0,&qslave->base->spi_data); >> >> +} >> + >> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const >> void >> *dout, >> + void *din, unsigned long flags) >> +{ >> + struct qspi_slave *qslave = to_qspi_slave(slave); >> + uint words = bitlen>> 3; /* fixed 8-bit word length */ >> + const uchar *txp = dout; >> + uchar *rxp = din; >> + uint status; >> + int timeout, val; >> + >> + debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", >> __func__, >> + slave->bus, slave->cs, bitlen, words, flags); >> + >> + qslave->dc = 0; >> + if (qslave->mode& SPI_CPHA) >> >> + qslave->dc |= QSPI_CKPHA(slave->cs); >> + if (qslave->mode& SPI_CPOL) >> >> + qslave->dc |= QSPI_CKPOL(slave->cs); >> + if (qslave->mode& SPI_CS_HIGH) >> >> + qslave->dc |= QSPI_CSPOL(slave->cs); >> + >> + writel(qslave->dc,&qslave->base->spi_dc); > Adjust this code in spi_claim_bus() > Ok. >> + >> + if (flags& SPI_XFER_MEM_MAP) { >> + writel(MM_SWITCH,&qslave->base->spi_switch); >> >> + val = readl(CORE_CTRL_IO); >> + val |= MEM_CS; >> + writel(val, CORE_CTRL_IO); >> + return 0; >> + } else if (flags& SPI_XFER_MEM_MAP_END) { >> + writel(~MM_SWITCH,&qslave->base->spi_switch); >> + val = readl(CORE_CTRL_IO); >> + val&= MEM_CS_UNSELECT; >> >> + writel(val, CORE_CTRL_IO); >> + return 0; >> + } > What is this your are returning from here directly for memory_map? is > it? Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000).
OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code.
I dont think so it make much sense to put it ib cs_activate apis, this portion does not really justify that. This code is more SOC specific control module parameters which allows you to switch between the configuaration port and memory mapped port.
Where is SPI_XFER_BEGIN are you not using this?
We dont need SPI_XFER_BEGIN here, for memory mapped we just need MEM_MAP flag, do the required memory mapped read and end the transfer. This is what differentiate a SPI based read and a memory mapped read.
OK. Understand, let me clear once are you trying to use erase/write from spi_flash or not? The reason for asking if yes, we need to use BEGIN on driver and also these is one more controller already exists on spi where the controller uses read for mmap and erase/write is common - are your driver looks same as this or different?
Yes, erase and write commands are also done. Generally, BEGIN flag is used sp that cs_activate can be called. But, in my case cs is activated in a different way, its enabled by writing in a command register. So, there is no need for a BEGIN.
OK, please do all modifications as we discussed so-far. and send the next level- so we can discuss more.
Please follow the driver code model- what I referred in previous mail.
Ok. I will send the v5 with all the discussed modification.
>> + > --TAG+ >> + if (bitlen == 0) >> + goto out; >> + >> + if (bitlen % 8) { >> + flags |= SPI_XFER_END; >> + goto out; >> + } > --TAG- > > TAG+ .. TAG- please move this code on start of the xfer..ie below > debug(); > I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out.
So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ?
Sound good, please keep here.
>> + >> + /* setup command reg */ >> + qslave->cmd = 0; >> + qslave->cmd |= QSPI_WLEN(8); >> + qslave->cmd |= QSPI_EN_CS(slave->cs); >> + if (flags& SPI_3WIRE) >> >> + qslave->cmd |= QSPI_3_PIN; >> + qslave->cmd |= 0xfff; >> + >> + while (words--) { >> + if (txp) { >> + debug("tx cmd %08x dc %08x data %02x\n", >> + qslave->cmd | QSPI_WR_SNGL, qslave->dc, >> *txp); >> + writel(*txp++,&qslave->base->spi_data); >> + writel(qslave->cmd | QSPI_WR_SNGL, >> +&qslave->base->spi_cmd); >> >> + status = readl(&qslave->base->spi_status); >> + timeout = QSPI_TIMEOUT; >> + while ((status& QSPI_WC_BUSY) != >> QSPI_XFER_DONE) >> { >> >> + if (--timeout< 0) { >> + printf("QSPI tx timed >> out\n"); >> + return -1; >> + } >> + status = >> readl(&qslave->base->spi_status); >> + } >> + debug("tx done, status %08x\n", status); >> + } >> + if (rxp) { >> + qslave->cmd |= QSPI_RD_SNGL; >> + debug("rx cmd %08x dc %08x\n", >> + qslave->cmd, qslave->dc); >> + writel(qslave->cmd,&qslave->base->spi_cmd); >> >> + status = readl(&qslave->base->spi_status); >> + timeout = QSPI_TIMEOUT; >> + while ((status& QSPI_WC_BUSY) != >> QSPI_XFER_DONE) >> { >> >> + if (--timeout< 0) { >> + printf("QSPI rx timed >> out\n"); >> + return -1; >> + } >> + status = >> readl(&qslave->base->spi_status); >> + } >> + *rxp++ = readl(&qslave->base->spi_data); >> + debug("rx done, status %08x, read %02x\n", >> + status, *(rxp-1)); >> + } >> + } >> + >> +out: >> + /* Terminate frame */ >> + if (flags& SPI_XFER_END) >> + writel(qslave->cmd | >> QSPI_INVAL,&qslave->base->spi_cmd); > Please palce this code in spi_cs_deactivate() > > I request you please follow the code structure in below thread. > I feel better if you use same prints that used in below example > driver. > http://patchwork.ozlabs.org/patch/265683/

What if this code is placed in cs_active() with BEGIN flag.? + /* setup command reg */ + qslave->cmd = 0; + qslave->cmd |= QSPI_WLEN(8); + qslave->cmd |= QSPI_EN_CS(slave->cs); + if (flags & SPI_3WIRE) + qslave->cmd |= QSPI_3_PIN; + qslave->cmd |= 0xfff;
On Sat, Oct 5, 2013 at 7:53 PM, Sourav Poddar sourav.poddar@ti.com wrote:
On Saturday 05 October 2013 05:10 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com wrote: > > On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote: >> >> On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com >> wrote: >>> >>> From: Matt Portermatt.porter@linaro.org >>> >>> Adds a SPI master driver for the TI QSPI peripheral. >>> >>> Signed-off-by: Matt Portermatt.porter@linaro.org >>> Signed-off-by: Sourav Poddarsourav.poddar@ti.com >>> [Added quad read support and memory mapped support). >> >> What is this comment, any specific? > > This simply tell the portion which i did in the patch.
May be not required, bcz it will come after i apply below s-o-b
>>> --- >> >> You missed change log for all patches, i think you have summarized >> in >> 0/6. >> I feel it's better write it on individual patches. >> > Ok. > >>> drivers/spi/Makefile | 1 + >>> drivers/spi/ti_qspi.c | 328 >>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 329 insertions(+), 0 deletions(-) >>> create mode 100644 drivers/spi/ti_qspi.c >>> >>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile >>> index 91d24ce..e5941b0 100644 >>> --- a/drivers/spi/Makefile >>> +++ b/drivers/spi/Makefile >>> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o >>> COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o >>> COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o >>> COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o >>> +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o >>> COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o >>> COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o >>> >>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >>> new file mode 100644 >>> index 0000000..d8a03a8 >>> --- /dev/null >>> +++ b/drivers/spi/ti_qspi.c >>> @@ -0,0 +1,328 @@ >>> +/* >>> + * TI QSPI driver >>> + * >>> + * Copyright (C) 2013, Texas Instruments, Incorporated >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >> >> Got below format after apply this patch - please check >> *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+ >> > ahh..I copied it from a patch on some list. May be something went > wrong, > I > will check. > >>> + */ >>> + >>> +#include<common.h> >>> +#include<asm/io.h> >>> +#include<asm/arch/omap.h> >>> +#include<malloc.h> >>> +#include<spi.h> >>> + >>> +struct qspi_regs { >>> +u32 pid; >>> +u32 pad0[3]; >>> +u32 sysconfig; >>> +u32 pad1[3]; >>> +u32 intr_status_raw_set; >>> +u32 intr_status_enabled_clear; >>> +u32 intr_enable_set; >>> +u32 intr_enable_clear; >>> +u32 intc_eoi; >>> +u32 pad2[3]; >>> +u32 spi_clock_cntrl; >>> +u32 spi_dc; >>> +u32 spi_cmd; >>> +u32 spi_status; >>> +u32 spi_data; >>> +u32 spi_setup0; >>> +u32 spi_setup1; >>> +u32 spi_setup2; >>> +u32 spi_setup3; >>> +u32 spi_switch; >>> +u32 spi_data1; >>> +u32 spi_data2; >>> +u32 spi_data3; >> >> Please add tab space. >> > ok > >>> +}; >>> + >>> +struct qspi_slave { >>> + struct spi_slave slave; >>> + struct qspi_regs *base; >>> + unsigned int mode; >>> + u32 cmd; >>> + u32 dc; >>> +}; >>> + >> >> -- TAG+ >>> >>> +#define QSPI_TIMEOUT 2000000 >>> + >>> +#define QSPI_FCLK 192000000 >>> + >>> +/* Clock Control */ >>> +#define QSPI_CLK_EN (1<< 31) >>> +#define QSPI_CLK_DIV_MAX 0xffff >>> + >>> +/* Command */ >>> +#define QSPI_EN_CS(n) (n<< 28) >>> +#define QSPI_WLEN(n) ((n-1)<< 19) >>> +#define QSPI_3_PIN (1<< 18) >>> +#define QSPI_RD_SNGL (1<< 16) >>> +#define QSPI_WR_SNGL (2<< 16) >>> +#define QSPI_INVAL (4<< 16) >>> +#define QSPI_RD_QUAD (7<< 16) >>> + >>> +/* Device Control */ >>> +#define QSPI_DD(m, n) (m<< (3 + n*8)) >>> +#define QSPI_CKPHA(n) (1<< (2 + n*8)) >>> +#define QSPI_CSPOL(n) (1<< (1 + n*8)) >>> +#define QSPI_CKPOL(n) (1<< (n*8)) >>> + >>> +/* Status */ >>> +#define QSPI_WC (1<< 1) >>> +#define QSPI_BUSY (1<< 0) >>> +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) >>> +#define QSPI_XFER_DONE QSPI_WC >>> + >>> +#define MM_SWITCH 0x01 >>> +#define MEM_CS 0x100 >>> +#define MEM_CS_UNSELECT 0xfffff0ff >>> +#define MMAP_START_ADDR 0x5c000000 >>> +#define CORE_CTRL_IO 0x4a002558 >>> + >>> +#define QSPI_CMD_READ (0x3<< 0) >>> +#define QSPI_CMD_READ_QUAD (0x6b<< 0) >>> +#define QSPI_CMD_READ_FAST (0x0b<< 0) >>> +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) >>> +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) >>> +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) >>> +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) >>> +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) >>> +#define QSPI_CMD_WRITE (0x2<< 16) >>> +#define QSPI_NUM_DUMMY_BITS (0x0<< 24) >> >> --TAG- >> >> TAG+ ... TAG- please move these macro definitions in below headers > > Ok. > >>> + >>> +static inline struct qspi_slave *to_qspi_slave(struct spi_slave >>> *slave) >>> +{ >>> + return container_of(slave, struct qspi_slave, slave); >>> +} >>> +static inline struct qspi_regs *get_qspi_bus(int dev) >>> +{ >>> + if (!dev) >>> + return (struct qspi_regs *)QSPI_BASE; >>> + else >>> + return NULL; >>> +} >> >> Is this function really required, how many bus controller you have? > > Actually one.
Ok, Please remove this function and assign directly.
>>> + >>> +int spi_cs_is_valid(unsigned int bus, unsigned int cs) >>> +{ >>> + return 1; >>> +} >>> + >>> +void spi_cs_activate(struct spi_slave *slave) >>> +{ >>> + /* CS handled in xfer */ >>> + return; >>> +} >>> + >>> +void spi_cs_deactivate(struct spi_slave *slave) >>> +{ >>> + /* CS handled in xfer */ >>> + return; >>> +} >>> + >>> +void spi_init(void) >>> +{ >>> + /* nothing to do */ >>> +} >>> + >>> +void spi_set_up_spi_register(struct qspi_slave *qslave) >>> +{ >>> + u32 memval = 0; >>> + struct spi_slave *slave =&qslave->slave; >>> >>> + >>> + slave->memory_map = (void *)MMAP_START_ADDR; >>> + >>> + memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES | >>> + QSPI_SETUP0_NUM_D_BYTES_NO_BITS | >>> QSPI_SETUP0_READ_NORMAL >>> | >>> + QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS); >>> + >>> + writel(memval,&qslave->base->spi_setup0); >>> >>> +} >>> + >>> +void spi_set_speed(struct spi_slave *slave, uint hz) >>> +{ >>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>> + >>> + uint clk_div; >>> + >>> + if (!hz) >>> + clk_div = 0; >>> + else >>> + clk_div = (QSPI_FCLK / hz) - 1; >>> + >>> + debug("%s: hz: %d, clock divider %d\n", __func__, hz, >>> clk_div); >>> + >>> + /* disable SCLK */ >>> + writel(readl(&qslave->base->spi_clock_cntrl)& >>> ~QSPI_CLK_EN, >>> +&qslave->base->spi_clock_cntrl); >>> >>> + >>> + if (clk_div< 0) { >>> + debug("%s: clock divider< 0, using /1 >>> divider\n", >>> __func__); >>> + clk_div = 0; >>> + } >>> + >>> + if (clk_div> QSPI_CLK_DIV_MAX) { >>> + debug("%s: clock divider>%d , using /%d divider\n", >>> + __func__, QSPI_CLK_DIV_MAX, >>> QSPI_CLK_DIV_MAX >>> + >>> 1); >>> + clk_div = QSPI_CLK_DIV_MAX; >>> + } >>> + >>> + /* enable SCLK */ >>> + writel(QSPI_CLK_EN | >>> clk_div,&qslave->base->spi_clock_cntrl); >>> >>> + debug("%s: spi_clock_cntrl %08x\n", __func__, >>> + readl(&qslave->base->spi_clock_cntrl)); >>> +} >>> + >>> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int >>> cs, >>> + unsigned int max_hz, unsigned int >>> mode) >>> +{ >>> + struct qspi_slave *qslave; >>> + >>> + qslave = spi_alloc_slave(struct qspi_slave, bus, cs); >>> + if (!qslave) >>> + return NULL; >>> + >>> + qslave->base = get_qspi_bus(bus); >>> + if (qslave->base) >>> + debug("No Qspi device found\n"); >>> + spi_set_speed(&qslave->slave, max_hz); >>> + qslave->mode = mode; >>> + >>> +#ifdef CONFIG_MMAP >>> + spi_set_up_spi_register(qslave); >>> +#endif >>> + >>> + debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, >>> mode); >>> + >>> + return&qslave->slave; >>> >>> +} >>> + >>> +void spi_free_slave(struct spi_slave *slave) >>> +{ >>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>> + free(qslave); >>> +} >>> + >>> +int spi_claim_bus(struct spi_slave *slave) >>> +{ >>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>> + >>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>> slave->cs); >>> + >>> + writel(0,&qslave->base->spi_dc); >>> + writel(0,&qslave->base->spi_cmd); >>> + writel(0,&qslave->base->spi_data); >>> >>> + >>> + return 0; >>> +} >>> + >>> +void spi_release_bus(struct spi_slave *slave) >>> +{ >>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>> + >>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>> slave->cs); >>> + >>> + writel(0,&qslave->base->spi_dc); >>> + writel(0,&qslave->base->spi_cmd); >>> + writel(0,&qslave->base->spi_data); >>> >>> +} >>> + >>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const >>> void >>> *dout, >>> + void *din, unsigned long flags) >>> +{ >>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>> + uint words = bitlen>> 3; /* fixed 8-bit word length */ >>> + const uchar *txp = dout; >>> + uchar *rxp = din; >>> + uint status; >>> + int timeout, val; >>> + >>> + debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", >>> __func__, >>> + slave->bus, slave->cs, bitlen, words, flags); >>> + >>> + qslave->dc = 0; >>> + if (qslave->mode& SPI_CPHA) >>> >>> + qslave->dc |= QSPI_CKPHA(slave->cs); >>> + if (qslave->mode& SPI_CPOL) >>> >>> + qslave->dc |= QSPI_CKPOL(slave->cs); >>> + if (qslave->mode& SPI_CS_HIGH) >>> >>> + qslave->dc |= QSPI_CSPOL(slave->cs); >>> + >>> + writel(qslave->dc,&qslave->base->spi_dc); >> >> Adjust this code in spi_claim_bus() >> > Ok. >>> >>> + >>> + if (flags& SPI_XFER_MEM_MAP) { >>> + writel(MM_SWITCH,&qslave->base->spi_switch); >>> >>> + val = readl(CORE_CTRL_IO); >>> + val |= MEM_CS; >>> + writel(val, CORE_CTRL_IO); >>> + return 0; >>> + } else if (flags& SPI_XFER_MEM_MAP_END) { >>> + writel(~MM_SWITCH,&qslave->base->spi_switch); >>> + val = readl(CORE_CTRL_IO); >>> + val&= MEM_CS_UNSELECT; >>> >>> + writel(val, CORE_CTRL_IO); >>> + return 0; >>> + } >> >> What is this your are returning from here directly for memory_map? >> is >> it? > > Yes, memory map does not care about setting the command register and > doing the transfer using the normal spi framework. > Memory ma has a different set of registers, set up > register(configured > above). > Here, we just switch the controller to memory mapped port and use > flash->memory_map > to read the data from the memory mapped port(0x5c000000).
OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code.
I dont think so it make much sense to put it ib cs_activate apis, this portion does not really justify that. This code is more SOC specific control module parameters which allows you to switch between the configuaration port and memory mapped port.
Where is SPI_XFER_BEGIN are you not using this?
We dont need SPI_XFER_BEGIN here, for memory mapped we just need MEM_MAP flag, do the required memory mapped read and end the transfer. This is what differentiate a SPI based read and a memory mapped read.
OK. Understand, let me clear once are you trying to use erase/write from spi_flash or not? The reason for asking if yes, we need to use BEGIN on driver and also these is one more controller already exists on spi where the controller uses read for mmap and erase/write is common - are your driver looks same as this or different?
Yes, erase and write commands are also done. Generally, BEGIN flag is used sp that cs_activate can be called. But, in my case cs is activated in a different way, its enabled by writing in a command register. So, there is no need for a BEGIN.
OK, please do all modifications as we discussed so-far. and send the next level- so we can discuss more.
Please follow the driver code model- what I referred in previous mail.
Ok. I will send the v5 with all the discussed modification.
>>> + >> >> --TAG+ >>> >>> + if (bitlen == 0) >>> + goto out; >>> + >>> + if (bitlen % 8) { >>> + flags |= SPI_XFER_END; >>> + goto out; >>> + } >> >> --TAG- >> >> TAG+ .. TAG- please move this code on start of the xfer..ie below >> debug(); >> > I understand this point, but it was done purposefully. I wanted to > hit > this > point only if memory map is not invoked. If you see sf_ops.c, I am > invoking > the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, > NULL, > SPI_XFER_MEM_MAP)" > If I place TAG+..TAG- before memory map stuff above, it will always > goto > out. > > So, > either I keep it here or > pass some dummy non zero value for bitlen in above mentioned > spi_xfer. > ?
Sound good, please keep here.
>>> + >>> + /* setup command reg */ >>> + qslave->cmd = 0; >>> + qslave->cmd |= QSPI_WLEN(8); >>> + qslave->cmd |= QSPI_EN_CS(slave->cs); >>> + if (flags& SPI_3WIRE) >>> >>> + qslave->cmd |= QSPI_3_PIN; >>> + qslave->cmd |= 0xfff; >>> + >>> + while (words--) { >>> + if (txp) { >>> + debug("tx cmd %08x dc %08x data %02x\n", >>> + qslave->cmd | QSPI_WR_SNGL, >>> qslave->dc, >>> *txp); >>> + writel(*txp++,&qslave->base->spi_data); >>> + writel(qslave->cmd | QSPI_WR_SNGL, >>> +&qslave->base->spi_cmd); >>> >>> + status = readl(&qslave->base->spi_status); >>> + timeout = QSPI_TIMEOUT; >>> + while ((status& QSPI_WC_BUSY) != >>> QSPI_XFER_DONE) >>> { >>> >>> + if (--timeout< 0) { >>> + printf("QSPI tx timed >>> out\n"); >>> + return -1; >>> + } >>> + status = >>> readl(&qslave->base->spi_status); >>> + } >>> + debug("tx done, status %08x\n", status); >>> + } >>> + if (rxp) { >>> + qslave->cmd |= QSPI_RD_SNGL; >>> + debug("rx cmd %08x dc %08x\n", >>> + qslave->cmd, qslave->dc); >>> + writel(qslave->cmd,&qslave->base->spi_cmd); >>> >>> + status = readl(&qslave->base->spi_status); >>> + timeout = QSPI_TIMEOUT; >>> + while ((status& QSPI_WC_BUSY) != >>> QSPI_XFER_DONE) >>> { >>> >>> + if (--timeout< 0) { >>> + printf("QSPI rx timed >>> out\n"); >>> + return -1; >>> + } >>> + status = >>> readl(&qslave->base->spi_status); >>> + } >>> + *rxp++ = readl(&qslave->base->spi_data); >>> + debug("rx done, status %08x, read %02x\n", >>> + status, *(rxp-1)); >>> + } >>> + } >>> + >>> +out: >>> + /* Terminate frame */ >>> + if (flags& SPI_XFER_END) >>> + writel(qslave->cmd | >>> QSPI_INVAL,&qslave->base->spi_cmd); >> >> Please palce this code in spi_cs_deactivate() >> >> I request you please follow the code structure in below thread. >> I feel better if you use same prints that used in below example >> driver. >> http://patchwork.ozlabs.org/patch/265683/

On Sunday 06 October 2013 02:14 PM, Jagan Teki wrote:
What if this code is placed in cs_active() with BEGIN flag.?
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
Functionality wise it wont effect. I am open to what you suggest here, whether to move it or not.
Though, just one thing you should note here, is that the above code just mask the cmd register bits. The actual cmd register write happens inside while loop and only when that write happens, then the cs gets activated. So, above code does not activate the cs, it just prepare the mask that will enable cs later.
On Sat, Oct 5, 2013 at 7:53 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 05:10 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote: > On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com > wrote: >> On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote: >>> On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com >>> wrote: >>>> From: Matt Portermatt.porter@linaro.org >>>> >>>> Adds a SPI master driver for the TI QSPI peripheral. >>>> >>>> Signed-off-by: Matt Portermatt.porter@linaro.org >>>> Signed-off-by: Sourav Poddarsourav.poddar@ti.com >>>> [Added quad read support and memory mapped support). >>> What is this comment, any specific? >> This simply tell the portion which i did in the patch. > May be not required, bcz it will come after i apply below s-o-b > >>>> --- >>> You missed change log for all patches, i think you have summarized >>> in >>> 0/6. >>> I feel it's better write it on individual patches. >>> >> Ok. >> >>>> drivers/spi/Makefile | 1 + >>>> drivers/spi/ti_qspi.c | 328 >>>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 329 insertions(+), 0 deletions(-) >>>> create mode 100644 drivers/spi/ti_qspi.c >>>> >>>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile >>>> index 91d24ce..e5941b0 100644 >>>> --- a/drivers/spi/Makefile >>>> +++ b/drivers/spi/Makefile >>>> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o >>>> COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o >>>> COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o >>>> COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o >>>> +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o >>>> COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o >>>> COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o >>>> >>>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >>>> new file mode 100644 >>>> index 0000000..d8a03a8 >>>> --- /dev/null >>>> +++ b/drivers/spi/ti_qspi.c >>>> @@ -0,0 +1,328 @@ >>>> +/* >>>> + * TI QSPI driver >>>> + * >>>> + * Copyright (C) 2013, Texas Instruments, Incorporated >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>> Got below format after apply this patch - please check >>> *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+ >>> >> ahh..I copied it from a patch on some list. May be something went >> wrong, >> I >> will check. >> >>>> + */ >>>> + >>>> +#include<common.h> >>>> +#include<asm/io.h> >>>> +#include<asm/arch/omap.h> >>>> +#include<malloc.h> >>>> +#include<spi.h> >>>> + >>>> +struct qspi_regs { >>>> +u32 pid; >>>> +u32 pad0[3]; >>>> +u32 sysconfig; >>>> +u32 pad1[3]; >>>> +u32 intr_status_raw_set; >>>> +u32 intr_status_enabled_clear; >>>> +u32 intr_enable_set; >>>> +u32 intr_enable_clear; >>>> +u32 intc_eoi; >>>> +u32 pad2[3]; >>>> +u32 spi_clock_cntrl; >>>> +u32 spi_dc; >>>> +u32 spi_cmd; >>>> +u32 spi_status; >>>> +u32 spi_data; >>>> +u32 spi_setup0; >>>> +u32 spi_setup1; >>>> +u32 spi_setup2; >>>> +u32 spi_setup3; >>>> +u32 spi_switch; >>>> +u32 spi_data1; >>>> +u32 spi_data2; >>>> +u32 spi_data3; >>> Please add tab space. >>> >> ok >> >>>> +}; >>>> + >>>> +struct qspi_slave { >>>> + struct spi_slave slave; >>>> + struct qspi_regs *base; >>>> + unsigned int mode; >>>> + u32 cmd; >>>> + u32 dc; >>>> +}; >>>> + >>> -- TAG+ >>>> +#define QSPI_TIMEOUT 2000000 >>>> + >>>> +#define QSPI_FCLK 192000000 >>>> + >>>> +/* Clock Control */ >>>> +#define QSPI_CLK_EN (1<< 31) >>>> +#define QSPI_CLK_DIV_MAX 0xffff >>>> + >>>> +/* Command */ >>>> +#define QSPI_EN_CS(n) (n<< 28) >>>> +#define QSPI_WLEN(n) ((n-1)<< 19) >>>> +#define QSPI_3_PIN (1<< 18) >>>> +#define QSPI_RD_SNGL (1<< 16) >>>> +#define QSPI_WR_SNGL (2<< 16) >>>> +#define QSPI_INVAL (4<< 16) >>>> +#define QSPI_RD_QUAD (7<< 16) >>>> + >>>> +/* Device Control */ >>>> +#define QSPI_DD(m, n) (m<< (3 + n*8)) >>>> +#define QSPI_CKPHA(n) (1<< (2 + n*8)) >>>> +#define QSPI_CSPOL(n) (1<< (1 + n*8)) >>>> +#define QSPI_CKPOL(n) (1<< (n*8)) >>>> + >>>> +/* Status */ >>>> +#define QSPI_WC (1<< 1) >>>> +#define QSPI_BUSY (1<< 0) >>>> +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) >>>> +#define QSPI_XFER_DONE QSPI_WC >>>> + >>>> +#define MM_SWITCH 0x01 >>>> +#define MEM_CS 0x100 >>>> +#define MEM_CS_UNSELECT 0xfffff0ff >>>> +#define MMAP_START_ADDR 0x5c000000 >>>> +#define CORE_CTRL_IO 0x4a002558 >>>> + >>>> +#define QSPI_CMD_READ (0x3<< 0) >>>> +#define QSPI_CMD_READ_QUAD (0x6b<< 0) >>>> +#define QSPI_CMD_READ_FAST (0x0b<< 0) >>>> +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) >>>> +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) >>>> +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) >>>> +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) >>>> +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) >>>> +#define QSPI_CMD_WRITE (0x2<< 16) >>>> +#define QSPI_NUM_DUMMY_BITS (0x0<< 24) >>> --TAG- >>> >>> TAG+ ... TAG- please move these macro definitions in below headers >> Ok. >> >>>> + >>>> +static inline struct qspi_slave *to_qspi_slave(struct spi_slave >>>> *slave) >>>> +{ >>>> + return container_of(slave, struct qspi_slave, slave); >>>> +} >>>> +static inline struct qspi_regs *get_qspi_bus(int dev) >>>> +{ >>>> + if (!dev) >>>> + return (struct qspi_regs *)QSPI_BASE; >>>> + else >>>> + return NULL; >>>> +} >>> Is this function really required, how many bus controller you have? >> Actually one. > Ok, Please remove this function and assign directly. > >>>> + >>>> +int spi_cs_is_valid(unsigned int bus, unsigned int cs) >>>> +{ >>>> + return 1; >>>> +} >>>> + >>>> +void spi_cs_activate(struct spi_slave *slave) >>>> +{ >>>> + /* CS handled in xfer */ >>>> + return; >>>> +} >>>> + >>>> +void spi_cs_deactivate(struct spi_slave *slave) >>>> +{ >>>> + /* CS handled in xfer */ >>>> + return; >>>> +} >>>> + >>>> +void spi_init(void) >>>> +{ >>>> + /* nothing to do */ >>>> +} >>>> + >>>> +void spi_set_up_spi_register(struct qspi_slave *qslave) >>>> +{ >>>> + u32 memval = 0; >>>> + struct spi_slave *slave =&qslave->slave; >>>> >>>> + >>>> + slave->memory_map = (void *)MMAP_START_ADDR; >>>> + >>>> + memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES | >>>> + QSPI_SETUP0_NUM_D_BYTES_NO_BITS | >>>> QSPI_SETUP0_READ_NORMAL >>>> | >>>> + QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS); >>>> + >>>> + writel(memval,&qslave->base->spi_setup0); >>>> >>>> +} >>>> + >>>> +void spi_set_speed(struct spi_slave *slave, uint hz) >>>> +{ >>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>> + >>>> + uint clk_div; >>>> + >>>> + if (!hz) >>>> + clk_div = 0; >>>> + else >>>> + clk_div = (QSPI_FCLK / hz) - 1; >>>> + >>>> + debug("%s: hz: %d, clock divider %d\n", __func__, hz, >>>> clk_div); >>>> + >>>> + /* disable SCLK */ >>>> + writel(readl(&qslave->base->spi_clock_cntrl)& >>>> ~QSPI_CLK_EN, >>>> +&qslave->base->spi_clock_cntrl); >>>> >>>> + >>>> + if (clk_div< 0) { >>>> + debug("%s: clock divider< 0, using /1 >>>> divider\n", >>>> __func__); >>>> + clk_div = 0; >>>> + } >>>> + >>>> + if (clk_div> QSPI_CLK_DIV_MAX) { >>>> + debug("%s: clock divider>%d , using /%d divider\n", >>>> + __func__, QSPI_CLK_DIV_MAX, >>>> QSPI_CLK_DIV_MAX >>>> + >>>> 1); >>>> + clk_div = QSPI_CLK_DIV_MAX; >>>> + } >>>> + >>>> + /* enable SCLK */ >>>> + writel(QSPI_CLK_EN | >>>> clk_div,&qslave->base->spi_clock_cntrl); >>>> >>>> + debug("%s: spi_clock_cntrl %08x\n", __func__, >>>> + readl(&qslave->base->spi_clock_cntrl)); >>>> +} >>>> + >>>> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int >>>> cs, >>>> + unsigned int max_hz, unsigned int >>>> mode) >>>> +{ >>>> + struct qspi_slave *qslave; >>>> + >>>> + qslave = spi_alloc_slave(struct qspi_slave, bus, cs); >>>> + if (!qslave) >>>> + return NULL; >>>> + >>>> + qslave->base = get_qspi_bus(bus); >>>> + if (qslave->base) >>>> + debug("No Qspi device found\n"); >>>> + spi_set_speed(&qslave->slave, max_hz); >>>> + qslave->mode = mode; >>>> + >>>> +#ifdef CONFIG_MMAP >>>> + spi_set_up_spi_register(qslave); >>>> +#endif >>>> + >>>> + debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, >>>> mode); >>>> + >>>> + return&qslave->slave; >>>> >>>> +} >>>> + >>>> +void spi_free_slave(struct spi_slave *slave) >>>> +{ >>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>> + free(qslave); >>>> +} >>>> + >>>> +int spi_claim_bus(struct spi_slave *slave) >>>> +{ >>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>> + >>>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>>> slave->cs); >>>> + >>>> + writel(0,&qslave->base->spi_dc); >>>> + writel(0,&qslave->base->spi_cmd); >>>> + writel(0,&qslave->base->spi_data); >>>> >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +void spi_release_bus(struct spi_slave *slave) >>>> +{ >>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>> + >>>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>>> slave->cs); >>>> + >>>> + writel(0,&qslave->base->spi_dc); >>>> + writel(0,&qslave->base->spi_cmd); >>>> + writel(0,&qslave->base->spi_data); >>>> >>>> +} >>>> + >>>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const >>>> void >>>> *dout, >>>> + void *din, unsigned long flags) >>>> +{ >>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>> + uint words = bitlen>> 3; /* fixed 8-bit word length */ >>>> + const uchar *txp = dout; >>>> + uchar *rxp = din; >>>> + uint status; >>>> + int timeout, val; >>>> + >>>> + debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", >>>> __func__, >>>> + slave->bus, slave->cs, bitlen, words, flags); >>>> + >>>> + qslave->dc = 0; >>>> + if (qslave->mode& SPI_CPHA) >>>> >>>> + qslave->dc |= QSPI_CKPHA(slave->cs); >>>> + if (qslave->mode& SPI_CPOL) >>>> >>>> + qslave->dc |= QSPI_CKPOL(slave->cs); >>>> + if (qslave->mode& SPI_CS_HIGH) >>>> >>>> + qslave->dc |= QSPI_CSPOL(slave->cs); >>>> + >>>> + writel(qslave->dc,&qslave->base->spi_dc); >>> Adjust this code in spi_claim_bus() >>> >> Ok. >>>> + >>>> + if (flags& SPI_XFER_MEM_MAP) { >>>> + writel(MM_SWITCH,&qslave->base->spi_switch); >>>> >>>> + val = readl(CORE_CTRL_IO); >>>> + val |= MEM_CS; >>>> + writel(val, CORE_CTRL_IO); >>>> + return 0; >>>> + } else if (flags& SPI_XFER_MEM_MAP_END) { >>>> + writel(~MM_SWITCH,&qslave->base->spi_switch); >>>> + val = readl(CORE_CTRL_IO); >>>> + val&= MEM_CS_UNSELECT; >>>> >>>> + writel(val, CORE_CTRL_IO); >>>> + return 0; >>>> + } >>> What is this your are returning from here directly for memory_map? >>> is >>> it? >> Yes, memory map does not care about setting the command register and >> doing the transfer using the normal spi framework. >> Memory ma has a different set of registers, set up >> register(configured >> above). >> Here, we just switch the controller to memory mapped port and use >> flash->memory_map >> to read the data from the memory mapped port(0x5c000000). > OK. can you readjust this code by placing existing spi_flash func like > spi_cs_activate() > Looks like this cs activate code. I dont think so it make much sense to put it ib cs_activate apis, this portion does not really justify that. This code is more SOC specific control module parameters which allows you to switch between the configuaration port and memory mapped port.
> Where is SPI_XFER_BEGIN are you not using this? We dont need SPI_XFER_BEGIN here, for memory mapped we just need MEM_MAP flag, do the required memory mapped read and end the transfer. This is what differentiate a SPI based read and a memory mapped read.
OK. Understand, let me clear once are you trying to use erase/write from spi_flash or not? The reason for asking if yes, we need to use BEGIN on driver and also these is one more controller already exists on spi where the controller uses read for mmap and erase/write is common - are your driver looks same as this or different?
Yes, erase and write commands are also done. Generally, BEGIN flag is used sp that cs_activate can be called. But, in my case cs is activated in a different way, its enabled by writing in a command register. So, there is no need for a BEGIN.
OK, please do all modifications as we discussed so-far. and send the next level- so we can discuss more.
Please follow the driver code model- what I referred in previous mail.
Ok. I will send the v5 with all the discussed modification.
>>>> + >>> --TAG+ >>>> + if (bitlen == 0) >>>> + goto out; >>>> + >>>> + if (bitlen % 8) { >>>> + flags |= SPI_XFER_END; >>>> + goto out; >>>> + } >>> --TAG- >>> >>> TAG+ .. TAG- please move this code on start of the xfer..ie below >>> debug(); >>> >> I understand this point, but it was done purposefully. I wanted to >> hit >> this >> point only if memory map is not invoked. If you see sf_ops.c, I am >> invoking >> the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, >> NULL, >> SPI_XFER_MEM_MAP)" >> If I place TAG+..TAG- before memory map stuff above, it will always >> goto >> out. >> >> So, >> either I keep it here or >> pass some dummy non zero value for bitlen in above mentioned >> spi_xfer. >> ? > Sound good, please keep here. > >>>> + >>>> + /* setup command reg */ >>>> + qslave->cmd = 0; >>>> + qslave->cmd |= QSPI_WLEN(8); >>>> + qslave->cmd |= QSPI_EN_CS(slave->cs); >>>> + if (flags& SPI_3WIRE) >>>> >>>> + qslave->cmd |= QSPI_3_PIN; >>>> + qslave->cmd |= 0xfff; >>>> + >>>> + while (words--) { >>>> + if (txp) { >>>> + debug("tx cmd %08x dc %08x data %02x\n", >>>> + qslave->cmd | QSPI_WR_SNGL, >>>> qslave->dc, >>>> *txp); >>>> + writel(*txp++,&qslave->base->spi_data); >>>> + writel(qslave->cmd | QSPI_WR_SNGL, >>>> +&qslave->base->spi_cmd); >>>> >>>> + status = readl(&qslave->base->spi_status); >>>> + timeout = QSPI_TIMEOUT; >>>> + while ((status& QSPI_WC_BUSY) != >>>> QSPI_XFER_DONE) >>>> { >>>> >>>> + if (--timeout< 0) { >>>> + printf("QSPI tx timed >>>> out\n"); >>>> + return -1; >>>> + } >>>> + status = >>>> readl(&qslave->base->spi_status); >>>> + } >>>> + debug("tx done, status %08x\n", status); >>>> + } >>>> + if (rxp) { >>>> + qslave->cmd |= QSPI_RD_SNGL; >>>> + debug("rx cmd %08x dc %08x\n", >>>> + qslave->cmd, qslave->dc); >>>> + writel(qslave->cmd,&qslave->base->spi_cmd); >>>> >>>> + status = readl(&qslave->base->spi_status); >>>> + timeout = QSPI_TIMEOUT; >>>> + while ((status& QSPI_WC_BUSY) != >>>> QSPI_XFER_DONE) >>>> { >>>> >>>> + if (--timeout< 0) { >>>> + printf("QSPI rx timed >>>> out\n"); >>>> + return -1; >>>> + } >>>> + status = >>>> readl(&qslave->base->spi_status); >>>> + } >>>> + *rxp++ = readl(&qslave->base->spi_data); >>>> + debug("rx done, status %08x, read %02x\n", >>>> + status, *(rxp-1)); >>>> + } >>>> + } >>>> + >>>> +out: >>>> + /* Terminate frame */ >>>> + if (flags& SPI_XFER_END) >>>> + writel(qslave->cmd | >>>> QSPI_INVAL,&qslave->base->spi_cmd); >>> Please palce this code in spi_cs_deactivate() >>> >>> I request you please follow the code structure in below thread. >>> I feel better if you use same prints that used in below example >>> driver. >>> http://patchwork.ozlabs.org/patch/265683/

On Sun, Oct 6, 2013 at 3:44 PM, Sourav Poddar sourav.poddar@ti.com wrote:
On Sunday 06 October 2013 02:14 PM, Jagan Teki wrote:
What if this code is placed in cs_active() with BEGIN flag.?
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
Functionality wise it wont effect. I am open to what you suggest here, whether to move it or not.
Though, just one thing you should note here, is that the above code just mask the cmd register bits. The actual cmd register write happens inside while loop and only when that write happens, then the cs gets activated. So, above code does not activate the cs, it just prepare the mask that will enable cs later.
OK, just park this as of now try to send next level patch-set we will discuss more.
On Sat, Oct 5, 2013 at 7:53 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 05:10 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddarsourav.poddar@ti.com wrote: > > On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote: >> >> On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com >> wrote: >>> >>> On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote: >>>> >>>> On Fri, Oct 4, 2013 at 8:21 PM, Sourav >>>> Poddarsourav.poddar@ti.com >>>> wrote: >>>>> >>>>> From: Matt Portermatt.porter@linaro.org >>>>> >>>>> Adds a SPI master driver for the TI QSPI peripheral. >>>>> >>>>> Signed-off-by: Matt Portermatt.porter@linaro.org >>>>> Signed-off-by: Sourav Poddarsourav.poddar@ti.com >>>>> [Added quad read support and memory mapped support). >>>> >>>> What is this comment, any specific? >>> >>> This simply tell the portion which i did in the patch. >> >> May be not required, bcz it will come after i apply below s-o-b >> >>>>> --- >>>> >>>> You missed change log for all patches, i think you have summarized >>>> in >>>> 0/6. >>>> I feel it's better write it on individual patches. >>>> >>> Ok. >>> >>>>> drivers/spi/Makefile | 1 + >>>>> drivers/spi/ti_qspi.c | 328 >>>>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 2 files changed, 329 insertions(+), 0 deletions(-) >>>>> create mode 100644 drivers/spi/ti_qspi.c >>>>> >>>>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile >>>>> index 91d24ce..e5941b0 100644 >>>>> --- a/drivers/spi/Makefile >>>>> +++ b/drivers/spi/Makefile >>>>> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o >>>>> COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o >>>>> COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o >>>>> COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o >>>>> +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o >>>>> COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o >>>>> COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o >>>>> >>>>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >>>>> new file mode 100644 >>>>> index 0000000..d8a03a8 >>>>> --- /dev/null >>>>> +++ b/drivers/spi/ti_qspi.c >>>>> @@ -0,0 +1,328 @@ >>>>> +/* >>>>> + * TI QSPI driver >>>>> + * >>>>> + * Copyright (C) 2013, Texas Instruments, Incorporated >>>>> + * >>>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> >>>> Got below format after apply this patch - please check >>>> *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+ >>>> >>> ahh..I copied it from a patch on some list. May be something went >>> wrong, >>> I >>> will check. >>> >>>>> + */ >>>>> + >>>>> +#include<common.h> >>>>> +#include<asm/io.h> >>>>> +#include<asm/arch/omap.h> >>>>> +#include<malloc.h> >>>>> +#include<spi.h> >>>>> + >>>>> +struct qspi_regs { >>>>> +u32 pid; >>>>> +u32 pad0[3]; >>>>> +u32 sysconfig; >>>>> +u32 pad1[3]; >>>>> +u32 intr_status_raw_set; >>>>> +u32 intr_status_enabled_clear; >>>>> +u32 intr_enable_set; >>>>> +u32 intr_enable_clear; >>>>> +u32 intc_eoi; >>>>> +u32 pad2[3]; >>>>> +u32 spi_clock_cntrl; >>>>> +u32 spi_dc; >>>>> +u32 spi_cmd; >>>>> +u32 spi_status; >>>>> +u32 spi_data; >>>>> +u32 spi_setup0; >>>>> +u32 spi_setup1; >>>>> +u32 spi_setup2; >>>>> +u32 spi_setup3; >>>>> +u32 spi_switch; >>>>> +u32 spi_data1; >>>>> +u32 spi_data2; >>>>> +u32 spi_data3; >>>> >>>> Please add tab space. >>>> >>> ok >>> >>>>> +}; >>>>> + >>>>> +struct qspi_slave { >>>>> + struct spi_slave slave; >>>>> + struct qspi_regs *base; >>>>> + unsigned int mode; >>>>> + u32 cmd; >>>>> + u32 dc; >>>>> +}; >>>>> + >>>> >>>> -- TAG+ >>>>> >>>>> +#define QSPI_TIMEOUT 2000000 >>>>> + >>>>> +#define QSPI_FCLK 192000000 >>>>> + >>>>> +/* Clock Control */ >>>>> +#define QSPI_CLK_EN (1<< 31) >>>>> +#define QSPI_CLK_DIV_MAX 0xffff >>>>> + >>>>> +/* Command */ >>>>> +#define QSPI_EN_CS(n) (n<< 28) >>>>> +#define QSPI_WLEN(n) ((n-1)<< 19) >>>>> +#define QSPI_3_PIN (1<< 18) >>>>> +#define QSPI_RD_SNGL (1<< 16) >>>>> +#define QSPI_WR_SNGL (2<< 16) >>>>> +#define QSPI_INVAL (4<< 16) >>>>> +#define QSPI_RD_QUAD (7<< 16) >>>>> + >>>>> +/* Device Control */ >>>>> +#define QSPI_DD(m, n) (m<< (3 + n*8)) >>>>> +#define QSPI_CKPHA(n) (1<< (2 + n*8)) >>>>> +#define QSPI_CSPOL(n) (1<< (1 + n*8)) >>>>> +#define QSPI_CKPOL(n) (1<< (n*8)) >>>>> + >>>>> +/* Status */ >>>>> +#define QSPI_WC (1<< 1) >>>>> +#define QSPI_BUSY (1<< 0) >>>>> +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) >>>>> +#define QSPI_XFER_DONE QSPI_WC >>>>> + >>>>> +#define MM_SWITCH 0x01 >>>>> +#define MEM_CS 0x100 >>>>> +#define MEM_CS_UNSELECT 0xfffff0ff >>>>> +#define MMAP_START_ADDR 0x5c000000 >>>>> +#define CORE_CTRL_IO 0x4a002558 >>>>> + >>>>> +#define QSPI_CMD_READ (0x3<< 0) >>>>> +#define QSPI_CMD_READ_QUAD (0x6b<< 0) >>>>> +#define QSPI_CMD_READ_FAST (0x0b<< 0) >>>>> +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) >>>>> +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) >>>>> +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) >>>>> +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) >>>>> +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) >>>>> +#define QSPI_CMD_WRITE (0x2<< 16) >>>>> +#define QSPI_NUM_DUMMY_BITS (0x0<< 24) >>>> >>>> --TAG- >>>> >>>> TAG+ ... TAG- please move these macro definitions in below headers >>> >>> Ok. >>> >>>>> + >>>>> +static inline struct qspi_slave *to_qspi_slave(struct spi_slave >>>>> *slave) >>>>> +{ >>>>> + return container_of(slave, struct qspi_slave, slave); >>>>> +} >>>>> +static inline struct qspi_regs *get_qspi_bus(int dev) >>>>> +{ >>>>> + if (!dev) >>>>> + return (struct qspi_regs *)QSPI_BASE; >>>>> + else >>>>> + return NULL; >>>>> +} >>>> >>>> Is this function really required, how many bus controller you >>>> have? >>> >>> Actually one. >> >> Ok, Please remove this function and assign directly. >> >>>>> + >>>>> +int spi_cs_is_valid(unsigned int bus, unsigned int cs) >>>>> +{ >>>>> + return 1; >>>>> +} >>>>> + >>>>> +void spi_cs_activate(struct spi_slave *slave) >>>>> +{ >>>>> + /* CS handled in xfer */ >>>>> + return; >>>>> +} >>>>> + >>>>> +void spi_cs_deactivate(struct spi_slave *slave) >>>>> +{ >>>>> + /* CS handled in xfer */ >>>>> + return; >>>>> +} >>>>> + >>>>> +void spi_init(void) >>>>> +{ >>>>> + /* nothing to do */ >>>>> +} >>>>> + >>>>> +void spi_set_up_spi_register(struct qspi_slave *qslave) >>>>> +{ >>>>> + u32 memval = 0; >>>>> + struct spi_slave *slave =&qslave->slave; >>>>> >>>>> + >>>>> + slave->memory_map = (void *)MMAP_START_ADDR; >>>>> + >>>>> + memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES | >>>>> + QSPI_SETUP0_NUM_D_BYTES_NO_BITS | >>>>> QSPI_SETUP0_READ_NORMAL >>>>> | >>>>> + QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS); >>>>> + >>>>> + writel(memval,&qslave->base->spi_setup0); >>>>> >>>>> +} >>>>> + >>>>> +void spi_set_speed(struct spi_slave *slave, uint hz) >>>>> +{ >>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>> + >>>>> + uint clk_div; >>>>> + >>>>> + if (!hz) >>>>> + clk_div = 0; >>>>> + else >>>>> + clk_div = (QSPI_FCLK / hz) - 1; >>>>> + >>>>> + debug("%s: hz: %d, clock divider %d\n", __func__, hz, >>>>> clk_div); >>>>> + >>>>> + /* disable SCLK */ >>>>> + writel(readl(&qslave->base->spi_clock_cntrl)& >>>>> ~QSPI_CLK_EN, >>>>> +&qslave->base->spi_clock_cntrl); >>>>> >>>>> + >>>>> + if (clk_div< 0) { >>>>> + debug("%s: clock divider< 0, using /1 >>>>> divider\n", >>>>> __func__); >>>>> + clk_div = 0; >>>>> + } >>>>> + >>>>> + if (clk_div> QSPI_CLK_DIV_MAX) { >>>>> + debug("%s: clock divider>%d , using /%d >>>>> divider\n", >>>>> + __func__, QSPI_CLK_DIV_MAX, >>>>> QSPI_CLK_DIV_MAX >>>>> + >>>>> 1); >>>>> + clk_div = QSPI_CLK_DIV_MAX; >>>>> + } >>>>> + >>>>> + /* enable SCLK */ >>>>> + writel(QSPI_CLK_EN | >>>>> clk_div,&qslave->base->spi_clock_cntrl); >>>>> >>>>> + debug("%s: spi_clock_cntrl %08x\n", __func__, >>>>> + readl(&qslave->base->spi_clock_cntrl)); >>>>> +} >>>>> + >>>>> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int >>>>> cs, >>>>> + unsigned int max_hz, unsigned >>>>> int >>>>> mode) >>>>> +{ >>>>> + struct qspi_slave *qslave; >>>>> + >>>>> + qslave = spi_alloc_slave(struct qspi_slave, bus, cs); >>>>> + if (!qslave) >>>>> + return NULL; >>>>> + >>>>> + qslave->base = get_qspi_bus(bus); >>>>> + if (qslave->base) >>>>> + debug("No Qspi device found\n"); >>>>> + spi_set_speed(&qslave->slave, max_hz); >>>>> + qslave->mode = mode; >>>>> + >>>>> +#ifdef CONFIG_MMAP >>>>> + spi_set_up_spi_register(qslave); >>>>> +#endif >>>>> + >>>>> + debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, >>>>> mode); >>>>> + >>>>> + return&qslave->slave; >>>>> >>>>> +} >>>>> + >>>>> +void spi_free_slave(struct spi_slave *slave) >>>>> +{ >>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>> + free(qslave); >>>>> +} >>>>> + >>>>> +int spi_claim_bus(struct spi_slave *slave) >>>>> +{ >>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>> + >>>>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>>>> slave->cs); >>>>> + >>>>> + writel(0,&qslave->base->spi_dc); >>>>> + writel(0,&qslave->base->spi_cmd); >>>>> + writel(0,&qslave->base->spi_data); >>>>> >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +void spi_release_bus(struct spi_slave *slave) >>>>> +{ >>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>> + >>>>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>>>> slave->cs); >>>>> + >>>>> + writel(0,&qslave->base->spi_dc); >>>>> + writel(0,&qslave->base->spi_cmd); >>>>> + writel(0,&qslave->base->spi_data); >>>>> >>>>> +} >>>>> + >>>>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const >>>>> void >>>>> *dout, >>>>> + void *din, unsigned long flags) >>>>> +{ >>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>> + uint words = bitlen>> 3; /* fixed 8-bit word length >>>>> */ >>>>> + const uchar *txp = dout; >>>>> + uchar *rxp = din; >>>>> + uint status; >>>>> + int timeout, val; >>>>> + >>>>> + debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", >>>>> __func__, >>>>> + slave->bus, slave->cs, bitlen, words, flags); >>>>> + >>>>> + qslave->dc = 0; >>>>> + if (qslave->mode& SPI_CPHA) >>>>> >>>>> + qslave->dc |= QSPI_CKPHA(slave->cs); >>>>> + if (qslave->mode& SPI_CPOL) >>>>> >>>>> + qslave->dc |= QSPI_CKPOL(slave->cs); >>>>> + if (qslave->mode& SPI_CS_HIGH) >>>>> >>>>> + qslave->dc |= QSPI_CSPOL(slave->cs); >>>>> + >>>>> + writel(qslave->dc,&qslave->base->spi_dc); >>>> >>>> Adjust this code in spi_claim_bus() >>>> >>> Ok. >>>>> >>>>> + >>>>> + if (flags& SPI_XFER_MEM_MAP) { >>>>> + writel(MM_SWITCH,&qslave->base->spi_switch); >>>>> >>>>> + val = readl(CORE_CTRL_IO); >>>>> + val |= MEM_CS; >>>>> + writel(val, CORE_CTRL_IO); >>>>> + return 0; >>>>> + } else if (flags& SPI_XFER_MEM_MAP_END) { >>>>> + writel(~MM_SWITCH,&qslave->base->spi_switch); >>>>> + val = readl(CORE_CTRL_IO); >>>>> + val&= MEM_CS_UNSELECT; >>>>> >>>>> + writel(val, CORE_CTRL_IO); >>>>> + return 0; >>>>> + } >>>> >>>> What is this your are returning from here directly for memory_map? >>>> is >>>> it? >>> >>> Yes, memory map does not care about setting the command register >>> and >>> doing the transfer using the normal spi framework. >>> Memory ma has a different set of registers, set up >>> register(configured >>> above). >>> Here, we just switch the controller to memory mapped port and use >>> flash->memory_map >>> to read the data from the memory mapped port(0x5c000000). >> >> OK. can you readjust this code by placing existing spi_flash func >> like >> spi_cs_activate() >> Looks like this cs activate code. > > I dont think so it make much sense to put it ib cs_activate apis, > this > portion > does not really justify that. This code is more SOC specific control > module > parameters which allows you to switch between the configuaration port > and memory mapped port. > >> Where is SPI_XFER_BEGIN are you not using this? > > We dont need SPI_XFER_BEGIN here, for memory mapped we just need > MEM_MAP flag, do the required memory mapped read and end the > transfer. > This is what differentiate a SPI based read and a memory mapped read.
OK. Understand, let me clear once are you trying to use erase/write from spi_flash or not? The reason for asking if yes, we need to use BEGIN on driver and also these is one more controller already exists on spi where the controller uses read for mmap and erase/write is common - are your driver looks same as this or different?
Yes, erase and write commands are also done. Generally, BEGIN flag is used sp that cs_activate can be called. But, in my case cs is activated in a different way, its enabled by writing in a command register. So, there is no need for a BEGIN.
OK, please do all modifications as we discussed so-far. and send the next level- so we can discuss more.
Please follow the driver code model- what I referred in previous mail.
Ok. I will send the v5 with all the discussed modification.
>>>>> + >>>> >>>> --TAG+ >>>>> >>>>> + if (bitlen == 0) >>>>> + goto out; >>>>> + >>>>> + if (bitlen % 8) { >>>>> + flags |= SPI_XFER_END; >>>>> + goto out; >>>>> + } >>>> >>>> --TAG- >>>> >>>> TAG+ .. TAG- please move this code on start of the xfer..ie below >>>> debug(); >>>> >>> I understand this point, but it was done purposefully. I wanted to >>> hit >>> this >>> point only if memory map is not invoked. If you see sf_ops.c, I am >>> invoking >>> the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, >>> NULL, >>> SPI_XFER_MEM_MAP)" >>> If I place TAG+..TAG- before memory map stuff above, it will always >>> goto >>> out. >>> >>> So, >>> either I keep it here or >>> pass some dummy non zero value for bitlen in above mentioned >>> spi_xfer. >>> ? >> >> Sound good, please keep here. >> >>>>> + >>>>> + /* setup command reg */ >>>>> + qslave->cmd = 0; >>>>> + qslave->cmd |= QSPI_WLEN(8); >>>>> + qslave->cmd |= QSPI_EN_CS(slave->cs); >>>>> + if (flags& SPI_3WIRE) >>>>> >>>>> + qslave->cmd |= QSPI_3_PIN; >>>>> + qslave->cmd |= 0xfff; >>>>> + >>>>> + while (words--) { >>>>> + if (txp) { >>>>> + debug("tx cmd %08x dc %08x data %02x\n", >>>>> + qslave->cmd | QSPI_WR_SNGL, >>>>> qslave->dc, >>>>> *txp); >>>>> + writel(*txp++,&qslave->base->spi_data); >>>>> + writel(qslave->cmd | QSPI_WR_SNGL, >>>>> +&qslave->base->spi_cmd); >>>>> >>>>> + status = >>>>> readl(&qslave->base->spi_status); >>>>> + timeout = QSPI_TIMEOUT; >>>>> + while ((status& QSPI_WC_BUSY) != >>>>> QSPI_XFER_DONE) >>>>> { >>>>> >>>>> + if (--timeout< 0) { >>>>> + printf("QSPI tx timed >>>>> out\n"); >>>>> + return -1; >>>>> + } >>>>> + status = >>>>> readl(&qslave->base->spi_status); >>>>> + } >>>>> + debug("tx done, status %08x\n", status); >>>>> + } >>>>> + if (rxp) { >>>>> + qslave->cmd |= QSPI_RD_SNGL; >>>>> + debug("rx cmd %08x dc %08x\n", >>>>> + qslave->cmd, qslave->dc); >>>>> + >>>>> writel(qslave->cmd,&qslave->base->spi_cmd); >>>>> >>>>> + status = >>>>> readl(&qslave->base->spi_status); >>>>> + timeout = QSPI_TIMEOUT; >>>>> + while ((status& QSPI_WC_BUSY) != >>>>> QSPI_XFER_DONE) >>>>> { >>>>> >>>>> + if (--timeout< 0) { >>>>> + printf("QSPI rx timed >>>>> out\n"); >>>>> + return -1; >>>>> + } >>>>> + status = >>>>> readl(&qslave->base->spi_status); >>>>> + } >>>>> + *rxp++ = readl(&qslave->base->spi_data); >>>>> + debug("rx done, status %08x, read >>>>> %02x\n", >>>>> + status, *(rxp-1)); >>>>> + } >>>>> + } >>>>> + >>>>> +out: >>>>> + /* Terminate frame */ >>>>> + if (flags& SPI_XFER_END) >>>>> + writel(qslave->cmd | >>>>> QSPI_INVAL,&qslave->base->spi_cmd); >>>> >>>> Please palce this code in spi_cs_deactivate() >>>> >>>> I request you please follow the code structure in below thread. >>>> I feel better if you use same prints that used in below example >>>> driver. >>>> http://patchwork.ozlabs.org/patch/265683/ > >

On Sunday 06 October 2013 09:00 PM, Jagan Teki wrote:
On Sun, Oct 6, 2013 at 3:44 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Sunday 06 October 2013 02:14 PM, Jagan Teki wrote:
What if this code is placed in cs_active() with BEGIN flag.?
/* setup command reg */
qslave->cmd = 0;
qslave->cmd |= QSPI_WLEN(8);
qslave->cmd |= QSPI_EN_CS(slave->cs);
if (flags& SPI_3WIRE)
qslave->cmd |= QSPI_3_PIN;
qslave->cmd |= 0xfff;
Functionality wise it wont effect. I am open to what you suggest here, whether to move it or not.
Though, just one thing you should note here, is that the above code just mask the cmd register bits. The actual cmd register write happens inside while loop and only when that write happens, then the cs gets activated. So, above code does not activate the cs, it just prepare the mask that will enable cs later.
OK, just park this as of now try to send next level patch-set we will discuss more.
Ok. I am working on the next version.
On Sat, Oct 5, 2013 at 7:53 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 05:10 PM, Jagan Teki wrote:
On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote: > On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddarsourav.poddar@ti.com > wrote: >> On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote: >>> On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddarsourav.poddar@ti.com >>> wrote: >>>> On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote: >>>>> On Fri, Oct 4, 2013 at 8:21 PM, Sourav >>>>> Poddarsourav.poddar@ti.com >>>>> wrote: >>>>>> From: Matt Portermatt.porter@linaro.org >>>>>> >>>>>> Adds a SPI master driver for the TI QSPI peripheral. >>>>>> >>>>>> Signed-off-by: Matt Portermatt.porter@linaro.org >>>>>> Signed-off-by: Sourav Poddarsourav.poddar@ti.com >>>>>> [Added quad read support and memory mapped support). >>>>> What is this comment, any specific? >>>> This simply tell the portion which i did in the patch. >>> May be not required, bcz it will come after i apply below s-o-b >>> >>>>>> --- >>>>> You missed change log for all patches, i think you have summarized >>>>> in >>>>> 0/6. >>>>> I feel it's better write it on individual patches. >>>>> >>>> Ok. >>>> >>>>>> drivers/spi/Makefile | 1 + >>>>>> drivers/spi/ti_qspi.c | 328 >>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> 2 files changed, 329 insertions(+), 0 deletions(-) >>>>>> create mode 100644 drivers/spi/ti_qspi.c >>>>>> >>>>>> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile >>>>>> index 91d24ce..e5941b0 100644 >>>>>> --- a/drivers/spi/Makefile >>>>>> +++ b/drivers/spi/Makefile >>>>>> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o >>>>>> COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o >>>>>> COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o >>>>>> COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o >>>>>> +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o >>>>>> COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o >>>>>> COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o >>>>>> >>>>>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c >>>>>> new file mode 100644 >>>>>> index 0000000..d8a03a8 >>>>>> --- /dev/null >>>>>> +++ b/drivers/spi/ti_qspi.c >>>>>> @@ -0,0 +1,328 @@ >>>>>> +/* >>>>>> + * TI QSPI driver >>>>>> + * >>>>>> + * Copyright (C) 2013, Texas Instruments, Incorporated >>>>>> + * >>>>>> + * SPDX-License-Identifier: GPL-2.0+ >>>>> Got below format after apply this patch - please check >>>>> *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+ >>>>> >>>> ahh..I copied it from a patch on some list. May be something went >>>> wrong, >>>> I >>>> will check. >>>> >>>>>> + */ >>>>>> + >>>>>> +#include<common.h> >>>>>> +#include<asm/io.h> >>>>>> +#include<asm/arch/omap.h> >>>>>> +#include<malloc.h> >>>>>> +#include<spi.h> >>>>>> + >>>>>> +struct qspi_regs { >>>>>> +u32 pid; >>>>>> +u32 pad0[3]; >>>>>> +u32 sysconfig; >>>>>> +u32 pad1[3]; >>>>>> +u32 intr_status_raw_set; >>>>>> +u32 intr_status_enabled_clear; >>>>>> +u32 intr_enable_set; >>>>>> +u32 intr_enable_clear; >>>>>> +u32 intc_eoi; >>>>>> +u32 pad2[3]; >>>>>> +u32 spi_clock_cntrl; >>>>>> +u32 spi_dc; >>>>>> +u32 spi_cmd; >>>>>> +u32 spi_status; >>>>>> +u32 spi_data; >>>>>> +u32 spi_setup0; >>>>>> +u32 spi_setup1; >>>>>> +u32 spi_setup2; >>>>>> +u32 spi_setup3; >>>>>> +u32 spi_switch; >>>>>> +u32 spi_data1; >>>>>> +u32 spi_data2; >>>>>> +u32 spi_data3; >>>>> Please add tab space. >>>>> >>>> ok >>>> >>>>>> +}; >>>>>> + >>>>>> +struct qspi_slave { >>>>>> + struct spi_slave slave; >>>>>> + struct qspi_regs *base; >>>>>> + unsigned int mode; >>>>>> + u32 cmd; >>>>>> + u32 dc; >>>>>> +}; >>>>>> + >>>>> -- TAG+ >>>>>> +#define QSPI_TIMEOUT 2000000 >>>>>> + >>>>>> +#define QSPI_FCLK 192000000 >>>>>> + >>>>>> +/* Clock Control */ >>>>>> +#define QSPI_CLK_EN (1<< 31) >>>>>> +#define QSPI_CLK_DIV_MAX 0xffff >>>>>> + >>>>>> +/* Command */ >>>>>> +#define QSPI_EN_CS(n) (n<< 28) >>>>>> +#define QSPI_WLEN(n) ((n-1)<< 19) >>>>>> +#define QSPI_3_PIN (1<< 18) >>>>>> +#define QSPI_RD_SNGL (1<< 16) >>>>>> +#define QSPI_WR_SNGL (2<< 16) >>>>>> +#define QSPI_INVAL (4<< 16) >>>>>> +#define QSPI_RD_QUAD (7<< 16) >>>>>> + >>>>>> +/* Device Control */ >>>>>> +#define QSPI_DD(m, n) (m<< (3 + n*8)) >>>>>> +#define QSPI_CKPHA(n) (1<< (2 + n*8)) >>>>>> +#define QSPI_CSPOL(n) (1<< (1 + n*8)) >>>>>> +#define QSPI_CKPOL(n) (1<< (n*8)) >>>>>> + >>>>>> +/* Status */ >>>>>> +#define QSPI_WC (1<< 1) >>>>>> +#define QSPI_BUSY (1<< 0) >>>>>> +#define QSPI_WC_BUSY (QSPI_WC | QSPI_BUSY) >>>>>> +#define QSPI_XFER_DONE QSPI_WC >>>>>> + >>>>>> +#define MM_SWITCH 0x01 >>>>>> +#define MEM_CS 0x100 >>>>>> +#define MEM_CS_UNSELECT 0xfffff0ff >>>>>> +#define MMAP_START_ADDR 0x5c000000 >>>>>> +#define CORE_CTRL_IO 0x4a002558 >>>>>> + >>>>>> +#define QSPI_CMD_READ (0x3<< 0) >>>>>> +#define QSPI_CMD_READ_QUAD (0x6b<< 0) >>>>>> +#define QSPI_CMD_READ_FAST (0x0b<< 0) >>>>>> +#define QSPI_SETUP0_NUM_A_BYTES (0x2<< 8) >>>>>> +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS (0x0<< 10) >>>>>> +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1<< 10) >>>>>> +#define QSPI_SETUP0_READ_NORMAL (0x0<< 12) >>>>>> +#define QSPI_SETUP0_READ_QUAD (0x3<< 12) >>>>>> +#define QSPI_CMD_WRITE (0x2<< 16) >>>>>> +#define QSPI_NUM_DUMMY_BITS (0x0<< 24) >>>>> --TAG- >>>>> >>>>> TAG+ ... TAG- please move these macro definitions in below headers >>>> Ok. >>>> >>>>>> + >>>>>> +static inline struct qspi_slave *to_qspi_slave(struct spi_slave >>>>>> *slave) >>>>>> +{ >>>>>> + return container_of(slave, struct qspi_slave, slave); >>>>>> +} >>>>>> +static inline struct qspi_regs *get_qspi_bus(int dev) >>>>>> +{ >>>>>> + if (!dev) >>>>>> + return (struct qspi_regs *)QSPI_BASE; >>>>>> + else >>>>>> + return NULL; >>>>>> +} >>>>> Is this function really required, how many bus controller you >>>>> have? >>>> Actually one. >>> Ok, Please remove this function and assign directly. >>> >>>>>> + >>>>>> +int spi_cs_is_valid(unsigned int bus, unsigned int cs) >>>>>> +{ >>>>>> + return 1; >>>>>> +} >>>>>> + >>>>>> +void spi_cs_activate(struct spi_slave *slave) >>>>>> +{ >>>>>> + /* CS handled in xfer */ >>>>>> + return; >>>>>> +} >>>>>> + >>>>>> +void spi_cs_deactivate(struct spi_slave *slave) >>>>>> +{ >>>>>> + /* CS handled in xfer */ >>>>>> + return; >>>>>> +} >>>>>> + >>>>>> +void spi_init(void) >>>>>> +{ >>>>>> + /* nothing to do */ >>>>>> +} >>>>>> + >>>>>> +void spi_set_up_spi_register(struct qspi_slave *qslave) >>>>>> +{ >>>>>> + u32 memval = 0; >>>>>> + struct spi_slave *slave =&qslave->slave; >>>>>> >>>>>> + >>>>>> + slave->memory_map = (void *)MMAP_START_ADDR; >>>>>> + >>>>>> + memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES | >>>>>> + QSPI_SETUP0_NUM_D_BYTES_NO_BITS | >>>>>> QSPI_SETUP0_READ_NORMAL >>>>>> | >>>>>> + QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS); >>>>>> + >>>>>> + writel(memval,&qslave->base->spi_setup0); >>>>>> >>>>>> +} >>>>>> + >>>>>> +void spi_set_speed(struct spi_slave *slave, uint hz) >>>>>> +{ >>>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>>> + >>>>>> + uint clk_div; >>>>>> + >>>>>> + if (!hz) >>>>>> + clk_div = 0; >>>>>> + else >>>>>> + clk_div = (QSPI_FCLK / hz) - 1; >>>>>> + >>>>>> + debug("%s: hz: %d, clock divider %d\n", __func__, hz, >>>>>> clk_div); >>>>>> + >>>>>> + /* disable SCLK */ >>>>>> + writel(readl(&qslave->base->spi_clock_cntrl)& >>>>>> ~QSPI_CLK_EN, >>>>>> +&qslave->base->spi_clock_cntrl); >>>>>> >>>>>> + >>>>>> + if (clk_div< 0) { >>>>>> + debug("%s: clock divider< 0, using /1 >>>>>> divider\n", >>>>>> __func__); >>>>>> + clk_div = 0; >>>>>> + } >>>>>> + >>>>>> + if (clk_div> QSPI_CLK_DIV_MAX) { >>>>>> + debug("%s: clock divider>%d , using /%d >>>>>> divider\n", >>>>>> + __func__, QSPI_CLK_DIV_MAX, >>>>>> QSPI_CLK_DIV_MAX >>>>>> + >>>>>> 1); >>>>>> + clk_div = QSPI_CLK_DIV_MAX; >>>>>> + } >>>>>> + >>>>>> + /* enable SCLK */ >>>>>> + writel(QSPI_CLK_EN | >>>>>> clk_div,&qslave->base->spi_clock_cntrl); >>>>>> >>>>>> + debug("%s: spi_clock_cntrl %08x\n", __func__, >>>>>> + readl(&qslave->base->spi_clock_cntrl)); >>>>>> +} >>>>>> + >>>>>> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int >>>>>> cs, >>>>>> + unsigned int max_hz, unsigned >>>>>> int >>>>>> mode) >>>>>> +{ >>>>>> + struct qspi_slave *qslave; >>>>>> + >>>>>> + qslave = spi_alloc_slave(struct qspi_slave, bus, cs); >>>>>> + if (!qslave) >>>>>> + return NULL; >>>>>> + >>>>>> + qslave->base = get_qspi_bus(bus); >>>>>> + if (qslave->base) >>>>>> + debug("No Qspi device found\n"); >>>>>> + spi_set_speed(&qslave->slave, max_hz); >>>>>> + qslave->mode = mode; >>>>>> + >>>>>> +#ifdef CONFIG_MMAP >>>>>> + spi_set_up_spi_register(qslave); >>>>>> +#endif >>>>>> + >>>>>> + debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, >>>>>> mode); >>>>>> + >>>>>> + return&qslave->slave; >>>>>> >>>>>> +} >>>>>> + >>>>>> +void spi_free_slave(struct spi_slave *slave) >>>>>> +{ >>>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>>> + free(qslave); >>>>>> +} >>>>>> + >>>>>> +int spi_claim_bus(struct spi_slave *slave) >>>>>> +{ >>>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>>> + >>>>>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>>>>> slave->cs); >>>>>> + >>>>>> + writel(0,&qslave->base->spi_dc); >>>>>> + writel(0,&qslave->base->spi_cmd); >>>>>> + writel(0,&qslave->base->spi_data); >>>>>> >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +void spi_release_bus(struct spi_slave *slave) >>>>>> +{ >>>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>>> + >>>>>> + debug("%s: bus:%i cs:%i\n", __func__, slave->bus, >>>>>> slave->cs); >>>>>> + >>>>>> + writel(0,&qslave->base->spi_dc); >>>>>> + writel(0,&qslave->base->spi_cmd); >>>>>> + writel(0,&qslave->base->spi_data); >>>>>> >>>>>> +} >>>>>> + >>>>>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const >>>>>> void >>>>>> *dout, >>>>>> + void *din, unsigned long flags) >>>>>> +{ >>>>>> + struct qspi_slave *qslave = to_qspi_slave(slave); >>>>>> + uint words = bitlen>> 3; /* fixed 8-bit word length >>>>>> */ >>>>>> + const uchar *txp = dout; >>>>>> + uchar *rxp = din; >>>>>> + uint status; >>>>>> + int timeout, val; >>>>>> + >>>>>> + debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", >>>>>> __func__, >>>>>> + slave->bus, slave->cs, bitlen, words, flags); >>>>>> + >>>>>> + qslave->dc = 0; >>>>>> + if (qslave->mode& SPI_CPHA) >>>>>> >>>>>> + qslave->dc |= QSPI_CKPHA(slave->cs); >>>>>> + if (qslave->mode& SPI_CPOL) >>>>>> >>>>>> + qslave->dc |= QSPI_CKPOL(slave->cs); >>>>>> + if (qslave->mode& SPI_CS_HIGH) >>>>>> >>>>>> + qslave->dc |= QSPI_CSPOL(slave->cs); >>>>>> + >>>>>> + writel(qslave->dc,&qslave->base->spi_dc); >>>>> Adjust this code in spi_claim_bus() >>>>> >>>> Ok. >>>>>> + >>>>>> + if (flags& SPI_XFER_MEM_MAP) { >>>>>> + writel(MM_SWITCH,&qslave->base->spi_switch); >>>>>> >>>>>> + val = readl(CORE_CTRL_IO); >>>>>> + val |= MEM_CS; >>>>>> + writel(val, CORE_CTRL_IO); >>>>>> + return 0; >>>>>> + } else if (flags& SPI_XFER_MEM_MAP_END) { >>>>>> + writel(~MM_SWITCH,&qslave->base->spi_switch); >>>>>> + val = readl(CORE_CTRL_IO); >>>>>> + val&= MEM_CS_UNSELECT; >>>>>> >>>>>> + writel(val, CORE_CTRL_IO); >>>>>> + return 0; >>>>>> + } >>>>> What is this your are returning from here directly for memory_map? >>>>> is >>>>> it? >>>> Yes, memory map does not care about setting the command register >>>> and >>>> doing the transfer using the normal spi framework. >>>> Memory ma has a different set of registers, set up >>>> register(configured >>>> above). >>>> Here, we just switch the controller to memory mapped port and use >>>> flash->memory_map >>>> to read the data from the memory mapped port(0x5c000000). >>> OK. can you readjust this code by placing existing spi_flash func >>> like >>> spi_cs_activate() >>> Looks like this cs activate code. >> I dont think so it make much sense to put it ib cs_activate apis, >> this >> portion >> does not really justify that. This code is more SOC specific control >> module >> parameters which allows you to switch between the configuaration port >> and memory mapped port. >> >>> Where is SPI_XFER_BEGIN are you not using this? >> We dont need SPI_XFER_BEGIN here, for memory mapped we just need >> MEM_MAP flag, do the required memory mapped read and end the >> transfer. >> This is what differentiate a SPI based read and a memory mapped read. > OK. > Understand, let me clear once are you trying to use erase/write from > spi_flash or not? > The reason for asking if yes, we need to use BEGIN on driver and also > these is one more controller already exists on spi > where the controller uses read for mmap and erase/write is common - > are your driver looks same as this or different? > Yes, erase and write commands are also done. Generally, BEGIN flag is used sp that cs_activate can be called. But, in my case cs is activated in a different way, its enabled by writing in a command register. So, there is no need for a BEGIN.
OK, please do all modifications as we discussed so-far. and send the next level- so we can discuss more.
Please follow the driver code model- what I referred in previous mail.
Ok. I will send the v5 with all the discussed modification.
>>>>>> + >>>>> --TAG+ >>>>>> + if (bitlen == 0) >>>>>> + goto out; >>>>>> + >>>>>> + if (bitlen % 8) { >>>>>> + flags |= SPI_XFER_END; >>>>>> + goto out; >>>>>> + } >>>>> --TAG- >>>>> >>>>> TAG+ .. TAG- please move this code on start of the xfer..ie below >>>>> debug(); >>>>> >>>> I understand this point, but it was done purposefully. I wanted to >>>> hit >>>> this >>>> point only if memory map is not invoked. If you see sf_ops.c, I am >>>> invoking >>>> the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, >>>> NULL, >>>> SPI_XFER_MEM_MAP)" >>>> If I place TAG+..TAG- before memory map stuff above, it will always >>>> goto >>>> out. >>>> >>>> So, >>>> either I keep it here or >>>> pass some dummy non zero value for bitlen in above mentioned >>>> spi_xfer. >>>> ? >>> Sound good, please keep here. >>> >>>>>> + >>>>>> + /* setup command reg */ >>>>>> + qslave->cmd = 0; >>>>>> + qslave->cmd |= QSPI_WLEN(8); >>>>>> + qslave->cmd |= QSPI_EN_CS(slave->cs); >>>>>> + if (flags& SPI_3WIRE) >>>>>> >>>>>> + qslave->cmd |= QSPI_3_PIN; >>>>>> + qslave->cmd |= 0xfff; >>>>>> + >>>>>> + while (words--) { >>>>>> + if (txp) { >>>>>> + debug("tx cmd %08x dc %08x data %02x\n", >>>>>> + qslave->cmd | QSPI_WR_SNGL, >>>>>> qslave->dc, >>>>>> *txp); >>>>>> + writel(*txp++,&qslave->base->spi_data); >>>>>> + writel(qslave->cmd | QSPI_WR_SNGL, >>>>>> +&qslave->base->spi_cmd); >>>>>> >>>>>> + status = >>>>>> readl(&qslave->base->spi_status); >>>>>> + timeout = QSPI_TIMEOUT; >>>>>> + while ((status& QSPI_WC_BUSY) != >>>>>> QSPI_XFER_DONE) >>>>>> { >>>>>> >>>>>> + if (--timeout< 0) { >>>>>> + printf("QSPI tx timed >>>>>> out\n"); >>>>>> + return -1; >>>>>> + } >>>>>> + status = >>>>>> readl(&qslave->base->spi_status); >>>>>> + } >>>>>> + debug("tx done, status %08x\n", status); >>>>>> + } >>>>>> + if (rxp) { >>>>>> + qslave->cmd |= QSPI_RD_SNGL; >>>>>> + debug("rx cmd %08x dc %08x\n", >>>>>> + qslave->cmd, qslave->dc); >>>>>> + >>>>>> writel(qslave->cmd,&qslave->base->spi_cmd); >>>>>> >>>>>> + status = >>>>>> readl(&qslave->base->spi_status); >>>>>> + timeout = QSPI_TIMEOUT; >>>>>> + while ((status& QSPI_WC_BUSY) != >>>>>> QSPI_XFER_DONE) >>>>>> { >>>>>> >>>>>> + if (--timeout< 0) { >>>>>> + printf("QSPI rx timed >>>>>> out\n"); >>>>>> + return -1; >>>>>> + } >>>>>> + status = >>>>>> readl(&qslave->base->spi_status); >>>>>> + } >>>>>> + *rxp++ = readl(&qslave->base->spi_data); >>>>>> + debug("rx done, status %08x, read >>>>>> %02x\n", >>>>>> + status, *(rxp-1)); >>>>>> + } >>>>>> + } >>>>>> + >>>>>> +out: >>>>>> + /* Terminate frame */ >>>>>> + if (flags& SPI_XFER_END) >>>>>> + writel(qslave->cmd | >>>>>> QSPI_INVAL,&qslave->base->spi_cmd); >>>>> Please palce this code in spi_cs_deactivate() >>>>> >>>>> I request you please follow the code structure in below thread. >>>>> I feel better if you use same prints that used in below example >>>>> driver. >>>>> http://patchwork.ozlabs.org/patch/265683/ >>

From: Matt Porter matt.porter@linaro.org
Enables support for SPI SPL, QSPI and Spansion serial flash device on the EVM. Configures pin muxes for QSPI mode.
Signed-off-by: Matt Porter matt.porter@linaro.org Signed-off-by: Sourav Poddar sourav.poddar@ti.com --- board/ti/dra7xx/mux_data.h | 10 ++++++++++ include/configs/dra7xx_evm.h | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/board/ti/dra7xx/mux_data.h b/board/ti/dra7xx/mux_data.h index 0a86594..6965cc5 100644 --- a/board/ti/dra7xx/mux_data.h +++ b/board/ti/dra7xx/mux_data.h @@ -51,5 +51,15 @@ const struct pad_conf_entry core_padconf_array_essential[] = { {RGMII0_RXD2, (IEN | M0) }, {RGMII0_RXD1, (IEN | M0) }, {RGMII0_RXD0, (IEN | M0) }, + {GPMC_A13, (IEN | PDIS | M1)}, /* QSPI1_RTCLK */ + {GPMC_A14, (IEN | PDIS | M1)}, /* QSPI1_D[3] */ + {GPMC_A15, (IEN | PDIS | M1)}, /* QSPI1_D[2] */ + {GPMC_A16, (IEN | PDIS | M1)}, /* QSPI1_D[1] */ + {GPMC_A17, (IEN | PDIS | M1)}, /* QSPI1_D[0] */ + {GPMC_A18, (M1)}, /* QSPI1_SCLK */ + {GPMC_A3, (IEN | PDIS | M1)}, /* QSPI1_CS2 */ + {GPMC_A4, (IEN | PDIS | M1)}, /* QSPI1_CS3 */ + {GPMC_CS2, (IEN | PTU | PDIS | M1)}, /* QSPI1_CS0 */ + {GPMC_CS3, (IEN | PTU | PDIS | M1)}, /* QSPI1_CS1*/ }; #endif /* _MUX_DATA_DRA7XX_H_ */ diff --git a/include/configs/dra7xx_evm.h b/include/configs/dra7xx_evm.h index 4fbe768..310c7ac 100644 --- a/include/configs/dra7xx_evm.h +++ b/include/configs/dra7xx_evm.h @@ -42,4 +42,23 @@ #define CONFIG_PHYLIB #define CONFIG_PHY_ADDR 2
+/* SPI */ +#undef CONFIG_OMAP3_SPI +#define CONFIG_TI_QSPI +#define CONFIG_SPI_FLASH +#define CONFIG_SPI_FLASH_SPANSION +#define CONFIG_CMD_SF +#define CONFIG_CMD_SPI +#define CONFIG_MMAP +#define CONFIG_SF_DEFAULT_SPEED 48000000 +#define CONFIG_DEFAULT_SPI_MODE SPI_MODE_3 + +/* SPI SPL */ +#define CONFIG_SPL_SPI_SUPPORT +#define CONFIG_SPL_SPI_LOAD +#define CONFIG_SPL_SPI_FLASH_SUPPORT +#define CONFIG_SPL_SPI_BUS 0 +#define CONFIG_SPL_SPI_CS 0 +#define CONFIG_SYS_SPI_U_BOOT_OFFS 0x20000 + #endif /* __CONFIG_DRA7XX_EVM_H */

Contains documentation and testing details for qspi flash interface.
Signed-off-by: Sourav Poddar sourav.poddar@ti.com --- doc/README.ti_qspi_dra_test | 38 ++++++++++++++++++++++++++++++++++ doc/README.ti_qspi_flash | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 0 deletions(-) create mode 100644 doc/README.ti_qspi_dra_test create mode 100644 doc/README.ti_qspi_flash
diff --git a/doc/README.ti_qspi_dra_test b/doc/README.ti_qspi_dra_test new file mode 100644 index 0000000..c4540ea --- /dev/null +++ b/doc/README.ti_qspi_dra_test @@ -0,0 +1,38 @@ +------------------------------------------------- + Simple steps used to test the QSPI at U-Boot +------------------------------------------------- + +For #1, build the patched U-Boot and load MLO/u-boot.img + +---------------------------------- +Boot from another medium like MMC +---------------------------------- + +DRA752 EVM # mmc dev 0 +DRA752 EVM # fatload mmc 0 0x82000000 MLO +DRA752 EVM # fatload mmc 0 0x83000000 u-boot.img + +-------------------------------------------------- +Commands to erase/write u-boot/mlo to flash device +-------------------------------------------------- + +DRA752 EVM # sf probe 0 +[should detect the S25FL256S serial flash device] + +DRA752 EVM # sf erase 0 10000 +DRA752 EVM # sf erase 10000 10000 +DRA752 EVM # sf erase 20000 10000 +DRA752 EVM # sf erase 30000 10000 +DRA752 EVM # sf erase 40000 10000 +DRA752 EVM # sf erase 50000 10000 +DRA752 EVM # sf erase 60000 10000 + +DRA752 EVM # sf write 82000000 0 10000 +DRA752 EVM # sf write 83000000 20000 70000 + +For #2, set sysboot to QSPI-1 boot mode(SYSBOOT[5:0] = 100110) and power +on. ROM should find the GP header at offset 0 and load/execute SPL. SPL +then detects that ROM was in QSPI-1 mode (boot code 10) and attempts to +find a U-Boot image header at offset 0x20000 (set in the config file) +and proceeds to load that image using the U-Boot image payload offset/size +from the header. It will then start U-Boot. diff --git a/doc/README.ti_qspi_flash b/doc/README.ti_qspi_flash new file mode 100644 index 0000000..1b86d01 --- /dev/null +++ b/doc/README.ti_qspi_flash @@ -0,0 +1,47 @@ +QSPI U-boot support +------------------ + +Host processor is connected to serial flash device via qpsi +interface. QSPI is a kind of spi module that allows single, +dual and quad read access to external spi devices. The module +has a memory mapped interface which provide direct interface +for accessing data form external spi devices. + +The one QSPI in the device is primarily intended for fast booting +from Quad SPI flash devices. + +Usecase +------- + +MLO/u-boot.img will be flashed from SD/MMC to the flash device +using serial flash erase and write commands. Then, switch settings +will be changed to qspi boot. Then, the ROM code will read MLO +from the predefined location in the flash, where it was flashed and +execute it after storing it in SDRAM. Then, the MLO will read +u-boot.img from flash and execute it from SDRAM. + +SPI mode +------- +SPI mode uses mtd spi framework for transfer and reception of data. +Can be used in: +1. Normal mode: use single pin for transfers +2. Dual Mode: use two pins for transfers. +3. Quad mode: use four pin for transfer + +Memory mapped read mode +----------------------- +In this, SPI controller is configured using configuration port and then +controler is switched to memory mapped port for data read. + +Driver +------ +drivers/qspi/ti_qspi.c + - Newly created file which is responsible for configuring the + qspi controller and also for providing the low level api which + is responsible for transferring the datas from host controller + to flash device and vice versa. + +Testing +------- +A seperated file named README.dra_qspi_test has been created which gives all the +details about the commands required to test qspi at u-boot level.

Hi Sourav,
Please place these these readme files in doc/SPI/* All these patches tested on top of u-boot-spi.git master-probe?
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar sourav.poddar@ti.com wrote:
Contains documentation and testing details for qspi flash interface.
Signed-off-by: Sourav Poddar sourav.poddar@ti.com
doc/README.ti_qspi_dra_test | 38 ++++++++++++++++++++++++++++++++++ doc/README.ti_qspi_flash | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 0 deletions(-) create mode 100644 doc/README.ti_qspi_dra_test create mode 100644 doc/README.ti_qspi_flash
diff --git a/doc/README.ti_qspi_dra_test b/doc/README.ti_qspi_dra_test new file mode 100644 index 0000000..c4540ea --- /dev/null +++ b/doc/README.ti_qspi_dra_test @@ -0,0 +1,38 @@ +-------------------------------------------------
- Simple steps used to test the QSPI at U-Boot
+-------------------------------------------------
+For #1, build the patched U-Boot and load MLO/u-boot.img
+---------------------------------- +Boot from another medium like MMC +----------------------------------
+DRA752 EVM # mmc dev 0 +DRA752 EVM # fatload mmc 0 0x82000000 MLO +DRA752 EVM # fatload mmc 0 0x83000000 u-boot.img
+-------------------------------------------------- +Commands to erase/write u-boot/mlo to flash device +--------------------------------------------------
+DRA752 EVM # sf probe 0 +[should detect the S25FL256S serial flash device]
+DRA752 EVM # sf erase 0 10000 +DRA752 EVM # sf erase 10000 10000 +DRA752 EVM # sf erase 20000 10000 +DRA752 EVM # sf erase 30000 10000 +DRA752 EVM # sf erase 40000 10000 +DRA752 EVM # sf erase 50000 10000 +DRA752 EVM # sf erase 60000 10000
+DRA752 EVM # sf write 82000000 0 10000 +DRA752 EVM # sf write 83000000 20000 70000
These test procedure steps were done in real hw. Seems like written once, could be generic if you test these steps on real hw and palce the same log here...
+For #2, set sysboot to QSPI-1 boot mode(SYSBOOT[5:0] = 100110) and power +on. ROM should find the GP header at offset 0 and load/execute SPL. SPL +then detects that ROM was in QSPI-1 mode (boot code 10) and attempts to +find a U-Boot image header at offset 0x20000 (set in the config file) +and proceeds to load that image using the U-Boot image payload offset/size +from the header. It will then start U-Boot. diff --git a/doc/README.ti_qspi_flash b/doc/README.ti_qspi_flash new file mode 100644 index 0000000..1b86d01 --- /dev/null +++ b/doc/README.ti_qspi_flash @@ -0,0 +1,47 @@ +QSPI U-boot support +------------------
+Host processor is connected to serial flash device via qpsi +interface. QSPI is a kind of spi module that allows single, +dual and quad read access to external spi devices. The module +has a memory mapped interface which provide direct interface +for accessing data form external spi devices.
+The one QSPI in the device is primarily intended for fast booting +from Quad SPI flash devices.
+Usecase +-------
+MLO/u-boot.img will be flashed from SD/MMC to the flash device +using serial flash erase and write commands. Then, switch settings +will be changed to qspi boot. Then, the ROM code will read MLO +from the predefined location in the flash, where it was flashed and +execute it after storing it in SDRAM. Then, the MLO will read +u-boot.img from flash and execute it from SDRAM.
+SPI mode +------- +SPI mode uses mtd spi framework for transfer and reception of data. +Can be used in: +1. Normal mode: use single pin for transfers +2. Dual Mode: use two pins for transfers. +3. Quad mode: use four pin for transfer
+Memory mapped read mode +----------------------- +In this, SPI controller is configured using configuration port and then +controler is switched to memory mapped port for data read.
+Driver +------ +drivers/qspi/ti_qspi.c
- Newly created file which is responsible for configuring the
qspi controller and also for providing the low level api which
is responsible for transferring the datas from host controller
to flash device and vice versa.
+Testing +------- +A seperated file named README.dra_qspi_test has been created which gives all the
+details about the commands required to test qspi at u-boot level.
1.7.1
Please use the doc/SPI/status.txt as an example format for writing new readme files.

On Saturday 05 October 2013 12:08 AM, Jagan Teki wrote:
Hi Sourav,
Please place these these readme files in doc/SPI/* All these patches tested on top of u-boot-spi.git master-probe?
Yes, this are tested on the above branch.
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
Contains documentation and testing details for qspi flash interface.
Signed-off-by: Sourav Poddarsourav.poddar@ti.com
doc/README.ti_qspi_dra_test | 38 ++++++++++++++++++++++++++++++++++ doc/README.ti_qspi_flash | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 0 deletions(-) create mode 100644 doc/README.ti_qspi_dra_test create mode 100644 doc/README.ti_qspi_flash
diff --git a/doc/README.ti_qspi_dra_test b/doc/README.ti_qspi_dra_test new file mode 100644 index 0000000..c4540ea --- /dev/null +++ b/doc/README.ti_qspi_dra_test @@ -0,0 +1,38 @@ +-------------------------------------------------
- Simple steps used to test the QSPI at U-Boot
+-------------------------------------------------
+For #1, build the patched U-Boot and load MLO/u-boot.img
+---------------------------------- +Boot from another medium like MMC +----------------------------------
+DRA752 EVM # mmc dev 0 +DRA752 EVM # fatload mmc 0 0x82000000 MLO +DRA752 EVM # fatload mmc 0 0x83000000 u-boot.img
+-------------------------------------------------- +Commands to erase/write u-boot/mlo to flash device +--------------------------------------------------
+DRA752 EVM # sf probe 0 +[should detect the S25FL256S serial flash device]
+DRA752 EVM # sf erase 0 10000 +DRA752 EVM # sf erase 10000 10000 +DRA752 EVM # sf erase 20000 10000 +DRA752 EVM # sf erase 30000 10000 +DRA752 EVM # sf erase 40000 10000 +DRA752 EVM # sf erase 50000 10000 +DRA752 EVM # sf erase 60000 10000
+DRA752 EVM # sf write 82000000 0 10000 +DRA752 EVM # sf write 83000000 20000 70000
These test procedure steps were done in real hw. Seems like written once, could be generic if you test these steps on real hw and palce the same log here...
+For #2, set sysboot to QSPI-1 boot mode(SYSBOOT[5:0] = 100110) and power +on. ROM should find the GP header at offset 0 and load/execute SPL. SPL +then detects that ROM was in QSPI-1 mode (boot code 10) and attempts to +find a U-Boot image header at offset 0x20000 (set in the config file) +and proceeds to load that image using the U-Boot image payload offset/size +from the header. It will then start U-Boot. diff --git a/doc/README.ti_qspi_flash b/doc/README.ti_qspi_flash new file mode 100644 index 0000000..1b86d01 --- /dev/null +++ b/doc/README.ti_qspi_flash @@ -0,0 +1,47 @@ +QSPI U-boot support +------------------
+Host processor is connected to serial flash device via qpsi +interface. QSPI is a kind of spi module that allows single, +dual and quad read access to external spi devices. The module +has a memory mapped interface which provide direct interface +for accessing data form external spi devices.
+The one QSPI in the device is primarily intended for fast booting +from Quad SPI flash devices.
+Usecase +-------
+MLO/u-boot.img will be flashed from SD/MMC to the flash device +using serial flash erase and write commands. Then, switch settings +will be changed to qspi boot. Then, the ROM code will read MLO +from the predefined location in the flash, where it was flashed and +execute it after storing it in SDRAM. Then, the MLO will read +u-boot.img from flash and execute it from SDRAM.
+SPI mode +------- +SPI mode uses mtd spi framework for transfer and reception of data. +Can be used in: +1. Normal mode: use single pin for transfers +2. Dual Mode: use two pins for transfers. +3. Quad mode: use four pin for transfer
+Memory mapped read mode +----------------------- +In this, SPI controller is configured using configuration port and then +controler is switched to memory mapped port for data read.
+Driver +------ +drivers/qspi/ti_qspi.c
- Newly created file which is responsible for configuring the
qspi controller and also for providing the low level api which
is responsible for transferring the datas from host controller
to flash device and vice versa.
+Testing +------- +A seperated file named README.dra_qspi_test has been created which gives all the
+details about the commands required to test qspi at u-boot level.
1.7.1
Please use the doc/SPI/status.txt as an example format for writing new readme files.

May be your are missing my comments in previous post. - Please place these these readme files in doc/SPI/* - Please use the doc/SPI/status.txt as an example format for writing new readme files.
On Sat, Oct 5, 2013 at 1:15 AM, Sourav Poddar sourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:08 AM, Jagan Teki wrote:
Hi Sourav,
Please place these these readme files in doc/SPI/* All these patches tested on top of u-boot-spi.git master-probe?
Yes, this are tested on the above branch.
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
Contains documentation and testing details for qspi flash interface.
Signed-off-by: Sourav Poddarsourav.poddar@ti.com
doc/README.ti_qspi_dra_test | 38 ++++++++++++++++++++++++++++++++++ doc/README.ti_qspi_flash | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 0 deletions(-) create mode 100644 doc/README.ti_qspi_dra_test create mode 100644 doc/README.ti_qspi_flash
diff --git a/doc/README.ti_qspi_dra_test b/doc/README.ti_qspi_dra_test new file mode 100644 index 0000000..c4540ea --- /dev/null +++ b/doc/README.ti_qspi_dra_test @@ -0,0 +1,38 @@ +-------------------------------------------------
- Simple steps used to test the QSPI at U-Boot
+-------------------------------------------------
+For #1, build the patched U-Boot and load MLO/u-boot.img
+---------------------------------- +Boot from another medium like MMC +----------------------------------
+DRA752 EVM # mmc dev 0 +DRA752 EVM # fatload mmc 0 0x82000000 MLO +DRA752 EVM # fatload mmc 0 0x83000000 u-boot.img
+-------------------------------------------------- +Commands to erase/write u-boot/mlo to flash device +--------------------------------------------------
+DRA752 EVM # sf probe 0 +[should detect the S25FL256S serial flash device]
+DRA752 EVM # sf erase 0 10000 +DRA752 EVM # sf erase 10000 10000 +DRA752 EVM # sf erase 20000 10000 +DRA752 EVM # sf erase 30000 10000 +DRA752 EVM # sf erase 40000 10000 +DRA752 EVM # sf erase 50000 10000 +DRA752 EVM # sf erase 60000 10000
+DRA752 EVM # sf write 82000000 0 10000 +DRA752 EVM # sf write 83000000 20000 70000
These test procedure steps were done in real hw. Seems like written once, could be generic if you test these steps on real hw and palce the same log here...
+For #2, set sysboot to QSPI-1 boot mode(SYSBOOT[5:0] = 100110) and power +on. ROM should find the GP header at offset 0 and load/execute SPL. SPL +then detects that ROM was in QSPI-1 mode (boot code 10) and attempts to +find a U-Boot image header at offset 0x20000 (set in the config file) +and proceeds to load that image using the U-Boot image payload offset/size +from the header. It will then start U-Boot. diff --git a/doc/README.ti_qspi_flash b/doc/README.ti_qspi_flash new file mode 100644 index 0000000..1b86d01 --- /dev/null +++ b/doc/README.ti_qspi_flash @@ -0,0 +1,47 @@ +QSPI U-boot support +------------------
+Host processor is connected to serial flash device via qpsi +interface. QSPI is a kind of spi module that allows single, +dual and quad read access to external spi devices. The module +has a memory mapped interface which provide direct interface +for accessing data form external spi devices.
+The one QSPI in the device is primarily intended for fast booting +from Quad SPI flash devices.
+Usecase +-------
+MLO/u-boot.img will be flashed from SD/MMC to the flash device +using serial flash erase and write commands. Then, switch settings +will be changed to qspi boot. Then, the ROM code will read MLO +from the predefined location in the flash, where it was flashed and +execute it after storing it in SDRAM. Then, the MLO will read +u-boot.img from flash and execute it from SDRAM.
+SPI mode +------- +SPI mode uses mtd spi framework for transfer and reception of data. +Can be used in: +1. Normal mode: use single pin for transfers +2. Dual Mode: use two pins for transfers. +3. Quad mode: use four pin for transfer
+Memory mapped read mode +----------------------- +In this, SPI controller is configured using configuration port and then +controler is switched to memory mapped port for data read.
+Driver +------ +drivers/qspi/ti_qspi.c
- Newly created file which is responsible for configuring the
qspi controller and also for providing the low level api which
is responsible for transferring the datas from host controller
to flash device and vice versa.
+Testing +------- +A seperated file named README.dra_qspi_test has been created which gives all the
+details about the commands required to test qspi at u-boot level.
1.7.1
Please use the doc/SPI/status.txt as an example format for writing new readme files.

On Saturday 05 October 2013 01:44 AM, Jagan Teki wrote:
May be your are missing my comments in previous post.
- Please place these these readme files in doc/SPI/*
- Please use the doc/SPI/status.txt as an example format for writing
new readme files.
ok.
On Sat, Oct 5, 2013 at 1:15 AM, Sourav Poddarsourav.poddar@ti.com wrote:
On Saturday 05 October 2013 12:08 AM, Jagan Teki wrote:
Hi Sourav,
Please place these these readme files in doc/SPI/* All these patches tested on top of u-boot-spi.git master-probe?
Yes, this are tested on the above branch.
On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddarsourav.poddar@ti.com wrote:
Contains documentation and testing details for qspi flash interface.
Signed-off-by: Sourav Poddarsourav.poddar@ti.com
doc/README.ti_qspi_dra_test | 38 ++++++++++++++++++++++++++++++++++ doc/README.ti_qspi_flash | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 0 deletions(-) create mode 100644 doc/README.ti_qspi_dra_test create mode 100644 doc/README.ti_qspi_flash
diff --git a/doc/README.ti_qspi_dra_test b/doc/README.ti_qspi_dra_test new file mode 100644 index 0000000..c4540ea --- /dev/null +++ b/doc/README.ti_qspi_dra_test @@ -0,0 +1,38 @@ +-------------------------------------------------
- Simple steps used to test the QSPI at U-Boot
+-------------------------------------------------
+For #1, build the patched U-Boot and load MLO/u-boot.img
+---------------------------------- +Boot from another medium like MMC +----------------------------------
+DRA752 EVM # mmc dev 0 +DRA752 EVM # fatload mmc 0 0x82000000 MLO +DRA752 EVM # fatload mmc 0 0x83000000 u-boot.img
+-------------------------------------------------- +Commands to erase/write u-boot/mlo to flash device +--------------------------------------------------
+DRA752 EVM # sf probe 0 +[should detect the S25FL256S serial flash device]
+DRA752 EVM # sf erase 0 10000 +DRA752 EVM # sf erase 10000 10000 +DRA752 EVM # sf erase 20000 10000 +DRA752 EVM # sf erase 30000 10000 +DRA752 EVM # sf erase 40000 10000 +DRA752 EVM # sf erase 50000 10000 +DRA752 EVM # sf erase 60000 10000
+DRA752 EVM # sf write 82000000 0 10000 +DRA752 EVM # sf write 83000000 20000 70000
These test procedure steps were done in real hw. Seems like written once, could be generic if you test these steps on real hw and palce the same log here...
+For #2, set sysboot to QSPI-1 boot mode(SYSBOOT[5:0] = 100110) and power +on. ROM should find the GP header at offset 0 and load/execute SPL. SPL +then detects that ROM was in QSPI-1 mode (boot code 10) and attempts to +find a U-Boot image header at offset 0x20000 (set in the config file) +and proceeds to load that image using the U-Boot image payload offset/size +from the header. It will then start U-Boot. diff --git a/doc/README.ti_qspi_flash b/doc/README.ti_qspi_flash new file mode 100644 index 0000000..1b86d01 --- /dev/null +++ b/doc/README.ti_qspi_flash @@ -0,0 +1,47 @@ +QSPI U-boot support +------------------
+Host processor is connected to serial flash device via qpsi +interface. QSPI is a kind of spi module that allows single, +dual and quad read access to external spi devices. The module +has a memory mapped interface which provide direct interface +for accessing data form external spi devices.
+The one QSPI in the device is primarily intended for fast booting +from Quad SPI flash devices.
+Usecase +-------
+MLO/u-boot.img will be flashed from SD/MMC to the flash device +using serial flash erase and write commands. Then, switch settings +will be changed to qspi boot. Then, the ROM code will read MLO +from the predefined location in the flash, where it was flashed and +execute it after storing it in SDRAM. Then, the MLO will read +u-boot.img from flash and execute it from SDRAM.
+SPI mode +------- +SPI mode uses mtd spi framework for transfer and reception of data. +Can be used in: +1. Normal mode: use single pin for transfers +2. Dual Mode: use two pins for transfers. +3. Quad mode: use four pin for transfer
+Memory mapped read mode +----------------------- +In this, SPI controller is configured using configuration port and then +controler is switched to memory mapped port for data read.
+Driver +------ +drivers/qspi/ti_qspi.c
- Newly created file which is responsible for configuring the
qspi controller and also for providing the low level api which
is responsible for transferring the datas from host controller
to flash device and vice versa.
+Testing +------- +A seperated file named README.dra_qspi_test has been created which gives all the
+details about the commands required to test qspi at u-boot level.
1.7.1
Please use the doc/SPI/status.txt as an example format for writing new readme files.
participants (3)
-
Gerhard Sittig
-
Jagan Teki
-
Sourav Poddar