login
Search
From 3e33a3cfc289e9baa41f6e7cf4d612d41b88e19b Mon Sep 17 00:00:00 2001 From: brucle <brucle@selinc.com> Date: Wed, 13 Aug 2008 18:07:19 -0700 Subject: [PATCH] Add support for > 2GiB MTD devices Signed-off-by: Bruce D. Leonard <brucle@selinc.com> --- drivers/mtd/mtdchar.c | 16 ++++++++-------- drivers/mtd/mtdcore.c | 6 +++--- drivers/mtd/nand/nand_base.c | 36 ++++++++++++++++++++++++------------ drivers/mtd/nand/nand_bbt.c | 30 +++++++++++++++--------------- drivers/mtd/ubi/build.c | 5 +++-- include/linux/mtd/mtd.h | 27 ++++++++++++++++++++++----- include/mtd/mtd-abi.h | 4 ++-- 7 files changed, 77 insertions(+), 47 deletions(-) diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c index d2f3318..2829faa 100644 --- a/drivers/mtd/mtdchar.c +++ b/drivers/mtd/mtdchar.c @@ -70,13 +70,13 @@ static loff_t mtd_lseek (struct file *file, loff_t offset, int orig) offset += file->f_pos; break; case SEEK_END: - offset += mtd->size; + offset += device_size(mtd); break; default: return -EINVAL; } - if (offset >= 0 && offset <= mtd->size) + if (offset >= 0 && offset <= device_size(mtd)) return file->f_pos = offset; return -EINVAL; @@ -173,8 +173,8 @@ static ssize_t mtd_read(struct file *file, char __user *buf, size_t count,loff_t DEBUG(MTD_DEBUG_LEVEL0,"MTD_read\n"); - if (*ppos + count > mtd->size) - count = mtd->size - *ppos; + if (*ppos + count > device_size(mtd)) + count = device_size(mtd) - *ppos; if (!count) return 0; @@ -266,11 +266,11 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count DEBUG(MTD_DEBUG_LEVEL0,"MTD_write\n"); - if (*ppos == mtd->size) + if (*ppos == device_size(mtd)) return -ENOSPC; - if (*ppos + count > mtd->size) - count = mtd->size - *ppos; + if (*ppos + count > device_size(mtd)) + count = device_size(mtd) - *ppos; if (!count) return 0; @@ -426,7 +426,7 @@ static int ...
On Thu, 21 Aug 2008 19:00:55 -0700 (GMT-07:00) This changes the kernel<->userspace ABI and is hence a big no-no. I assume that this change will cause old userspace to malfunction on new kernels, and vice versa. Supporting >2Gb MTD devices sounds useful (I'm surprised that we don't already do so). Please cc linux-mtd@lists.infradead.org (at least) on MTD-related patches, thanks. --
Well, in my posting I noted that the mtd-utils were broken because of this but I didn't really have any idea as to how to fix things. I can see why it would be a big no-no to change this. Do you have any suggestions on There was a LOT of interest in this over the last few months while I was working on it, but a very suprising silence has developed since I posted I started with the MTD list and then also posted to lkml when I realized I had forgotten to CC it. Thanks. Bruce --
Bruce, Since you don't want to change the ABI you need to extend it. Andrew, should he add an new ioctl that passes a new structure definition? That way the --
Yes, I expect that a new ioctl would be best. Sometimes it is possible to extend an existing one (and its associated payload) but that requires versioning info which usually wasn't thought of on day one. But David's the man. --
Now there's something I hadn't thought of. Thanks, I'll look into doing it that way. --
Hi Bruce, Unfortunately, it is unacceptable to break existing stuff, even old crappy mtd utils :-). That's simply how we work in the kernel community. Stuff which you do is always not simple because you have to make sure all legacy stuff does not break. So you'll need to invest more time into Yeah, sometimes MTD ML just ignores patches, I am in the same boat, but even if you've decided to send stuff to lkml, please, CC MTD ML anyway. -- Best regards, Artem Bityutskiy (Битюцкий Артём) --
Bruce, I have been experimenting with this. I think there is at least 3 maybe 4 IOCTLS that are going to need to have a LARGE mode. The 2 most obvious are MEMGETINFO and MEMERASE. I was starting to code something up that passes the size as blocks as opposed to bytes. That may be more consistent with UBI. I am doing a little experimenting if you want some help there. In any case, it can be done where the old utility can work (for 2GB and less size devices) and a new call not supported by the kernel will let the utility try the new interface and revert to the old if the kernel does not support block mode interfaces. Then we can either re-use the existing structures passed through another ioctl, changing the definition of START and SIZE to be blocks or define new erase_info_user and mtd_info_user structures for the large call. David, would you thing re-using the same structure would be to obscure? I have started looking at a MEMGETLINFO defining size as blocks instead of --
dwmw2 vetoed any MEMGETINFO ioctl changes, or new similar ioctls, and it kind of make sense, because we should use sysfs instead. But this, in turn, means implementing sysfs support for MTD, because MTD is not LinuxDeviceModel-enabled at the moment. -- Best regards, Artem Bityutskiy (Битюцкий Артём) --
Artem, I see your point. Ioctls are going away after all. So we should have something like: /sys/class/mtd/mtd0/ --
Tim, please stop top-posting. I'm not sure I'd say I _vetoed_ new ioctls, but I certainly expressed a desire to do all the information stuff through sysfs. We would need a new MEMERASE64 ioctl though. -- David Woodhouse Open Source Technology Centre David.Woodhouse@intel.com Intel Corporation --
If the (lack of) sysfs stuff is standing between us and a nice >4GiB mtd I for one would think that a set of 64-bit ioctls would be a reasonable halfway house. /Ricard -- Ricard Wolf Wanderlöf ricardw(at)axis.com Axis Communications AB, Lund, Sweden www.axis.com Phone +46 46 272 2016 Fax +46 46 13 61 30 --
Having started this mess, I'll throw my half euros worth in :). While I agree that sysfs is the right way to go, I'm not going to be able to pull it off. Not withstanding my total lack of knowledge about sysfs, I've been very fortunate in that I've been doing all of this work on company time. Due to project constraints, that company time is coming to an end. I could eek out another week or two and get a set of 64- bit ioctls, but I will not be able to spend a couple more months of company time learning sysfs and implementing it. And I just don't have any personal time to spare right now. I'm sorry, if it's anything other than ioctl I just can't do it. That being said, if a set of ioctls is acceptable, I'm more than happy to take that on as soon as we reach a consensus. Bruce --
On the other hand we'll stay with half-solutions forever if we allow them. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) --
I am not very good in LDM, but it does not look like MTD stuff should go to /sys/class/. I'd say it should be in /sys/devices/. -- Best regards, Artem Bityutskiy (Битюцкий Артём) --
On Tue, 26 Aug 2008 23:20:46 -0700 I don't know where that stupid story keeps coming from. Ioctl is alive and well and there are more not less of them. There are lots of things you *cannot* do with sysfs, including synchronization and handling many kinds of changes to objects that can appear and disappear. Ditto there are problems with getting a consistent snapshot via sysfs because you can't atomically read multiple fields. So please stop this 'ioctls are going away' stuff, its bunkum. Alan --
True, and we'll definitely need a new MEMERASE64 ioctl. But for the _informational_ parts, those can happily be done through sysfs. -- David Woodhouse Open Source Technology Centre David.Woodhouse@intel.com Intel Corporation --
They certainly can, but should they? There may be some reason to prefer sysfs that should be self-evident - except that I'm a bit thick and seem to need it spelled out. Or maybe I've just become disillusioned with the practice of replacing crappy interfaces (ioctl here) with other crappy interfaces (sysfs here) and having to support both for all eternity. sysctl, ioctl, proc, sysfs, debugfs, netlink, ... - individually they all suck in their own peculiar way. But together they create a mess I no longer dare to name. So what was the reason again why mtd needs two userspace interfaces instead of just one? Jörn -- It does not require a majority to prevail, but rather an irate, tireless minority keen to set brush fires in people's minds. -- Samuel Adams --
The plus of sysfs I see is that I can add more files to expose more information in sysfs, while I can not change MEMGETINFO ioctl. E.g., I need to expose sub-page size to user-space, and I cannot do this with I would like to make udev creating MTD devices, instead of creating them by hands. Adding MTD to LDM would anyway introduce corresponding sysfs files, right? This means we would have one more interface anyway. -- Best regards, Artem Bityutskiy (Битюцкий Артём) --
sysfs makes adding new attributes easier, yes. But once added you cannot remove the attribute again - ever. Which means that either way you need to tread carefully and think twice before making a rash Could be useful, I don't mind you sending a patch. However, does this means that MEMGETINFO64 or some other ioctl should not be done? Should flash_erase open, read and close 8 seperate files instead of doing a single ioctl? And should our support for large devices wait for the sysfs support that has been talked about and not done for about two years already? Call me a sceptic. Jörn -- The cheapest, fastest and most reliable components of a computer system are those that aren't there. -- Gordon Bell, DEC labratories --
Up to dwmw2, but I do not mind if the answer to all the above questions is "yes" :-) -- Best regards, Artem Bityutskiy (Битюцкий Артём) --
Well, I personally think a "yes" to the last question would be utter madness. Whoever answers that should better come up with an alternative patch for sysfs support pronto. Large flashes are not a one-off cases where a single manufacturer had a rather bizarre design. Their numbers will continually increase. And not supporting an ever-increasing class of hardware is simply not an option. Jörn -- on a false concept. This concept is that people actually want to look at source code. -- Rob Enderle --
That's not _necessarily_ true, although it should certainly be done with
care. Attributes in sysfs can be optional (in a way that they can't
really be optional if they're part of a binary ioctl payload), and
userspace can cope with them being absent. The sub-page size attribute
is something which wouldn't always be present, and we could happily just
It's hardly a fast path. And we don't have to worry about the fact that
You whine too much, Jörn. It doesn't take very long, as a proof of
concept, to add some attributes to the existing class support in
mtdchar.c ...
--- drivers/mtd/mtdchar.c~ 2008-07-13 22:51:29.000000000 +0100
+++ drivers/mtd/mtdchar.c 2008-08-29 13:15:08.000000000 +0100
@@ -22,12 +22,32 @@
static struct class *mtd_class;
+static ssize_t mtd_show_size(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct mtd_info *mtd = dev_get_drvdata(dev);
+ return sprintf(buf, "0x%x\n", mtd->size);
+}
+static ssize_t mtd_show_erasesize(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct mtd_info *mtd = dev_get_drvdata(dev);
+ return sprintf(buf, "0x%x\n", mtd->erasesize);
+}
+
+static DEVICE_ATTR(size, S_IRUGO, mtd_show_size, NULL);
+static DEVICE_ATTR(erasesize, S_IRUGO, mtd_show_erasesize, NULL);
+
static void mtd_notify_add(struct mtd_info* mtd)
{
+ struct device *dev;
if (!mtd)
return;
- device_create(mtd_class, NULL, MKDEV(MTD_CHAR_MAJOR, mtd->index*2), "mtd%d", mtd->index);
+ dev = device_create(mtd_class, NULL, MKDEV(MTD_CHAR_MAJOR, mtd->index*2), "mtd%d", mtd->index);
+ dev_set_drvdata(dev, mtd);
+ device_create_file(dev, &dev_attr_size);
+ device_create_file(dev, &dev_attr_erasesize);
device_create(mtd_class, NULL,
MKDEV(MTD_CHAR_MAJOR, mtd->index*2+1), "mtd%dro", mtd->index);
Ok, so it shouldn't be only for mtdchar -- it should be generic, so we
should shift some of that into the mtd core code. And we should let
people hook up the 'parent' correctly, and ...You can do this with ioctls, if you designed them with extra space and versioning from the beginning.
| Greg KH | Og dreams of kernels |
| Jens Axboe | [PATCH 31/33] Fusion: sg chaining support |
| Arnd Bergmann | Re: finding your own dead "CONFIG_" variables |
| Mark Brown | [PATCH 2/2] Subject: natsemi: Allow users to disable workaround for DspCfg reset |
| Tony Breeds | [LGUEST] Look in object dir for .config |
git: | |
| Brian Downing | Re: Git in a Nutshell guide |