From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Herring Subject: Re: [PATCH v2 1/2] doc/devicetree: Add Aspeed VIC bindings Date: Fri, 20 May 2016 15:13:31 -0500 Message-ID: References: <462797239-14765-1-git-send-email-joel@jms.id.au> <1462802317-27086-1-git-send-email-joel@jms.id.au> <1462802317-27086-2-git-send-email-joel@jms.id.au> <20160511143313.GA7426@rob-hp-laptop> <1463014882.20290.203.camel@kernel.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Joel Stanley Cc: Benjamin Herrenschmidt , Thomas Gleixner , Jason Cooper , Marc Zyngier , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , Jeremy Kerr , Arnd Bergmann , Baruch Siach List-Id: devicetree@vger.kernel.org On Wed, May 18, 2016 at 8:50 AM, Joel Stanley wrote: > On Fri, May 13, 2016 at 2:50 AM, Rob Herring wrote: >> On Wed, May 11, 2016 at 8:01 PM, Benjamin Herrenschmidt >> wrote: >>> On Wed, 2016-05-11 at 09:33 -0500, Rob Herring wrote: >>>> > +- interrupt-controller : Identifies the node as an interrupt controller >>>> > +- #interrupt-cells : Specifies the number of cells needed to encode an >>>> > + interrupt source. The value shall be 1. >>>> >>>> No need for level vs. edge flags? >>> >>> That's an open question. Most interrupts are fixed. A handful of GPIOs >>> can be configured either way. For now I am relying on uboot setting up >>> the right config for them and I read it back at boot time, but we could >>> make it part of the binding I suppose. >> >> It is almost standard to just use 2 cells here even if reserved for >> future use. Especially since the IP block seems to have registers to >> control that. > > Yep, makes sense. I've added this to the bindings document. > > I was trying to use the second cell in our driver: > > static struct irq_domain_ops avic_dom_ops = { > .map = avic_map, > .xlate = irq_domain_xlate_twocell, > }; > > So we have irq_domain_xlate_twocell to parse the device tree and > grabs the type property from the second cell. > > In avic_map we set the irq handler: > > if (vic->edge_sources[sidx] & sbit) { > /* TODO: Check aginst type from dt and warn if not edge */ > irq_set_chip_and_handler(irq, &avic_chip, handle_edge_irq); > } else { > /* TODO: Check aginst type from dt and warn if not level */ > irq_set_chip_and_handler(irq, &avic_chip, handle_level_irq); > } > > However, we don't have the type here, so we can't use it to check that > we're setting the correct edge/level callback (or use it to set the > register in the future if we want to use the device tree to override > the bootloader settings). > > I had a look at some other drivers and they don't seem to use the dt > property when mapping the irq. What am I missing here? The type will be set by the irqdomain core when the mapping is created and this should result in irq_set_type() being called on your irqchip. The map function doesn't need to deal with type. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html