[PATCH] mtd: physmap_of: Add "map-indirect" DT property

[PATCH] mtd: physmap_of: Add "map-indirect" DT property

[Stefan Roese]

On some platforms (e.g. MPC5200) a direct 1:1 mapping may cause
problems with JFFS2 usage, as the local bus (LPB) doesn't support
unaligned accesses as implemented in the JFFS2 code via memcpy().
By defining "map-indirect", the flash will not be exposed directly
to the MTD users (e.g. JFFS2) any more.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Stephan Gatzka <Stephan.Gatzka@hbm.com>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Albrecht Dress <albrecht.dress@arcor.de>
---
 Documentation/devicetree/bindings/mtd/mtd-physmap.txt |  7 +++++++
 drivers/mtd/maps/physmap_of.c                         | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
index a63c2bd7..97a7245 100644
--- a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
@@ -16,6 +16,13 @@ file systems on embedded devices.
  - #address-cells, #size-cells : Must be present if the device has
    sub-nodes representing partitions (see below).  In this case
    both #address-cells and #size-cells must be equal to 1.
+ - map-indirect: boolean to disable the default direct mapping of the
+   flash.
+   On some platforms (e.g. MPC5200) a direct 1:1 mapping may cause
+   problems with JFFS2 usage, as the local bus (LPB) doesn't support
+   unaligned accesses as implemented in the JFFS2 code via memcpy().
+   By defining "map-indirect", the flash will not be exposed directly
+   to the MTD users (e.g. JFFS2) any more.
 
 For JEDEC compatible devices, the following additional properties
 are defined:
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 2e6fb68..a96011d 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -169,6 +169,7 @@ static int __devinit of_flash_probe(struct platform_device *dev)
 	struct mtd_info **mtd_list = NULL;
 	resource_size_t res_size;
 	struct mtd_part_parser_data ppdata;
+	bool map_indirect;
 
 	match = of_match_device(of_flash_match, &dev->dev);
 	if (!match)
@@ -192,6 +193,8 @@ static int __devinit of_flash_probe(struct platform_device *dev)
 	}
 	count /= reg_tuple_size;
 
+	map_indirect = of_property_read_bool(dp, "map-indirect");
+
 	err = -ENOMEM;
 	info = kzalloc(sizeof(struct of_flash) +
 		       sizeof(struct of_flash_list) * count, GFP_KERNEL);
@@ -247,6 +250,17 @@ static int __devinit of_flash_probe(struct platform_device *dev)
 
 		simple_map_init(&info->list[i].map);
 
+		/*
+		 * On some platforms (e.g. MPC5200) a direct 1:1 mapping
+		 * may cause problems with JFFS2 usage, as the local bus (LPB)
+		 * doesn't support unaligned accesses as implemented in the
+		 * JFFS2 code via memcpy(). By setting NO_XIP, the
+		 * flash will not be exposed directly to the MTD users
+		 * (e.g. JFFS2) any more.
+		 */
+		if (map_indirect)
+			info->list[i].map.phys = NO_XIP;
+
 		if (probe_type) {
 			info->list[i].mtd = do_map_probe(probe_type,
 							 &info->list[i].map);
-- 
1.7.11.1

Re: [PATCH] mtd: physmap_of: Add "map-indirect" DT property

[Stephan Gatzka]

[-- Attachment #1: Type: text/plain, Size: 489 bytes --]

Hello all,

this patch works very well for us (of course, because Stefan made it for 
us). Are there any concerns against it? Probably against introducing a 
new DT property? Of course we can make this fix much more MPC5200 
specific but this will result in largely copying physmap_of.c and some 
additional code for access routines and Kconfig.

I'm wondering a bit about this silence because I consider this LPB issue 
a major concern for the MPC5200.

Regards,

Stephan


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4931 bytes --]

Re: [PATCH] mtd: physmap_of: Add "map-indirect" DT property

[Anatolij Gustschin]

Hi Stefan,

On Tue,  3 Jul 2012 10:26:31 +0200
Stefan Roese <sr@denx.de> wrote:

> On some platforms (e.g. MPC5200) a direct 1:1 mapping may cause
> problems with JFFS2 usage, as the local bus (LPB) doesn't support
> unaligned accesses as implemented in the JFFS2 code via memcpy().
> By defining "map-indirect", the flash will not be exposed directly
> to the MTD users (e.g. JFFS2) any more.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Stephan Gatzka <Stephan.Gatzka@hbm.com>
> Cc: Anatolij Gustschin <agust@denx.de>
> Cc: Albrecht Dress <albrecht.dress@arcor.de>
> ---
>  Documentation/devicetree/bindings/mtd/mtd-physmap.txt |  7 +++++++
>  drivers/mtd/maps/physmap_of.c                         | 14 ++++++++++++++
>  2 files changed, 21 insertions(+)

The patch looks good, so

Acked-by: Anatolij Gustschin <agust@denx.de>

Thanks,
Anatolij

Re: [PATCH] mtd: physmap_of: Add "map-indirect" DT property

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 447 bytes --]

On Tue, 2012-07-03 at 10:26 +0200, Stefan Roese wrote:
> On some platforms (e.g. MPC5200) a direct 1:1 mapping may cause
> problems with JFFS2 usage, as the local bus (LPB) doesn't support
> unaligned accesses as implemented in the JFFS2 code via memcpy().
> By defining "map-indirect", the flash will not be exposed directly
> to the MTD users (e.g. JFFS2) any more.

Pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mtd: physmap_of: Add "map-indirect" DT property

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Sat, 2012-07-07 at 09:16 +0200, Stephan Gatzka wrote:
> this patch works very well for us (of course, because Stefan made it for 
> us). Are there any concerns against it? Probably against introducing a 
> new DT property? Of course we can make this fix much more MPC5200 
> specific but this will result in largely copying physmap_of.c and some 
> additional code for access routines and Kconfig. 

I don't much like the "map-indirect" name. If it's actually unaligned
access, or non-word-sized access, that's forbidden, then that's what the
DT property should be.

The term "map-indirect" is more a description of how the software
currently behaves... which is exactly that DT bindings *shouldn't* be.

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] mtd: physmap_of: Add "map-indirect" DT property

[Stefan Roese]

On Monday 16 July 2012 22:41:52 David Woodhouse wrote:
> On Sat, 2012-07-07 at 09:16 +0200, Stephan Gatzka wrote:
> > this patch works very well for us (of course, because Stefan made it for
> > us). Are there any concerns against it? Probably against introducing a
> > new DT property? Of course we can make this fix much more MPC5200
> > specific but this will result in largely copying physmap_of.c and some
> > additional code for access routines and Kconfig.
> 
> I don't much like the "map-indirect" name. If it's actually unaligned
> access, or non-word-sized access, that's forbidden, then that's what the
> DT property should be.
>
> The term "map-indirect" is more a description of how the software
> currently behaves... which is exactly that DT bindings *shouldn't* be.

Okay. I've chosen "map-indirect" because it might be used by other platforms 
as well, perhaps because of different reasons (so not restricting this to the 
unaligned access problem of the MPC5200). But I have no strong feelings here, 
so I can prepare a new patch version with a different name.

How about "no-unaligned-direct-access"? Pretty long though. Any other 
suggestions here?

Thanks,
Stefan

Re: [PATCH] mtd: physmap_of: Add "map-indirect" DT property

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 241 bytes --]

On Tue, 2012-07-17 at 11:56 +0200, Stefan Roese wrote:
> How about "no-unaligned-direct-access"? Pretty long though. Any other 
> suggestions here?

FYI, I am dropping this patch from l2-mtd.git.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]