* [PATCH v2 RESEND] gpiolib: Add pin change notification
From: Ben Nizette @ 2008-10-20 22:50 UTC (permalink / raw)
To: David Brownell; +Cc: Andrew Morton, linux-kernel, linux-embedded
[-- Attachment #1: Type: text/plain, Size: 826 bytes --]
This adds pin change notification to the gpiolib sysfs interface. It
requires 16 extra bytes in gpio_desc iff CONFIG_GPIO_SYSFS which in turn
means, eg, 4k of .bss usage on AVR32. Due to limitations in sysfs, this
patch makes poll(2) and friends work as expected on the "value"
attribute, though reads on "value" will never block and there is no
facility for async reads and writes.
Signed-off-by: Ben Nizette <bn@niasdigital.com>
---
Right, there seems to be a new outgoing mail server between me and you
which viciously attacks tabulations. Until I find the monkey
responsible please accept the patch as an attachment along with my
apologies.
Documentation/gpio.txt | 48 +++++++++
drivers/gpio/gpiolib.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 289 insertions(+), 5 deletions(-)
[-- Attachment #2: pinchange.patch --]
[-- Type: text/x-patch, Size: 11349 bytes --]
diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
index b1b9887..998a40f 100644
--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -529,6 +529,21 @@ and have the following read/write attributes:
is configured as an output, this value may be written;
any nonzero value is treated as high.
+ "notify" ... Selects a method for the detection of pin change
+ events. This can be written or read as one of "none",
+ "irq" or a number. If "none", no detection will be
+ done, if "irq" then the change detection will be done
+ by registering an interrupt handler on the line given
+ by gpio_to_irq for that gpio. If a number is written,
+ the gpio will be polled for a state change every n
+ milliseconds where n is the number written. The
+ minimum interval is 10 milliseconds.
+
+ "notify_filter" ... This can be written and read as one of
+ "rising", "falling" or "both". If "rising", only
+ rising edges will be reported, similarly for "falling"
+ and "both".
+
GPIO controllers have paths like /sys/class/gpio/chipchip42/ (for the
controller implementing GPIOs starting at #42) and have the following
read-only attributes:
@@ -549,6 +564,39 @@ gpiochip nodes (possibly in conjunction with schematics) to determine
the correct GPIO number to use for a given signal.
+Pin Change Notification
+-----------------------
+The "value" attribute of a gpio is pollable. For this to work, you
+must have set up the notify attribute of that gpio to the type of
+detection suitable for the underlying hardware.
+
+If the hardware supports interrupts on both edges and the gpio to
+interrupt line mapping is unique, you should write "irq" to the notify
+attribute.
+
+If the hardware doesn't support this, or you're unsure, you can write
+a number to the notify attribute. This number is the delay between
+successive polls of the gpio state in milliseconds. You may not poll
+more often than 10ms intervals.
+
+You must use poll mode if communication with the gpio's chip requires
+sleeping. This is the case if, for example, the gpio is part of a
+I2C or SPI GPIO expander.
+
+By default, both rising and falling edges are reported. To filter
+this, you can write one of "rising" or "falling" to the notify_filter
+attribute. To go back to being notified of both edge changes, write
+"both" to this attribute.
+
+Once the notification has been set up, you may use poll(2) and friends
+on the "value" attribute. This attribute will register as having new
+data ready to be read if and only if there has been a state change
+since the last read(2) to the "value" attribute.
+
+Note that unlike a regular device file, a read on the "value" attribute
+will never block whether or not there's new data to be read.
+
+
Exporting from Kernel code
--------------------------
Kernel code can explicitly manage exports of GPIOs which have already been
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 9112830..f43f231 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1,6 +1,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/irq.h>
+#include <linux/interrupt.h>
#include <linux/spinlock.h>
#include <linux/device.h>
#include <linux/err.h>
@@ -49,13 +50,35 @@ struct gpio_desc {
#define FLAG_RESERVED 2
#define FLAG_EXPORT 3 /* protected by sysfs_lock */
#define FLAG_SYSFS 4 /* exported via /sys/class/gpio/control */
+#define ASYNC_MODE_IRQ 5 /* using interrupts for async notification */
+#define ASYNC_MODE_POLL 6 /* using polling for async notification */
+#define ASYNC_RISING 7 /* will notify on rising edges */
+#define ASYNC_FALLING 8 /* will notify on falling edges */
#ifdef CONFIG_DEBUG_FS
const char *label;
#endif
+
+#ifdef CONFIG_GPIO_SYSFS
+ struct device *dev;
+ struct poll_desc *poll;
+
+ /* Poll interval in jiffies, here (rather than in struct poll_desc
+ * so the user can change the value no matter what their current
+ * notification mode is */
+ long timeout;
+
+ /* Last known value */
+ int val;
+#endif
};
static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];
+struct poll_desc {
+ struct delayed_work work;
+ unsigned gpio;
+};
+
static inline void desc_set_label(struct gpio_desc *d, const char *label)
{
#ifdef CONFIG_DEBUG_FS
@@ -184,12 +207,56 @@ static DEFINE_MUTEX(sysfs_lock);
* /value
* * always readable, subject to hardware behavior
* * may be writable, as zero/nonzero
- *
- * REVISIT there will likely be an attribute for configuring async
- * notifications, e.g. to specify polling interval or IRQ trigger type
- * that would for example trigger a poll() on the "value".
+ * /notify
+ * * read/write as "irq", numeric or "none"
+ * /notify_filter
+ * * read/write as "rising", "falling" or "both"
*/
+struct poll_desc *work_to_poll(struct work_struct *ws)
+{
+ return container_of((struct delayed_work *)ws, struct poll_desc, work);
+}
+
+void gpio_poll_work(struct work_struct *ws)
+{
+ struct poll_desc *poll = work_to_poll(ws);
+
+ unsigned gpio = poll->gpio;
+ struct gpio_desc *desc = &gpio_desc[gpio];
+
+ int new = gpio_get_value_cansleep(gpio);
+ int old = desc->val;
+
+ if ((new && !old && test_bit(ASYNC_RISING, &desc->flags)) ||
+ (!new && old && test_bit(ASYNC_FALLING, &desc->flags)))
+ sysfs_notify(&desc->dev->kobj, NULL, "value");
+
+ desc->val = new;
+ schedule_delayed_work(&poll->work, desc->timeout);
+}
+
+static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
+{
+ struct gpio_desc *desc = dev_id;
+ int gpio = desc - gpio_desc;
+ int new, old;
+
+ if (!gpio_is_valid(gpio))
+ return IRQ_NONE;
+
+ new = gpio_get_value(gpio);
+ old = desc->val;
+
+ if ((new && !old && test_bit(ASYNC_RISING, &desc->flags)) ||
+ (!new && old && test_bit(ASYNC_FALLING, &desc->flags)))
+ sysfs_notify(&desc->dev->kobj, NULL, "value");
+
+ desc->val = new;
+
+ return IRQ_HANDLED;
+}
+
static ssize_t gpio_direction_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -284,9 +351,162 @@ static ssize_t gpio_value_store(struct device *dev,
static /*const*/ DEVICE_ATTR(value, 0644,
gpio_value_show, gpio_value_store);
+static ssize_t gpio_notify_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ const struct gpio_desc *desc = dev_get_drvdata(dev);
+ ssize_t ret;
+
+ mutex_lock(&sysfs_lock);
+
+ if (test_bit(ASYNC_MODE_IRQ, &desc->flags))
+ ret = sprintf(buf, "irq\n");
+ else if (test_bit(ASYNC_MODE_POLL, &desc->flags))
+ ret = sprintf(buf, "%ld\n", desc->timeout * 1000 / HZ);
+ else
+ ret = sprintf(buf, "none\n");
+
+ mutex_unlock(&sysfs_lock);
+ return ret;
+}
+
+static ssize_t gpio_notify_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t size)
+{
+ struct gpio_desc *desc = dev_get_drvdata(dev);
+ unsigned gpio = desc - gpio_desc;
+ ssize_t status = 0;
+ int new;
+ long value;
+
+ mutex_lock(&sysfs_lock);
+
+ if (sysfs_streq(buf, "irq"))
+ new = ASYNC_MODE_IRQ;
+ else if (sysfs_streq(buf, "none"))
+ new = -1;
+ else {
+ status = strict_strtol(buf, 0, &value);
+
+ if (status || value < 10) {
+ status = -EINVAL;
+ goto out;
+ }
+
+ /* value will be in ms, convert to jiffies. */
+ desc->timeout = DIV_ROUND_UP(value * HZ, 1000);
+ new = ASYNC_MODE_POLL;
+ }
+
+ if (test_and_clear_bit(ASYNC_MODE_IRQ, &desc->flags))
+ free_irq(gpio_to_irq(gpio), desc);
+ else if (test_and_clear_bit(ASYNC_MODE_POLL, &desc->flags)) {
+ BUG_ON(!desc->poll);
+
+ cancel_delayed_work(&desc->poll->work);
+ kfree(desc->poll);
+ }
+
+ if (new == ASYNC_MODE_IRQ) {
+ if (desc->chip->can_sleep) {
+ /* -EINVAL probably isn't appropriate here,
+ * what's code for "not supported by
+ * underlying hardware"? */
+ status = -EINVAL;
+ goto out;
+ }
+
+ desc->val = gpio_get_value(gpio);
+
+ /* REVISIT: We always request both edges then filter in the
+ * irq handler. How many devices don't support this..?? */
+ status = request_irq(gpio_to_irq(gpio), gpio_irq_handler,
+ IRQF_SHARED | IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING,
+ "gpiolib", desc);
+ } else if (new == ASYNC_MODE_POLL) {
+
+ desc->poll = kmalloc(sizeof(struct poll_desc), GFP_KERNEL);
+ if (!desc->poll) {
+ status = -ENOMEM;
+ goto out;
+ }
+
+ desc->poll->gpio = gpio;
+
+ desc->val = gpio_get_value_cansleep(gpio);
+
+ INIT_DELAYED_WORK(&desc->poll->work, gpio_poll_work);
+ schedule_delayed_work(&desc->poll->work, desc->timeout);
+ }
+
+ if (new >= 0 && !status)
+ set_bit(new, &desc->flags);
+
+out:
+ if (status)
+ dev_dbg(dev, "gpio notification mode set fail, err %d\n",
+ (int)status);
+
+ mutex_unlock(&sysfs_lock);
+ return status ? : size;
+}
+
+static const DEVICE_ATTR(notify, 0644,
+ gpio_notify_show, gpio_notify_store);
+
+static ssize_t gpio_notify_filter_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct gpio_desc *desc = dev_get_drvdata(dev);
+ ssize_t status;
+
+ mutex_lock(&sysfs_lock);
+
+ if (test_bit(ASYNC_FALLING, &desc->flags) &&
+ test_bit(ASYNC_RISING, &desc->flags))
+ status = sprintf(buf, "both\n");
+ else if (test_bit(ASYNC_RISING, &desc->flags))
+ status = sprintf(buf, "rising\n");
+ else
+ status = sprintf(buf, "falling\n");
+
+ mutex_unlock(&sysfs_lock);
+ return status;
+}
+
+static ssize_t gpio_notify_filter_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t size)
+{
+ struct gpio_desc *desc = dev_get_drvdata(dev);
+ ssize_t status = 0;
+
+ mutex_lock(&sysfs_lock);
+
+ if (sysfs_streq(buf, "rising")) {
+ set_bit(ASYNC_RISING, &desc->flags);
+ clear_bit(ASYNC_FALLING, &desc->flags);
+ } else if (sysfs_streq(buf, "falling")) {
+ clear_bit(ASYNC_RISING, &desc->flags);
+ set_bit(ASYNC_FALLING, &desc->flags);
+ } else if (sysfs_streq(buf, "both")) {
+ set_bit(ASYNC_RISING, &desc->flags);
+ set_bit(ASYNC_FALLING, &desc->flags);
+ } else
+ status = -EINVAL;
+
+ mutex_unlock(&sysfs_lock);
+ return status ? : size;
+}
+
+static const DEVICE_ATTR(notify_filter, 0644,
+ gpio_notify_filter_show, gpio_notify_filter_store);
+
static const struct attribute *gpio_attrs[] = {
&dev_attr_direction.attr,
&dev_attr_value.attr,
+ &dev_attr_notify.attr,
+ &dev_attr_notify_filter.attr,
NULL,
};
@@ -398,6 +618,17 @@ static ssize_t unexport_store(struct class *class, const char *buf, size_t len)
status = 0;
gpio_free(gpio);
}
+
+ if (test_and_clear_bit(ASYNC_MODE_IRQ, &gpio_desc[gpio].flags))
+ free_irq(gpio_to_irq(gpio), &gpio_desc[gpio]);
+
+ if (test_and_clear_bit(ASYNC_MODE_POLL, &gpio_desc[gpio].flags)) {
+ BUG_ON(!gpio_desc[gpio].poll);
+
+ cancel_delayed_work(&gpio_desc[gpio].poll->work);
+ kfree(gpio_desc[gpio].poll);
+ }
+
done:
if (status)
pr_debug("%s: status %d\n", __func__, status);
@@ -479,6 +710,12 @@ int gpio_export(unsigned gpio, bool direction_may_change)
status = -ENODEV;
if (status == 0)
set_bit(FLAG_EXPORT, &desc->flags);
+
+ desc->dev = dev;
+ /* 100ms default poll interval */
+ desc->timeout = HZ / 10;
+ set_bit(ASYNC_RISING, &desc->flags);
+ set_bit(ASYNC_FALLING, &desc->flags);
}
mutex_unlock(&sysfs_lock);
@@ -626,7 +863,6 @@ static int __init gpiolib_sysfs_init(void)
}
spin_unlock_irqrestore(&gpio_lock, flags);
-
return status;
}
postcore_initcall(gpiolib_sysfs_init);
^ permalink raw reply related
* Re: Subject: [PATCH 01/16] Squashfs: inode operations
From: Phillip Lougher @ 2008-10-21 0:32 UTC (permalink / raw)
To: Jörn Engel
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <20081017165300.GA8076@logfs.org>
Jörn Engel wrote:
> None of the comments below are a reason against mainline inclusion, imo.
> They should get handled, but whether that happens before or after a
> merge doesn't really matter.
Yeah you're right regarding your comments. That's where code-review
comes in handy, to spot things you don't see because you're too used to
the code.
I'm working on a re-spin incorporating your comments, and it should be
ready tomorrow.
Thanks for the code-review.
Phillip
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: Phillip Lougher @ 2008-10-21 1:12 UTC (permalink / raw)
To: David P. Quigley
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <1224268027.18940.78.camel@moss-terrapins.epoch.ncsc.mil>
David P. Quigley wrote:
> Looking through the code I see two references to xattrs, one is the
> index of the xattr table in the superblock and there seems to be struct
> member in one of the inode structures that is an index into this table.
> Looking through the code I don't see either of these used at all. Do you
> intend to add xattr support at some point? I saw reference to the desire
> to add xattr support in an email from 2004 but you said that the code
> has been rewritten since then. If you are going to add xattr support you
> probably want to add it to more than just regular files. In SELinux and
> other LSMs symlinks and directories are also labeled so they will need
> xattr entries.
Yes and yes. I am intending to add xattr support, something that's been
on my to-do list for a long time (since 2004 as you said), but it's been
something which I've never got the time to do. Once (if) Squashfs is
mainlined, it will be the next thing.
The xattr references in the layout is my attempt at forward planning to
avoid making an incompatible layout change when I finally get around to
implementing it. My plan is to put xattrs in a table (referenced by the
superblock), and then put indexes in "extended" inodes which index
into the table (as you noticed). The general idea in Squashfs is that
inodes get optimised for normally occurring cases, and less common cases
(that would need a bigger inode) get to use an extended inode.
Squashfs currently has an extended regular file inode, which is where
the xattr index will sit, and so this has had an xattr index added. The
other inodes don't currently have extended inodes, these will be defined
when I implement xattrs (which is why they're missing).
Having said that, I've fscked up and forgotten to add an xattr field to
the extended directory inode which is currently defined :)
Thanks for spotting this.
Phillip
> Dave
>
>
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: Phillip Lougher @ 2008-10-21 1:21 UTC (permalink / raw)
To: David P. Quigley
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <1224268027.18940.78.camel@moss-terrapins.epoch.ncsc.mil>
David P. Quigley wrote:
In SELinux and
> other LSMs symlinks and directories are also labeled so they will need
> xattr entries.
BTW you don't mention device, fifo and socket inodes... Do they ever
get labelled? It's something I was going to look into closer to an
implementation, but it would be interesting to know.
Phillip
>
> Dave
>
>
^ permalink raw reply
* Re: UIO - interrupt performance
From: Wolfgang Grandegger @ 2008-10-21 6:57 UTC (permalink / raw)
To: Ben Nizette; +Cc: Nicholas Mc Guire, linux-embedded
In-Reply-To: <1224540757.3954.59.camel@moss.renham>
Ben Nizette wrote:
> On Mon, 2008-10-20 at 03:06 -0800, Nicholas Mc Guire wrote:
>>> On Mon, 2008-10-20 at 10:55 +0100, Douglas, Jim (Jim) wrote:
>>>> We are contemplating porting a large number of device drivers to Linux.
>>>> The pragmatic solution is to keep them in user mode (using the UIO
>>>> framework) where possible ... they are written in C++ for a start.
>>>>
>>>> The obvious disadvantages of user mode device drivers are security /
>>>> isolation. The main benefit is ease of development.
>>>>
>>>> Do you know what the *technical* disadvantages of this approach might
>>>> be? I am most concerned about possible impact on interrupt handling.
>>>>
>>>> For example, I assume the context switching overhead is higher, and that
>>>> interrupt latency is more difficult to predict?
>>> Userspace drivers certainly aren't first class citizens; uio and kernel
>>> mode drivers generally aren't really interchangeable.
>>>
>>> The technical disadvantages of userspace drivers are that you don't have
>>> access to kernel subsystems, you can't run any userspace content in irq
>>> context so everything needs to be scheduled before it can be dealt with.
>>> A UIO driver still needs a kernel component to do acknowledge the
>>> interrupt. As such when you say "interrupt latency" you need to define
>>> the end point. A UIO driver will have it's in-kernel handler called
>>> just as quickly as any other driver but the userspace app will need to
>>> be scheduled before it receives notification that the IRQ has fired.
>>>
>>> The technical advantage of a UIO driver is that devices which only need
>>> to shift data don't have to double-handle it. e.g. an ADC card doesn't
>>> need to move ADC results from hardware to kernel, kernel to userspace,
>>> it's just one fluid movement.
>>>
>>> What kind of device drivers are you talking about? They have to be of a
>>> fairly specific flavour to fit in to a UIO model. Linux isn't a
>>> microkernel, userspace drivers are quite restricted in their power.
>>>
>> are these claims based on benchmarks of a specific driver ? I only know
>> of a singe UIO driver for a Hilscher CIF card and one for a SMX
>> Cryptengine (I guess thats yours any way) but none for a AD/DIO card - if
>> you know of such a driver I would be interested in seeing its performance.
>
> When UIO was being discussed for inclusion, the example case being
> thrown around was for such an ADC card. They claimed to have seen
> significant improvements in speed by avoiding the double-handling of
> data. Come to think of it, I can't see that this specific driver has
> shown up...
>
> But what kind of benchmarks do you want? When I say "restricted in
> their power" I mean more in a feature-set kind of way than a raw speed
> way. Userspace drivers can't plug in to kernel subsystems so can't, for
> example, be SPI hosts or terminal devices or network hardware or
> anything else which sits in the middle of a standard stack. All they
> can do is be notified of an interrupt and have direct access to a lump
> of memory.
>
> As I asked before, what's your use-case? It tends to be fairly obvious
> whether the hardware is suitable for a UIO-based driver or whether it's
> going to have to live in kernel.
>
>> Also if you know of any simple UIO sample drivers that would also help.
>
> As in examples of the userspace half? Unfortunately uio-smx isn't ready
> to fly thanks to some significant production delays but the userspace
> half of the Hilscher CIF driver can be found at
> http://www.osadl.org/projects/downloads/UIO/user/
As I see it, mainly the license conditions attract people to use UIO.
Performance is not that important.
Wolfgang.
^ permalink raw reply
* Re: UIO - interrupt performance
From: Marco Stornelli @ 2008-10-21 8:36 UTC (permalink / raw)
To: Bill Gatliff; +Cc: Douglas, Jim (Jim), Embedded Linux mailing list
In-Reply-To: <48FCAE16.70509@billgatliff.com>
As I said I think UIO drivers are a "young feature" from kernel point of
view but I haven't problems to use it. Jim, however, was talking about
to do a complete porting of drivers. I don't know if it'd be a good
idea. UIO drivers, however, has been inserted mainly for one reason: the
problem with GPL, so I prefer, but it's only my opinion, at least for
now, to write a "classic" driver if there aren't GPL problems.
Bill Gatliff ha scritto:
> Marco Stornelli wrote:
>> I quite agree with Ben and Christian. I think UIO drivers are usable for
>> simple devices, I think they aren't mature (will it ever be?) to use it
>> with complicated devices or with strict requirement.
>
> I disagree. Completely.
>
> I recall seeing a report from the Gelato project, where they reimplemented an
> IDE driver under UIO. IIRC, their test results showed at least 80% of the
> in-kernel performance--- and this was a very early UIO implementation, I would
> guess that things are much improved since then. I know that IDE isn't a USBH,
> but it isn't a GPIO LED, either. :)
>
> If you are concerned about timeliness of execution, then if you have never heard
> of POSIX.1b then you shouldn't be writing Linux code anyway. But if you do use
> the features that POSIX.1b gives you, then I haven't found UIO to be objectionable.
>
>>From a performance standpoint, the major differences between UIO vs. in-kernel
> are (a) a _possible_ additional context switch at each interrupt, to transfer
> control back to the userspace responder, and (b) the elimination of a syscall to
> push data through an in-kernel interface back to the device. Only your own
> testing with your own hardware and application can tell you if that's a net
> improvement or regression and where--- if anywhere--- you take the hit.
>
> The general upsides with UIO are huge: you can debug your driver with gdb, and
> you can bind your driver tightly to your application if it makes sense to do so.
> Every i/o action is potentially zero-copy straight into the correct data
> structures, for example. For some workloads, that puts UIO way ahead of an
> in-kernel driver without the complexity of mmap().
>
> As an aside, if you really need an interface that resembles a device node then
> you can emulate that in userspace with a FIFO. That lets you put the driver in
> a standalone program if you like, and other user applications can't tell the
> difference between that and a true device node. (They can figure it out if they
> need to, but if they just are using open/close/read/write then they don't care).
>
> The social downside to UIO is that you'll never get your driver(s) mainlined,
> since Linux-the-kernel doesn't run in userspace. :)
>
> Put simply, you can't dismiss UIO lightly unless you haven't worked with or
> reviewed the code behind it.
>
>
>
> b.g.
--
Marco Stornelli
Embedded Software Engineer
CoRiTeL - Consorzio di Ricerca sulle Telecomunicazioni
http://www.coritel.it
marco.stornelli@coritel.it
+39 06 72582838
^ permalink raw reply
* Re: UIO - interrupt performance
From: Alessio Igor Bogani @ 2008-10-21 9:01 UTC (permalink / raw)
To: Marco Stornelli
Cc: Bill Gatliff, Douglas, Jim (Jim), Embedded Linux mailing list
In-Reply-To: <48FD94AA.8070900@coritel.it>
Hi All,
Sorry for my (very) bad english.
2008/10/21 Marco Stornelli <marco.stornelli@coritel.it>:
[...]
> idea. UIO drivers, however, has been inserted mainly for one reason: the
> problem with GPL, so I prefer, but it's only my opinion, at least for
> now, to write a "classic" driver if there aren't GPL problems.
No, that isn't the main objective of the UIO Authors.
AFAIK They want achieve:
1) Fast prototyping for device drivers
2) Easy and fast debug
3) Avoid "fast and furious" changes of the internal kernel interfaces
for their drivers
Ciao,
Alessio
^ permalink raw reply
* Re: UIO - interrupt performance
From: Marco Stornelli @ 2008-10-21 9:30 UTC (permalink / raw)
To: Alessio Igor Bogani
Cc: Bill Gatliff, Douglas, Jim (Jim), Embedded Linux mailing list
In-Reply-To: <63a49ef40810210201x36249476tcc75141d87a610f2@mail.gmail.com>
I could agree, but "the facto" due to UIO license condition, a company
often uses UIO drivers, regardless performance, debug, etc, only as not
to public the code under GPL.
Alessio Igor Bogani ha scritto:
> Hi All,
>
> Sorry for my (very) bad english.
>
> 2008/10/21 Marco Stornelli <marco.stornelli@coritel.it>:
> [...]
>> idea. UIO drivers, however, has been inserted mainly for one reason: the
>> problem with GPL, so I prefer, but it's only my opinion, at least for
>> now, to write a "classic" driver if there aren't GPL problems.
>
> No, that isn't the main objective of the UIO Authors.
>
> AFAIK They want achieve:
>
> 1) Fast prototyping for device drivers
> 2) Easy and fast debug
> 3) Avoid "fast and furious" changes of the internal kernel interfaces
> for their drivers
>
> Ciao,
> Alessio
>
--
Marco Stornelli
Embedded Software Engineer
CoRiTeL - Consorzio di Ricerca sulle Telecomunicazioni
http://www.coritel.it
marco.stornelli@coritel.it
+39 06 72582838
^ permalink raw reply
* Re: UIO - interrupt performance
From: Ben Nizette @ 2008-10-21 9:32 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Nicholas Mc Guire, linux-embedded
In-Reply-To: <48FD7D65.1070409@grandegger.com>
On Tue, 2008-10-21 at 08:57 +0200, Wolfgang Grandegger wrote:
> Ben Nizette wrote:
> >
> > As in examples of the userspace half? Unfortunately uio-smx isn't ready
> > to fly thanks to some significant production delays but the userspace
> > half of the Hilscher CIF driver can be found at
> > http://www.osadl.org/projects/downloads/UIO/user/
>
> As I see it, mainly the license conditions attract people to use UIO.
> Performance is not that important.
>
Ohw god I hope not. If people want to keep their stuff proprietary they
can supply binary-only modules a la nvidia. Thankfully such nonsense is
fairly uncommon these days.
UIO is not a set of hooks for general userspace drivers, it can't
replace 99% of kernel drivers. It exists to allow easy and
high-performance interfacing to a particular family of devices - those
which simply need to shift data around and have an interrupt telling you
when they're done.
--Ben.
^ permalink raw reply
* Re: UIO - interrupt performance
From: Ben Nizette @ 2008-10-21 9:37 UTC (permalink / raw)
To: Marco Stornelli
Cc: Alessio Igor Bogani, Bill Gatliff, Douglas, Jim (Jim),
Embedded Linux mailing list
In-Reply-To: <48FDA13F.3030303@coritel.it>
On Tue, 2008-10-21 at 11:30 +0200, Marco Stornelli wrote:
> I could agree, but "the facto" due to UIO license condition, a company
> often uses UIO drivers, regardless performance, debug, etc, only as not
> to public the code under GPL.
It sounds to me like you think that driver authors can sit down and
decide whether they want to implement their driver in userspace or
kernel space. For 99% of drivers that's simply not true. You *cannot*
write userspace drivers for most hardware, the hooks just aren't
available. UIO is Userspace I/O, not a set of general hooks for
userspace drivers.
If people want drivers not under the GPL then they can distribute a
binary-only module (though thank $DEITY there aren't many of those
left). Userspace I/O exists to provide good performance interfacing to
a family of devices - those which exist just to shuffle data around and
have an interrupt to tell you when they're done.
Do you have any example of a userspace i/o driver which exists to get
around licencing constraints?
--Ben.
^ permalink raw reply
* Re: UIO - interrupt performance
From: Marco Stornelli @ 2008-10-21 10:24 UTC (permalink / raw)
To: Ben Nizette
Cc: Alessio Igor Bogani, Bill Gatliff, Douglas, Jim (Jim),
Embedded Linux mailing list
In-Reply-To: <1224581853.3954.127.camel@moss.renham>
No I don't think you can decide kernel or user space, indeed you can
read my previous posts, I quite agree with you, I meant the same to Bill
Gatliff.
Ben Nizette ha scritto:
> On Tue, 2008-10-21 at 11:30 +0200, Marco Stornelli wrote:
>> I could agree, but "the facto" due to UIO license condition, a company
>> often uses UIO drivers, regardless performance, debug, etc, only as not
>> to public the code under GPL.
>
> It sounds to me like you think that driver authors can sit down and
> decide whether they want to implement their driver in userspace or
> kernel space. For 99% of drivers that's simply not true. You *cannot*
> write userspace drivers for most hardware, the hooks just aren't
> available. UIO is Userspace I/O, not a set of general hooks for
> userspace drivers.
>
> If people want drivers not under the GPL then they can distribute a
> binary-only module (though thank $DEITY there aren't many of those
> left). Userspace I/O exists to provide good performance interfacing to
> a family of devices - those which exist just to shuffle data around and
> have an interrupt to tell you when they're done.
>
> Do you have any example of a userspace i/o driver which exists to get
> around licencing constraints?
>
> --Ben.
>
>
--
Marco Stornelli
Embedded Software Engineer
CoRiTeL - Consorzio di Ricerca sulle Telecomunicazioni
http://www.coritel.it
marco.stornelli@coritel.it
+39 06 72582838
^ permalink raw reply
* Re: UIO - interrupt performance
From: Wolfgang Grandegger @ 2008-10-21 10:28 UTC (permalink / raw)
To: Ben Nizette
Cc: Marco Stornelli, Alessio Igor Bogani, Bill Gatliff,
Douglas, Jim (Jim), Embedded Linux mailing list
In-Reply-To: <1224581853.3954.127.camel@moss.renham>
Ben Nizette wrote:
> On Tue, 2008-10-21 at 11:30 +0200, Marco Stornelli wrote:
>> I could agree, but "the facto" due to UIO license condition, a company
>> often uses UIO drivers, regardless performance, debug, etc, only as not
>> to public the code under GPL.
>
> It sounds to me like you think that driver authors can sit down and
> decide whether they want to implement their driver in userspace or
> kernel space. For 99% of drivers that's simply not true. You *cannot*
> write userspace drivers for most hardware, the hooks just aren't
> available. UIO is Userspace I/O, not a set of general hooks for
> userspace drivers.
I known, fortunately it's not that simple or even feasible. Image a
network driver with I/O multiplexing used by various processes.
> If people want drivers not under the GPL then they can distribute a
> binary-only module (though thank $DEITY there aren't many of those
> left). Userspace I/O exists to provide good performance interfacing to
That's *not* an option, please read the GPL license conditions. At least
it's legal gray area. Note that it's not my intention to start a
discussion on that.
> a family of devices - those which exist just to shuffle data around and
> have an interrupt to tell you when they're done.
>
> Do you have any example of a userspace i/o driver which exists to get
> around licencing constraints?
There will be plenty sooner than later. What you can do currently with
UIO is very limited.
Wolfgang.
^ permalink raw reply
* Re: UIO - interrupt performance
From: Bill Gatliff @ 2008-10-21 11:39 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Ben Nizette, Marco Stornelli, Alessio Igor Bogani,
Douglas, Jim (Jim), Embedded Linux mailing list
In-Reply-To: <48FDAEE1.9080503@grandegger.com>
Wolfgang Grandegger wrote:
> Ben Nizette wrote:
>> On Tue, 2008-10-21 at 11:30 +0200, Marco Stornelli wrote:
>>> I could agree, but "the facto" due to UIO license condition, a company
>>> often uses UIO drivers, regardless performance, debug, etc, only as not
>>> to public the code under GPL.
>> It sounds to me like you think that driver authors can sit down and
>> decide whether they want to implement their driver in userspace or
>> kernel space. For 99% of drivers that's simply not true. You *cannot*
>> write userspace drivers for most hardware, the hooks just aren't
>> available. UIO is Userspace I/O, not a set of general hooks for
>> userspace drivers.
>
> I known, fortunately it's not that simple or even feasible. Image a
> network driver with I/O multiplexing used by various processes.
Actually, UIO is pretty useful for that when combined with tun/tap.
> That's *not* an option, please read the GPL license conditions. At least
> it's legal gray area. Note that it's not my intention to start a
> discussion on that.
Then I will only contradict you, and not cite my supporting evidence. :)
b.g.
--
Bill Gatliff
bgat@billgatliff.com
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: Stephen Smalley @ 2008-10-21 12:07 UTC (permalink / raw)
To: Phillip Lougher
Cc: David P. Quigley, akpm, linux-embedded, linux-fsdevel,
linux-kernel, tim.bird
In-Reply-To: <48FD2C74.2070608@lougher.demon.co.uk>
On Tue, 2008-10-21 at 02:12 +0100, Phillip Lougher wrote:
> David P. Quigley wrote:
> > Looking through the code I see two references to xattrs, one is the
> > index of the xattr table in the superblock and there seems to be struct
> > member in one of the inode structures that is an index into this table.
> > Looking through the code I don't see either of these used at all. Do you
> > intend to add xattr support at some point? I saw reference to the desire
> > to add xattr support in an email from 2004 but you said that the code
> > has been rewritten since then. If you are going to add xattr support you
> > probably want to add it to more than just regular files. In SELinux and
> > other LSMs symlinks and directories are also labeled so they will need
> > xattr entries.
>
> Yes and yes. I am intending to add xattr support, something that's been
> on my to-do list for a long time (since 2004 as you said), but it's been
> something which I've never got the time to do. Once (if) Squashfs is
> mainlined, it will be the next thing.
>
> The xattr references in the layout is my attempt at forward planning to
> avoid making an incompatible layout change when I finally get around to
> implementing it. My plan is to put xattrs in a table (referenced by the
> superblock), and then put indexes in "extended" inodes which index
> into the table (as you noticed). The general idea in Squashfs is that
> inodes get optimised for normally occurring cases, and less common cases
> (that would need a bigger inode) get to use an extended inode.
> Squashfs currently has an extended regular file inode, which is where
> the xattr index will sit, and so this has had an xattr index added. The
> other inodes don't currently have extended inodes, these will be defined
> when I implement xattrs (which is why they're missing).
>
> Having said that, I've fscked up and forgotten to add an xattr field to
> the extended directory inode which is currently defined :)
>
> Thanks for spotting this.
Just to clarify: When using a labeled MAC solution like SELinux or
SMACK, every file (of every type, including device nodes, symlinks,
fifos, etc) will have a security attribute on it. In the case of ext3,
we have benefited from inlining of small attributes into the inode.
--
Stephen Smalley
National Security Agency
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: David P. Quigley @ 2008-10-21 16:09 UTC (permalink / raw)
To: Phillip Lougher
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <48FD2C74.2070608@lougher.demon.co.uk>
On Tue, 2008-10-21 at 02:12 +0100, Phillip Lougher wrote:
> David P. Quigley wrote:
> > Looking through the code I see two references to xattrs, one is the
> > index of the xattr table in the superblock and there seems to be struct
> > member in one of the inode structures that is an index into this table.
> > Looking through the code I don't see either of these used at all. Do you
> > intend to add xattr support at some point? I saw reference to the desire
> > to add xattr support in an email from 2004 but you said that the code
> > has been rewritten since then. If you are going to add xattr support you
> > probably want to add it to more than just regular files. In SELinux and
> > other LSMs symlinks and directories are also labeled so they will need
> > xattr entries.
>
> Yes and yes. I am intending to add xattr support, something that's been
> on my to-do list for a long time (since 2004 as you said), but it's been
> something which I've never got the time to do. Once (if) Squashfs is
> mainlined, it will be the next thing.
>
> The xattr references in the layout is my attempt at forward planning to
> avoid making an incompatible layout change when I finally get around to
> implementing it. My plan is to put xattrs in a table (referenced by the
> superblock), and then put indexes in "extended" inodes which index
> into the table (as you noticed). The general idea in Squashfs is that
> inodes get optimised for normally occurring cases, and less common cases
> (that would need a bigger inode) get to use an extended inode.
> Squashfs currently has an extended regular file inode, which is where
> the xattr index will sit, and so this has had an xattr index added. The
> other inodes don't currently have extended inodes, these will be defined
> when I implement xattrs (which is why they're missing).
>
> Having said that, I've fscked up and forgotten to add an xattr field to
> the extended directory inode which is currently defined :)
>
> Thanks for spotting this.
>
> Phillip
>
> > Dave
> >
> >
Looking through the code I noticed that you give certain object types
the same inode number for all instances of it (devices, fifo/sockets).
How is this done internally? Do these types of objects occupy the same
position on the inode table? If so how do you differentiate between a
device and a socket?
I have some other comments but I'll post them under the specific
patches.
I use to work on Unionfs and we used CVS initially for our SCM. When we
started working on mainlining Unionfs we moved over to a GIT based
system and we found it worked a lot better. You might want to consider
moving your patches to a GIT tree that you make publically available so
people can just clone, compile, and test them. I don't see anything that
stops Squashfs from being compiled and loaded as a module so it might
not be necessary but it makes it easier for people who want to test the
code or even contribute patches.
Dave
^ permalink raw reply
* Re: Subject: [PATCH 01/16] Squashfs: inode operations
From: David P. Quigley @ 2008-10-21 16:14 UTC (permalink / raw)
To: Jörn Engel
Cc: Phillip Lougher, akpm, linux-embedded, linux-fsdevel,
linux-kernel, tim.bird
In-Reply-To: <20081017165300.GA8076@logfs.org>
On Fri, 2008-10-17 at 18:53 +0200, Jörn Engel wrote:
> None of the comments below are a reason against mainline inclusion, imo.
> They should get handled, but whether that happens before or after a
> merge doesn't really matter.
>
> On Fri, 17 October 2008 16:42:50 +0100, Phillip Lougher wrote:
> >
> > +#include <linux/squashfs_fs.h>
> > +#include <linux/squashfs_fs_sb.h>
> > +#include <linux/squashfs_fs_i.h>
>
> Current verdict seems to be that these files should live in fs/squashfs/,
> not include/linux/. No kernel code beside squashfs needs the headers
> and userspace tools should have a private copy.
>
[Snip]
I looked at where filesystems such as ext3 store these and it seems to
be in include/linux. I'm assuming this is because usespace utilities
like fsck need them. It seems wrong for userspace tools to have their
own private copy since you can potentially have them out of sync with
the kernel you are running and it provides more chance for you
forgetting to update a structure somewhere.
Dave
^ permalink raw reply
* Re: Subject: [PATCH 12/16] Squashfs: header files
From: David P. Quigley @ 2008-10-21 16:25 UTC (permalink / raw)
To: Phillip Lougher
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <E1KqrTX-0001y4-JE@dylan>
On Fri, 2008-10-17 at 16:42 +0100, Phillip Lougher wrote:
[snip]
> +
> +struct squashfs_reg_inode {
> + __le16 inode_type;
> + __le16 mode;
> + __le16 uid;
> + __le16 guid;
> + __le32 mtime;
> + __le32 inode_number;
> + __le32 start_block;
> + __le32 fragment;
> + __le32 offset;
> + __le32 file_size;
> + __le16 block_list[0];
> +};
> +
> +struct squashfs_lreg_inode {
> + __le16 inode_type;
> + __le16 mode;
> + __le16 uid;
> + __le16 guid;
> + __le32 mtime;
> + __le32 inode_number;
> + __le64 start_block;
> + __le64 file_size;
> + __le64 sparse;
> + __le32 nlink;
> + __le32 fragment;
> + __le32 offset;
> + __le32 xattr;
> + __le16 block_list[0];
> +};
> +
> +struct squashfs_dir_inode {
> + __le16 inode_type;
> + __le16 mode;
> + __le16 uid;
> + __le16 guid;
> + __le32 mtime;
> + __le32 inode_number;
> + __le32 start_block;
> + __le32 nlink;
> + __le16 file_size;
> + __le16 offset;
> + __le32 parent_inode;
> +};
> +
> +struct squashfs_ldir_inode {
> + __le16 inode_type;
> + __le16 mode;
> + __le16 uid;
> + __le16 guid;
> + __le32 mtime;
> + __le32 inode_number;
> + __le32 nlink;
> + __le32 file_size;
> + __le32 start_block;
> + __le32 parent_inode;
> + __le16 i_count;
> + __le16 offset;
> + struct squashfs_dir_index index[0];
> +};
> +
[snip]
Something that seems weird is the inconsistency in the ordering of these
structs. The base part is the same across all inodes but for your
reg/lreg dir/ldir pairs you seem to shuffle the order of the added
parts. Is there a reason for this? Is their layout the same on disk
(baring the extra data in the l versions)? If so they probably should be
the same in the struct.
^ permalink raw reply
* Re: Subject: [PATCH 01/16] Squashfs: inode operations
From: Jörn Engel @ 2008-10-21 18:03 UTC (permalink / raw)
To: David P. Quigley
Cc: Phillip Lougher, akpm, linux-embedded, linux-fsdevel,
linux-kernel, tim.bird
In-Reply-To: <1224605666.31157.71.camel@moss-terrapins.epoch.ncsc.mil>
On Tue, 21 October 2008 12:14:26 -0400, David P. Quigley wrote:
> On Fri, 2008-10-17 at 18:53 +0200, Jörn Engel wrote:
> > None of the comments below are a reason against mainline inclusion, imo.
> > They should get handled, but whether that happens before or after a
> > merge doesn't really matter.
> >
> > On Fri, 17 October 2008 16:42:50 +0100, Phillip Lougher wrote:
> > >
> > > +#include <linux/squashfs_fs.h>
> > > +#include <linux/squashfs_fs_sb.h>
> > > +#include <linux/squashfs_fs_i.h>
> >
> > Current verdict seems to be that these files should live in fs/squashfs/,
> > not include/linux/. No kernel code beside squashfs needs the headers
> > and userspace tools should have a private copy.
> >
> [Snip]
>
> I looked at where filesystems such as ext3 store these and it seems to
> be in include/linux. I'm assuming this is because usespace utilities
> like fsck need them. It seems wrong for userspace tools to have their
> own private copy since you can potentially have them out of sync with
> the kernel you are running and it provides more chance for you
> forgetting to update a structure somewhere.
Existing headers remain where they are. New headers are supposed to
go... or at least that's what I was told to do.
And being out of sync is definitely not an argument you can use with a
filesystem. The data on your disk doesn't magically change when you
upgrade a kernel. Nor can you assume that any given filesystem is
accessed only by Linux. If you change the format, then locating
external copies of the header will be the least of your problems.
Jörn
--
Do not stop an army on its way home.
-- Sun Tzu
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Subject: [PATCH 12/16] Squashfs: header files
From: Phillip Lougher @ 2008-10-21 18:17 UTC (permalink / raw)
To: David P. Quigley
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <1224606318.31157.80.camel@moss-terrapins.epoch.ncsc.mil>
David P. Quigley wrote:
>
> Something that seems weird is the inconsistency in the ordering of these
> structs. The base part is the same across all inodes but for your
> reg/lreg dir/ldir pairs you seem to shuffle the order of the added
> parts. Is there a reason for this? Is their layout the same on disk
> (baring the extra data in the l versions)? If so they probably should be
> the same in the struct.
>
>
They're deliberately shuffled about to eliminate holes (due to alignment
contraints), and to maximise compression. Shifting to cluster similar
fields can get better compression, and the current layout is the result
of a lot of work to to get the best ordering.
For example:
>> +struct squashfs_reg_inode {
>> + __le16 inode_type;
>> + __le16 mode;
>> + __le16 uid;
>> + __le16 guid;
>> + __le32 mtime;
>> + __le32 inode_number;
>> + __le32 start_block;
>> + __le32 fragment;
>> + __le32 offset;
>> + __le32 file_size;
>> + __le16 block_list[0];
>> +};
Inode_number, start_block, fragment clustered together because in most
filesystems they'll contain a lot of zero bits (filesystems mainly being
small). Better compression.
>> +
>> +struct squashfs_lreg_inode {
>> + __le16 inode_type;
>> + __le16 mode;
>> + __le16 uid;
>> + __le16 guid;
>> + __le32 mtime;
>> + __le32 inode_number;
>> + __le64 start_block;
>> + __le64 file_size;
>> + __le64 sparse;
>> + __le32 nlink;
>> + __le32 fragment;
>> + __le32 offset;
>> + __le32 xattr;
>> + __le16 block_list[0];
>> +};
Start_block, file_size have been doubled, and the fragment field
consequently moved to preserve 64-bit alignment constraints on 64-bit
quantities (no holes). Plus moving fragment means it can be grouped
with the new nlink field giving a nice run of zero bits (non-extended
regular files have an implicit nlink of 1).
^ permalink raw reply
* Re: [PATCH v2 RESEND] gpiolib: Add pin change notification
From: Andrew Morton @ 2008-10-21 20:31 UTC (permalink / raw)
To: Ben Nizette; +Cc: david-b, linux-kernel, linux-embedded
In-Reply-To: <1224543007.3954.71.camel@moss.renham>
On Tue, 21 Oct 2008 09:50:06 +1100
Ben Nizette <bn@niasdigital.com> wrote:
>
> This adds pin change notification to the gpiolib sysfs interface. It
> requires 16 extra bytes in gpio_desc iff CONFIG_GPIO_SYSFS which in turn
> means, eg, 4k of .bss usage on AVR32. Due to limitations in sysfs, this
> patch makes poll(2) and friends work as expected on the "value"
> attribute, though reads on "value" will never block and there is no
> facility for async reads and writes.
>
>
> ...
>
> +struct poll_desc *work_to_poll(struct work_struct *ws)
static
> +{
> + return container_of((struct delayed_work *)ws, struct poll_desc, work);
> +}
> +
> +void gpio_poll_work(struct work_struct *ws)
static
> +{
> + struct poll_desc *poll = work_to_poll(ws);
> +
> + unsigned gpio = poll->gpio;
> + struct gpio_desc *desc = &gpio_desc[gpio];
> +
> + int new = gpio_get_value_cansleep(gpio);
> + int old = desc->val;
> +
> + if ((new && !old && test_bit(ASYNC_RISING, &desc->flags)) ||
> + (!new && old && test_bit(ASYNC_FALLING, &desc->flags)))
> + sysfs_notify(&desc->dev->kobj, NULL, "value");
> +
> + desc->val = new;
> + schedule_delayed_work(&poll->work, desc->timeout);
> +}
> +
> +static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
> +{
> + struct gpio_desc *desc = dev_id;
> + int gpio = desc - gpio_desc;
> + int new, old;
> +
> + if (!gpio_is_valid(gpio))
> + return IRQ_NONE;
> +
> + new = gpio_get_value(gpio);
> + old = desc->val;
> +
> + if ((new && !old && test_bit(ASYNC_RISING, &desc->flags)) ||
> + (!new && old && test_bit(ASYNC_FALLING, &desc->flags)))
> + sysfs_notify(&desc->dev->kobj, NULL, "value");
eekeekeek! sysfs_notify() does mutex_lock() and will die horridly if
called from an interrupt handler.
You should have got a storm of warnings when runtime testing this code.
Please ensure that all debug options are enabled when testing code.
Documentation/SubmitChecklist has help.
> + desc->val = new;
> +
> + return IRQ_HANDLED;
> +}
> +
> static ssize_t gpio_direction_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -284,9 +351,162 @@ static ssize_t gpio_value_store(struct d
> static /*const*/ DEVICE_ATTR(value, 0644,
> gpio_value_show, gpio_value_store);
>
> +static ssize_t gpio_notify_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + const struct gpio_desc *desc = dev_get_drvdata(dev);
> + ssize_t ret;
> +
> + mutex_lock(&sysfs_lock);
> +
> + if (test_bit(ASYNC_MODE_IRQ, &desc->flags))
> + ret = sprintf(buf, "irq\n");
> + else if (test_bit(ASYNC_MODE_POLL, &desc->flags))
> + ret = sprintf(buf, "%ld\n", desc->timeout * 1000 / HZ);
> + else
> + ret = sprintf(buf, "none\n");
> +
> + mutex_unlock(&sysfs_lock);
> + return ret;
> +}
<panics>
oh, sysfs_lock is a gpiolib-local thing, not a sysfs-core thing.
Crappy name.
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: Alex Riesen @ 2008-10-21 22:29 UTC (permalink / raw)
To: Phillip Lougher
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <E1KqrTW-0001x8-3h@dylan>
2008/10/17 Phillip Lougher <phillip@lougher.demon.co.uk>:
> There are 16 patches in the patch set, and the patches are against the
> latest linux-next tree (linux 2.6.27-next-20081016).
You better don't base anything off linux-next. These are not stable: there
can be even something in the tree you mentioned which will never end
up in the mainline and if your patches depend on it they wont apply to
something like v2.6.26.2.
Use something tagged (v2.6.27, v2.6.27-rc*) in _mainline_ instead.
Than it can be applied to anything from stable releases to vendor trees
(which mostly are based off mainline).
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: Phillip Lougher @ 2008-10-21 23:15 UTC (permalink / raw)
To: Alex Riesen; +Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <81b0412b0810211529x3fc85567yc34fd369ff7c8518@mail.gmail.com>
Alex Riesen wrote:
> 2008/10/17 Phillip Lougher <phillip@lougher.demon.co.uk>:
>> There are 16 patches in the patch set, and the patches are against the
>> latest linux-next tree (linux 2.6.27-next-20081016).
>
> You better don't base anything off linux-next. These are not stable: there
> can be even something in the tree you mentioned which will never end
> up in the mainline and if your patches depend on it they wont apply to
> something like v2.6.26.2.
Definately, there's some d_obtain_alias stuff in linux-next which has
been there since linux-2.6.27-rc4-next. I thought it would make it into
the final 2.6.27 but it didn't.
I thought linux-next *was* the tree that new patches should be based
off. However, the relationship between linux-2.6.git, linux-next.git,
and the -mm patch series seems to be a little vague to me, not to
mention where the linux-staging tree fits into all this.
Phillip
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: David P. Quigley @ 2008-10-21 23:36 UTC (permalink / raw)
To: Phillip Lougher
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <48FE68E9.2020401@lougher.demon.co.uk>
On Wed, 2008-10-22 at 00:42 +0100, Phillip Lougher wrote:
> David P. Quigley wrote:
>
> >
> > Looking through the code I noticed that you give certain object types
> > the same inode number for all instances of it (devices, fifo/sockets).
> > How is this done internally? Do these types of objects occupy the same
> > position on the inode table? If so how do you differentiate between a
> > device and a socket?
> >
>
> No, devices and fifo/sockets get their own unique inode numbers:
>
> root@slackware:/mnt# mount -t squashfs test.sqsh /mnt2 -o loop
> root@slackware:/mnt# ls -li /mnt2
> total 0
> 2 crw-r--r-- 1 root root 1, 1 2008-10-22 00:31 device
> 4 prw-r--r-- 1 root root 0 2008-10-22 00:31 fifo
> 3 srwxr-xr-x 1 root root 0 2008-10-17 16:25 socket
[Snip]
My mistake I misread your statement in email 0. You said that squashfs
has real inode numbers and that cramfs didn't. Good luck with your
mainlining attempt. Once you get xattr support this would definitely
make life better for people who want to make SELinux enabled LiveCDs and
other small devices.
Dave
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: Phillip Lougher @ 2008-10-21 23:42 UTC (permalink / raw)
To: David P. Quigley
Cc: akpm, linux-embedded, linux-fsdevel, linux-kernel, tim.bird
In-Reply-To: <1224605348.31157.67.camel@moss-terrapins.epoch.ncsc.mil>
David P. Quigley wrote:
>
> Looking through the code I noticed that you give certain object types
> the same inode number for all instances of it (devices, fifo/sockets).
> How is this done internally? Do these types of objects occupy the same
> position on the inode table? If so how do you differentiate between a
> device and a socket?
>
No, devices and fifo/sockets get their own unique inode numbers:
root@slackware:/mnt# mount -t squashfs test.sqsh /mnt2 -o loop
root@slackware:/mnt# ls -li /mnt2
total 0
2 crw-r--r-- 1 root root 1, 1 2008-10-22 00:31 device
4 prw-r--r-- 1 root root 0 2008-10-22 00:31 fifo
3 srwxr-xr-x 1 root root 0 2008-10-17 16:25 socket
struct squashfs_ipc_inode {
__le16 inode_type;
__le16 mode;
__le16 uid;
__le16 guid;
__le32 mtime;
__le32 inode_number;
__le32 nlink;
};
struct squashfs_dev_inode {
__le16 inode_type;
__le16 mode;
__le16 uid;
__le16 guid;
__le32 mtime;
__le32 inode_number;
__le32 nlink;
__le32 rdev;
};
> I use to work on Unionfs and we used CVS initially for our SCM. When we
> started working on mainlining Unionfs we moved over to a GIT based
> system and we found it worked a lot better. You might want to consider
> moving your patches to a GIT tree that you make publically available so
> people can just clone, compile, and test them. I don't see anything that
> stops Squashfs from being compiled and loaded as a module so it might
> not be necessary but it makes it easier for people who want to test the
> code or even contribute patches.
Yeah, Git is much better than CVS, however, I've got nowhere to host a
public Git repository. If someone were to offer hosting I'd be only too
happy to move over to Git.
Phillip
^ permalink raw reply
* Re: Subject: [PATCH 00/16] Squashfs: compressed read-only filesystem
From: Peter Korsgaard @ 2008-10-22 7:21 UTC (permalink / raw)
To: Phillip Lougher
Cc: David P. Quigley, akpm, linux-embedded, linux-fsdevel,
linux-kernel, tim.bird
In-Reply-To: <48FE68E9.2020401@lougher.demon.co.uk>
>>>>> "Phillip" == Phillip Lougher <phillip@lougher.demon.co.uk> writes:
Hi,
Phillip> Yeah, Git is much better than CVS, however, I've got nowhere
Phillip> to host a public Git repository. If someone were to offer
Phillip> hosting I'd be only too happy to move over to Git.
You could apply for a kernel.org account:
http://kernel.org/faq/#account
--
Bye, Peter Korsgaard
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox