lkml.org 
[lkml]   [2008]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 2/2][MTD] Add support for > 2GiB MTD devices
From
Date
On Wed, 2008-08-27 at 17:25 +0200, Jörn Engel wrote:
> On Wed, 27 August 2008 17:47:43 +0300, Artem Bityutskiy wrote:
> >
> > 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
> > MEMGETINFO.
>
> 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
> decision.

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
drop it and forget about it in future if we really wanted to.

> > > So what was the reason again why mtd needs two userspace interfaces
> > > instead of just one?
> >
> > 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.
>
> 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?

It's hardly a fast path. And we don't have to worry about the fact that
it's non-atomic -- these things aren't exactly _changing_ over time.

> 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?

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 there are a few other things
we should do to tidy it up. But it isn't exactly hard.

--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2008-08-29 16:55    [W:0.255 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site