[PATCH v4] gpio: lib-sysfs: Add 'wakeup' attribute

Add an attribute 'wakeup' to the GPIO sysfs interface which allows
marking/unmarking a GPIO as wake IRQ.
The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
is associated with that GPIO and the irqchip implements set_wake().
Writing 'enabled' to that file will enable wake for that GPIO, while
writing 'disabled' will disable wake.
Reading that file will return either 'disabled' or 'enabled' depening on
the currently set flag for the GPIO's IRQ.

Signed-off-by: Soren Brinkmann <redacted>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
---
Hi Linus, Johan,

I rebased my patch. And things look good. But the 'is_visible' things does not
behave the way I expected it to. It seems to be only triggered on an export but
not when attributes change. Hence, in my case, everything was visiible since the
inital state matches that, but even when changing the direction or things like
that, attributes don't disappear. Is that something still worked on? Expected
behavior?

	Thanks,
	Sören

v4:
 - rebased onto gpio/fixes
   - fit into the new attribute framework
   - extend is_visible to limit the wakeup attributes visibility
     according to usability
v3:
 - add documentation
v2:
 - fix error path to unlock mutex before return
---
 Documentation/ABI/testing/sysfs-gpio |  1 +
 Documentation/gpio/sysfs.txt         |  8 +++++
 drivers/gpio/gpiolib-sysfs.c         | 62 ++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-gpio b/Documentation/ABI/testing/sysfs-gpio
index 80f4c94c7bef..4cc7f4b3f724 100644
--- a/Documentation/ABI/testing/sysfs-gpio
+++ b/Documentation/ABI/testing/sysfs-gpio

@@ -20,6 +20,7 @@ Description:
 	    /value ... always readable, writes fail for input GPIOs
 	    /direction ... r/w as: in, out (default low); write: high, low
 	    /edge ... r/w as: none, falling, rising, both
+	    /wakeup ... r/w as: enabled, disabled
 	/gpiochipN ... for each gpiochip; #N is its first GPIO
 	    /base ... (r/o) same as N
 	    /label ... (r/o) descriptive, not necessarily unique

diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
index c2c3a97f8ff7..f703377d528f 100644
--- a/Documentation/gpio/sysfs.txt
+++ b/Documentation/gpio/sysfs.txt

@@ -97,6 +97,14 @@ and have the following read/write attributes:
 		for "rising" and "falling" edges will follow this
 		setting.
 
