[PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Xilinx Spartan-3AN FPGAs contain an In-System Flash where they keep
their configuration data and (optionally) some user data.

The protocol of this flash follows most of the spi-nor standard. With
the following differences:

- Page size might not be a power of two.
- The address calculation (default addressing mode).
- The spi nor commands used.

Protocol is described on Xilinx User Guide UG333

Reviewed-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
v6:
Suggested by: Boris Brezillon <boris.brezillon@free-electrons.com>
-Replace triple operator with if/else

v5:
-Rebase on top of l2-mtd/master
Suggested by: Cyrille Pitchen <cyrille.pitchen@atmel.com>:
-Fix to+1 bug
-Move all address conversions to spi-nor
-Replace pr_dev with dev_err

v4:
-Rebase on top of l2-mtd/master

v3:
-Rebase on top of mtd-next
-Rename ADDR_NATIVE to ADDR_DEFAULT to follow UG333 naming
-Fix bug on probe

v2: Suggested by Brian Norris <computersforpeace@gmail.com>

-Remove inline qualifier
-Improve documentation of Default Addressing Mode
-Convert function callbacks into SNOR_F_
-Fix missmatch braces
-Improve documentation of SPI_S3AN flag

 drivers/mtd/spi-nor/spi-nor.c | 124 ++++++++++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |  12 ++++
 2 files changed, 131 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d0fc165d7d66..d29842177808 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -75,6 +75,12 @@ struct flash_info {
 					 * bit. Must be used with
 					 * SPI_NOR_HAS_LOCK.
 					 */
+#define	SPI_S3AN		BIT(10)	/*
+					 * Xilinx Spartan 3AN In-System Flash
+					 * (MFR cannot be used for probing
+					 * because it has the same value as
+					 * ATMEL flashes)
+					*/
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -217,6 +223,21 @@ static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
 		return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
 	}
 }
+
+static int s3an_sr_ready(struct spi_nor *nor)
+{
+	int ret;
+	u8 val;
+
+	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
+		return ret;
+	}
+
+	return !!(val & XSR_RDY);
+}
+
 static inline int spi_nor_sr_ready(struct spi_nor *nor)
 {
 	int sr = read_sr(nor);
@@ -238,7 +259,11 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
 static int spi_nor_ready(struct spi_nor *nor)
 {
 	int sr, fsr;
-	sr = spi_nor_sr_ready(nor);
+
+	if (nor->flags & SNOR_F_READY_XSR_RDY)
+		sr = s3an_sr_ready(nor);
+	else
+		sr = spi_nor_sr_ready(nor);
 	if (sr < 0)
 		return sr;
 	fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
@@ -320,6 +345,22 @@ static void spi_nor_unlock_and_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
 }
 
 /*
+ * This code converts an address to the Default Address Mode, that has non
+ * power of two page sizes. We must support this mode because it is the default
+ * mode supported by Xilinx tools, it can access the whole flash area and
+ * changing over to the Power-of-two mode is irreversible and corrupts the
+ * original data.
+ */
+static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
+{
+	unsigned int offset;
+
+	offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);
+
+	return ((addr - offset) << 1) | offset;
+}
+
+/*
  * Initiate the erasure of a single sector
  */
 static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
@@ -327,6 +368,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
 	u8 buf[SPI_NOR_MAX_ADDR_WIDTH];
 	int i;
 
+	if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
+		addr = spi_nor_s3an_addr_convert(nor, addr);
+
 	if (nor->erase)
 		return nor->erase(nor, addr);
 
@@ -368,7 +412,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 		return ret;
 
 	/* whole-chip erase? */
