ASPEED SPI driver support for dual IO mode
Subsystems (3)
cover ASPEED SPI driver support for dual IO mode
Adds support for dual IO SPI mode. This speeds up SPI accesses, dropping time to dump an entire 64MB Macronix chip from ~12s to ~6s using:
dd if=/dev/mtdblock0 bs=64MB count=1 of=/dev/null
The dual IO bit mode should not depend on any board-specific properties as it uses the same IO lines as single bit mode. Only issue could be if a board used a one-way level tranlator or mux on the MOSI line but that seems unlikely to me.
Tested on Zaius board with AST2500 chip, not tested on AST2400 but I expect it to work since registers are identical.
Note dropped TODO about quad mode since ASPEED SPI controller does not support quad mode.
Robert Lippert (3):
mtd: spi-nor: add SPI_NOR_DUAL_READ to mx66l51235l
mtd: spi-nor: aspeed: add support for SPI dual IO read mode
mtd: spi-nor: aspeed: fix DMA access on AST2500
drivers/mtd/spi-nor/aspeed-smc.c | 30 +++++++++++++++++++++++------- drivers/mtd/spi-nor/spi-nor.c | 2 +- 2 files changed, 24 insertions(+), 8 deletions(-)
Signature · 1 line
2.11.0.390.gc69c2f50cf-goog
PATCH 1/3 mtd: spi-nor: add SPI_NOR_DUAL_READ to mx66l51235l
---
drivers/mtd/spi-nor/spi-nor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/spi-nor/spi-nor.c +1 -1
@@ -857,7 +857,7 @@ static const struct flash_info spi_nor_ids[] = { { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, 0) }, { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, - { "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_QUAD_READ) }, + { "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "mx66l1g45g", INFO(0xc2201b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_QUAD_READ) }, { "mx66l1g55g", INFO(0xc2261b, 0, 64 * 1024, 2048, SPI_NOR_QUAD_READ) },
Signature · 1 line
2.11.0.390.gc69c2f50cf-goog
Re: [PATCH linux 1/3] mtd: spi-nor: add SPI_NOR_DUAL_READ to mx66l51235l
Quoted 1 line · Robert Lippert
Signed-off-by Robert Lippert <[email protected]>
This would be good to upstream I think. As I need to send a little patchset for the label support in mainline Linux, I can include it if you want.
C.
Quoted 19 lines
---
drivers/mtd/spi-nor/spi-nor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/mtd/spi-nor/spi-nor.c +1
-1@@ -857,7 +857,7 @@ static const struct flash_info spi_nor_ids[] = { { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, 0) }, { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, - { "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_QUAD_READ) }, + { "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "mx66l1g45g", INFO(0xc2201b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_QUAD_READ) }, { "mx66l1g55g", INFO(0xc2261b, 0, 64 * 1024, 2048, SPI_NOR_QUAD_READ) },
Re: [PATCH linux 1/3] mtd: spi-nor: add SPI_NOR_DUAL_READ to mx66l51235l
Quoted 8 lines · Cédric Le Goater
Quoted 1 line · Robert Lippert
Signed-off-by Robert Lippert <[email protected]>Reviewed-by Cédric Le Goater <[email protected]>This would be good to upstream I think. As I need to send a little patchset for the label support in mainline Linux, I can include it if you want.
Sounds good to me, no issue with including this change in your patchset.
Quoted 23 lines
C.
Quoted 19 lines
---
drivers/mtd/spi-nor/spi-nor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/mtd/spi-nor/spi-nor.c +1
-1@@ -857,7 +857,7 @@ static const struct flash_info spi_nor_ids[] = { { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) }, { "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512, 0) }, { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) }, - { "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_QUAD_READ) }, + { "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, { "mx66l1g45g", INFO(0xc2201b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_QUAD_READ) }, { "mx66l1g55g", INFO(0xc2261b, 0, 64 * 1024, 2048, SPI_NOR_QUAD_READ) },
PATCH 2/3 mtd: spi-nor: aspeed: add support for SPI dual IO read mode
In reply to Robert LippertImplements support for the dual IO read mode on aspeed SMC/FMC controllers which uses both MISO and MOSI lines for data during a read to double the read bandwidth.
---
drivers/mtd/spi-nor/aspeed-smc.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)
drivers/mtd/spi-nor/aspeed-smc.c +22 -6
@@ -540,6 +540,9 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, {struct aspeed_smc_chip *chip = nor->priv;int ret; +u32 ctl; +int i; +u8 dummy =0 ;mutex_lock (&chip->controller->mutex);
@@ -557,6 +560,18 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,aspeed_smc_start_user (nor);aspeed_smc_send_cmd_addr (nor, nor->read_opcode, from); + +/* Send dummy bytes */ +for (i =0 ; i < (chip->nor.read_dummy /8 ); ++i) +aspeed_smc_write_to_ahb (chip->base, &dummy,1 ); + +if (chip->nor.flash_read ==SPI_NOR_DUAL ) { +/* Switch to dual I/O mode for data cycle */ + ctl =readl (chip->ctl) & ~CONTROL_SPI_IO_MODE_MASK ; + ctl |=CONTROL_SPI_IO_DUAL_DATA ; +writel (ctl, chip->ctl); + } +aspeed_smc_read_from_ahb (read_buf, chip->base, len);aspeed_smc_stop_user (nor);
@@ -868,6 +883,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) cmd =CONTROL_SPI_COMMAND_MODE_NORMAL ;break ;case SPI_NOR_FAST : +case SPI_NOR_DUAL : cmd =CONTROL_SPI_COMMAND_MODE_FREAD ;break ;default :
@@ -876,9 +892,13 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) } chip->ctl_val[smc_read] |= cmd | +spi_control_fill_opcode (chip->nor.read_opcode) |CONTROL_SPI_IO_DUMMY_CYCLES_SET (chip->nor.read_dummy /8 ); -dev_dbg (controller->dev,"base control register: %08x\n" , +if (chip->nor.flash_read ==SPI_NOR_DUAL ) + chip->ctl_val[smc_read] |=CONTROL_SPI_IO_DUAL_DATA ; + +dev_info (controller->dev,"read control register: %08x\n" , chip->ctl_val[smc_read]);return 0 ; }
@@ -980,11 +1000,7 @@ static int aspeed_smc_probe(struct platform_device *pdev)if (err)continue ; -/* -* XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach -* when board support is present as determined by of property. -*/ - err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_NORMAL ); + err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_DUAL );if (err)continue ;
Signature · 1 line
2.11.0.390.gc69c2f50cf-goog
Re: [PATCH linux 2/3] mtd: spi-nor: aspeed: add support for SPI dual IO read mode
Hello Robert,
Quoted 15 lines · Robert Lippert
Implements support for the dual IO read mode on aspeed SMC/FMC controllers which uses both MISO and MOSI lines for data during a read to double the read bandwidth.
Signed-off-by Robert Lippert <[email protected]>---
drivers/mtd/spi-nor/aspeed-smc.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)drivers/mtd/spi-nor/aspeed-smc.c +0
-0
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 545
Quoted 6 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0@@ -540,6 +540,9 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, {struct aspeed_smc_chip *chip = nor->priv;int ret; +u32 ctl; +int i; +u8 dummy =0 ;
OxFF would be a better value.
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 565
Quoted 9 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0mutex_lock (&chip->controller->mutex);@@ -557,6 +560,18 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,aspeed_smc_start_user (nor);aspeed_smc_send_cmd_addr (nor, nor->read_opcode, from); + +/* Send dummy bytes */ +for (i =0 ; i < (chip->nor.read_dummy /8 ); ++i)
the parentheses are not needed I think.
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 573
Quoted 8 lines
drivers/mtd/spi-nor/aspeed-smc.c +8
-0+aspeed_smc_write_to_ahb (chip->base, &dummy,1 ); + +if (chip->nor.flash_read ==SPI_NOR_DUAL ) { +/* Switch to dual I/O mode for data cycle */ + ctl =readl (chip->ctl) & ~CONTROL_SPI_IO_MODE_MASK ; + ctl |=CONTROL_SPI_IO_DUAL_DATA ; +writel (ctl, chip->ctl); + }
So why can not we set the controller in aspeed_smc_chip_setup_finish() once and for all ?
Quoted 13 lines
drivers/mtd/spi-nor/aspeed-smc.c +2
-0+aspeed_smc_read_from_ahb (read_buf, chip->base, len);aspeed_smc_stop_user (nor);@@ -868,6 +883,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) cmd =CONTROL_SPI_COMMAND_MODE_NORMAL ;break ;case SPI_NOR_FAST : +case SPI_NOR_DUAL : cmd =CONTROL_SPI_COMMAND_MODE_FREAD ;break ;default :
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 899
Quoted 9 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-1@@ -876,9 +892,13 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) } chip->ctl_val[smc_read] |= cmd | +spi_control_fill_opcode (chip->nor.read_opcode) |CONTROL_SPI_IO_DUMMY_CYCLES_SET (chip->nor.read_dummy /8 ); - dev_dbg(controller->dev, "base control register: %08x\n", +if (chip->nor.flash_read ==SPI_NOR_DUAL ) + chip->ctl_val[smc_read] |=CONTROL_SPI_IO_DUAL_DATA ;
may be we can move that part in the switch setting above ?
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 902
Quoted 2 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-0+dev_info (controller->dev,"read control register: %08x\n" , chip->ctl_val[smc_read]);
may be keep dev_dbg, but the change of 'base' by 'read' is ok.
Thanks,
C.
Quoted 16 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-5return 0 ; }@@ -980,11 +1000,7 @@ static int aspeed_smc_probe(struct platform_device *pdev)if (err)continue ; -/* -* XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach -* when board support is present as determined by of property. -*/ - err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_NORMAL ); + err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_DUAL );if (err)continue ;
Re: [PATCH linux 2/3] mtd: spi-nor: aspeed: add support for SPI dual IO read mode
Quoted 14 lines · Cédric Le Goater
Hello Robert,
Quoted 10 lines · Robert Lippert
Implements support for the dual IO read mode on aspeed SMC/FMC controllers which uses both MISO and MOSI lines for data during a read to double the read bandwidth.
Signed-off-by Robert Lippert <[email protected]>---
drivers/mtd/spi-nor/aspeed-smc.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)
Quoted 13 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 545
Quoted 11 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0@@ -540,6 +540,9 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, {struct aspeed_smc_chip *chip = nor->priv;int ret; +u32 ctl; +int i; +u8 dummy =0 ;OxFF would be a better value.
OK.
Quoted 5 lines
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +0
-0mutex_lock (&chip->controller->mutex);@@ -557,6 +560,18 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,
Quoted 7 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 565
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0aspeed_smc_start_user (nor);aspeed_smc_send_cmd_addr (nor, nor->read_opcode, from); + +/* Send dummy bytes */ +for (i =0 ; i < (chip->nor.read_dummy /8 ); ++i)the parentheses are not needed I think.
Yes not needed, removed.
Quoted 10 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 573
Quoted 9 lines
drivers/mtd/spi-nor/aspeed-smc.c +8
-0+aspeed_smc_write_to_ahb (chip->base, &dummy,1 ); + +if (chip->nor.flash_read ==SPI_NOR_DUAL ) { +/* Switch to dual I/O mode for data cycle */ + ctl =readl (chip->ctl) & ~CONTROL_SPI_IO_MODE_MASK ; + ctl |=CONTROL_SPI_IO_DUAL_DATA ; +writel (ctl, chip->ctl); + }
Quoted 2 lines
So why can not we set the controller in
aspeed_smc_chip_setup_finish()once and for all ?
It needs to make sure to only effect the data part of the operation (the command+addr writes to the device should not be dual-IO mode), and also needs to not interfere with the aspeed_smc_read_reg read calls in which the data phase cannot be dual IO mode.
Quoted 20 lines
Quoted 20 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0+aspeed_smc_read_from_ahb (read_buf, chip->base, len);aspeed_smc_stop_user (nor);@@ -868,6 +883,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) cmd =CONTROL_SPI_COMMAND_MODE_NORMAL ;break ;case SPI_NOR_FAST : +case SPI_NOR_DUAL : cmd =CONTROL_SPI_COMMAND_MODE_FREAD ;break ;default :@@ -876,9 +892,13 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) } chip->ctl_val[smc_read] |= cmd | +spi_control_fill_opcode (chip->nor.read_opcode) |CONTROL_SPI_IO_DUMMY_CYCLES_SET (chip->nor.read_dummy /8 );
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 899
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +2
-1- dev_dbg(controller->dev, "base control register: %08x\n", +if (chip->nor.flash_read ==SPI_NOR_DUAL ) + chip->ctl_val[smc_read] |=CONTROL_SPI_IO_DUAL_DATA ;may be we can move that part in the switch setting above ?
Done.
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 902
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-0+dev_info (controller->dev,"read control register: %08x\n" , chip->ctl_val[smc_read]);may be keep
dev_dbg, but the change of 'base' by 'read' is ok.
OK.
Quoted 22 lines
Thanks,
C.
Quoted 16 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-5return 0 ; }@@ -980,11 +1000,7 @@ static int aspeed_smc_probe(struct platform_device *pdev)if (err)continue ; -/* -* XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach -* when board support is present as determined by of property. -*/ - err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_NORMAL ); + err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_DUAL );if (err)continue ;
Re: [PATCH linux 2/3] mtd: spi-nor: aspeed: add support for SPI dual IO read mode
Quoted 56 lines · Rob Lippert
Quoted 14 lines · Cédric Le Goater
Hello Robert,
Quoted 10 lines · Robert Lippert
Implements support for the dual IO read mode on aspeed SMC/FMC controllers which uses both MISO and MOSI lines for data during a read to double the read bandwidth.
Signed-off-by Robert Lippert <[email protected]>---
drivers/mtd/spi-nor/aspeed-smc.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)Quoted 13 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 545
Quoted 11 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0@@ -540,6 +540,9 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, {struct aspeed_smc_chip *chip = nor->priv;int ret; +u32 ctl; +int i; +u8 dummy =0 ;OxFF would be a better value.
OK.
Quoted 5 lines
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +0
-0mutex_lock (&chip->controller->mutex);@@ -557,6 +560,18 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,Quoted 7 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 565
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0aspeed_smc_start_user (nor);aspeed_smc_send_cmd_addr (nor, nor->read_opcode, from); + +/* Send dummy bytes */ +for (i =0 ; i < (chip->nor.read_dummy /8 ); ++i)the parentheses are not needed I think.
Yes not needed, removed.
Quoted 10 lines
Quoted 9 lines
drivers/mtd/spi-nor/aspeed-smc.c +8
-0+aspeed_smc_write_to_ahb (chip->base, &dummy,1 ); + +if (chip->nor.flash_read ==SPI_NOR_DUAL ) { +/* Switch to dual I/O mode for data cycle */ + ctl =readl (chip->ctl) & ~CONTROL_SPI_IO_MODE_MASK ; + ctl |=CONTROL_SPI_IO_DUAL_DATA ; +writel (ctl, chip->ctl); + }
Quoted 7 lines
Quoted 2 lines
So why can not we set the controller in
aspeed_smc_chip_setup_finish()once and for all ?It needs to make sure to only effect the data part of the operation (the command+addr writes to the device should not be dual-IO mode), and also needs to not interfere with the
aspeed_smc_read_regread calls in which the data phase cannot be dual IO mode.
ok. it makes sense. Is it much faster ?
Thanks,
C.
Quoted 58 lines
Quoted 20 lines
Quoted 20 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0+aspeed_smc_read_from_ahb (read_buf, chip->base, len);aspeed_smc_stop_user (nor);@@ -868,6 +883,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) cmd =CONTROL_SPI_COMMAND_MODE_NORMAL ;break ;case SPI_NOR_FAST : +case SPI_NOR_DUAL : cmd =CONTROL_SPI_COMMAND_MODE_FREAD ;break ;default :@@ -876,9 +892,13 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) } chip->ctl_val[smc_read] |= cmd | +spi_control_fill_opcode (chip->nor.read_opcode) |CONTROL_SPI_IO_DUMMY_CYCLES_SET (chip->nor.read_dummy /8 );Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 899
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +2
-1- dev_dbg(controller->dev, "base control register: %08x\n", +if (chip->nor.flash_read ==SPI_NOR_DUAL ) + chip->ctl_val[smc_read] |=CONTROL_SPI_IO_DUAL_DATA ;may be we can move that part in the switch setting above ?
Done.
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 902
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-0+dev_info (controller->dev,"read control register: %08x\n" , chip->ctl_val[smc_read]);may be keep
dev_dbg, but the change of 'base' by 'read' is ok.OK.
Quoted 22 lines
Thanks,
C.
Quoted 16 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-5return 0 ; }@@ -980,11 +1000,7 @@ static int aspeed_smc_probe(struct platform_device *pdev)if (err)continue ; -/* -* XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach -* when board support is present as determined by of property. -*/ - err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_NORMAL ); + err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_DUAL );if (err)continue ;
Re: [PATCH linux 2/3] mtd: spi-nor: aspeed: add support for SPI dual IO read mode
Quoted 60 lines · Cédric Le Goater
Quoted 59 lines · Rob Lippert
Quoted 14 lines · Cédric Le Goater
Hello Robert,
Quoted 10 lines · Robert Lippert
Implements support for the dual IO read mode on aspeed SMC/FMC controllers which uses both MISO and MOSI lines for data during a read to double the read bandwidth.
Signed-off-by Robert Lippert <[email protected]>---
drivers/mtd/spi-nor/aspeed-smc.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)Quoted 13 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 545
Quoted 11 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0@@ -540,6 +540,9 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, {struct aspeed_smc_chip *chip = nor->priv;int ret; +u32 ctl; +int i; +u8 dummy =0 ;OxFF would be a better value.
OK.
Quoted 5 lines
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +0
-0mutex_lock (&chip->controller->mutex);@@ -557,6 +560,18 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,Quoted 7 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 565
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0aspeed_smc_start_user (nor);aspeed_smc_send_cmd_addr (nor, nor->read_opcode, from); + +/* Send dummy bytes */ +for (i =0 ; i < (chip->nor.read_dummy /8 ); ++i)the parentheses are not needed I think.
Yes not needed, removed.
Quoted 12 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 573
Quoted 9 lines
drivers/mtd/spi-nor/aspeed-smc.c +8
-0+aspeed_smc_write_to_ahb (chip->base, &dummy,1 ); + +if (chip->nor.flash_read ==SPI_NOR_DUAL ) { +/* Switch to dual I/O mode for data cycle */ + ctl =readl (chip->ctl) & ~CONTROL_SPI_IO_MODE_MASK ; + ctl |=CONTROL_SPI_IO_DUAL_DATA ; +writel (ctl, chip->ctl); + }So why can not we set the controller in
aspeed_smc_chip_setup_finish()once and for all ?
Quoted 6 lines
Quoted 4 lines
It needs to make sure to only effect the data part of the operation (the command+addr writes to the device should not be dual-IO mode), and also needs to not interfere with the
aspeed_smc_read_regread calls in which the data phase cannot be dual IO mode.ok. it makes sense. Is it much faster ?
Yea it is about twice as much bandwidth, as I mentioned in the cover letter it drops the time to dump an entire 64MB flash from 12s to 7s.
With a companion change to u-boot to double the SPI frequency to 104Mhz that time drops to 3s and speeds up the entire bmc boot process (including uboot) by about 15s.
Thanks,
-Rob
Quoted 64 lines
Thanks,
C.
Quoted 58 lines
Quoted 20 lines
Quoted 20 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0+aspeed_smc_read_from_ahb (read_buf, chip->base, len);aspeed_smc_stop_user (nor);@@ -868,6 +883,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) cmd =CONTROL_SPI_COMMAND_MODE_NORMAL ;break ;case SPI_NOR_FAST : +case SPI_NOR_DUAL : cmd =CONTROL_SPI_COMMAND_MODE_FREAD ;break ;default :@@ -876,9 +892,13 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) } chip->ctl_val[smc_read] |= cmd | +spi_control_fill_opcode (chip->nor.read_opcode) |CONTROL_SPI_IO_DUMMY_CYCLES_SET (chip->nor.read_dummy /8 );Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 899
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +2
-1- dev_dbg(controller->dev, "base control register: %08x\n", +if (chip->nor.flash_read ==SPI_NOR_DUAL ) + chip->ctl_val[smc_read] |=CONTROL_SPI_IO_DUAL_DATA ;may be we can move that part in the switch setting above ?
Done.
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 902
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-0+dev_info (controller->dev,"read control register: %08x\n" , chip->ctl_val[smc_read]);may be keep
dev_dbg, but the change of 'base' by 'read' is ok.OK.
Quoted 22 lines
Thanks,
C.
Quoted 16 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-5return 0 ; }@@ -980,11 +1000,7 @@ static int aspeed_smc_probe(struct platform_device *pdev)if (err)continue ; -/* -* XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach -* when board support is present as determined by of property. -*/ - err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_NORMAL ); + err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_DUAL );if (err)continue ;
Re: [PATCH linux 2/3] mtd: spi-nor: aspeed: add support for SPI dual IO read mode
Quoted 66 lines · Rob Lippert
Quoted 65 lines · Cédric Le Goater
Quoted 63 lines · Rob Lippert
Quoted 14 lines · Cédric Le Goater
Hello Robert,
Quoted 10 lines · Robert Lippert
Implements support for the dual IO read mode on aspeed SMC/FMC controllers which uses both MISO and MOSI lines for data during a read to double the read bandwidth.
Signed-off-by Robert Lippert <[email protected]>---
drivers/mtd/spi-nor/aspeed-smc.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)Quoted 13 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 545
Quoted 11 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0@@ -540,6 +540,9 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, {struct aspeed_smc_chip *chip = nor->priv;int ret; +u32 ctl; +int i; +u8 dummy =0 ;OxFF would be a better value.
OK.
Quoted 5 lines
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +0
-0mutex_lock (&chip->controller->mutex);@@ -557,6 +560,18 @@ static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len,Quoted 7 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 565
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0aspeed_smc_start_user (nor);aspeed_smc_send_cmd_addr (nor, nor->read_opcode, from); + +/* Send dummy bytes */ +for (i =0 ; i < (chip->nor.read_dummy /8 ); ++i)the parentheses are not needed I think.
Yes not needed, removed.
Quoted 10 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_read_user(struct spi_nor *nor, loff_t from, size_t len, · line 573
Quoted 9 lines
drivers/mtd/spi-nor/aspeed-smc.c +8
-0+aspeed_smc_write_to_ahb (chip->base, &dummy,1 ); + +if (chip->nor.flash_read ==SPI_NOR_DUAL ) { +/* Switch to dual I/O mode for data cycle */ + ctl =readl (chip->ctl) & ~CONTROL_SPI_IO_MODE_MASK ; + ctl |=CONTROL_SPI_IO_DUAL_DATA ; +writel (ctl, chip->ctl); + }Quoted 2 lines
So why can not we set the controller in
aspeed_smc_chip_setup_finish()once and for all ?It needs to make sure to only effect the data part of the operation (the command+addr writes to the device should not be dual-IO mode), and also needs to not interfere with the
aspeed_smc_read_regread calls in which the data phase cannot be dual IO mode.
Quoted 4 lines
Quoted 1 line
ok. it makes sense. Is it much faster ?
Yea it is about twice as much bandwidth, as I mentioned in the cover letter it drops the time to dump an entire 64MB flash from 12s to 7s.
Good. Sorry for asking, I scanned the cover a bit quickly it seems.
Quoted 3 lines
With a companion change to u-boot to double the SPI frequency to 104Mhz that time drops to 3s and speeds up the entire bmc boot process (including uboot) by about 15s.
yes we need to add that to the driver also. I have a few other patches improving the timing settings but I have been focusing on upstreaming first.
Thanks,
C.
Quoted 67 lines
Thanks,
-RobQuoted 64 lines
Thanks,
C.
Quoted 58 lines
Quoted 20 lines
Quoted 20 lines
drivers/mtd/spi-nor/aspeed-smc.c +3
-0+aspeed_smc_read_from_ahb (read_buf, chip->base, len);aspeed_smc_stop_user (nor);@@ -868,6 +883,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) cmd =CONTROL_SPI_COMMAND_MODE_NORMAL ;break ;case SPI_NOR_FAST : +case SPI_NOR_DUAL : cmd =CONTROL_SPI_COMMAND_MODE_FREAD ;break ;default :@@ -876,9 +892,13 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) } chip->ctl_val[smc_read] |= cmd | +spi_control_fill_opcode (chip->nor.read_opcode) |CONTROL_SPI_IO_DUMMY_CYCLES_SET (chip->nor.read_dummy /8 );Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 899
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +2
-1- dev_dbg(controller->dev, "base control register: %08x\n", +if (chip->nor.flash_read ==SPI_NOR_DUAL ) + chip->ctl_val[smc_read] |=CONTROL_SPI_IO_DUAL_DATA ;may be we can move that part in the switch setting above ?
Done.
Quoted 5 lines
drivers/mtd/spi-nor/aspeed-smc.c static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) · line 902
Quoted 3 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-0+dev_info (controller->dev,"read control register: %08x\n" , chip->ctl_val[smc_read]);may be keep
dev_dbg, but the change of 'base' by 'read' is ok.OK.
Quoted 22 lines
Thanks,
C.
Quoted 16 lines
drivers/mtd/spi-nor/aspeed-smc.c +1
-5return 0 ; }@@ -980,11 +1000,7 @@ static int aspeed_smc_probe(struct platform_device *pdev)if (err)continue ; -/* -* XXX Add support for SPI_NOR_QUAD and SPI_NOR_DUAL attach -* when board support is present as determined by of property. -*/ - err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_NORMAL ); + err =spi_nor_scan (&chip->nor,NULL ,SPI_NOR_DUAL );if (err)continue ;
PATCH 3/3 mtd: spi-nor: aspeed: fix DMA access on AST2500
In reply to Robert LippertAST2500 has additional bits in the dma_addr field. Its easier to just write the full address into the register as the hardware will handle the masking properly.
---
drivers/mtd/spi-nor/aspeed-smc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/spi-nor/aspeed-smc.c +1 -1
@@ -349,7 +349,7 @@ static int aspeed_smc_dma_wait(struct aspeed_smc_chip *chip) }#define DMA_LENGTH (x) (((x) - 4) & ~0xFE000003) -#define DMA_ADDR (x) ((x) & ~0xE0000003) +#define DMA_ADDR (x) ((x) & ~0x00000003)static inline void aspeed_smc_chip_configure (struct aspeed_smc_chip *chip,u32 ctl)
Signature · 1 line
2.11.0.390.gc69c2f50cf-goog
Re: [PATCH linux 3/3] mtd: spi-nor: aspeed: fix DMA access on AST2500
Quoted 3 lines · Robert Lippert
AST2500 has additional bits in the
dma_addrfield. Its easier to just write the full address into the register as the hardware will handle the masking properly.
Yes definitely,
Not for mainline yet though.
C.
Quoted 20 lines
Signed-off-by Robert Lippert <[email protected]>---
drivers/mtd/spi-nor/aspeed-smc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/mtd/spi-nor/aspeed-smc.c +1
-1@@ -349,7 +349,7 @@ static int aspeed_smc_dma_wait(struct aspeed_smc_chip *chip) }#define DMA_LENGTH (x) (((x) - 4) & ~0xFE000003) -#define DMA_ADDR (x) ((x) & ~0xE0000003) +#define DMA_ADDR (x) ((x) & ~0x00000003)static inline void aspeed_smc_chip_configure (struct aspeed_smc_chip *chip,u32 ctl)