From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sourav Poddar Date: Mon, 6 Jan 2014 13:52:28 +0530 Subject: [U-Boot] [PATCH v6 11/12] sf: Divide flash register ops from QEB code In-Reply-To: References: <1388847888-19474-1-git-send-email-jaganna@xilinx.com> <52CA568E.5080602@ti.com> Message-ID: <52CA67C4.1060006@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Monday 06 January 2014 01:10 PM, Jagan Teki wrote: > On Mon, Jan 6, 2014 at 12:39 PM, Sourav Poddar wrote: >> On Saturday 04 January 2014 08:34 PM, Jagannadha Sutradharudu Teki wrote: >>> QEB code comprises of couple of flash register read/write operations, >>> this patch moved flash register operations on to sf_op >>> >>> Signed-off-by: Jagannadha Sutradharudu Teki >>> --- >>> drivers/mtd/spi/sf_internal.h | 11 ++++--- >>> drivers/mtd/spi/sf_ops.c | 75 >>> +++++++++++++++---------------------------- >>> drivers/mtd/spi/sf_probe.c | 44 +++++++++++++++++++++++++ >>> 3 files changed, 77 insertions(+), 53 deletions(-) >>> >>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h >>> index c69b53d..c77961f 100644 >>> --- a/drivers/mtd/spi/sf_internal.h >>> +++ b/drivers/mtd/spi/sf_internal.h >>> @@ -101,14 +101,17 @@ int spi_flash_cmd_write(struct spi_slave *spi, const >>> u8 *cmd, size_t cmd_len, >>> /* Flash erase(sectors) operation, support all possible erase commands >>> */ >>> int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t >>> len); >>> >>> +/* Read the status register */ >>> +int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs); >>> + >>> /* Program the status register */ >>> int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); >>> >>> -/* Set quad enbale bit for macronix flashes */ >>> -int spi_flash_set_qeb_mxic(struct spi_flash *flash); >>> +/* Read the config register */ >>> +int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc); >>> >>> -/* Set quad enbale bit for winbond and spansion flashes */ >>> -int spi_flash_set_qeb_winspan(struct spi_flash *flash); >>> +/* Program the config register */ >>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc); >>> >>> /* Enable writing on the SPI flash */ >>> static inline int spi_flash_cmd_write_enable(struct spi_flash *flash) >>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c >>> index 9681042..658294c 100644 >>> --- a/drivers/mtd/spi/sf_ops.c >>> +++ b/drivers/mtd/spi/sf_ops.c >>> @@ -24,94 +24,71 @@ static void spi_flash_addr(u32 addr, u8 *cmd) >>> cmd[3] = addr>> 0; >>> } >>> >>> -int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >>> +int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs) >>> { >>> - u8 cmd; >>> int ret; >>> + u8 cmd; >>> >>> - cmd = CMD_WRITE_STATUS; >>> - ret = spi_flash_write_common(flash,&cmd, 1,&sr, 1); >>> + cmd = CMD_READ_STATUS; >>> + ret = spi_flash_read_common(flash,&cmd, 1, rs, 1); >>> >>> if (ret< 0) { >>> - debug("SF: fail to write status register\n"); >>> + debug("SF: fail to read status register\n"); >>> return ret; >>> } >>> >>> return 0; >>> } >>> >>> -#ifdef CONFIG_SPI_FLASH_MACRONIX >>> -int spi_flash_set_qeb_mxic(struct spi_flash *flash) >>> +int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >>> { >>> - u8 qeb_status; >>> u8 cmd; >>> int ret; >>> >>> - cmd = CMD_READ_STATUS; >>> - ret = spi_flash_read_common(flash,&cmd, 1,&qeb_status, 1); >>> + cmd = CMD_WRITE_STATUS; >>> + ret = spi_flash_write_common(flash,&cmd, 1,&sr, 1); >>> >>> if (ret< 0) { >>> - debug("SF: fail to read status register\n"); >>> + debug("SF: fail to write status register\n"); >>> return ret; >>> } >>> >>> - if (qeb_status& STATUS_QEB_MXIC) { >>> >>> - debug("SF: Quad enable bit is already set\n"); >>> - } else { >>> - ret = spi_flash_cmd_write_status(flash, STATUS_QEB_MXIC); >>> - if (ret< 0) >>> - return ret; >>> - } >>> - >>> - return ret; >>> + return 0; >>> } >>> -#endif >>> >>> #if defined(CONFIG_SPI_FLASH_SPANSION) || >>> defined(CONFIG_SPI_FLASH_WINBOND) >>> -static int spi_flash_cmd_write_config(struct spi_flash *flash, u8 cr) >>> +int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc) >>> { >>> - u8 data[2]; >>> - u8 cmd; >>> int ret; >>> + u8 cmd; >>> >>> - cmd = CMD_READ_STATUS; >>> - ret = spi_flash_read_common(flash,&cmd, 1,&data[0], 1); >>> + cmd = CMD_READ_CONFIG; >>> + ret = spi_flash_read_common(flash,&cmd, 1, rc, 1); >>> >>> if (ret< 0) { >>> - debug("SF: fail to read status register\n"); >>> - return ret; >>> - } >>> - >>> - cmd = CMD_WRITE_STATUS; >>> - data[1] = cr; >>> - ret = spi_flash_write_common(flash,&cmd, 1,&data, 2); >>> >>> - if (ret) { >>> - debug("SF: fail to write config register\n"); >>> + debug("SF: fail to read config register\n"); >>> return ret; >>> } >>> >>> return 0; >>> } >>> >>> -int spi_flash_set_qeb_winspan(struct spi_flash *flash) >>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc) >>> { >>> - u8 qeb_status; >>> + u8 data[2]; >>> u8 cmd; >>> int ret; >>> >>> - cmd = CMD_READ_CONFIG; >>> - ret = spi_flash_read_common(flash,&cmd, 1,&qeb_status, 1); >>> >>> - if (ret< 0) { >>> - debug("SF: fail to read config register\n"); >>> + ret = spi_flash_cmd_read_status(flash,&data[0]); >>> >>> + if (ret< 0) >>> return ret; >>> - } >>> >>> - if (qeb_status& STATUS_QEB_WINSPAN) { >>> >>> - debug("SF: Quad enable bit is already set\n"); >>> - } else { >>> - ret = spi_flash_cmd_write_config(flash, >>> STATUS_QEB_WINSPAN); >>> - if (ret< 0) >>> - return ret; >>> + cmd = CMD_WRITE_STATUS; >>> + data[1] = wc; >>> + ret = spi_flash_write_common(flash,&cmd, 1,&data, 2); >>> >>> + if (ret) { >>> + debug("SF: fail to write config register\n"); >>> + return ret; >>> } >>> >>> - return ret; >>> + return 0; >>> } >>> #endif >>> >>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c >>> index cbde350..8715b7c 100644 >>> --- a/drivers/mtd/spi/sf_probe.c >>> +++ b/drivers/mtd/spi/sf_probe.c >>> @@ -28,6 +28,50 @@ static u8 spi_read_cmds_array[] = { >>> CMD_READ_QUAD_IO_FAST, >>> }; >>> >>> +#ifdef CONFIG_SPI_FLASH_MACRONIX >>> +static int spi_flash_set_qeb_mxic(struct spi_flash *flash) >>> +{ >>> + u8 qeb_status; >>> + int ret; >>> + >>> + ret = spi_flash_cmd_read_status(flash,&qeb_status); >>> + if (ret< 0) >>> + return ret; >>> + >>> + if (qeb_status& STATUS_QEB_MXIC) { >>> >>> + debug("SF: mxic: QEB is already set\n"); >>> + } else { >>> + ret = spi_flash_cmd_write_status(flash, STATUS_QEB_MXIC); >>> + if (ret< 0) >>> + return ret; >>> + } >>> + >>> + return ret; >>> +} >>> +#endif >>> + >>> +#if defined(CONFIG_SPI_FLASH_SPANSION) || >>> defined(CONFIG_SPI_FLASH_WINBOND) >>> +static int spi_flash_set_qeb_winspan(struct spi_flash *flash) >>> +{ >>> + u8 qeb_status; >>> + int ret; >>> + >>> + ret = spi_flash_cmd_read_config(flash,&qeb_status); >>> + if (ret< 0) >>> + return ret; >>> + >>> + if (qeb_status& STATUS_QEB_WINSPAN) { >>> >>> + debug("SF: winspan: QEB is already set\n"); >>> + } else { >>> + ret = spi_flash_cmd_write_config(flash, >>> STATUS_QEB_WINSPAN); >>> + if (ret< 0) >>> + return ret; >>> + } >>> + >>> + return ret; >>> +} >>> +#endif >>> + >> >> This looks to be the generic function used to set quad mode for both >> spansion and >> winbond flashes? If its the case, function name should be more generic and >> without a >> _winbond appended to it. Same goes with the other macros used in the >> function. > Thats what I reflected on the name right - as this code specific to > winbond and spansion - > I named as *_winspan() and macro looks the same *_WINSPAN > > Any comments? > Ok, yes, overlooked its winspan. its fine then.