+	"wakeup" ... reads as either "enabled" or "disabled". Write these
+		strings to set/clear the 'wakeup' flag of the IRQ associated
+		with this GPIO. If the IRQ has the 'wakeup' flag set, it can
+		wake the system from sleep states.
+
+		This file exists only if the pin can generate interrupts and
+		the driver implements the required infrastructure.
+
 GPIO controllers have paths like /sys/class/gpio/gpiochip42/ (for the
 controller implementing GPIOs starting at #42) and have the following
 read-only attributes:

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index f62aa115d79a..14b34d15e61c 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c

@@ -286,6 +286,58 @@ found:
 
 static DEVICE_ATTR(edge, 0644, gpio_edge_show, gpio_edge_store);
 
+static ssize_t gpio_wakeup_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	ssize_t			status;
+	const struct gpio_desc	*desc = dev_get_drvdata(dev);
+	int			irq = gpiod_to_irq(desc);
+	struct irq_desc		*irq_desc = irq_to_desc(irq);
+
+	mutex_lock(&sysfs_lock);
+
+	if (irqd_is_wakeup_set(&irq_desc->irq_data))
+		status = sprintf(buf, "enabled\n");
+	else
+		status = sprintf(buf, "disabled\n");
+
+	mutex_unlock(&sysfs_lock);
+
+	return status;
+}
+
+static ssize_t gpio_wakeup_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	int			ret;
+	unsigned int		on;
+	struct gpio_desc	*desc = dev_get_drvdata(dev);
+	int			irq = gpiod_to_irq(desc);
+
+	mutex_lock(&sysfs_lock);
+
+	if (sysfs_streq("enabled", buf)) {
+		on = true;
+	} else if (sysfs_streq("disabled", buf)) {
+		on = false;
+	} else {
+		mutex_unlock(&sysfs_lock);
+		return -EINVAL;
+	}
+
+	ret = irq_set_irq_wake(irq, on);
+
+	mutex_unlock(&sysfs_lock);
+
+	if (ret)
+		pr_warn("%s: failed to %s wake\n", __func__,
+				on ? "enable" : "disable");
+
+	return size;
+}
+
+static DEVICE_ATTR(wakeup, 0644, gpio_wakeup_show, gpio_wakeup_store);
+
 static int sysfs_set_active_low(struct gpio_desc *desc, struct device *dev,
 				int value)
 {

@@ -361,6 +413,7 @@ static umode_t gpio_is_visible(struct kobject *kobj, struct attribute *attr,
 {
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct gpio_desc *desc = dev_get_drvdata(dev);
+	struct irq_chip *irqchip = desc->chip->irqchip;
 	umode_t mode = attr->mode;
 	bool show_direction = test_bit(FLAG_SYSFS_DIR, &desc->flags);
 

@@ -372,6 +425,14 @@ static umode_t gpio_is_visible(struct kobject *kobj, struct attribute *attr,
 			mode = 0;
 		if (!show_direction && test_bit(FLAG_IS_OUT, &desc->flags))
 			mode = 0;
+	} else if (attr == &dev_attr_wakeup.attr) {
+		if (gpiod_to_irq(desc) < 0)
+			mode = 0;
+		if (!show_direction && test_bit(FLAG_IS_OUT, &desc->flags))
+			mode = 0;
+		if (!test_bit(IRQCHIP_SKIP_SET_WAKE, &irqchip->flags) &&
+				!irqchip->irq_set_wake)
+			mode = 0;
 	}
 
 	return mode;

@@ -382,6 +443,7 @@ static struct attribute *gpio_attrs[] = {
 	&dev_attr_edge.attr,
 	&dev_attr_value.attr,
 	&dev_attr_active_low.attr,
+	&dev_attr_wakeup.attr,
 	NULL,
 };
 

-- 
2.2.1.1.gb42cc81

On Thu, Jan 15, 2015 at 11:49:49AM -0800, Soren Brinkmann wrote:

Add an attribute 'wakeup' to the GPIO sysfs interface which allows
marking/unmarking a GPIO as wake IRQ.
The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
is associated with that GPIO and the irqchip implements set_wake().
Writing 'enabled' to that file will enable wake for that GPIO, while
writing 'disabled' will disable wake.
Reading that file will return either 'disabled' or 'enabled' depening on
the currently set flag for the GPIO's IRQ.

Signed-off-by: Soren Brinkmann <redacted>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
---
Hi Linus, Johan,

I rebased my patch. And things look good.

I took at closer look at this patch now and I really don't think it
should be merged at all.

We have a mechanism for handling wake-up sources (documented in
Documentation/power/devices.txt) as well as an ABI to enable/disable
them using the power/wakeup device attribute from userspace.

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

[ And we really shouldn't be adding anything to the broken gpio sysfs
interface until it's been redesigned. ]

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

But the 'is_visible' things does not behave the way I expected it to.
It seems to be only triggered on an export but not when attributes
change. Hence, in my case, everything was visiible since the inital
state matches that, but even when changing the direction or things
like that, attributes don't disappear. Is that something still worked
on? Expected

That's expected. We generally don't want attributes to appear or
disappear after the device has been registered (although there is a
mechanism for cases were it makes sense). This is no different from
how your v3 patch worked either.

Johan

On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

On Thu, Jan 15, 2015 at 11:49:49AM -0800, Soren Brinkmann wrote:

quoted

Add an attribute 'wakeup' to the GPIO sysfs interface which allows
marking/unmarking a GPIO as wake IRQ.
The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
is associated with that GPIO and the irqchip implements set_wake().
Writing 'enabled' to that file will enable wake for that GPIO, while
writing 'disabled' will disable wake.
Reading that file will return either 'disabled' or 'enabled' depening on
the currently set flag for the GPIO's IRQ.

Signed-off-by: Soren Brinkmann <redacted>
Reviewed-by: Alexandre Courbot <redacted>
---
Hi Linus, Johan,

I rebased my patch. And things look good.

I took at closer look at this patch now and I really don't think it
should be merged at all.

We have a mechanism for handling wake-up sources (documented in
Documentation/power/devices.txt) as well as an ABI to enable/disable
them using the power/wakeup device attribute from userspace.

Doesn't work for GPIOs AFAIK.

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

These are claimed, by the sysfs interface.

[ And we really shouldn't be adding anything to the broken gpio sysfs
interface until it's been redesigned. ]

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

We had that discussion and I don't think GPIO keys is the right solution
for every use-case.

quoted

But the 'is_visible' things does not behave the way I expected it to.
It seems to be only triggered on an export but not when attributes
change. Hence, in my case, everything was visiible since the inital
state matches that, but even when changing the direction or things
like that, attributes don't disappear. Is that something still worked
on? Expected

That's expected. We generally don't want attributes to appear or
disappear after the device has been registered (although there is a
mechanism for cases were it makes sense). This is no different from
how your v3 patch worked either.

Sure, but the is_visible thing is effectively broken for GPIO. I think a
GPIO is in a defined state when exported and the checks all work on that
state during export. But then this state can be changed through the
sysfs interface. So, if the initial state hides something it becomes
unavailable for all times and, vice versa, if the initial state makes
something visible, it will stay even when it is no longer a valid
property to change.

	Sören

On Sat, Jan 17, 2015 at 1:49 AM, Sören Brinkmann
[off-list ref] wrote:

On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

quoted

On Thu, Jan 15, 2015 at 11:49:49AM -0800, Soren Brinkmann wrote:

quoted

Add an attribute 'wakeup' to the GPIO sysfs interface which allows
marking/unmarking a GPIO as wake IRQ.
The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
is associated with that GPIO and the irqchip implements set_wake().
Writing 'enabled' to that file will enable wake for that GPIO, while
writing 'disabled' will disable wake.
Reading that file will return either 'disabled' or 'enabled' depening on
the currently set flag for the GPIO's IRQ.

Signed-off-by: Soren Brinkmann <redacted>
Reviewed-by: Alexandre Courbot <redacted>
---
Hi Linus, Johan,

I rebased my patch. And things look good.

I took at closer look at this patch now and I really don't think it
should be merged at all.

We have a mechanism for handling wake-up sources (documented in
Documentation/power/devices.txt) as well as an ABI to enable/disable
them using the power/wakeup device attribute from userspace.

Doesn't work for GPIOs AFAIK.

quoted

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

These are claimed, by the sysfs interface.

quoted

[ And we really shouldn't be adding anything to the broken gpio sysfs
interface until it's been redesigned. ]

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

We had that discussion and I don't think GPIO keys is the right solution
for every use-case.

Sorry, it has been a while - can you remind us of why?

quoted

quoted

But the 'is_visible' things does not behave the way I expected it to.
It seems to be only triggered on an export but not when attributes
change. Hence, in my case, everything was visiible since the inital
state matches that, but even when changing the direction or things
like that, attributes don't disappear. Is that something still worked
on? Expected

That's expected. We generally don't want attributes to appear or
disappear after the device has been registered (although there is a
mechanism for cases were it makes sense). This is no different from
how your v3 patch worked either.

Sure, but the is_visible thing is effectively broken for GPIO. I think a
GPIO is in a defined state when exported and the checks all work on that
state during export. But then this state can be changed through the
sysfs interface. So, if the initial state hides something it becomes
unavailable for all times and, vice versa, if the initial state makes
something visible, it will stay even when it is no longer a valid
property to change.

Is there anything that prevents you from patching other GPIO sysfs
hooks to remove/reenable the wakeup property as needed?

On Mon, Jan 19, 2015 at 5:20 AM, Alexandre Courbot [off-list ref] wrote:

On Sat, Jan 17, 2015 at 1:49 AM, Sören Brinkmann [off-list ref] wrote:

quoted

On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

quoted

quoted

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

(...)

quoted

quoted

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

We had that discussion and I don't think GPIO keys is the right solution
for every use-case.

Sorry, it has been a while - can you remind us of why?

There are such cases. Of course keys should be handled by GPIO-keys
and these will trigger the right wakeup events in such cases.

This is for more esoteric cases: we cannot have a kernel module for
everything people want to do with GPIOs, and the use case I accept
is GPIOs used in automatic control etc, think factory lines or doors.
We can't have a "door" driver or "punch arm" or "fire alarm" driver
in the kernel. Those are userspace things.

Still such embedded systems need to be able to go to idle and
sleep to conerve power, and then they need to put wakeups on
these GPIOs.

So it is a feature userspace needs, though as with much of the
sysfs ABI it is very often abused for things like keys and LEDs which
is an abomination but we can't do much about it :(

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Jan 16, 2015 at 08:49:17AM -0800, Sören Brinkmann wrote:

On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

quoted

On Thu, Jan 15, 2015 at 11:49:49AM -0800, Soren Brinkmann wrote:

quoted

Add an attribute 'wakeup' to the GPIO sysfs interface which allows
marking/unmarking a GPIO as wake IRQ.
The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
is associated with that GPIO and the irqchip implements set_wake().
Writing 'enabled' to that file will enable wake for that GPIO, while
writing 'disabled' will disable wake.
Reading that file will return either 'disabled' or 'enabled' depening on
the currently set flag for the GPIO's IRQ.

Signed-off-by: Soren Brinkmann <redacted>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
---
Hi Linus, Johan,

I rebased my patch. And things look good.

I took at closer look at this patch now and I really don't think it
should be merged at all.

We have a mechanism for handling wake-up sources (documented in
Documentation/power/devices.txt) as well as an ABI to enable/disable
them using the power/wakeup device attribute from userspace.

Doesn't work for GPIOs AFAIK.

Not today no, that's why I said it would take some work.

quoted

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

These are claimed, by the sysfs interface.

Unclaimed by a proper device and driver in the driver model.

quoted

[ And we really shouldn't be adding anything to the broken gpio sysfs
interface until it's been redesigned. ]

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

We had that discussion and I don't think GPIO keys is the right solution
for every use-case.

I can see that, but this still needs to be implemented properly and not
just as a quick hack on top of the already fragile gpio sysfs-interface.

Since pretty much everyone agrees that the current interface needs to be
replaced, we really shouldn't be adding more stuff to the broken
interface before that happens.

quoted

quoted

But the 'is_visible' things does not behave the way I expected it to.
It seems to be only triggered on an export but not when attributes
change. Hence, in my case, everything was visiible since the inital
state matches that, but even when changing the direction or things
like that, attributes don't disappear. Is that something still worked
on? Expected

That's expected. We generally don't want attributes to appear or
disappear after the device has been registered (although there is a
mechanism for cases were it makes sense). This is no different from
how your v3 patch worked either.

Sure, but the is_visible thing is effectively broken for GPIO. I think a
GPIO is in a defined state when exported and the checks all work on that
state during export. But then this state can be changed through the
sysfs interface. So, if the initial state hides something it becomes
unavailable for all times and, vice versa, if the initial state makes
something visible, it will stay even when it is no longer a valid
property to change.

Again, this is exactly how the interface has always worked, and that's
exactly how your v3, which added the attributes manually, also worked.

The group-visibility mechanism is not broken. What's broken is interface
designs based on attributes magically disappearing and reappearing after
the device has been created.

Johan

Hi Linus,

On Mon, 2015-01-19 at 09:54AM +0100, Linus Walleij wrote:

On Mon, Jan 19, 2015 at 5:20 AM, Alexandre Courbot [off-list ref] wrote:

quoted

On Sat, Jan 17, 2015 at 1:49 AM, Sören Brinkmann [off-list ref] wrote:

quoted

On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

quoted

quoted

quoted

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

(...)

quoted

quoted

quoted

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

We had that discussion and I don't think GPIO keys is the right solution
for every use-case.

Sorry, it has been a while - can you remind us of why?

There are such cases. Of course keys should be handled by GPIO-keys
and these will trigger the right wakeup events in such cases.

This is for more esoteric cases: we cannot have a kernel module for
everything people want to do with GPIOs, and the use case I accept
is GPIOs used in automatic control etc, think factory lines or doors.
We can't have a "door" driver or "punch arm" or "fire alarm" driver
in the kernel. Those are userspace things.

Still such embedded systems need to be able to go to idle and
sleep to conerve power, and then they need to put wakeups on
these GPIOs.

So it is a feature userspace needs, though as with much of the
sysfs ABI it is very often abused for things like keys and LEDs which
is an abomination but we can't do much about it :(

Thanks for clearing that up.
What does that mean for this patch? Are we going ahead, accepting the
extension of this API or do all these use-cases have to wait for the
rewrite of a proper GPIO userspace interface?

	Thanks,
	Sören
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Jan 29, 2015 at 6:23 PM, Sören Brinkmann
[off-list ref] wrote:

On Mon, 2015-01-19 at 09:54AM +0100, Linus Walleij wrote:

quoted

On Mon, Jan 19, 2015 at 5:20 AM, Alexandre Courbot [off-list ref] wrote:

quoted

On Sat, Jan 17, 2015 at 1:49 AM, Sören Brinkmann [off-list ref] wrote:

quoted

On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

quoted

quoted

quoted

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

(...)

quoted

quoted

quoted

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

We had that discussion and I don't think GPIO keys is the right solution
for every use-case.

Sorry, it has been a while - can you remind us of why?

There are such cases. Of course keys should be handled by GPIO-keys
and these will trigger the right wakeup events in such cases.

This is for more esoteric cases: we cannot have a kernel module for
everything people want to do with GPIOs, and the use case I accept
is GPIOs used in automatic control etc, think factory lines or doors.
We can't have a "door" driver or "punch arm" or "fire alarm" driver
in the kernel. Those are userspace things.

Still such embedded systems need to be able to go to idle and
sleep to conerve power, and then they need to put wakeups on
these GPIOs.

So it is a feature userspace needs, though as with much of the
sysfs ABI it is very often abused for things like keys and LEDs which
is an abomination but we can't do much about it :(

Thanks for clearing that up.
What does that mean for this patch? Are we going ahead, accepting the
extension of this API or do all these use-cases have to wait for the
rewrite of a proper GPIO userspace interface?

What needs to happen (IMHO) is to make gpio_chips properly obeying
the device model, and then add the attributes for fiddling around with
GPIOs to either the *real* device or create a new char device interface.
Whatever works best. These mock devices are fragile and never
worked properly especially in the removal path as Johans recent
fixes has shown.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, 2015-02-04 at 10:19AM +0100, Linus Walleij wrote:

On Thu, Jan 29, 2015 at 6:23 PM, Sören Brinkmann
[off-list ref] wrote:

quoted

On Mon, 2015-01-19 at 09:54AM +0100, Linus Walleij wrote:

quoted

On Mon, Jan 19, 2015 at 5:20 AM, Alexandre Courbot [off-list ref] wrote:

quoted

On Sat, Jan 17, 2015 at 1:49 AM, Sören Brinkmann [off-list ref] wrote:

quoted

On Fri, 2015-01-16 at 12:11PM +0100, Johan Hovold wrote:

quoted

quoted

quoted

Implementing proper wakeup support for unclaimed GPIOs would take some
work (if at all desired), but that is not a reason to be adding custom
implementations that violates the kernel's power policies and new ABIs
that would need to be maintained forever.

(...)

quoted

quoted

quoted

Meanwhile you can (should) use gpio-keys if you need to wake your system
on gpio events.

We had that discussion and I don't think GPIO keys is the right solution
for every use-case.

Sorry, it has been a while - can you remind us of why?

There are such cases. Of course keys should be handled by GPIO-keys
and these will trigger the right wakeup events in such cases.

This is for more esoteric cases: we cannot have a kernel module for
everything people want to do with GPIOs, and the use case I accept
is GPIOs used in automatic control etc, think factory lines or doors.
We can't have a "door" driver or "punch arm" or "fire alarm" driver
in the kernel. Those are userspace things.

Still such embedded systems need to be able to go to idle and
sleep to conerve power, and then they need to put wakeups on
these GPIOs.

So it is a feature userspace needs, though as with much of the
sysfs ABI it is very often abused for things like keys and LEDs which
is an abomination but we can't do much about it :(

Thanks for clearing that up.
What does that mean for this patch? Are we going ahead, accepting the
extension of this API or do all these use-cases have to wait for the
rewrite of a proper GPIO userspace interface?

What needs to happen (IMHO) is to make gpio_chips properly obeying
the device model, and then add the attributes for fiddling around with
GPIOs to either the *real* device or create a new char device interface.
Whatever works best. These mock devices are fragile and never
worked properly especially in the removal path as Johans recent
fixes has shown.

Sure, that would be a nice long-term solution. But until then this patch
would probably be welcomed by some people, without making the brokenness
of this interface much worse.

	Sören

On Wed, Feb 04, 2015 at 10:27:15AM -0800, Sören Brinkmann wrote:

On Wed, 2015-02-04 at 10:19AM +0100, Linus Walleij wrote:

quoted

What needs to happen (IMHO) is to make gpio_chips properly obeying
the device model, and then add the attributes for fiddling around with
GPIOs to either the *real* device or create a new char device interface.
Whatever works best. These mock devices are fragile and never
worked properly especially in the removal path as Johans recent
fixes has shown.

Sure, that would be a nice long-term solution. But until then this patch
would probably be welcomed by some people, without making the brokenness
of this interface much worse.

We don't knowingly add broken functionality, and especially not when it
will become ABI that needs to be maintained forever.

And "this might be useful for someone at some point" is certainly not a
reason to break that rule.

Johan