[U-Boot] [PATCH resend 0/7] mtd/st_smi: Add fixes for smi driver

[U-Boot] [PATCH resend 0/7] mtd/st_smi: Add fixes for smi driver

[Vipin Kumar]

Hello,

This patch-set adds simple fixes for smi driver used in spear devices.

Modifications include
- Add MTD support for smi driver
- Write to flash in a tight loop
- Alphabetically sort the list of devices
- Avoid issuing multiple write enable commands

PS: Wolfgang, Stefan, I am directing these to you as there is no direct
maintainer of mtd in u-boot. I can add these in my pull-request once you review
these patches

Thanks in advance
Vipin

Armando Visconti (1):
  mtd/st_smi: Add support for Micron N25Q128 Flash

Vipin Kumar (6):
  mtd/st_smi: Clear error flags while initiating a fresh write
  mtd/st_smi: Rearrange the supported devices in alphabetical order
  mtd/st_smi: Avoid issuing multiple WE commands
  mtd/st_smi: Write to flash in a tight loop
  mtd/st_smi: Use page sizes respective to flash
  mtd/st_smi: Add mtd support for smi

 drivers/mtd/st_smi.c       | 260 ++++++++++++++++++++++++++++++++++++++-------
 include/linux/mtd/st_smi.h |   1 -
 2 files changed, 222 insertions(+), 39 deletions(-)

-- 
1.8.0

[U-Boot] [PATCH resend 1/7] mtd/st_smi: Clear error flags while initiating a fresh write

[Vipin Kumar]

SMI controller reports an error when the code tries to write on the flash area
with Write Enable command not issued or the bank has come out of the write mode.

This error is reported even with a fresh write once the ERF1 or ERF2 is set.
Clear these flags while initiating a fresh write

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
---
 drivers/mtd/st_smi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
index 7507e5d..fad4420 100644
--- a/drivers/mtd/st_smi.c
+++ b/drivers/mtd/st_smi.c
@@ -392,6 +392,8 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
 		return -1;
 	}
 
+	writel(readl(&smicntl->smi_sr) & ~(ERF1 | ERF2), &smicntl->smi_sr);
+
 	if (smi_wait_till_ready(banknum, CONFIG_SYS_FLASH_WRITE_TOUT))
 		return -EBUSY;
 
-- 
1.8.0

[U-Boot] [PATCH resend 1/7] mtd/st_smi: Clear error flags while initiating a fresh write

[Stefan Roese]

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> SMI controller reports an error when the code tries to write on the flash area
> with Write Enable command not issued or the bank has come out of the write mode.
> 
> This error is reported even with a fresh write once the ERF1 or ERF2 is set.
> Clear these flags while initiating a fresh write
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> ---
>  drivers/mtd/st_smi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
> index 7507e5d..fad4420 100644
> --- a/drivers/mtd/st_smi.c
> +++ b/drivers/mtd/st_smi.c
> @@ -392,6 +392,8 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
>  		return -1;
>  	}
>  
> +	writel(readl(&smicntl->smi_sr) & ~(ERF1 | ERF2), &smicntl->smi_sr);

Better use:

	clrbits_le32(&smicntl->smi_sr, ERF1 | ERF2)

Thanks,
Stefan

[U-Boot] [PATCH resend 1/7] mtd/st_smi: Clear error flags while initiating a fresh write

[Vipin Kumar]

On 12/6/2012 1:07 PM, Stefan Roese wrote:
> On 12/06/2012 07:21 AM, Vipin Kumar wrote:
>> SMI controller reports an error when the code tries to write on the flash area
>> with Write Enable command not issued or the bank has come out of the write mode.
>>
>> This error is reported even with a fresh write once the ERF1 or ERF2 is set.
>> Clear these flags while initiating a fresh write
>>
>> Signed-off-by: Vipin Kumar<vipin.kumar@st.com>
>> ---
>>   drivers/mtd/st_smi.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
>> index 7507e5d..fad4420 100644
>> --- a/drivers/mtd/st_smi.c
>> +++ b/drivers/mtd/st_smi.c
>> @@ -392,6 +392,8 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
>>   		return -1;
>>   	}
>>
>> +	writel(readl(&smicntl->smi_sr)&  ~(ERF1 | ERF2),&smicntl->smi_sr);
>
> Better use:
>
> 	clrbits_le32(&smicntl->smi_sr, ERF1 | ERF2)
>

Thanks.. I would use it and send a v2

> Thanks,
> Stefan
>
>

[U-Boot] [PATCH resend 2/7] mtd/st_smi: Rearrange the supported devices in alphabetical order

[Vipin Kumar]

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
---
 drivers/mtd/st_smi.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
