linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mtd: spi-nor: add Kconfig option to disable 4K sectors
@ 2014-08-16 20:17 Rafał Miłecki
  2014-08-16 21:06 ` Kevin Cernekee
  2014-08-17  9:27 ` [PATCH V2] " Rafał Miłecki
  0 siblings, 2 replies; 6+ messages in thread
From: Rafał Miłecki @ 2014-08-16 20:17 UTC (permalink / raw)
  To: linux-mtd, David Woodhouse, Artem Bityutskiy, Brian Norris
  Cc: Rafał Miłecki

Current situation with 4K sectors is quite messy. First of all, some
MTD "users" don't work with such small size. An example may be UBIFS
which requires 15 KiB erase blocks as a minimum. In theory spi-nor
should provide multiple erase regions and MTD "users" should use the
one they need. Unforunately that is not implemented.

In the result our flashes database in spi-nor is hackish. For some
flashes we pretend they don't support 4K sectors just because some
distribution users UBIFS on it. This ofc leads to conflicts, like
Samsung using w25q128 with 4K sectors vs. OpenWrt requiring it to
pretend it's 64 KiB blocks only.

My idea (plan?) for fixing this situation:
1) Use real hw info (this requires a way for disabling 4K for now)
2) Provide detailed info about erase regions
3) Make UBIFS work with devices that support 4K sectors

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/spi-nor/Kconfig   | 11 +++++++++++
 drivers/mtd/spi-nor/spi-nor.c |  5 ++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index f8acfa4..601879e 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -7,6 +7,17 @@ menuconfig MTD_SPI_NOR
 
 if MTD_SPI_NOR
 
+config MTD_SPI_NOR_USE_4K_SECTORS
+	bool "Use small 4096 B erase sectors"
+	default y
+	help
+	  Many flash memories support erasing small (4096 B) sectors. Depending
+	  on the usage this feature may provide performance gain in comparison
+	  to erasing whole blocks (32/64 KiB).
+
+	  Please note that some tools/drivers/filesystems may not work with
+	  4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
+
 config SPI_FSL_QUADSPI
 	tristate "Freescale Quad SPI controller"
 	depends on ARCH_MXC
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ba3f61e..ae16aa2 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -987,6 +987,7 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	    nor->wait_till_ready == spi_nor_wait_till_ready)
 		nor->wait_till_ready = spi_nor_wait_till_fsr_ready;
 
+#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
 	/* prefer "small sector" erase if possible */
 	if (info->flags & SECT_4K) {
 		nor->erase_opcode = SPINOR_OP_BE_4K;
@@ -994,7 +995,9 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	} else if (info->flags & SECT_4K_PMC) {
 		nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
 		mtd->erasesize = 4096;
-	} else {
+	} else
+#endif
+	{
 		nor->erase_opcode = SPINOR_OP_SE;
 		mtd->erasesize = info->sector_size;
 	}
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: spi-nor: add Kconfig option to disable 4K sectors
  2014-08-16 20:17 [PATCH] mtd: spi-nor: add Kconfig option to disable 4K sectors Rafał Miłecki
@ 2014-08-16 21:06 ` Kevin Cernekee
  2014-08-17  9:27 ` [PATCH V2] " Rafał Miłecki
  1 sibling, 0 replies; 6+ messages in thread
From: Kevin Cernekee @ 2014-08-16 21:06 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Brian Norris, linux-mtd@lists.infradead.org, David Woodhouse,
	Artem Bityutskiy

On Sat, Aug 16, 2014 at 1:17 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> +config MTD_SPI_NOR_USE_4K_SECTORS
> +       bool "Use small 4096 B erase sectors"
> +       default y
> +       help
> +         Many flash memories support erasing small (4096 B) sectors. Depending
> +         on the usage this feature may provide performance gain in comparison
> +         to erasing whole blocks (32/64 KiB).

I would also note that on some SPI-NOR devices, erasing 16x 4K sectors
is significantly slower than erasing 1x 64K block.  There is also the
question of balancing filesystem overhead versus internal
fragmentation.

