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

[Sören Brinkmann <soren.brinkmann@xxxxxxxxxx>]

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:
> > 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 <soren.brinkmann@xxxxxxxxxx>
> > Reviewed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> > ---
> > 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.

> 
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html