On Mon, Sep 14, 2015 at 10:23 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > On 14/09/15 16:06, Y Vo wrote: >> On Mon, Sep 14, 2015 at 9:47 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: >>> On Monday 14 September 2015 16:39:43 Y Vo wrote: >>>> On Mon, Sep 14, 2015 at 4:11 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: >>>>> On Saturday 12 September 2015 12:55:55 Y Vo wrote: >>>>>> On Fri, Sep 11, 2015 at 11:45 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: >>>>>>> On Friday 11 September 2015 22:06:58 Y Vo wrote: >>>>>> >>>>>> Example for configure GPIO_DS13 as interrupt and use as button with >>>>>> the current gpio driver: >>>>>> gpio-keys { >>>>>> compatible = "gpio-keys"; >>>>>> button@1 { >>>>>> label = "POWER"; >>>>>> linux,code = <116>; >>>>>> linux,input-type = <0x1>; >>>>>> interrupts = <0x0 0x2d 0x1>; >>>>>> }; >>>>>> }; >>>>> >>>>> Wait, this looks wrong: the gpio driver doesn't actually see >>>>> the connection here and won't be able to configure the interrupt >>>>> correctly. The interrupt is already owned by the gpio driver, so >>>>> you cannot use it in the button node. >>>> >>>> In summary: >>>> - Our GPIO doesn't support interrupt controller. >>>> - There are 6 pins which used the external interrupt from GIC, so all >>>> setup for those irqs are from gic driver. The GPIO driver only >>>> configure to wire those lines. >>>> >>>> For your concern: >>>> - That's correct: if we use that defined, the gpio driver never saw >>>> the connection here (That's why it already is configued at the >>>> beginning). >>>> - At the first time, we tried to use the define: <&sbgpio 13 1>, it >>>> means using the GPIO_DS13, it will go into the GPIO driver to setup, >>>> but there is another problem which I have sent out to all of you: >>>> + It will go into gpio_keys_setup_key (gpio_keys.c driver) function, >>>> then set the irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, >>>> but the gic only support IRQ_TYPE_LEVEL_HIGH && IRQF_TRIGGER_RISING, >>>> so it always returns failed at gpio_keys_setup_key function. Please >>>> see the gic_set_type at gic driver. >>> >>> Hmm, I see now how the event handling in the gpio-keys driver differs >>> between irq mode and gpio mode, where gpio mode relies on getting >>> a separate event for the release. This is certainly something that >>> could be changed in the gpio-keys driver as an extension, but that >>> seems to be what Laxman Dewangan did when he introduced the irq-mode. >>> >>>> static int gic_set_type(struct irq_data *d, unsigned int type) >>>> { >>>> void __iomem *base = gic_dist_base(d); >>>> unsigned int gicirq = gic_irq(d); >>>> >>>> /* Interrupt configuration for SGIs can't be changed */ >>>> if (gicirq < 16) >>>> return -EINVAL; >>>> >>>> /* SPIs have restrictions on the supported types */ >>>> if (gicirq >= 32 && type != IRQ_TYPE_LEVEL_HIGH && >>>> type != IRQ_TYPE_EDGE_RISING) >>>> return -EINVAL; >>>> >>>> return gic_configure_irq(gicirq, type, base, NULL); >>>> } >>>> + Another issue: in order gpio_key works it needs the status of GPIO. >>>> For our chip, when the GPIO is configued as interrupt, we need to >>>> access to GIC register to read the real status, it is not acceptable >>>> to implement accessing GIC registers at gpio driver. The function >>>> irq_get_irqchip_state(..) also doesn't work in our chip too. Because >>>> it needs to access different offset. >>> >>> I thought we had solved that problem long ago when you first >>> submitted the driver. >>> >>> 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. > > And I asked Feng Kan to explain *why* it doesn't work, but nobody ever > bothered giving me a straight answer on that: > > https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg892972.html > > The GIC400 TRM explicitly states that reading GICD_SPISRn is the same as > reading GICD_ICPENDRn. if that doesn't work for you please explain why. > > If I can get a reasonable explanation on *why* it doesn't work, then we > can look at having an X-Gene specific workaround. > I am checking and let you know soon. Thanks > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html