It probably does make sense to let the user weigh the tradeoffs and
make the right decision for his application instead of hardcoding the
policy into the driver.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2] mtd: spi-nor: add Kconfig option to disable 4K sectors
  2014-08-16 20:17 [PATCH] mtd: spi-nor: add Kconfig option to disable 4K sectors Rafał Miłecki
  2014-08-16 21:06 ` Kevin Cernekee
@ 2014-08-17  9:27 ` Rafał Miłecki
  2014-09-28  0:28   ` Brian Norris
  1 sibling, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2014-08-17  9:27 UTC (permalink / raw)
  To: linux-mtd, David Woodhouse, Artem Bityutskiy, Brian Norris
  Cc: Rafał Miłecki

Current situation with 4K sectors is quite messy. First of all, some
MTD "users" don't work with such small size. An example may be UBIFS
which requires 15 KiB erase blocks as a minimum. In theory spi-nor
should provide multiple erase regions and MTD "users" should use the
one they need. Unforunately that is not implemented.

In the result our flashes database in spi-nor is hackish. For some
flashes we pretend they don't support 4K sectors just because some
distribution uses UBIFS on it. This ofc leads to conflicts, like
Samsung using w25q128 with 4K sectors vs. OpenWrt requiring it to
pretend it's 64 KiB blocks only.

My idea (plan?) for fixing this situation:
1) Use real hw info (this requires a way for disabling 4K for now)
2) Provide detailed info about erase regions
3) Make UBIFS work with devices that support 4K sectors

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
V2: Add extra info about performance in Kconfig. Thanks Kevin.
    Fix typo s/users/uses/ in commit message.
---
 drivers/mtd/spi-nor/Kconfig   | 14 ++++++++++++++
 drivers/mtd/spi-nor/spi-nor.c |  5 ++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index f8acfa4..abab223 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -7,6 +7,20 @@ menuconfig MTD_SPI_NOR
 
 if MTD_SPI_NOR
 
+config MTD_SPI_NOR_USE_4K_SECTORS
+	bool "Use small 4096 B erase sectors"
+	default y
+	help
+	  Many flash memories support erasing small (4096 B) sectors. Depending
+	  on the usage this feature may provide performance gain in comparison
+	  to erasing whole blocks (32/64 KiB).
+	  Changing small part of flash content is usually faster with small
+	  sectors. On the other hand erasing should be faster when using 64 KiB
+	  block instead of 16 × 4 KiB sectors.
+
+	  Please note that some tools/drivers/filesystems may not work with
+	  4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
+
 config SPI_FSL_QUADSPI
 	tristate "Freescale Quad SPI controller"
 	depends on ARCH_MXC
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ba3f61e..ae16aa2 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -987,6 +987,7 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	    nor->wait_till_ready == spi_nor_wait_till_ready)
 		nor->wait_till_ready = spi_nor_wait_till_fsr_ready;
 
+#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
 	/* prefer "small sector" erase if possible */
 	if (info->flags & SECT_4K) {
 		nor->erase_opcode = SPINOR_OP_BE_4K;
@@ -994,7 +995,9 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	} else if (info->flags & SECT_4K_PMC) {
 		nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
 		mtd->erasesize = 4096;
-	} else {
+	} else
+#endif
+	{
 		nor->erase_opcode = SPINOR_OP_SE;
 		mtd->erasesize = info->sector_size;
 	}
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] mtd: spi-nor: add Kconfig option to disable 4K sectors
  2014-08-17  9:27 ` [PATCH V2] " Rafał Miłecki
@ 2014-09-28  0:28   ` Brian Norris
  2014-09-28 20:29     ` Rafał Miłecki
  0 siblings, 1 reply; 6+ messages in thread
From: Brian Norris @ 2014-09-28  0:28 UTC (permalink / raw)
  To: Rafał Miłecki; +Cc: linux-mtd, David Woodhouse, Artem Bityutskiy