-	if (len == mtd->size) {
+	if (len == mtd->size && !(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) {
 		unsigned long timeout;
 
 		write_enable(nor);
@@ -782,6 +826,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 		.addr_width = (_addr_width),				\
 		.flags = (_flags),
 
+#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)			\
+		.id = {							\
+			((_jedec_id) >> 16) & 0xff,			\
+			((_jedec_id) >> 8) & 0xff,			\
+			(_jedec_id) & 0xff				\
+			},						\
+		.id_len = 3,						\
+		.sector_size = (8*_page_size),				\
+		.n_sectors = (_n_sectors),				\
+		.page_size = _page_size,				\
+		.addr_width = 3,					\
+		.flags = SPI_NOR_NO_FR | SPI_S3AN,
+
 /* NOTE: double check command sets and memory organization when you add
  * more nor chips.  This current list focusses on newer chips, which
  * have been converging on command sets which including JEDEC ID.
@@ -1009,6 +1066,13 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
 	{ "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
 	{ "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
+
+	/* Xilinx S3AN Internal Flash */
+	{ "3S50AN", S3AN_INFO(0x1f2200, 64, 264) },
+	{ "3S200AN", S3AN_INFO(0x1f2400, 256, 264) },
+	{ "3S400AN", S3AN_INFO(0x1f2400, 256, 264) },
+	{ "3S700AN", S3AN_INFO(0x1f2500, 512, 264) },
+	{ "3S1400AN", S3AN_INFO(0x1f2600, 512, 528) },
 	{ },
 };
 
@@ -1049,7 +1113,12 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
 		return ret;
 
 	while (len) {
-		ret = nor->read(nor, from, len, buf);
+		loff_t addr = from;
+
+		if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
+			addr = spi_nor_s3an_addr_convert(nor, addr);
+
+		ret = nor->read(nor, addr, len, buf);
 		if (ret == 0) {
 			/* We shouldn't see 0-length reads */
 			ret = -EIO;
@@ -1170,8 +1239,15 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 	for (i = 0; i < len; ) {
 		ssize_t written;
+		loff_t addr = to + i;
 
-		page_offset = (to + i) & (nor->page_size - 1);
+		if (hweight32(nor->page_size) == 1) {
+			page_offset = addr & (nor->page_size - 1);
+		} else {
+			uint64_t aux = addr;
+
+			page_offset = do_div(aux, nor->page_size);
+		}
 		WARN_ONCE(page_offset,
 			  "Writing at offset %zu into a NOR page. Writing partial pages may decrease reliability and increase wear of NOR flash.",
 			  page_offset);
@@ -1179,8 +1255,11 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 		page_remain = min_t(size_t,
 				    nor->page_size - page_offset, len - i);
 
+		if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
+			addr = spi_nor_s3an_addr_convert(nor, addr);
+
 		write_enable(nor);
-		ret = nor->write(nor, to + i, page_remain, buf + i);
+		ret = nor->write(nor, addr, page_remain, buf + i);
 		if (ret < 0)
 			goto write_err;
 		written = ret;
@@ -1300,6 +1379,35 @@ static int spi_nor_check(struct spi_nor *nor)
 	return 0;
 }
 
+static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
+{
+	int ret;
+	u8 val;
+
+	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
+	if (ret < 0) {
+		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
+		return ret;
+	}
+
+	nor->erase_opcode = SPINOR_OP_XSE;
+	nor->program_opcode = SPINOR_OP_XPP;
+	nor->read_opcode = SPINOR_OP_READ;
+	nor->flags |= SNOR_F_NO_OP_CHIP_ERASE | SNOR_F_READY_XSR_RDY;
+
+	/* Flash in Power of 2 mode */
+	if (val & XSR_PAGESIZE) {
+		nor->page_size = (nor->page_size == 264) ? 256 : 512;
+		nor->mtd.writebufsize = nor->page_size;
+		nor->mtd.size = 8 * nor->page_size * info->n_sectors;
+		nor->mtd.erasesize = 8 * nor->page_size;
+	} else {
+		nor->flags |= SNOR_F_S3AN_ADDR_DEFAULT;
+	}
+
+	return 0;
+}
+
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
 	const struct flash_info *info = NULL;
@@ -1505,6 +1613,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	nor->read_dummy = spi_nor_read_dummy_cycles(nor);
 
+	if (info->flags & SPI_S3AN) {
+		ret = s3an_nor_scan(info, nor);
+		if (ret)
+			return ret;
+	}
+
 	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
 			(long long)mtd->size >> 10);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index c425c7b4c2a0..4950b2ef08c0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -68,6 +68,15 @@
 #define SPINOR_OP_WRDI		0x04	/* Write disable */
 #define SPINOR_OP_AAI_WP	0xad	/* Auto address increment word program */
 
+/* Used for S3AN flashes only */
+#define SPINOR_OP_XSE		0x50	/* Sector erase */
+#define SPINOR_OP_XPP		0x82	/* Page program */
+#define SPINOR_OP_XRDSR		0xd7	/* Read status register */
+
+#define XSR_PAGESIZE		BIT(0)	/* Page size in Po2 or Linear */
+#define XSR_RDY			BIT(7)	/* Ready */
+
+
 /* Used for Macronix and Winbond flashes. */
 #define SPINOR_OP_EN4B		0xb7	/* Enter 4-byte mode */
 #define SPINOR_OP_EX4B		0xe9	/* Exit 4-byte mode */
@@ -119,6 +128,9 @@ enum spi_nor_ops {
 enum spi_nor_option_flags {
 	SNOR_F_USE_FSR		= BIT(0),
 	SNOR_F_HAS_SR_TB	= BIT(1),
+	SNOR_F_NO_OP_CHIP_ERASE	= BIT(2),
+	SNOR_F_S3AN_ADDR_DEFAULT = BIT(3),
+	SNOR_F_READY_XSR_RDY	= BIT(4),
 };
 
 /**
-- 
2.9.3

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

Hi Ricardo,

Please try to pass the version in your subject prefix (pass
--subject-prefix="PATCH vX" to git format-patch).

On Tue, 20 Sep 2016 17:45:51 +0200
Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:

> Xilinx Spartan-3AN FPGAs contain an In-System Flash where they keep
> their configuration data and (optionally) some user data.
> 
> The protocol of this flash follows most of the spi-nor standard. With
> the following differences:
> 
> - Page size might not be a power of two.
> - The address calculation (default addressing mode).
> - The spi nor commands used.
> 
> Protocol is described on Xilinx User Guide UG333
> 
> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> v6:
> Suggested by: Boris Brezillon <boris.brezillon@free-electrons.com>
> -Replace triple operator with if/else
> 
> v5:
> -Rebase on top of l2-mtd/master
> Suggested by: Cyrille Pitchen <cyrille.pitchen@atmel.com>:
> -Fix to+1 bug
> -Move all address conversions to spi-nor
> -Replace pr_dev with dev_err
> 
> v4:
> -Rebase on top of l2-mtd/master
> 
> v3:
> -Rebase on top of mtd-next
> -Rename ADDR_NATIVE to ADDR_DEFAULT to follow UG333 naming
> -Fix bug on probe
> 
> v2: Suggested by Brian Norris <computersforpeace@gmail.com>
> 
> -Remove inline qualifier
> -Improve documentation of Default Addressing Mode
> -Convert function callbacks into SNOR_F_
> -Fix missmatch braces
> -Improve documentation of SPI_S3AN flag
> 
>  drivers/mtd/spi-nor/spi-nor.c | 124 ++++++++++++++++++++++++++++++++++++++++--
>  include/linux/mtd/spi-nor.h   |  12 ++++
>  2 files changed, 131 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d0fc165d7d66..d29842177808 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c

[...]

>  /*
> + * This code converts an address to the Default Address Mode, that has non
> + * power of two page sizes. We must support this mode because it is the default
> + * mode supported by Xilinx tools, it can access the whole flash area and
> + * changing over to the Power-of-two mode is irreversible and corrupts the
> + * original data.
> + */
> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
> +{
> +	unsigned int offset;
> +
> +	offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);

Can you send a new version with

	offset = addr % nor->page_size;

to see if kbuild test robot still complains.

> +
> +	return ((addr - offset) << 1) | offset;
> +}
> +

Thanks,

Boris

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi Boris

On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Hi Ricardo,
>
> Please try to pass the version in your subject prefix (pass
> --subject-prefix="PATCH vX" to git format-patch).
>

Sorry about that.


> On Tue, 20 Sep 2016 17:45:51 +0200
> Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:

>>  /*
>> + * This code converts an address to the Default Address Mode, that has non
>> + * power of two page sizes. We must support this mode because it is the default
>> + * mode supported by Xilinx tools, it can access the whole flash area and
>> + * changing over to the Power-of-two mode is irreversible and corrupts the
>> + * original data.
>> + */
>> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
>> +{
>> +     unsigned int offset;
>> +
>> +     offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);
>
> Can you send a new version with
>
>         offset = addr % nor->page_size;
>
> to see if kbuild test robot still complains.
>

This code:

static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
{
unsigned int offset;

offset = addr % nor->page_size;

return ((addr - offset) << 1) | offset;
}


When built like:

   wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=blackfin

Produces this error:


drivers/built-in.o: In function `spi_nor_write':
drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to `__moddi3'
drivers/built-in.o: In function `spi_nor_read':
drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to `__moddi3'
drivers/built-in.o: In function `spi_nor_erase':
drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to `__moddi3'
Makefile:956: recipe for target 'vmlinux' failed


But I think I found the right combination:


static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
{
unsigned int offset = addr;

offset %= nor->page_size;

return ((addr - offset) << 1) | offset;
}


This one works fine on x86 and blackfin

Sending v7


Thanks!


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 09:57:23 +0200
Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:

> Hi Boris
> 
> On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Hi Ricardo,
> >
> > Please try to pass the version in your subject prefix (pass
> > --subject-prefix="PATCH vX" to git format-patch).
> >  
> 
> Sorry about that.
> 
> 
> > On Tue, 20 Sep 2016 17:45:51 +0200
> > Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:  
> 
> >>  /*
> >> + * This code converts an address to the Default Address Mode, that has non
> >> + * power of two page sizes. We must support this mode because it is the default
> >> + * mode supported by Xilinx tools, it can access the whole flash area and
> >> + * changing over to the Power-of-two mode is irreversible and corrupts the
> >> + * original data.
> >> + */
> >> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
> >> +{
> >> +     unsigned int offset;
> >> +
> >> +     offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);  
> >
> > Can you send a new version with
> >
> >         offset = addr % nor->page_size;
> >
> > to see if kbuild test robot still complains.
> >  
> 
> This code:
> 
> static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> {
> unsigned int offset;
> 
> offset = addr % nor->page_size;
> 
> return ((addr - offset) << 1) | offset;
> }
> 
> 
> When built like:
> 
>    wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=blackfin
> 
> Produces this error:
> 
> 
> drivers/built-in.o: In function `spi_nor_write':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to `__moddi3'
> drivers/built-in.o: In function `spi_nor_read':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to `__moddi3'
> drivers/built-in.o: In function `spi_nor_erase':
> drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to `__moddi3'
> Makefile:956: recipe for target 'vmlinux' failed
> 
> 
> But I think I found the right combination:
> 
> 
> static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> {
> unsigned int offset = addr;
> 
> offset %= nor->page_size;
> 
> return ((addr - offset) << 1) | offset;
> }
> 
> 
> This one works fine on x86 and blackfin
> 
> Sending v7

Wait. If you really want to manipulate an loff_t variable, you can do

	offset = do_div(addr, nor->page_size);

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi Boris

On Wed, Sep 21, 2016 at 10:14 AM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Wait. If you really want to manipulate an loff_t variable, you can do
>
>         offset = do_div(addr, nor->page_size);
>
>
>
that leads to:

  CC      drivers/mtd/spi-nor/spi-nor.o
drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
drivers/mtd/spi-nor/spi-nor.c:359:11: warning: comparison of distinct
pointer types lacks a cast [enabled by default]


and if I do the casting

offset = do_div((uint64_t)addr, nor->page_size);


drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
operand of assignment
drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as unary
'&' operand
scr

Unsinged long is more than enough for s3an. I think v7 is ok.


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 10:19:10 +0200
Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:

> Hi Boris
> 
> On Wed, Sep 21, 2016 at 10:14 AM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Wait. If you really want to manipulate an loff_t variable, you can do
> >
> >         offset = do_div(addr, nor->page_size);
> >
> >
> >  
> that leads to:
> 
>   CC      drivers/mtd/spi-nor/spi-nor.o
> drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
> drivers/mtd/spi-nor/spi-nor.c:359:11: warning: comparison of distinct
> pointer types lacks a cast [enabled by default]
> 
> 
> and if I do the casting
> 
> offset = do_div((uint64_t)addr, nor->page_size);
> 
> 
> drivers/mtd/spi-nor/spi-nor.c: In function 'spi_nor_s3an_addr_convert':
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as left
> operand of assignment
> drivers/mtd/spi-nor/spi-nor.c:359:11: error: lvalue required as unary
> '&' operand
> scr
> 
> Unsinged long is more than enough for s3an. I think v7 is ok.

Then, why did you change the spi_nor_s3an_addr_convert() prototype? 

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hi

>>
>> Unsinged long is more than enough for s3an. I think v7 is ok.
>
> Then, why did you change the spi_nor_s3an_addr_convert() prototype?
>

Because it is used in spi_nor_read and spi_nor_write, which use loff_t.

I think it is more clean to have only one comment explaining the max
size of the s3an, instead of having them scattered around. But if
think that it is better to have unsinged int as prototype I have no
problem in changing it.

Regards!


-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Ping?

On Wed, Sep 21, 2016 at 10:26 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Hi
>
>>>
>>> Unsinged long is more than enough for s3an. I think v7 is ok.
>>
>> Then, why did you change the spi_nor_s3an_addr_convert() prototype?
>>
>
> Because it is used in spi_nor_read and spi_nor_write, which use loff_t.
>
> I think it is more clean to have only one comment explaining the max
> size of the s3an, instead of having them scattered around. But if
> think that it is better to have unsinged int as prototype I have no
> problem in changing it.
>
> Regards!
>
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Boris Brezillon]

On Wed, 21 Sep 2016 10:14:20 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Wed, 21 Sep 2016 09:57:23 +0200
> Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:
> 
> > Hi Boris
> > 
> > On Wed, Sep 21, 2016 at 9:07 AM, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:  
> > > Hi Ricardo,
> > >
> > > Please try to pass the version in your subject prefix (pass
> > > --subject-prefix="PATCH vX" to git format-patch).
> > >    
> > 
> > Sorry about that.
> > 
> >   
> > > On Tue, 20 Sep 2016 17:45:51 +0200
> > > Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:    
> >   
> > >>  /*
> > >> + * This code converts an address to the Default Address Mode, that has non
> > >> + * power of two page sizes. We must support this mode because it is the default
> > >> + * mode supported by Xilinx tools, it can access the whole flash area and
> > >> + * changing over to the Power-of-two mode is irreversible and corrupts the
> > >> + * original data.
> > >> + */
> > >> +static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
> > >> +{
> > >> +     unsigned int offset;
> > >> +
> > >> +     offset = (nor->page_size == 264) ? (addr % 264) : (addr % 528);    
> > >
> > > Can you send a new version with
> > >
> > >         offset = addr % nor->page_size;
> > >
> > > to see if kbuild test robot still complains.
> > >    
> > 
> > This code:
> > 
> > static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> > {
> > unsigned int offset;
> > 
> > offset = addr % nor->page_size;
> > 
> > return ((addr - offset) << 1) | offset;
> > }
> > 
> > 
> > When built like:
> > 
> >    wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
> > -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=blackfin
> > 
> > Produces this error:
> > 
> > 
> > drivers/built-in.o: In function `spi_nor_write':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0xfeb8): undefined reference to `__moddi3'
> > drivers/built-in.o: In function `spi_nor_read':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0x10248): undefined reference to `__moddi3'
> > drivers/built-in.o: In function `spi_nor_erase':
> > drivers/mtd/spi-nor/spi-nor.c:(.text+0x1034e): undefined reference to `__moddi3'
> > Makefile:956: recipe for target 'vmlinux' failed
> > 
> > 
> > But I think I found the right combination:
> > 
> > 
> > static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, loff_t addr)
> > {
> > unsigned int offset = addr;
> > 
> > offset %= nor->page_size;
> > 
> > return ((addr - offset) << 1) | offset;
> > }
> > 
> > 
> > This one works fine on x86 and blackfin
> > 
> > Sending v7  
> 
> Wait. If you really want to manipulate an loff_t variable, you can do
> 
> 	offset = do_div(addr, nor->page_size);
> 
> 

Actually, you should have an intermediate u64 var, or just turn addr
into an u64 (loff_t is a long long not an unsigned long long).

[PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Xilinx Spartan-3AN contain an embedded spi device where they keep their
configuration data and optionally some user data.

The protocol of this flash follows most of the spi-nor standard. With
the following differences:

- Page size might not be a power of two.
- The address calculation.
- The spi nor commands used.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/mtd/devices/m25p80.c  |  3 ++
 drivers/mtd/spi-nor/spi-nor.c | 97 +++++++++++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h   | 13 ++++++
 3 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index c9c3b7fa3051..869576735978 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -49,6 +49,9 @@ static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
 
 static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
 {
+	if (nor->addr_convert)
+		addr = nor->addr_convert(nor, addr);
+
 	/* opcode is in cmd[0] */
 	cmd[1] = addr >> (nor->addr_width * 8 -  8);
 	cmd[2] = addr >> (nor->addr_width * 8 - 16);
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ed0c19c558b5..90215f053916 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -69,6 +69,7 @@ struct flash_info {
 #define	SPI_NOR_DUAL_READ	0x20    /* Flash supports Dual Read */
 #define	SPI_NOR_QUAD_READ	0x40    /* Flash supports Quad Read */
 #define	USE_FSR			0x80	/* use flag status register */
+#define	SPI_S3AN		0x100	/* Xililnx S3AN Embedded SPI nor*/
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -211,6 +212,21 @@ static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
 		return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
 	}
 }
+
+static inline int s3an_sr_ready(struct spi_nor *nor)
+{
+	int ret;
+	u8 val;
+
+	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
+	if (ret < 0) {
+		pr_err("error %d reading SR\n", (int) ret);
+		return ret;
+	}
+
+	return !!(val & XSR_RDY);
+}
+
 static inline int spi_nor_sr_ready(struct spi_nor *nor)
 {
 	int sr = read_sr(nor);
@@ -232,7 +248,7 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
 static int spi_nor_ready(struct spi_nor *nor)
 {
 	int sr, fsr;
-	sr = spi_nor_sr_ready(nor);
+	sr = nor->sr_ready(nor);
 	if (sr < 0)
 		return sr;
 	fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
@@ -328,6 +344,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
 	 * Default implementation, if driver doesn't have a specialized HW
 	 * control
 	 */
+	if (nor->addr_convert)
+		addr = nor->addr_convert(nor, addr);
+
 	for (i = nor->addr_width - 1; i >= 0; i--) {
 		buf[i] = addr & 0xff;
 		addr >>= 8;
@@ -362,7 +381,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 		return ret;
 
 	/* whole-chip erase? */
-	if (len == mtd->size) {
+	if (len == mtd->size && !(nor->flags & SNOR_NO_OP_CHIP_ERASE)) {
 		unsigned long timeout;
 
 		write_enable(nor);
@@ -680,6 +699,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 		.addr_width = (_addr_width),				\
 		.flags = (_flags),
 
+#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)			\
+		.id = {							\
+			((_jedec_id) >> 16) & 0xff,			\
+			((_jedec_id) >> 8) & 0xff,			\
+			(_jedec_id) & 0xff				\
+			},						\
+		.id_len = 3,						\
+		.sector_size = (8*_page_size),				\
+		.n_sectors = (_n_sectors),				\
+		.page_size = _page_size,				\
+		.addr_width = 3,					\
+		.flags = SPI_NOR_NO_FR | SPI_S3AN,
+
 /* NOTE: double check command sets and memory organization when you add
  * more nor chips.  This current list focusses on newer chips, which
  * have been converging on command sets which including JEDEC ID.
@@ -877,6 +909,14 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
 	{ "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
 	{ },
+
+	/* Xilinx S3AN Internal Flash */
+	{ "3S50AN", S3AN_INFO(0x1f2200, 64, 264) },
+	{ "3S200AN", S3AN_INFO(0x1f2400, 256, 264) },
+	{ "3S400AN", S3AN_INFO(0x1f2400, 256, 264) },
+	{ "3S700AN", S3AN_INFO(0x1f2500, 512, 264) },
+	{ "3S1400AN", S3AN_INFO(0x1f2600, 512, 528) },
+	{ },
 };
 
 static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
@@ -1007,7 +1047,13 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 	write_enable(nor);
 
-	page_offset = to & (nor->page_size - 1);
+	if (hweight32(nor->page_size) == 1)
+		page_offset = to & (nor->page_size - 1);
+	else {
+		uint64_t aux = to;
+
+		page_offset = do_div(aux, nor->page_size);
+	}
 
 	/* do all the bytes fit onto one page? */
 	if (page_offset + len <= nor->page_size) {
@@ -1179,6 +1225,43 @@ static int spi_nor_check(struct spi_nor *nor)
 	return 0;
 }
 
+static unsigned int s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
+{
+	if (nor->page_size == 264)
+		return (addr / 264) * 512 + (addr % 264);
+
+	return (addr / 528) * 1024 + (addr % 528);
+}
+
+static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
+{
+	int ret;
+	u8 val;
+
+	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
+	if (ret < 0) {
+		pr_err("error %d reading SR\n", (int) ret);
+		return ret;
+	}
+
+	nor->erase_opcode = SPINOR_OP_XSE;
+	nor->program_opcode = SPINOR_OP_XPP;
+	nor->read_opcode = SPINOR_OP_READ;
+	nor->sr_ready = s3an_sr_ready;
+	nor->flags |= SNOR_NO_OP_CHIP_ERASE;
+
+	/*Flash in Power of 2 mode*/
+	if (val & XSR_PAGESIZE) {
+		nor->page_size = (nor->page_size == 264) ? 256 : 512;
+		nor->mtd.writebufsize = nor->page_size;
+		nor->mtd.size = 8 * nor->page_size * info->n_sectors;
+		nor->mtd.erasesize = 8 * nor->page_size;
+	} else
+		nor->addr_convert = s3an_addr_convert;
+
+	return 0;
+}
+
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
 	const struct flash_info *info = NULL;
@@ -1339,6 +1422,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	}
 
 	nor->program_opcode = SPINOR_OP_PP;
+	nor->sr_ready = spi_nor_sr_ready;
 
 	if (info->addr_width)
 		nor->addr_width = info->addr_width;
@@ -1379,6 +1463,13 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	nor->read_dummy = spi_nor_read_dummy_cycles(nor);
 
+	if (info->flags & SPI_S3AN) {
+		ret = s3an_nor_scan(info, nor);
+
+		if (ret)
+			return ret;
+	}
+
 	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
 			(long long)mtd->size >> 10);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 62356d50815b..8946fa936b46 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -67,6 +67,15 @@
 #define SPINOR_OP_WRDI		0x04	/* Write disable */
 #define SPINOR_OP_AAI_WP	0xad	/* Auto address increment word program */
 
+/* Used for S3AN flashes only */
+#define SPINOR_OP_XSE		0x50	/* Sector erase */
+#define SPINOR_OP_XPP		0x82	/* Page program */
+#define SPINOR_OP_XRDSR		0xd7	/* Read status register */
+
+#define XSR_PAGESIZE		BIT(0)	/* Page size in Po2 or Linear */
+#define XSR_RDY			BIT(7)	/* Ready */
+
+
 /* Used for Macronix and Winbond flashes. */
 #define SPINOR_OP_EN4B		0xb7	/* Enter 4-byte mode */
 #define SPINOR_OP_EX4B		0xe9	/* Exit 4-byte mode */
@@ -116,6 +125,7 @@ enum spi_nor_ops {
 
 enum spi_nor_option_flags {
 	SNOR_F_USE_FSR		= BIT(0),
+	SNOR_NO_OP_CHIP_ERASE	= BIT(1),
 };
 
 /**
@@ -170,6 +180,9 @@ struct spi_nor {
 	int (*read_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
 	int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
 
+	int (*sr_ready)(struct spi_nor *nor);
+	unsigned int (*addr_convert)(struct spi_nor *nor, unsigned int addr);
+
 	int (*read)(struct spi_nor *nor, loff_t from,
 			size_t len, size_t *retlen, u_char *read_buf);
 	void (*write)(struct spi_nor *nor, loff_t to,
-- 
2.6.4

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

ping?


On Fri, Jan 15, 2016 at 5:25 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Xilinx Spartan-3AN contain an embedded spi device where they keep their
> configuration data and optionally some user data.
>
> The protocol of this flash follows most of the spi-nor standard. With
> the following differences:
>
> - Page size might not be a power of two.
> - The address calculation.
> - The spi nor commands used.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/mtd/devices/m25p80.c  |  3 ++
>  drivers/mtd/spi-nor/spi-nor.c | 97 +++++++++++++++++++++++++++++++++++++++++--
>  include/linux/mtd/spi-nor.h   | 13 ++++++
>  3 files changed, 110 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c9c3b7fa3051..869576735978 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -49,6 +49,9 @@ static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
>
>  static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>  {
> +       if (nor->addr_convert)
> +               addr = nor->addr_convert(nor, addr);
> +
>         /* opcode is in cmd[0] */
>         cmd[1] = addr >> (nor->addr_width * 8 -  8);
>         cmd[2] = addr >> (nor->addr_width * 8 - 16);
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index ed0c19c558b5..90215f053916 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -69,6 +69,7 @@ struct flash_info {
>  #define        SPI_NOR_DUAL_READ       0x20    /* Flash supports Dual Read */
>  #define        SPI_NOR_QUAD_READ       0x40    /* Flash supports Quad Read */
>  #define        USE_FSR                 0x80    /* use flag status register */
> +#define        SPI_S3AN                0x100   /* Xililnx S3AN Embedded SPI nor*/
>  };
>
>  #define JEDEC_MFR(info)        ((info)->id[0])
> @@ -211,6 +212,21 @@ static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>                 return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
>         }
>  }
> +
> +static inline int s3an_sr_ready(struct spi_nor *nor)
> +{
> +       int ret;
> +       u8 val;
> +
> +       ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
> +       if (ret < 0) {
> +               pr_err("error %d reading SR\n", (int) ret);
> +               return ret;
> +       }
> +
> +       return !!(val & XSR_RDY);
> +}
> +
>  static inline int spi_nor_sr_ready(struct spi_nor *nor)
>  {
>         int sr = read_sr(nor);
> @@ -232,7 +248,7 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
>  static int spi_nor_ready(struct spi_nor *nor)
>  {
>         int sr, fsr;
> -       sr = spi_nor_sr_ready(nor);
> +       sr = nor->sr_ready(nor);
>         if (sr < 0)
>                 return sr;
>         fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
> @@ -328,6 +344,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
>          * Default implementation, if driver doesn't have a specialized HW
>          * control
>          */
> +       if (nor->addr_convert)
> +               addr = nor->addr_convert(nor, addr);
> +
>         for (i = nor->addr_width - 1; i >= 0; i--) {
>                 buf[i] = addr & 0xff;
>                 addr >>= 8;
> @@ -362,7 +381,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>                 return ret;
>
>         /* whole-chip erase? */
> -       if (len == mtd->size) {
> +       if (len == mtd->size && !(nor->flags & SNOR_NO_OP_CHIP_ERASE)) {
>                 unsigned long timeout;
>
>                 write_enable(nor);
> @@ -680,6 +699,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>                 .addr_width = (_addr_width),                            \
>                 .flags = (_flags),
>
> +#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)                   \
> +               .id = {                                                 \
> +                       ((_jedec_id) >> 16) & 0xff,                     \
> +                       ((_jedec_id) >> 8) & 0xff,                      \
> +                       (_jedec_id) & 0xff                              \
> +                       },                                              \
> +               .id_len = 3,                                            \
> +               .sector_size = (8*_page_size),                          \
> +               .n_sectors = (_n_sectors),                              \
> +               .page_size = _page_size,                                \
> +               .addr_width = 3,                                        \
> +               .flags = SPI_NOR_NO_FR | SPI_S3AN,
> +
>  /* NOTE: double check command sets and memory organization when you add
>   * more nor chips.  This current list focusses on newer chips, which
>   * have been converging on command sets which including JEDEC ID.
> @@ -877,6 +909,14 @@ static const struct flash_info spi_nor_ids[] = {
>         { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>         { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>         { },
> +
> +       /* Xilinx S3AN Internal Flash */
> +       { "3S50AN", S3AN_INFO(0x1f2200, 64, 264) },
> +       { "3S200AN", S3AN_INFO(0x1f2400, 256, 264) },
> +       { "3S400AN", S3AN_INFO(0x1f2400, 256, 264) },
> +       { "3S700AN", S3AN_INFO(0x1f2500, 512, 264) },
> +       { "3S1400AN", S3AN_INFO(0x1f2600, 512, 528) },
> +       { },
>  };
>
>  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> @@ -1007,7 +1047,13 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>
>         write_enable(nor);
>
> -       page_offset = to & (nor->page_size - 1);
> +       if (hweight32(nor->page_size) == 1)
> +               page_offset = to & (nor->page_size - 1);
> +       else {
> +               uint64_t aux = to;
> +
> +               page_offset = do_div(aux, nor->page_size);
> +       }
>
>         /* do all the bytes fit onto one page? */
>         if (page_offset + len <= nor->page_size) {
> @@ -1179,6 +1225,43 @@ static int spi_nor_check(struct spi_nor *nor)
>         return 0;
>  }
>
> +static unsigned int s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
> +{
> +       if (nor->page_size == 264)
> +               return (addr / 264) * 512 + (addr % 264);
> +
> +       return (addr / 528) * 1024 + (addr % 528);
> +}
> +
> +static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
> +{
> +       int ret;
> +       u8 val;
> +
> +       ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
> +       if (ret < 0) {
> +               pr_err("error %d reading SR\n", (int) ret);
> +               return ret;
> +       }
> +
> +       nor->erase_opcode = SPINOR_OP_XSE;
> +       nor->program_opcode = SPINOR_OP_XPP;
> +       nor->read_opcode = SPINOR_OP_READ;
> +       nor->sr_ready = s3an_sr_ready;
> +       nor->flags |= SNOR_NO_OP_CHIP_ERASE;
> +
> +       /*Flash in Power of 2 mode*/
> +       if (val & XSR_PAGESIZE) {
> +               nor->page_size = (nor->page_size == 264) ? 256 : 512;
> +               nor->mtd.writebufsize = nor->page_size;
> +               nor->mtd.size = 8 * nor->page_size * info->n_sectors;
> +               nor->mtd.erasesize = 8 * nor->page_size;
> +       } else
> +               nor->addr_convert = s3an_addr_convert;
> +
> +       return 0;
> +}
> +
>  int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  {
>         const struct flash_info *info = NULL;
> @@ -1339,6 +1422,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>         }
>
>         nor->program_opcode = SPINOR_OP_PP;
> +       nor->sr_ready = spi_nor_sr_ready;
>
>         if (info->addr_width)
>                 nor->addr_width = info->addr_width;
> @@ -1379,6 +1463,13 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>
>         nor->read_dummy = spi_nor_read_dummy_cycles(nor);
>
> +       if (info->flags & SPI_S3AN) {
> +               ret = s3an_nor_scan(info, nor);
> +
> +               if (ret)
> +                       return ret;
> +       }
> +
>         dev_info(dev, "%s (%lld Kbytes)\n", info->name,
>                         (long long)mtd->size >> 10);
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 62356d50815b..8946fa936b46 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -67,6 +67,15 @@
>  #define SPINOR_OP_WRDI         0x04    /* Write disable */
>  #define SPINOR_OP_AAI_WP       0xad    /* Auto address increment word program */
>
> +/* Used for S3AN flashes only */
> +#define SPINOR_OP_XSE          0x50    /* Sector erase */
> +#define SPINOR_OP_XPP          0x82    /* Page program */
> +#define SPINOR_OP_XRDSR                0xd7    /* Read status register */
> +
> +#define XSR_PAGESIZE           BIT(0)  /* Page size in Po2 or Linear */
> +#define XSR_RDY                        BIT(7)  /* Ready */
> +
> +
>  /* Used for Macronix and Winbond flashes. */
>  #define SPINOR_OP_EN4B         0xb7    /* Enter 4-byte mode */
>  #define SPINOR_OP_EX4B         0xe9    /* Exit 4-byte mode */
> @@ -116,6 +125,7 @@ enum spi_nor_ops {
>
>  enum spi_nor_option_flags {
>         SNOR_F_USE_FSR          = BIT(0),
> +       SNOR_NO_OP_CHIP_ERASE   = BIT(1),
>  };
>
>  /**
> @@ -170,6 +180,9 @@ struct spi_nor {
>         int (*read_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
>         int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
>
> +       int (*sr_ready)(struct spi_nor *nor);
> +       unsigned int (*addr_convert)(struct spi_nor *nor, unsigned int addr);
> +
>         int (*read)(struct spi_nor *nor, loff_t from,
>                         size_t len, size_t *retlen, u_char *read_buf);
>         void (*write)(struct spi_nor *nor, loff_t to,
> --
> 2.6.4
>



-- 
Ricardo Ribalda

Re: [PATCH] mtd: spi-nor: Add support for S3AN spi-nor devices

[Ricardo Ribalda Delgado]

Hello

It has been almost two months :)... Any feedback about the patch
before I port it to the next version and resend?

Regards!

On Fri, Jan 22, 2016 at 3:52 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> ping?
>
>
> On Fri, Jan 15, 2016 at 5:25 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
>> Xilinx Spartan-3AN contain an embedded spi device where they keep their
>> configuration data and optionally some user data.
>>
>> The protocol of this flash follows most of the spi-nor standard. With
>> the following differences:
>>
>> - Page size might not be a power of two.
>> - The address calculation.
>> - The spi nor commands used.
>>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>>  drivers/mtd/devices/m25p80.c  |  3 ++
>>  drivers/mtd/spi-nor/spi-nor.c | 97 +++++++++++++++++++++++++++++++++++++++++--
>>  include/linux/mtd/spi-nor.h   | 13 ++++++
>>  3 files changed, 110 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index c9c3b7fa3051..869576735978 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -49,6 +49,9 @@ static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
>>
>>  static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>>  {
>> +       if (nor->addr_convert)
>> +               addr = nor->addr_convert(nor, addr);
>> +
>>         /* opcode is in cmd[0] */
>>         cmd[1] = addr >> (nor->addr_width * 8 -  8);
>>         cmd[2] = addr >> (nor->addr_width * 8 - 16);
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index ed0c19c558b5..90215f053916 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -69,6 +69,7 @@ struct flash_info {
>>  #define        SPI_NOR_DUAL_READ       0x20    /* Flash supports Dual Read */
>>  #define        SPI_NOR_QUAD_READ       0x40    /* Flash supports Quad Read */
>>  #define        USE_FSR                 0x80    /* use flag status register */
>> +#define        SPI_S3AN                0x100   /* Xililnx S3AN Embedded SPI nor*/
>>  };
>>
>>  #define JEDEC_MFR(info)        ((info)->id[0])
>> @@ -211,6 +212,21 @@ static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>>                 return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
>>         }
>>  }
>> +
>> +static inline int s3an_sr_ready(struct spi_nor *nor)
>> +{
>> +       int ret;
>> +       u8 val;
>> +
>> +       ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
>> +       if (ret < 0) {
>> +               pr_err("error %d reading SR\n", (int) ret);
>> +               return ret;
>> +       }
>> +
>> +       return !!(val & XSR_RDY);
>> +}
>> +
>>  static inline int spi_nor_sr_ready(struct spi_nor *nor)
>>  {
>>         int sr = read_sr(nor);
>> @@ -232,7 +248,7 @@ static inline int spi_nor_fsr_ready(struct spi_nor *nor)
>>  static int spi_nor_ready(struct spi_nor *nor)
>>  {
>>         int sr, fsr;
>> -       sr = spi_nor_sr_ready(nor);
>> +       sr = nor->sr_ready(nor);
>>         if (sr < 0)
>>                 return sr;
>>         fsr = nor->flags & SNOR_F_USE_FSR ? spi_nor_fsr_ready(nor) : 1;
>> @@ -328,6 +344,9 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
>>          * Default implementation, if driver doesn't have a specialized HW
>>          * control
>>          */
>> +       if (nor->addr_convert)
>> +               addr = nor->addr_convert(nor, addr);
>> +
>>         for (i = nor->addr_width - 1; i >= 0; i--) {
>>                 buf[i] = addr & 0xff;
>>                 addr >>= 8;
>> @@ -362,7 +381,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>>                 return ret;
>>
>>         /* whole-chip erase? */
>> -       if (len == mtd->size) {
>> +       if (len == mtd->size && !(nor->flags & SNOR_NO_OP_CHIP_ERASE)) {
>>                 unsigned long timeout;
>>
>>                 write_enable(nor);
>> @@ -680,6 +699,19 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>>                 .addr_width = (_addr_width),                            \
>>                 .flags = (_flags),
>>
>> +#define S3AN_INFO(_jedec_id, _n_sectors, _page_size)                   \
>> +               .id = {                                                 \
>> +                       ((_jedec_id) >> 16) & 0xff,                     \
>> +                       ((_jedec_id) >> 8) & 0xff,                      \
>> +                       (_jedec_id) & 0xff                              \
>> +                       },                                              \
>> +               .id_len = 3,                                            \
>> +               .sector_size = (8*_page_size),                          \
>> +               .n_sectors = (_n_sectors),                              \
>> +               .page_size = _page_size,                                \
>> +               .addr_width = 3,                                        \
>> +               .flags = SPI_NOR_NO_FR | SPI_S3AN,
>> +
>>  /* NOTE: double check command sets and memory organization when you add
>>   * more nor chips.  This current list focusses on newer chips, which
>>   * have been converging on command sets which including JEDEC ID.
>> @@ -877,6 +909,14 @@ static const struct flash_info spi_nor_ids[] = {
>>         { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>>         { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>>         { },
>> +
>> +       /* Xilinx S3AN Internal Flash */
>> +       { "3S50AN", S3AN_INFO(0x1f2200, 64, 264) },
>> +       { "3S200AN", S3AN_INFO(0x1f2400, 256, 264) },
>> +       { "3S400AN", S3AN_INFO(0x1f2400, 256, 264) },
>> +       { "3S700AN", S3AN_INFO(0x1f2500, 512, 264) },
>> +       { "3S1400AN", S3AN_INFO(0x1f2600, 512, 528) },
>> +       { },
>>  };
>>
>>  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>> @@ -1007,7 +1047,13 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>>
>>         write_enable(nor);
>>
>> -       page_offset = to & (nor->page_size - 1);
>> +       if (hweight32(nor->page_size) == 1)
>> +               page_offset = to & (nor->page_size - 1);
>> +       else {
>> +               uint64_t aux = to;
>> +
>> +               page_offset = do_div(aux, nor->page_size);
>> +       }
>>
>>         /* do all the bytes fit onto one page? */
>>         if (page_offset + len <= nor->page_size) {
>> @@ -1179,6 +1225,43 @@ static int spi_nor_check(struct spi_nor *nor)
>>         return 0;
>>  }
>>
>> +static unsigned int s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
>> +{
>> +       if (nor->page_size == 264)
>> +               return (addr / 264) * 512 + (addr % 264);
>> +
>> +       return (addr / 528) * 1024 + (addr % 528);
>> +}
>> +
>> +static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
>> +{
>> +       int ret;
>> +       u8 val;
>> +
>> +       ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
>> +       if (ret < 0) {
>> +               pr_err("error %d reading SR\n", (int) ret);
>> +               return ret;
>> +       }
>> +
>> +       nor->erase_opcode = SPINOR_OP_XSE;
>> +       nor->program_opcode = SPINOR_OP_XPP;
>> +       nor->read_opcode = SPINOR_OP_READ;
>> +       nor->sr_ready = s3an_sr_ready;
>> +       nor->flags |= SNOR_NO_OP_CHIP_ERASE;
>> +
>> +       /*Flash in Power of 2 mode*/
>> +       if (val & XSR_PAGESIZE) {
>> +               nor->page_size = (nor->page_size == 264) ? 256 : 512;
>> +               nor->mtd.writebufsize = nor->page_size;
>> +               nor->mtd.size = 8 * nor->page_size * info->n_sectors;
>> +               nor->mtd.erasesize = 8 * nor->page_size;
>> +       } else
>> +               nor->addr_convert = s3an_addr_convert;
>> +
>> +       return 0;
>> +}
>> +
>>  int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>  {
>>         const struct flash_info *info = NULL;
>> @@ -1339,6 +1422,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>         }
>>
>>         nor->program_opcode = SPINOR_OP_PP;
>> +       nor->sr_ready = spi_nor_sr_ready;
>>
>>         if (info->addr_width)
>>                 nor->addr_width = info->addr_width;
>> @@ -1379,6 +1463,13 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>
>>         nor->read_dummy = spi_nor_read_dummy_cycles(nor);
>>
>> +       if (info->flags & SPI_S3AN) {
>> +               ret = s3an_nor_scan(info, nor);
>> +
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>>         dev_info(dev, "%s (%lld Kbytes)\n", info->name,
>>                         (long long)mtd->size >> 10);
>>
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index 62356d50815b..8946fa936b46 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -67,6 +67,15 @@
>>  #define SPINOR_OP_WRDI         0x04    /* Write disable */
>>  #define SPINOR_OP_AAI_WP       0xad    /* Auto address increment word program */
>>
>> +/* Used for S3AN flashes only */
>> +#define SPINOR_OP_XSE          0x50    /* Sector erase */
>> +#define SPINOR_OP_XPP          0x82    /* Page program */
>> +#define SPINOR_OP_XRDSR                0xd7    /* Read status register */
>> +
>> +#define XSR_PAGESIZE           BIT(0)  /* Page size in Po2 or Linear */
>> +#define XSR_RDY                        BIT(7)  /* Ready */
>> +
>> +
>>  /* Used for Macronix and Winbond flashes. */
>>  #define SPINOR_OP_EN4B         0xb7    /* Enter 4-byte mode */
>>  #define SPINOR_OP_EX4B         0xe9    /* Exit 4-byte mode */
>> @@ -116,6 +125,7 @@ enum spi_nor_ops {
>>
>>  enum spi_nor_option_flags {
>>         SNOR_F_USE_FSR          = BIT(0),
>> +       SNOR_NO_OP_CHIP_ERASE   = BIT(1),
>>  };
>>
>>  /**
>> @@ -170,6 +180,9 @@ struct spi_nor {
>>         int (*read_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
>>         int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
>>
>> +       int (*sr_ready)(struct spi_nor *nor);
>> +       unsigned int (*addr_convert)(struct spi_nor *nor, unsigned int addr);
>> +
>>         int (*read)(struct spi_nor *nor, loff_t from,
>>                         size_t len, size_t *retlen, u_char *read_buf);
>>         void (*write)(struct spi_nor *nor, loff_t to,
>> --
>> 2.6.4
>>
>
>
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda