All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] Problem with attaching UBI partition
Date: Mon, 29 Feb 2016 23:44:40 +0100	[thread overview]
Message-ID: <56D4C9D8.7060402@denx.de> (raw)
In-Reply-To: <BFEE4BD273371E48B6EB70F96CEC5B54040B20B7@DEMUMBX007.nsn-intra.net>

On 02/29/2016 05:44 PM, Bakhvalov, Denis (Nokia - PL/Wroclaw) wrote:
> Hi Jagan, Heiko,
> 
>> Did you enable CONFIG_SPI_FLASH_SPANSION on your config, becuase
>> S25FL512S is already been added u-boot. Pls- check the same and let me
>> know if you find any issues while detecting the flash.
> 
>> Whether the driver detecting flash or not with RDID, please define
>> DEBUG on drivers/mtd/spi/spi_flash.c
> 
> I had no problems with detecting the flash. It was detected like this:
> SF: Detected S25FL512S_256K with page size 512 Bytes, erase size 256 KiB, total 64 MiB
> 
> Yes, I have CONFIG_SPI_FLASH_SPANSION enabled in configs/socfpga_arria5_dbrrh_defconfig (dbrrh - is my custom board).
> 
> Actually, today I solved the problem, although in not very clean way, I think.
> 
> As I mentioned in previous mail I had problems with reading from the flash. Some bytes were read incorrectly.
> I'm migrating from U-Boot 2013 to mainline U-Boot. And on previous version there were no such issue.
> I found out that there was some workaround introduced in order to fix this.
> 
> So, I just apply some part of that workaround to mainline U-Boot and now it works.
> 
> Here is the problem in more detail:
> 
> On previous version (2013):
> U-BOOT # sf read 0x1B000000 0x03800000 4
>      cadence_qspi_apb_indirect_read_setup. addr_value = 0x3800000
>      cadence_qspi_apb_indirect_read_setup. rd_reg = 0x800000b
>      cadence_qspi_apb_indirect_read_setup. device size = 0x101003
> 
> U-BOOT # md 0x1B000000 1
> 1b000000: 55424923
> 
> But on mainline U-Boot(2016.03-rc1) I had:
> U-BOOT # sf read 0x1B000000 0x03800000 4
>      cadence_qspi_apb_indirect_read_setup. addr_value = 0x800000
>      cadence_qspi_apb_indirect_read_setup. rd_reg = 0x0b
>      cadence_qspi_apb_indirect_read_setup. device size = 0x101002
> 
> U-BOOT # md 0x1B000000 1
> 1b000000: 554249ff
> 
> You can see the difference in the register values that were written in QSPI registers.
> In case with mainline U-Boot I had address value not properly set.
> 
> Here is the workaround that I've made:
> 
> Index: drivers/mtd/spi/spi_flash.c
> ===================================================================
> --- drivers/mtd/spi/spi_flash.c	(revision 608)
> +++ drivers/mtd/spi/spi_flash.c	(revision 609)
> @@ -29,6 +29,17 @@
>  	cmd[3] = addr >> 0;
>  }
>  
> +#ifdef CONFIG_DBRRH_WORKAROUND                
> +static void spi_flash_addr32(u32 addr, u8 *cmd)
> +{
> +	/* cmd[0] is actual command */
> +	cmd[1] = (addr >> 24) & 0xFF;
> +	cmd[2] = (addr >> 16) & 0xFF;
> +	cmd[3] = (addr >> 8) & 0xFF;
> +	cmd[4] = (addr >> 0) & 0xFF;
> +}
> +#endif
> +
>  static int read_sr(struct spi_flash *flash, u8 *rs)
>  {
>  	int ret;
> @@ -510,8 +521,14 @@
>  		else
>  			read_len = remain_len;
>  
> +#ifdef CONFIG_DBRRH_WORKAROUND                
> +        		spi_flash_addr32(read_addr, cmd);
> +#else
>  		spi_flash_addr(read_addr, cmd);
> +#endif
>  
> 		ret = spi_flash_read_common(flash, cmd, cmdsz, data, read_len);
>  		if (ret < 0) {
>  			debug("SF: read failed\n");
> 
> Index: drivers/spi/cadence_qspi_apb.c
> ===================================================================
> --- drivers/spi/cadence_qspi_apb.c	(revision 608)
> +++ drivers/spi/cadence_qspi_apb.c	(revision 609)
> @@ -30,6 +30,10 @@
>  #include <asm/errno.h>
>  #include "cadence_qspi.h"
>  
> +#ifdef CONFIG_DBRRH_WORKAROUND
> +	#define CMD_OPTION_DUMMY_CYCLES		0x7F	/* Dummy Cycles for Read Command */
> +#endif
> +
> @@ -706,9 +710,25 @@
>  #endif
>  
>  	/* Get address */
> +#ifdef CONFIG_DBRRH_WORKAROUND
> +	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], 4);
> +#else
>  	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
> +#endif
> 
> +#ifdef CONFIG_DBRRH_WORKAROUND
> +	/* Setting Dummy Clock Cycle */
> +	dummy_clk = (0x08 & CMD_OPTION_DUMMY_CYCLES);
> +	if (dummy_clk) 
> +	{
> +		rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
> +			<< CQSPI_REG_RD_INSTR_DUMMY_LSB;
> +	}
> +#else
> +
>  	/* The remaining lenght is dummy bytes. */
>  	dummy_bytes = cmdlen - addr_bytes - 1;
>  	if (dummy_bytes) {
> @@ -731,7 +751,9 @@
>  			rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
>  				<< CQSPI_REG_RD_INSTR_DUMMY_LSB;
>  	}
> +#endif
> 
> 
> With this correction I can read contents of the flash properly.
> However, I'm a bit surprised that I was forced to make such correction like storing 4 bytes of address (see spi_flash_addr32() above).
> On the other hand I haven't found any switch that could be turned on to fix my problem in a clean and nice way.
> 
> With 24 bytes we can address only 16 MB. How cadence driver is supposed to work for larger spaces?
> Is this 4th byte comes from somewhere else?
> 
> Jagan, Heiko, please evaluate my correction.

