All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Y Vo <yvo@apm.com>
Cc: Laxman Dewangan <ldewangan@nvidia.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, Phong Vo <pvo@apm.com>,
	Toan Le <toanle@apm.com>, Loc Ho <lho@apm.com>,
	Feng Kan <fkan@apm.com>, Quan Nguyen <qnguyen@apm.com>,
	Duc Dang <dhdang@apm.com>, patches <patches@apm.com>
Subject: Re: [PATCH 2/2] Documentation: gpio: Update description for X-Gene standby GPIO controller DTS binding
Date: Mon, 14 Sep 2015 17:18:03 +0200	[thread overview]
Message-ID: <8906562.oG8T3AYRWq@wuerfel> (raw)
In-Reply-To: <CAL4ahLdOtJ=W2xZbORYFi9ocnqB3oX0OzfC9zGiarBezjuxGwQ@mail.gmail.com>

On Monday 14 September 2015 22:06:15 Y Vo wrote:
> >
> > Did 1b7047edfcfb25 ("genirq: Allow the irqchip state of an IRQ to be
> > save/restored") not address the problem for you? You were on
> > Cc to that patch and should have spoken up when the code that was
> > merged was not sufficient.
> 
> Yes, I am in this mail-list too, but I also had a issue on this, I
> think you are still in my submitted for this.
> Currently, irq_get|set_irqchip_state(..) supports access to
> GIC_DIST_ENABLE_SET, GIC_DIST_ACTIVE_SET, GIC_DIST_PENDING_SET. But
> our hw only has the valid value at SPISR register ("[PATCH v4 2/3]
> irqchip: GIC: Add support for irq_{get,set}_irqchip_state"), so I
> still can not use it.

Ok.

> >> >> > It also seems to me that the binding cannot distinguish between a
> >> >> > line configured as an input and one that is configured as an
> >> >> > interrupt, which are for other gpio chips the same thing, but
> >> >> > not on this one.  Could this be rectified by using another bit
> >> >> > of the second gpio cell? The low bit is used for active-high/active-low,
> >> >> > so you could use the second bit for irq/input.
> >> >> >
> >> >> Do you mean #gpio-cells property and using the high bit of the second
> >> >> bit for irq/input  ?
> >> >
> >> > Yes, that would be an option.
> >> I will look into it.
> >>
> >> Is there possible if:
> >> - Keep GPIO8..GPIO as interrupt by default.
> >> - Anyone want to use these GPIO pins as GPIO, we will re-configure
> >> them to GPIO mode ?
> >
> > That's not perfect but better than the patch you sent here.
> > The main disadvantage is that you end up with two references
> > to the same IRQ. It can still work, but only as long as nothing
> > tries to walk the entire DT to parse all the interrupts properties.
> >
> Let me think how.
> 
> > It would be ok for gpio-keys, as that does not need both the state
> > and the event together, but for other gpio users, you still need a
> > working driver that supports reading the state and getting an
> > interrupt.
> >
> In irq mode, if I reconfigured that gpio pin to gpio mode, then
> reading -> the value is valid.
> Could I do that way badly ? It means switch to gpio mode to read
> value, then switch back to  irq mode.

I don't see any downsides of this at the moment, other than it being
a bit slow. As long as we don't try to do any high-speed communication
over this gpio line, that seems like the best workaround given the
various constraints of the hardware.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] Documentation: gpio: Update description for X-Gene standby GPIO controller DTS binding
Date: Mon, 14 Sep 2015 17:18:03 +0200	[thread overview]
Message-ID: <8906562.oG8T3AYRWq@wuerfel> (raw)
In-Reply-To: <CAL4ahLdOtJ=W2xZbORYFi9ocnqB3oX0OzfC9zGiarBezjuxGwQ@mail.gmail.com>

On Monday 14 September 2015 22:06:15 Y Vo wrote:
> >
> > Did 1b7047edfcfb25 ("genirq: Allow the irqchip state of an IRQ to be
> > save/restored") not address the problem for you? You were on
> > Cc to that patch and should have spoken up when the code that was
> > merged was not sufficient.
> 
> Yes, I am in this mail-list too, but I also had a issue on this, I
> think you are still in my submitted for this.
> Currently, irq_get|set_irqchip_state(..) supports access to
> GIC_DIST_ENABLE_SET, GIC_DIST_ACTIVE_SET, GIC_DIST_PENDING_SET. But
> our hw only has the valid value at SPISR register ("[PATCH v4 2/3]
> irqchip: GIC: Add support for irq_{get,set}_irqchip_state"), so I
> still can not use it.

Ok.

> >> >> > It also seems to me that the binding cannot distinguish between a
> >> >> > line configured as an input and one that is configured as an
> >> >> > interrupt, which are for other gpio chips the same thing, but
> >> >> > not on this one.  Could this be rectified by using another bit
> >> >> > of the second gpio cell? The low bit is used for active-high/active-low,
> >> >> > so you could use the second bit for irq/input.
> >> >> >
> >> >> Do you mean #gpio-cells property and using the high bit of the second
> >> >> bit for irq/input  ?
> >> >
> >> > Yes, that would be an option.
> >> I will look into it.
> >>
> >> Is there possible if:
> >> - Keep GPIO8..GPIO as interrupt by default.
> >> - Anyone want to use these GPIO pins as GPIO, we will re-configure
> >> them to GPIO mode ?
> >
> > That's not perfect but better than the patch you sent here.
> > The main disadvantage is that you end up with two references
> > to the same IRQ. It can still work, but only as long as nothing
> > tries to walk the entire DT to parse all the interrupts properties.
> >
> Let me think how.
> 
> > It would be ok for gpio-keys, as that does not need both the state
> > and the event together, but for other gpio users, you still need a
> > working driver that supports reading the state and getting an
> > interrupt.
> >
> In irq mode, if I reconfigured that gpio pin to gpio mode, then
> reading -> the value is valid.
> Could I do that way badly ? It means switch to gpio mode to read
> value, then switch back to  irq mode.

I don't see any downsides of this at the moment, other than it being
a bit slow. As long as we don't try to do any high-speed communication
over this gpio line, that seems like the best workaround given the
various constraints of the hardware.

	Arnd

  reply	other threads:[~2015-09-14 15:18 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-11  9:22 [PATCH 0/2] gpio: xgene: add support to configure GPIO line as input, output or external IRQ pin Y Vo
2015-09-11  9:22 ` Y Vo
2015-09-11  9:22 ` [PATCH 1/2] " Y Vo
2015-09-11  9:22   ` Y Vo
2015-10-02  9:51   ` Linus Walleij
2015-10-02  9:51     ` Linus Walleij
2015-10-02 14:03     ` Y Vo
2015-10-02 14:03       ` Y Vo
2015-09-11  9:22 ` [PATCH 2/2] Documentation: gpio: Update description for X-Gene standby GPIO controller DTS binding Y Vo
2015-09-11  9:22   ` Y Vo
2015-09-11  9:35   ` Arnd Bergmann
2015-09-11  9:35     ` Arnd Bergmann
2015-09-11 11:24     ` Y Vo
2015-09-11 11:24       ` Y Vo
2015-09-11 12:46       ` Arnd Bergmann
2015-09-11 12:46         ` Arnd Bergmann
2015-09-11 14:23         ` Y Vo
2015-09-11 14:23           ` Y Vo
2015-09-11 14:47           ` Arnd Bergmann
2015-09-11 14:47             ` Arnd Bergmann
2015-09-11 15:06             ` Y Vo
2015-09-11 15:06               ` Y Vo
2015-09-11 16:45               ` Arnd Bergmann
2015-09-11 16:45                 ` Arnd Bergmann
2015-09-12  5:55                 ` Y Vo
2015-09-12  5:55                   ` Y Vo
2015-09-14  9:11                   ` Arnd Bergmann
2015-09-14  9:11                     ` Arnd Bergmann
2015-09-14  9:39                     ` Y Vo
2015-09-14  9:39                       ` Y Vo
2015-09-14 14:47                       ` Arnd Bergmann
2015-09-14 14:47                         ` Arnd Bergmann
2015-09-14 15:06                         ` Y Vo
2015-09-14 15:06                           ` Y Vo
2015-09-14 15:18                           ` Arnd Bergmann [this message]
2015-09-14 15:18                             ` Arnd Bergmann
2015-09-14 15:23                           ` Marc Zyngier
2015-09-14 15:23                             ` Marc Zyngier
2015-09-16  1:48                             ` Y Vo
2015-09-16  1:48                               ` Y Vo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8906562.oG8T3AYRWq@wuerfel \
    --to=arnd@arndb.de \
    --cc=devicetree@vger.kernel.org \
    --cc=dhdang@apm.com \
    --cc=fkan@apm.com \
    --cc=ldewangan@nvidia.com \
    --cc=lho@apm.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=patches@apm.com \
    --cc=pvo@apm.com \
    --cc=qnguyen@apm.com \
    --cc=toanle@apm.com \
    --cc=yvo@apm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.