* [PATCH RFC] mtdpart: don't force alignment to eraseblock if flags have MTD_NO_ERASE
@ 2017-07-14 10:20 Uwe Kleine-König
2017-07-14 11:17 ` Boris Brezillon
0 siblings, 1 reply; 2+ messages in thread
From: Uwe Kleine-König @ 2017-07-14 10:20 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen
Cc: kernel, linux-mtd
Some mtd devices don't need to be erased before writing to them. For
these it doesn't make sense to force partition alignment to erase
blocks.
This patch allows partitioning an Everspin mr25h256 that has
erasesize=32768, size=32768 and writesize=1.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,
this is my followup suggestion for
From: Bastian Stender <bst@pengutronix.de>
Subject: Re: [PATCH 1/2] mtd: spi-nor: make n_sectors in flash_info 32 bit wide
Message-Id: <afb06b83-e97b-48db-9875-9f76b365a5f4@pengutronix.de>
It's IMHO ugly because the two bodys of the newly introduced if look very
similar, but I think there is not much we can do about this.
To ease your reviews: The else body is exactly what we had before.
I'm a bit unsure about the check
slave->mtd.erasesize >= slave->mtd.writesize
because if that is false, the old code is not only inconvenient by not allowing
partitions, but also wrong. So probably the check can safely be dropped?
Otherwise the logic should be:
if (MTD_NO_ERASE):
alignto = writesize
else:
alignto = max(writesize, erasesize)
Best regards
Uwe
drivers/mtd/mtdpart.c | 47 +++++++++++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index ea5e5307f667..956c8f0ce2dd 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -567,20 +567,39 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
slave->mtd.erasesize = master->erasesize;
}
- if ((slave->mtd.flags & MTD_WRITEABLE) &&
- mtd_mod_by_eb(slave->offset, &slave->mtd)) {
- /* Doesn't start on a boundary of major erase size */
- /* FIXME: Let it be writable if it is on a boundary of
- * _minor_ erase size though */
- slave->mtd.flags &= ~MTD_WRITEABLE;
- printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase block boundary -- force read-only\n",
- part->name);
- }
- if ((slave->mtd.flags & MTD_WRITEABLE) &&
- mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
- slave->mtd.flags &= ~MTD_WRITEABLE;
- printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase block -- force read-only\n",
- part->name);
+ if (slave->mtd.flags & MTD_NO_ERASE &&
+ slave->mtd.erasesize >= slave->mtd.writesize) {
+ /* If we don't need to erase, then align to writesize */
+ if ((slave->mtd.flags & MTD_WRITEABLE) &&
+ mtd_mod_by_ws(slave->offset, &slave->mtd)) {
+ /* Doesn't start on a boundary of page */
+ /* FIXME: Can a minor erase block be smaller than a page? */
+ slave->mtd.flags &= ~MTD_WRITEABLE;
+ printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on a page boundary -- force read-only\n",
+ part->name);
+ }
+ if ((slave->mtd.flags & MTD_WRITEABLE) &&
+ mtd_mod_by_ws(slave->mtd.size, &slave->mtd)) {
+ slave->mtd.flags &= ~MTD_WRITEABLE;
+ printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on a page boundary -- force read-only\n",
+ part->name);
+ }
+ } else {
+ if ((slave->mtd.flags & MTD_WRITEABLE) &&
+ mtd_mod_by_eb(slave->offset, &slave->mtd)) {
+ /* Doesn't start on a boundary of major erase size */
+ /* FIXME: Let it be writable if it is on a boundary of
+ * _minor_ erase size though */
+ slave->mtd.flags &= ~MTD_WRITEABLE;
+ printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase block boundary -- force read-only\n",
+ part->name);
+ }
+ if ((slave->mtd.flags & MTD_WRITEABLE) &&
+ mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
+ slave->mtd.flags &= ~MTD_WRITEABLE;
+ printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase block -- force read-only\n",
+ part->name);
+ }
}
mtd_set_ooblayout(&slave->mtd, &part_ooblayout_ops);
--
2.11.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH RFC] mtdpart: don't force alignment to eraseblock if flags have MTD_NO_ERASE
2017-07-14 10:20 [PATCH RFC] mtdpart: don't force alignment to eraseblock if flags have MTD_NO_ERASE Uwe Kleine-König
@ 2017-07-14 11:17 ` Boris Brezillon
0 siblings, 0 replies; 2+ messages in thread
From: Boris Brezillon @ 2017-07-14 11:17 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Cyrille Pitchen, kernel, linux-mtd
Le Fri, 14 Jul 2017 12:20:27 +0200,
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> a écrit :
> Some mtd devices don't need to be erased before writing to them. For
> these it doesn't make sense to force partition alignment to erase
> blocks.
>
> This patch allows partitioning an Everspin mr25h256 that has
> erasesize=32768, size=32768 and writesize=1.
I might be wrong but it seems this patch is more or less addressing the
problem fixed by [1].
Can you try linux-next (or l2-mtd/master)?
[1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/mtd/mtdpart.c?h=next-20170714&id=1eeef2d7483a7e3f8d2dd2a5b9939b3b814dc549
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> this is my followup suggestion for
>
> From: Bastian Stender <bst@pengutronix.de>
> Subject: Re: [PATCH 1/2] mtd: spi-nor: make n_sectors in flash_info 32 bit wide
> Message-Id: <afb06b83-e97b-48db-9875-9f76b365a5f4@pengutronix.de>
>
>
>
> It's IMHO ugly because the two bodys of the newly introduced if look very
> similar, but I think there is not much we can do about this.
>
> To ease your reviews: The else body is exactly what we had before.
>
> I'm a bit unsure about the check
>
> slave->mtd.erasesize >= slave->mtd.writesize
>
> because if that is false, the old code is not only inconvenient by not allowing
> partitions, but also wrong. So probably the check can safely be dropped?
>
> Otherwise the logic should be:
>
> if (MTD_NO_ERASE):
> alignto = writesize
> else:
> alignto = max(writesize, erasesize)
>
> Best regards
> Uwe
>
> drivers/mtd/mtdpart.c | 47 +++++++++++++++++++++++++++++++++--------------
> 1 file changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index ea5e5307f667..956c8f0ce2dd 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -567,20 +567,39 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
> slave->mtd.erasesize = master->erasesize;
> }
>
> - if ((slave->mtd.flags & MTD_WRITEABLE) &&
> - mtd_mod_by_eb(slave->offset, &slave->mtd)) {
> - /* Doesn't start on a boundary of major erase size */
> - /* FIXME: Let it be writable if it is on a boundary of
> - * _minor_ erase size though */
> - slave->mtd.flags &= ~MTD_WRITEABLE;
> - printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase block boundary -- force read-only\n",
> - part->name);
> - }
> - if ((slave->mtd.flags & MTD_WRITEABLE) &&
> - mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
> - slave->mtd.flags &= ~MTD_WRITEABLE;
> - printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase block -- force read-only\n",
> - part->name);
> + if (slave->mtd.flags & MTD_NO_ERASE &&
> + slave->mtd.erasesize >= slave->mtd.writesize) {
> + /* If we don't need to erase, then align to writesize */
> + if ((slave->mtd.flags & MTD_WRITEABLE) &&
> + mtd_mod_by_ws(slave->offset, &slave->mtd)) {
> + /* Doesn't start on a boundary of page */
> + /* FIXME: Can a minor erase block be smaller than a page? */
> + slave->mtd.flags &= ~MTD_WRITEABLE;
> + printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on a page boundary -- force read-only\n",
> + part->name);
> + }
> + if ((slave->mtd.flags & MTD_WRITEABLE) &&
> + mtd_mod_by_ws(slave->mtd.size, &slave->mtd)) {
> + slave->mtd.flags &= ~MTD_WRITEABLE;
> + printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on a page boundary -- force read-only\n",
> + part->name);
> + }
> + } else {
> + if ((slave->mtd.flags & MTD_WRITEABLE) &&
> + mtd_mod_by_eb(slave->offset, &slave->mtd)) {
> + /* Doesn't start on a boundary of major erase size */
> + /* FIXME: Let it be writable if it is on a boundary of
> + * _minor_ erase size though */
> + slave->mtd.flags &= ~MTD_WRITEABLE;
> + printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase block boundary -- force read-only\n",
> + part->name);
> + }
> + if ((slave->mtd.flags & MTD_WRITEABLE) &&
> + mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
> + slave->mtd.flags &= ~MTD_WRITEABLE;
> + printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase block -- force read-only\n",
> + part->name);
> + }
> }
>
> mtd_set_ooblayout(&slave->mtd, &part_ooblayout_ops);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-07-14 11:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-14 10:20 [PATCH RFC] mtdpart: don't force alignment to eraseblock if flags have MTD_NO_ERASE Uwe Kleine-König
2017-07-14 11:17 ` Boris Brezillon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.