Try enabling CONFIG_SPI_FLASH_BAR in your board config ;-)

I normally use the cadence QSPI driver with N25Q00AA, which is 128MiB
part. I also had the 4-byte addressing support in the works for a while,
but never got around to cleaning that up. It'd be real nice if you could
send a proper patch.

-- 
Best regards,
Marek Vasut

  parent reply	other threads:[~2016-02-29 22:44 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-22 14:17 [U-Boot] Problem with attaching UBI partition Bakhvalov, Denis
2016-02-23 18:11 ` Heiko Schocher
2016-02-24  8:59   ` Bakhvalov, Denis
2016-02-29  6:36     ` Heiko Schocher
2016-02-29  6:47     ` Jagan Teki
2016-02-29 16:44       ` Bakhvalov, Denis
2016-02-29 17:17         ` Jagan Teki
2016-02-29 22:44         ` Marek Vasut [this message]
2016-03-01 13:04           ` Bakhvalov, Denis
2016-03-01 13:32             ` Jagan Teki
2016-03-01 13:53               ` Bakhvalov, Denis
2016-03-01 13:58                 ` Marek Vasut
2016-03-04  9:03                   ` Bakhvalov, Denis
2016-03-04 12:20                     ` Marek Vasut
2016-03-04 12:24                       ` Bakhvalov, Denis
2016-03-04 12:28                         ` Marek Vasut
2016-03-22 13:18                   ` Bakhvalov, Denis
2016-03-22 14:24                     ` Marek Vasut
2016-03-01 13:46             ` Marek Vasut
2016-02-29 22:55     ` Marek Vasut
2016-03-01  6:53       ` Chin Liang See
2016-03-01  7:23         ` Stefan Roese
2016-03-01 13:38           ` Chin Liang See
2016-03-01 15:35             ` Stefan Roese
2016-03-02 12:24               ` Chin Liang See
2016-03-03 11:51                 ` Stefan Roese
2016-03-03 12:43                   ` Marek Vasut
  -- strict thread matches above, loose matches on Subject: below --
2016-02-23 13:31 Bakhvalov, Denis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56D4C9D8.7060402@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.