index fad4420..c3e35c0 100644
--- a/drivers/mtd/st_smi.c
+++ b/drivers/mtd/st_smi.c
@@ -62,24 +62,24 @@ struct flash_device {
  * Currently the erase_cmd field is not used in this driver.
  */
 static struct flash_device flash_devices[] = {
-	FLASH_ID("st m25p16"     , 0xd8, 0x00152020, 0x100, 0x10000, 0x200000),
-	FLASH_ID("st m25p32"     , 0xd8, 0x00162020, 0x100, 0x10000, 0x400000),
-	FLASH_ID("st m25p64"     , 0xd8, 0x00172020, 0x100, 0x10000, 0x800000),
-	FLASH_ID("st m25p128"    , 0xd8, 0x00182020, 0x100, 0x40000, 0x1000000),
+	FLASH_ID("sp s25fl004"   , 0xd8, 0x00120201, 0x100, 0x10000, 0x80000),
+	FLASH_ID("sp s25fl008"   , 0xd8, 0x00130201, 0x100, 0x10000, 0x100000),
+	FLASH_ID("sp s25fl016"   , 0xd8, 0x00140201, 0x100, 0x10000, 0x200000),
+	FLASH_ID("sp s25fl032"   , 0xd8, 0x00150201, 0x100, 0x10000, 0x400000),
+	FLASH_ID("sp s25fl064"   , 0xd8, 0x00160201, 0x100, 0x10000, 0x800000),
 	FLASH_ID("st m25p05"     , 0xd8, 0x00102020, 0x80 , 0x8000 , 0x10000),
 	FLASH_ID("st m25p10"     , 0xd8, 0x00112020, 0x80 , 0x8000 , 0x20000),
+	FLASH_ID("st m25p16"     , 0xd8, 0x00152020, 0x100, 0x10000, 0x200000),
 	FLASH_ID("st m25p20"     , 0xd8, 0x00122020, 0x100, 0x10000, 0x40000),
+	FLASH_ID("st m25p32"     , 0xd8, 0x00162020, 0x100, 0x10000, 0x400000),
 	FLASH_ID("st m25p40"     , 0xd8, 0x00132020, 0x100, 0x10000, 0x80000),
+	FLASH_ID("st m25p64"     , 0xd8, 0x00172020, 0x100, 0x10000, 0x800000),
 	FLASH_ID("st m25p80"     , 0xd8, 0x00142020, 0x100, 0x10000, 0x100000),
+	FLASH_ID("st m25p128"    , 0xd8, 0x00182020, 0x100, 0x40000, 0x1000000),
 	FLASH_ID("st m45pe10"    , 0xd8, 0x00114020, 0x100, 0x10000, 0x20000),
 	FLASH_ID("st m45pe20"    , 0xd8, 0x00124020, 0x100, 0x10000, 0x40000),
 	FLASH_ID("st m45pe40"    , 0xd8, 0x00134020, 0x100, 0x10000, 0x80000),
 	FLASH_ID("st m45pe80"    , 0xd8, 0x00144020, 0x100, 0x10000, 0x100000),
-	FLASH_ID("sp s25fl004"   , 0xd8, 0x00120201, 0x100, 0x10000, 0x80000),
-	FLASH_ID("sp s25fl008"   , 0xd8, 0x00130201, 0x100, 0x10000, 0x100000),
-	FLASH_ID("sp s25fl016"   , 0xd8, 0x00140201, 0x100, 0x10000, 0x200000),
-	FLASH_ID("sp s25fl032"   , 0xd8, 0x00150201, 0x100, 0x10000, 0x400000),
-	FLASH_ID("sp s25fl064"   , 0xd8, 0x00160201, 0x100, 0x10000, 0x800000),
 	FLASH_ID("mac 25l512"    , 0xd8, 0x001020C2, 0x010, 0x10000, 0x10000),
 	FLASH_ID("mac 25l1005"   , 0xd8, 0x001120C2, 0x010, 0x10000, 0x20000),
 	FLASH_ID("mac 25l2005"   , 0xd8, 0x001220C2, 0x010, 0x10000, 0x40000),
@@ -91,7 +91,7 @@ static struct flash_device flash_devices[] = {
 	FLASH_ID("mac 25l3205"   , 0xd8, 0x001620C2, 0x100, 0x10000, 0x400000),
 	FLASH_ID("mac 25l3205a"  , 0xd8, 0x001620C2, 0x100, 0x10000, 0x400000),
 	FLASH_ID("mac 25l6405"   , 0xd8, 0x001720C2, 0x100, 0x10000, 0x800000),
-	FLASH_ID("wbd w25q128" , 0xd8, 0x001840EF, 0x100, 0x10000, 0x1000000),
+	FLASH_ID("wbd w25q128"   , 0xd8, 0x001840EF, 0x100, 0x10000, 0x1000000),
 };
 
 /*
-- 
1.8.0

[U-Boot] [PATCH resend 2/7] mtd/st_smi: Rearrange the supported devices in alphabetical order

[Stefan Roese]

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH resend 3/7] mtd/st_smi: Add support for Micron N25Q128 Flash

[Vipin Kumar]

From: Armando Visconti <armando.visconti@st.com>

Signed-off-by: Armando Visconti <armando.visconti@st.com>
---
 drivers/mtd/st_smi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
index c3e35c0..a46b273 100644
--- a/drivers/mtd/st_smi.c
+++ b/drivers/mtd/st_smi.c
@@ -91,6 +91,7 @@ static struct flash_device flash_devices[] = {
 	FLASH_ID("mac 25l3205"   , 0xd8, 0x001620C2, 0x100, 0x10000, 0x400000),
 	FLASH_ID("mac 25l3205a"  , 0xd8, 0x001620C2, 0x100, 0x10000, 0x400000),
 	FLASH_ID("mac 25l6405"   , 0xd8, 0x001720C2, 0x100, 0x10000, 0x800000),
+	FLASH_ID("mcr n25q128"   , 0xd8, 0x0018BA20, 0x100, 0x10000, 0x1000000),
 	FLASH_ID("wbd w25q128"   , 0xd8, 0x001840EF, 0x100, 0x10000, 0x1000000),
 };
 
-- 
1.8.0

[U-Boot] [PATCH resend 3/7] mtd/st_smi: Add support for Micron N25Q128 Flash

[Stefan Roese]

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> From: Armando Visconti <armando.visconti@st.com>
> 
> Signed-off-by: Armando Visconti <armando.visconti@st.com>

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH resend 4/7] mtd/st_smi: Avoid issuing multiple WE commands

[Vipin Kumar]

At the start of an smi_write, if the destination address is page aligned, the
Write Enable command is getting issued twice. This patch fixes it by keeping a
flag.

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
---
 drivers/mtd/st_smi.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
index a46b273..a84802a 100644
--- a/drivers/mtd/st_smi.c
+++ b/drivers/mtd/st_smi.c
@@ -374,7 +374,7 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
 	u8 *src_addr8 = (u8 *)src_addr;
 	u8 *dst_addr8 = (u8 *)dst_addr;
 	int banknum;
-	int i;
+	int i, issue_we;
 
 	switch (bank_addr) {
 	case SMIBANK0_BASE:
@@ -394,19 +394,16 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
 	}
 
 	writel(readl(&smicntl->smi_sr) & ~(ERF1 | ERF2), &smicntl->smi_sr);
-
-	if (smi_wait_till_ready(banknum, CONFIG_SYS_FLASH_WRITE_TOUT))
-		return -EBUSY;
+	issue_we = 1;
 
 	/* Set SMI in Hardware Mode */
 	writel(readl(&smicntl->smi_cr1) & ~SW_MODE, &smicntl->smi_cr1);
 
-	if (smi_write_enable(banknum))
-		return -EIO;
-
 	/* Perform the write command */
 	for (i = 0; i < length; i += 4) {
-		if (((ulong) (dst_addr) % SFLASH_PAGE_SIZE) == 0) {
+		if (issue_we || (((ulong)(dst_addr) % SFLASH_PAGE_SIZE) == 0)) {
+			issue_we = 0;
+
 			if (smi_wait_till_ready(banknum,
 						CONFIG_SYS_FLASH_WRITE_TOUT))
 				return -EBUSY;
-- 
1.8.0

[U-Boot] [PATCH resend 4/7] mtd/st_smi: Avoid issuing multiple WE commands

[Stefan Roese]

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> At the start of an smi_write, if the destination address is page aligned, the
> Write Enable command is getting issued twice. This patch fixes it by keeping a
> flag.
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH resend 5/7] mtd/st_smi: Write to flash in a tight loop

[Vipin Kumar]

The write loop is checking for dest_addr alignment with page size. This
sometimes leads to smi controller coming out of write mode and eventually the
next write failing with ERF1 being set.

To avoid this, write to flash in a tight loop and write bytewise to also support
not word aligned data bytes to be written. Additionally, enable burst mode
before this loop so that the controller does not deselect the chip if the
transfer is finished.

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
---
 drivers/mtd/st_smi.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
index a84802a..998bacb 100644
--- a/drivers/mtd/st_smi.c
+++ b/drivers/mtd/st_smi.c
@@ -368,13 +368,11 @@ static int smi_sector_erase(flash_info_t *info, unsigned int sector)
  *
  * Write to SMI flash
  */
-static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
+static int smi_write(unsigned char *src_addr, unsigned char *dst_addr,
 		     unsigned int length, ulong bank_addr)
 {
-	u8 *src_addr8 = (u8 *)src_addr;
-	u8 *dst_addr8 = (u8 *)dst_addr;
 	int banknum;
-	int i, issue_we;
+	int issue_we;
 
 	switch (bank_addr) {
 	case SMIBANK0_BASE:
@@ -400,7 +398,10 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
 	writel(readl(&smicntl->smi_cr1) & ~SW_MODE, &smicntl->smi_cr1);
 
 	/* Perform the write command */
-	for (i = 0; i < length; i += 4) {
+	while (length) {
+		int k;
+		unsigned int wlen = min(SFLASH_PAGE_SIZE, length);
+
 		if (issue_we || (((ulong)(dst_addr) % SFLASH_PAGE_SIZE) == 0)) {
 			issue_we = 0;
 
@@ -412,19 +413,14 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
 				return -EIO;
 		}
 
-		if (length < 4) {
-			int k;
+		writel(readl(&smicntl->smi_cr1) | WB_MODE, &smicntl->smi_cr1);
 
-			/*
-			 * Handle special case, where length < 4 (redundant env)
-			 */
-			for (k = 0; k < length; k++)
-				*dst_addr8++ = *src_addr8++;
-		} else {
-			/* Normal 32bit write */
+		for (k = 0; k < wlen; k++)
 			*dst_addr++ = *src_addr++;
-		}
 
+		writel(readl(&smicntl->smi_cr1) & ~WB_MODE, &smicntl->smi_cr1);
+
+		length -= wlen;
 		if ((readl(&smicntl->smi_sr) & (ERF1 | ERF2)))
 			return -EIO;
 	}
@@ -448,8 +444,8 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
  */
 int write_buff(flash_info_t *info, uchar *src, ulong dest_addr, ulong length)
 {
-	return smi_write((unsigned int *)src, (unsigned int *)dest_addr,
-			 length, info->start[0]);
+	return smi_write(src, (unsigned char *)dest_addr, length,
+			info->start[0]);
 }
 
 /*
-- 
1.8.0

[U-Boot] [PATCH resend 5/7] mtd/st_smi: Write to flash in a tight loop

[Stefan Roese]

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> The write loop is checking for dest_addr alignment with page size. This
> sometimes leads to smi controller coming out of write mode and eventually the
> next write failing with ERF1 being set.
> 
> To avoid this, write to flash in a tight loop and write bytewise to also support
> not word aligned data bytes to be written. Additionally, enable burst mode
> before this loop so that the controller does not deselect the chip if the
> transfer is finished.
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> ---
>  drivers/mtd/st_smi.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
> index a84802a..998bacb 100644
> --- a/drivers/mtd/st_smi.c
> +++ b/drivers/mtd/st_smi.c
> @@ -368,13 +368,11 @@ static int smi_sector_erase(flash_info_t *info, unsigned int sector)
>   *
>   * Write to SMI flash
>   */
> -static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
> +static int smi_write(unsigned char *src_addr, unsigned char *dst_addr,
>  		     unsigned int length, ulong bank_addr)
>  {
> -	u8 *src_addr8 = (u8 *)src_addr;
> -	u8 *dst_addr8 = (u8 *)dst_addr;
>  	int banknum;
> -	int i, issue_we;
> +	int issue_we;
>  
>  	switch (bank_addr) {
>  	case SMIBANK0_BASE:
> @@ -400,7 +398,10 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
>  	writel(readl(&smicntl->smi_cr1) & ~SW_MODE, &smicntl->smi_cr1);
>  
>  	/* Perform the write command */
> -	for (i = 0; i < length; i += 4) {
> +	while (length) {
> +		int k;
> +		unsigned int wlen = min(SFLASH_PAGE_SIZE, length);
> +
>  		if (issue_we || (((ulong)(dst_addr) % SFLASH_PAGE_SIZE) == 0)) {
>  			issue_we = 0;
>  
> @@ -412,19 +413,14 @@ static int smi_write(unsigned int *src_addr, unsigned int *dst_addr,
>  				return -EIO;
>  		}
>  
> -		if (length < 4) {
> -			int k;
> +		writel(readl(&smicntl->smi_cr1) | WB_MODE, &smicntl->smi_cr1);

Use setbits_le32() please.

> -			/*
> -			 * Handle special case, where length < 4 (redundant env)
> -			 */
> -			for (k = 0; k < length; k++)
> -				*dst_addr8++ = *src_addr8++;
> -		} else {
> -			/* Normal 32bit write */
> +		for (k = 0; k < wlen; k++)
>  			*dst_addr++ = *src_addr++;
> -		}
>  
> +		writel(readl(&smicntl->smi_cr1) & ~WB_MODE, &smicntl->smi_cr1);

clrbits_le32()

Thanks,
Stefan

[U-Boot] [PATCH resend 6/7] mtd/st_smi: Use page sizes respective to flash

[Vipin Kumar]

The page size is a flash dependent property and the driver was using a macro in
place of page size. This patch uses the proper page size wrt the flash device
connected on board

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
---
 drivers/mtd/st_smi.c       | 41 +++++++++++++++++++++++++++++++++--------
 include/linux/mtd/st_smi.h |  1 -
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
index 998bacb..0e529a4 100644
--- a/drivers/mtd/st_smi.c
+++ b/drivers/mtd/st_smi.c
@@ -96,6 +96,25 @@ static struct flash_device flash_devices[] = {
 };
 
 /*
+ * get_flash_device - Return flash_device pointer for a particular device id
+ * @id:	 Device id
+ *
+ * Return flash_device pointer for a particular device id
+ */
+static struct flash_device *get_flash_device(u32 id)
+{
+	struct flash_device *flash_dev_p = &flash_devices[0];
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(flash_devices); i++, flash_dev_p++) {
+		if (flash_dev_p->device_id == id)
+			return flash_dev_p;
+	}
+
+	return NULL;
+}
+
+/*
  * smi_wait_xfer_finish - Wait until TFF is set in status register
  * @timeout:	 timeout in milliseconds
  *
@@ -361,20 +380,27 @@ static int smi_sector_erase(flash_info_t *info, unsigned int sector)
 
 /*
  * smi_write - Write to SMI flash
+ * @info:	 flash info structure
  * @src_addr:	 source buffer
  * @dst_addr:	 destination buffer
  * @length:	 length to write in bytes
- * @bank:	 bank base address
  *
  * Write to SMI flash
  */
-static int smi_write(unsigned char *src_addr, unsigned char *dst_addr,
-		     unsigned int length, ulong bank_addr)
+static int smi_write(flash_info_t *info, unsigned char *src_addr,
+		unsigned char *dst_addr, unsigned int length)
 {
+	struct flash_device *flash_device_p = get_flash_device(info->flash_id);
+	u32 page_size;
 	int banknum;
 	int issue_we;
 
-	switch (bank_addr) {
+	if (!flash_device_p)
+		return -EIO;
+
+	page_size = flash_device_p->pagesize;
+
+	switch (info->start[0]) {
 	case SMIBANK0_BASE:
 		banknum = BANK0;
 		break;
@@ -400,9 +426,9 @@ static int smi_write(unsigned char *src_addr, unsigned char *dst_addr,
 	/* Perform the write command */
 	while (length) {
 		int k;
-		unsigned int wlen = min(SFLASH_PAGE_SIZE, length);
+		unsigned int wlen = min(page_size, length);
 
-		if (issue_we || (((ulong)(dst_addr) % SFLASH_PAGE_SIZE) == 0)) {
+		if (issue_we || (((ulong)(dst_addr) % page_size) == 0)) {
 			issue_we = 0;
 
 			if (smi_wait_till_ready(banknum,
@@ -444,8 +470,7 @@ static int smi_write(unsigned char *src_addr, unsigned char *dst_addr,
  */
 int write_buff(flash_info_t *info, uchar *src, ulong dest_addr, ulong length)
 {
-	return smi_write(src, (unsigned char *)dest_addr, length,
-			info->start[0]);
+	return smi_write(info, src, (unsigned char *)dest_addr, length);
 }
 
 /*
diff --git a/include/linux/mtd/st_smi.h b/include/linux/mtd/st_smi.h
index 04f81ea..5837493 100644
--- a/include/linux/mtd/st_smi.h
+++ b/include/linux/mtd/st_smi.h
@@ -108,7 +108,6 @@ struct flash_dev {
 	ushort sector_count;
 };
 
-#define SFLASH_PAGE_SIZE	0x100	/* flash page size */
 #define XFER_FINISH_TOUT	15	/* xfer finish timeout(in ms) */
 #define WMODE_TOUT		15	/* write enable timeout(in ms) */
 
-- 
1.8.0

[U-Boot] [PATCH resend 6/7] mtd/st_smi: Use page sizes respective to flash

[Stefan Roese]

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> The page size is a flash dependent property and the driver was using a macro in
> place of page size. This patch uses the proper page size wrt the flash device
> connected on board
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>

Acked-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH resend 7/7] mtd/st_smi: Add mtd support for smi

[Vipin Kumar]

This patch adds mtd device support for smi devices

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
---
 drivers/mtd/st_smi.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 165 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
index 0e529a4..4d1a6bd 100644
--- a/drivers/mtd/st_smi.c
+++ b/drivers/mtd/st_smi.c
@@ -24,6 +24,7 @@
 #include <common.h>
 #include <flash.h>
 #include <linux/err.h>
+#include <linux/mtd/mtd.h>
 #include <linux/mtd/st_smi.h>
 
 #include <asm/io.h>
@@ -37,6 +38,14 @@ static ulong bank_base[CONFIG_SYS_MAX_FLASH_BANKS] =
     CONFIG_SYS_FLASH_ADDR_BASE;
 flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];
 
+#if defined(CONFIG_MTD_DEVICE)
+/* MTD interface for SMI devices */
+static struct mtd_info smi_mtd_info[CONFIG_SYS_MAX_FLASH_BANKS];
+static char smi_mtd_names[CONFIG_SYS_MAX_FLASH_BANKS][16];
+
+static int smi_mtd_init(void);
+#endif
+
 /* data structure to maintain flash ids from different vendors */
 struct flash_device {
 	char *name;
@@ -482,7 +491,9 @@ unsigned long flash_init(void)
 {
 	unsigned long size = 0;
 	int i, j;
-
+#if defined(CONFIG_MTD_DEVICE)
+	int error;
+#endif
 	smi_init();
 
 	for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++) {
@@ -498,6 +509,12 @@ unsigned long flash_init(void)
 
 	}
 
+#if defined(CONFIG_MTD_DEVICE)
+	error = smi_mtd_init();
+	if (error < 0)
+		return 0;
+#endif
+
 	return size;
 }
 
@@ -599,4 +616,150 @@ int flash_erase(flash_info_t *info, int s_first, int s_last)
 	puts(" done\n");
 	return rcode;
 }
-#endif
+
+#if defined(CONFIG_MTD_DEVICE)
+static int smi_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
+{
+	flash_info_t *fi = mtd->priv;
+	size_t a_start = fi->start[0] + instr->addr;
+	size_t a_end = a_start + instr->len;
+	int s_first = -1;
+	int s_last = -1;
+	int error, sect;
+
+	for (sect = 0; sect < fi->sector_count; sect++) {
+		if (a_start == fi->start[sect])
+			s_first = sect;
+
+		if (sect < fi->sector_count - 1) {
+			if (a_end == fi->start[sect + 1]) {
+				s_last = sect;
+				break;
+			}
+		} else {
+			s_last = sect;
+			break;
+		}
+	}
+
+	if (s_first >= 0 && s_first <= s_last) {
+		instr->state = MTD_ERASING;
+
+		error = flash_erase(fi, s_first, s_last);
+		if (error) {
+			instr->state = MTD_ERASE_FAILED;
+			return -EIO;
+		}
+
+		instr->state = MTD_ERASE_DONE;
+		mtd_erase_callback(instr);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int smi_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
+	size_t *retlen, u_char *buf)
+{
+	flash_info_t *fi = mtd->priv;
+	u_char *f = (u_char *)(fi->start[0]) + from;
+
+	memcpy(buf, f, len);
+	*retlen = len;
+
+	return 0;
+}
+
+static int smi_mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
+	size_t *retlen, const u_char *buf)
+{
+	flash_info_t *fi = mtd->priv;
+	u_long t = fi->start[0] + to;
+	int error;
+
+	error = write_buff(fi, (u_char *)buf, t, len);
+	if (!error) {
+		*retlen = len;
+		return 0;
+	}
+
+	return -EIO;
+}
+
+static void smi_mtd_sync(struct mtd_info *mtd)
+{
+	/*
+	 * This function should wait until all pending operations
+	 * finish. However this driver is fully synchronous, so
+	 * this function returns immediately
+	 */
+}
+
+static int smi_mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+	flash_info_t *fi = mtd->priv;
+
+	flash_protect(FLAG_PROTECT_SET, fi->start[0] + ofs,
+					fi->start[0] + ofs + len - 1, fi);
+	return 0;
+}
+
+static int smi_mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+	flash_info_t *fi = mtd->priv;
+
+	flash_protect(FLAG_PROTECT_CLEAR, fi->start[0] + ofs,
+					fi->start[0] + ofs + len - 1, fi);
+	return 0;
+}
+
+static int smi_mtd_init(void)
+{
+	struct flash_device *flash_device_p;
+	struct mtd_info *mtd;
+	flash_info_t *fi;
+	int i;
+
+	for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++) {
+		fi = &flash_info[i];
+
+		if (!fi->size)
+			continue;
+
+		flash_device_p = get_flash_device(fi->flash_id);
+		if (!flash_device_p)
+			return -EIO;
+
+		mtd = &smi_mtd_info[i];
+
+		memset(mtd, 0, sizeof(struct mtd_info));
+
+		/* Uniform erase sizes for all sectors */
+		mtd->numeraseregions = 0;
+		mtd->erasesize = flash_device_p->sectorsize;
+
+		sprintf(smi_mtd_names[i], "nor%d", i);
+		mtd->name		= smi_mtd_names[i];
+		mtd->type		= MTD_NORFLASH;
+		mtd->flags		= MTD_CAP_NORFLASH;
+		mtd->size		= fi->size;
+		mtd->writesize		= 1;
+
+		mtd->erase		= smi_mtd_erase;
+		mtd->read		= smi_mtd_read;
+		mtd->write		= smi_mtd_write;
+		mtd->sync		= smi_mtd_sync;
+		mtd->lock		= smi_mtd_lock;
+		mtd->unlock		= smi_mtd_unlock;
+		mtd->priv		= fi;
+
+		if (add_mtd_device(mtd))
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+#endif /* CONFIG_MTD_DEVICE */
+
+#endif /* CONFIG_SYS_NO_FLASH */
-- 
1.8.0

[U-Boot] [PATCH resend 7/7] mtd/st_smi: Add mtd support for smi

[Stefan Roese]

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> This patch adds mtd device support for smi devices
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> ---
>  drivers/mtd/st_smi.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 165 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
> index 0e529a4..4d1a6bd 100644
> --- a/drivers/mtd/st_smi.c
> +++ b/drivers/mtd/st_smi.c
> @@ -24,6 +24,7 @@
>  #include <common.h>
>  #include <flash.h>
>  #include <linux/err.h>
> +#include <linux/mtd/mtd.h>
>  #include <linux/mtd/st_smi.h>
>  
>  #include <asm/io.h>
> @@ -37,6 +38,14 @@ static ulong bank_base[CONFIG_SYS_MAX_FLASH_BANKS] =
>      CONFIG_SYS_FLASH_ADDR_BASE;
>  flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];
>  
> +#if defined(CONFIG_MTD_DEVICE)
> +/* MTD interface for SMI devices */
> +static struct mtd_info smi_mtd_info[CONFIG_SYS_MAX_FLASH_BANKS];
> +static char smi_mtd_names[CONFIG_SYS_MAX_FLASH_BANKS][16];
> +
> +static int smi_mtd_init(void);
> +#endif
> +
>  /* data structure to maintain flash ids from different vendors */
>  struct flash_device {
>  	char *name;
> @@ -482,7 +491,9 @@ unsigned long flash_init(void)
>  {
>  	unsigned long size = 0;
>  	int i, j;
> -
> +#if defined(CONFIG_MTD_DEVICE)
> +	int error;
> +#endif

Hmm, I don't like all those new #ifdef's. Can't we instead add a dummy
smi_mtd_init() functions for the non-MTD version? Something like:

#if defined(CONFIG_MTD_DEVICE)
/* MTD interface for SMI devices */
static struct mtd_info smi_mtd_info[CONFIG_SYS_MAX_FLASH_BANKS];
static char smi_mtd_names[CONFIG_SYS_MAX_FLASH_BANKS][16];

static int smi_mtd_init(void);
#else
static int smi_mtd_init(void)
{
	return 0;
}
#endif

What do you think?

Thanks,
Stefan

[U-Boot] [PATCH resend 7/7] mtd/st_smi: Add mtd support for smi

[Vipin Kumar]

On 12/6/2012 1:32 PM, Stefan Roese wrote:
> On 12/06/2012 07:21 AM, Vipin Kumar wrote:
>> This patch adds mtd device support for smi devices
>>
>> Signed-off-by: Vipin Kumar<vipin.kumar@st.com>
>> ---
>>   drivers/mtd/st_smi.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 165 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/st_smi.c b/drivers/mtd/st_smi.c
>> index 0e529a4..4d1a6bd 100644
>> --- a/drivers/mtd/st_smi.c
>> +++ b/drivers/mtd/st_smi.c
>> @@ -24,6 +24,7 @@
>>   #include<common.h>
>>   #include<flash.h>
>>   #include<linux/err.h>
>> +#include<linux/mtd/mtd.h>
>>   #include<linux/mtd/st_smi.h>
>>
>>   #include<asm/io.h>
>> @@ -37,6 +38,14 @@ static ulong bank_base[CONFIG_SYS_MAX_FLASH_BANKS] =
>>       CONFIG_SYS_FLASH_ADDR_BASE;
>>   flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];
>>
>> +#if defined(CONFIG_MTD_DEVICE)
>> +/* MTD interface for SMI devices */
>> +static struct mtd_info smi_mtd_info[CONFIG_SYS_MAX_FLASH_BANKS];
>> +static char smi_mtd_names[CONFIG_SYS_MAX_FLASH_BANKS][16];
>> +
>> +static int smi_mtd_init(void);
>> +#endif
>> +
>>   /* data structure to maintain flash ids from different vendors */
>>   struct flash_device {
>>   	char *name;
>> @@ -482,7 +491,9 @@ unsigned long flash_init(void)
>>   {
>>   	unsigned long size = 0;
>>   	int i, j;
>> -
>> +#if defined(CONFIG_MTD_DEVICE)
>> +	int error;
>> +#endif
>
> Hmm, I don't like all those new #ifdef's. Can't we instead add a dummy
> smi_mtd_init() functions for the non-MTD version? Something like:
>
> #if defined(CONFIG_MTD_DEVICE)
> /* MTD interface for SMI devices */
> static struct mtd_info smi_mtd_info[CONFIG_SYS_MAX_FLASH_BANKS];
> static char smi_mtd_names[CONFIG_SYS_MAX_FLASH_BANKS][16];
>
> static int smi_mtd_init(void);
> #else
> static int smi_mtd_init(void)
> {
> 	return 0;
> }
> #endif
>
> What do you think?
>

I agree. That would be more readable. I would send out a v2

> Thanks,
> Stefan
>
>

[U-Boot] [PATCH resend 0/7] mtd/st_smi: Add fixes for smi driver

[Stefan Roese]

Hi Vipin,

On 12/06/2012 07:21 AM, Vipin Kumar wrote:
> Hello,
> 
> This patch-set adds simple fixes for smi driver used in spear devices.
> 
> Modifications include
> - Add MTD support for smi driver
> - Write to flash in a tight loop
> - Alphabetically sort the list of devices
> - Avoid issuing multiple write enable commands
> 
> PS: Wolfgang, Stefan, I am directing these to you as there is no direct
> maintainer of mtd in u-boot. I can add these in my pull-request once you review
> these patches

Since I'm the CFI (NOR parallel) flash custodian, I can take these
patches through my repository as well. If there is a dependency with
some of your platforms patches, then the patches can go through your ST
repo as well, after successful review.

Reviewing in a minute...

Thanks,
Stefan

[U-Boot] [PATCH resend 0/7] mtd/st_smi: Add fixes for smi driver

[Vipin Kumar]

On 12/6/2012 1:05 PM, Stefan Roese wrote:
> Hi Vipin,
>
> On 12/06/2012 07:21 AM, Vipin Kumar wrote:
>> Hello,
>>
>> This patch-set adds simple fixes for smi driver used in spear devices.
>>
>> Modifications include
>> - Add MTD support for smi driver
>> - Write to flash in a tight loop
>> - Alphabetically sort the list of devices
>> - Avoid issuing multiple write enable commands
>>
>> PS: Wolfgang, Stefan, I am directing these to you as there is no direct
>> maintainer of mtd in u-boot. I can add these in my pull-request once you review
>> these patches
>
> Since I'm the CFI (NOR parallel) flash custodian, I can take these
> patches through my repository as well. If there is a dependency with
> some of your platforms patches, then the patches can go through your ST
> repo as well, after successful review.
>

These patches do not have any dependency with the platform patches and 
they can go with your repository as well. Let me know if you plan to add 
these through your repo

Vipin

> Reviewing in a minute...
>
> Thanks,
> Stefan
> .
>

[U-Boot] [PATCH resend 0/7] mtd/st_smi: Add fixes for smi driver

[Stefan Roese]

On 12/06/2012 09:19 AM, Vipin Kumar wrote:
>> Since I'm the CFI (NOR parallel) flash custodian, I can take these
>> patches through my repository as well. If there is a dependency with
>> some of your platforms patches, then the patches can go through your ST
>> repo as well, after successful review.
>>
> 
> These patches do not have any dependency with the platform patches and 
> they can go with your repository as well. Let me know if you plan to add 
> these through your repo

Please push them to your repository after final review.

Thanks,
Stefan

[U-Boot] [PATCH resend 0/7] mtd/st_smi: Add fixes for smi driver

[Vipin Kumar]

On 12/6/2012 2:02 PM, Stefan Roese wrote:
> On 12/06/2012 09:19 AM, Vipin Kumar wrote:
>>> Since I'm the CFI (NOR parallel) flash custodian, I can take these
>>> patches through my repository as well. If there is a dependency with
>>> some of your platforms patches, then the patches can go through your ST
>>> repo as well, after successful review.
>>>
>>
>> These patches do not have any dependency with the platform patches and
>> they can go with your repository as well. Let me know if you plan to add
>> these through your repo
>
> Please push them to your repository after final review.
>

OK

> Thanks,
> Stefan
>
>