| Message ID | 1341303991-6219-1-git-send-email-sr@denx.de |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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!
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.
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
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.
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);
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(+)