On Sun, Aug 17, 2014 at 11:27:26AM +0200, Rafał Miłecki wrote:
> Current situation with 4K sectors is quite messy. First of all, some
> MTD "users" don't work with such small size. An example may be UBIFS
> which requires 15 KiB erase blocks as a minimum. In theory spi-nor
> should provide multiple erase regions and MTD "users" should use the
> one they need. Unforunately that is not implemented.
> 
> In the result our flashes database in spi-nor is hackish. For some
> flashes we pretend they don't support 4K sectors just because some
> distribution uses UBIFS on it. This ofc leads to conflicts, like
> Samsung using w25q128 with 4K sectors vs. OpenWrt requiring it to
> pretend it's 64 KiB blocks only.
> 
> My idea (plan?) for fixing this situation:
> 1) Use real hw info (this requires a way for disabling 4K for now)
> 2) Provide detailed info about erase regions
> 3) Make UBIFS work with devices that support 4K sectors
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> V2: Add extra info about performance in Kconfig. Thanks Kevin.
>     Fix typo s/users/uses/ in commit message.

Pushed to l2-mtd.git. Thanks!

Now we should probably try to handle (1), at least, so that the data
table holds fully accurate info about the flash.

Brian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] mtd: spi-nor: add Kconfig option to disable 4K sectors
  2014-09-28  0:28   ` Brian Norris
@ 2014-09-28 20:29     ` Rafał Miłecki
  2014-09-28 21:17       ` Brian Norris
  0 siblings, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2014-09-28 20:29 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd@lists.infradead.org, David Woodhouse, Artem Bityutskiy

On 28 September 2014 02:28, Brian Norris <computersforpeace@gmail.com> wrote:
> On Sun, Aug 17, 2014 at 11:27:26AM +0200, Rafał Miłecki wrote:
>> Current situation with 4K sectors is quite messy. First of all, some
>> MTD "users" don't work with such small size. An example may be UBIFS
>> which requires 15 KiB erase blocks as a minimum. In theory spi-nor
>> should provide multiple erase regions and MTD "users" should use the
>> one they need. Unforunately that is not implemented.
>>
>> In the result our flashes database in spi-nor is hackish. For some
>> flashes we pretend they don't support 4K sectors just because some
>> distribution uses UBIFS on it. This ofc leads to conflicts, like
>> Samsung using w25q128 with 4K sectors vs. OpenWrt requiring it to
>> pretend it's 64 KiB blocks only.
>>
>> My idea (plan?) for fixing this situation:
>> 1) Use real hw info (this requires a way for disabling 4K for now)
>> 2) Provide detailed info about erase regions
>> 3) Make UBIFS work with devices that support 4K sectors
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>> V2: Add extra info about performance in Kconfig. Thanks Kevin.
>>     Fix typo s/users/uses/ in commit message.
>
> Pushed to l2-mtd.git. Thanks!

Ooops, for some reason the whole diff is part of the commit message :|
http://git.infradead.org/l2-mtd.git/commitdiff/f83cd098095c0e2967e15914f3f9521bb122bc0a


> Now we should probably try to handle (1), at least, so that the data
> table holds fully accurate info about the flash.

I agree, will try to collect real info and submit a patch!

-- 
Rafał

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] mtd: spi-nor: add Kconfig option to disable 4K sectors
  2014-09-28 20:29     ` Rafał Miłecki
@ 2014-09-28 21:17       ` Brian Norris
  0 siblings, 0 replies; 6+ messages in thread
From: Brian Norris @ 2014-09-28 21:17 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-mtd@lists.infradead.org, David Woodhouse, Artem Bityutskiy

On Sun, Sep 28, 2014 at 10:29:42PM +0200, Rafał Miłecki wrote:
> On 28 September 2014 02:28, Brian Norris <computersforpeace@gmail.com> wrote:
> > Pushed to l2-mtd.git. Thanks!
> 
> Ooops, for some reason the whole diff is part of the commit message :|
> http://git.infradead.org/l2-mtd.git/commitdiff/f83cd098095c0e2967e15914f3f9521bb122bc0a

Oops indeed. Not sure how I did that. Fixed now, thanks for reporting.

Brian

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-09-28 21:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-16 20:17 [PATCH] mtd: spi-nor: add Kconfig option to disable 4K sectors Rafał Miłecki
2014-08-16 21:06 ` Kevin Cernekee
2014-08-17  9:27 ` [PATCH V2] " Rafał Miłecki
2014-09-28  0:28   ` Brian Norris
2014-09-28 20:29     ` Rafał Miłecki
2014-09-28 21:17       ` Brian Norris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).