From: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
To: Marc Zyngier <maz@kernel.org>
Cc: tglx@linutronix.de, jason@lakedaemon.net, "Anna,
Suman" <s-anna@ti.com>,
robh+dt@kernel.org, Lee Jones <lee.jones@linaro.org>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
David Lechner <david@lechnology.com>,
"Bajjuri, Praneeth" <praneeth@ti.com>,
Roger Quadros <rogerq@ti.com>
Subject: Re: [PATCH v6 2/5] irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts
Date: Wed, 16 Sep 2020 17:13:46 +0200 [thread overview]
Message-ID: <CAMxfBF6=eu+o8ZLOra16X8M=Yv4QmSgr1umk2hVO7jANEfN9Jw@mail.gmail.com> (raw)
In-Reply-To: <ab6858f56cf47e48f167d6893bcd3043@kernel.org>
On Tue, 15 Sep 2020 at 17:19, Marc Zyngier <maz@kernel.org> wrote:
>
> [ Dropping afd@ti.com from the Cc list, as this address bounces]
>
> On 2020-09-15 12:00, Grzegorz Jaszczyk wrote:
> > The Programmable Real-Time Unit Subsystem (PRUSS) contains a local
> > interrupt controller (INTC) that can handle various system input events
> > and post interrupts back to the device-level initiators. The INTC can
> > support upto 64 input events with individual control configuration and
> > hardware prioritization. These events are mapped onto 10 output
> > interrupt
> > lines through two levels of many-to-one mapping support. Different
> > interrupt lines are routed to the individual PRU cores or to the host
> > CPU, or to other devices on the SoC. Some of these events are sourced
> > from peripherals or other sub-modules within that PRUSS, while a few
> > others are sourced from SoC-level peripherals/devices.
> >
> > The PRUSS INTC platform driver manages this PRUSS interrupt controller
> > and implements an irqchip driver to provide a Linux standard way for
> > the PRU client users to enable/disable/ack/re-trigger a PRUSS system
> > event. The system events to interrupt channels and output interrupts
> > relies on the mapping configuration provided either through the PRU
> > firmware blob (for interrupts routed to PRU cores) or via the PRU
> > application's device tree node (for interrupt routed to the main CPU).
> > In the first case the mappings will be programmed on PRU remoteproc
> > driver demand (via irq_create_fwspec_mapping) during the boot of a PRU
> > core and cleaned up after the PRU core is stopped.
> >
> > Reference counting is used to allow multiple system events to share a
> > single channel and to allow multiple channels to share a single host
> > event.
> >
> > The PRUSS INTC module is reference counted during the interrupt
> > setup phase through the irqchip's irq_request_resources() and
> > irq_release_resources() ops. This restricts the module from being
> > removed as long as there are active interrupt users.
> >
> > The driver currently supports and can be built for OMAP architecture
> > based AM335x, AM437x and AM57xx SoCs; Keystone2 architecture based
> > 66AK2G SoCs and Davinci architecture based OMAP-L13x/AM18x/DA850 SoCs.
> > All of these SoCs support 64 system events, 10 interrupt channels and
> > 10 output interrupt lines per PRUSS INTC with a few SoC integration
> > differences.
> >
> > NOTE:
> > Each PRU-ICSS's INTC on AM57xx SoCs is preceded by a Crossbar that
> > enables multiple external events to be routed to a specific number
> > of input interrupt events. Any non-default external interrupt event
> > directed towards PRUSS needs this crossbar to be setup properly.
> >
> > Signed-off-by: Suman Anna <s-anna@ti.com>
> > Signed-off-by: Andrew F. Davis <afd@ti.com>
> > Signed-off-by: Roger Quadros <rogerq@ti.com>
> > Signed-off-by: David Lechner <david@lechnology.com>
> > Signed-off-by: Grzegorz Jaszczyk <grzegorz.jaszczyk@linaro.org>
>
> Please see the use of the Co-developed-by: tag.
Ok, thank you.
>
> > ---
> > v5->v6:
> > 1) Address Marc Zyngier comments:
> > - Use unsigned types for variables used to compute masks/shifts (ch,
> > evt, host).
> > - Move part responsible for enabling global interrupt from
> > pruss_intc_map to pruss_intc_init.
> > - Improve coding style in pruss_intc_init with regards to variable
> > assignments.
> > - Align the '=' signs vertically in pruss_irqchip structure.
> > - Change the irq type in xlate handler from IRQ_TYPE_NONE to
> > IRQ_TYPE_LEVEL_MASK
>
> Gruik? (yes, that's approximately the noise I made reading this)
>
> [...]
>
> > +static void pruss_intc_init(struct pruss_intc *intc)
> > +{
> > + const struct pruss_intc_match_data *soc_config = intc->soc_config;
> > + int num_chnl_map_regs, num_host_intr_regs, num_event_type_regs, i;
> > +
> > + num_chnl_map_regs = DIV_ROUND_UP(soc_config->num_system_events,
> > + CMR_EVT_PER_REG);
> > + num_host_intr_regs = DIV_ROUND_UP(soc_config->num_host_events,
> > + HMR_CH_PER_REG);
> > + num_event_type_regs = DIV_ROUND_UP(soc_config->num_system_events,
> > 32);
> > +
> > + /*
> > + * configure polarity (SIPR register) to active high and
> > + * type (SITR register) to level interrupt for all system events
> > + */
>
> So I read this...
>
> [...]
>
> > +static int
> > +pruss_intc_irq_domain_xlate(struct irq_domain *d, struct device_node
> > *node,
> > + const u32 *intspec, unsigned int intsize,
> > + unsigned long *out_hwirq, unsigned int *out_type)
> > +{
> > + struct pruss_intc *intc = d->host_data;
> > + struct device *dev = intc->dev;
> > + int ret, sys_event, channel, host;
> > +
> > + if (intsize < 3)
> > + return -EINVAL;
> > +
> > + sys_event = intspec[0];
> > + if (sys_event < 0 || sys_event >=
> > intc->soc_config->num_system_events) {
> > + dev_err(dev, "%d is not valid event number\n", sys_event);
> > + return -EINVAL;
> > + }
> > +
> > + channel = intspec[1];
> > + if (channel < 0 || channel >= intc->soc_config->num_host_events) {
> > + dev_err(dev, "%d is not valid channel number", channel);
> > + return -EINVAL;
> > + }
> > +
> > + host = intspec[2];
> > + if (host < 0 || host >= intc->soc_config->num_host_events) {
> > + dev_err(dev, "%d is not valid host irq number\n", host);
> > + return -EINVAL;
> > + }
> > +
> > + /* check if requested sys_event was already mapped, if so validate it
> > */
> > + ret = pruss_intc_validate_mapping(intc, sys_event, channel, host);
> > + if (ret)
> > + return ret;
> > +
> > + *out_hwirq = sys_event;
> > + *out_type = IRQ_TYPE_LEVEL_MASK;
>
> ... and then I see that.
>
> What does IRQ_TYPE_LEVEL_MASK even mean? Can the interrupt be triggered
> as
> level high and low *at the same time*? (this is a rhetorical question).
Really sorry for that, my mistake. I will change it to
IRQ_TYPE_LEVEL_HIGH in v7.
Thank you for your review,
Grzegorz
next prev parent reply other threads:[~2020-09-16 21:05 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-15 11:00 [PATCH v6 0/5] Add TI PRUSS Local Interrupt Controller IRQChip driver Grzegorz Jaszczyk
2020-09-15 11:00 ` [PATCH v6 1/5] dt-bindings: irqchip: Add PRU-ICSS interrupt controller bindings Grzegorz Jaszczyk
2020-09-15 11:00 ` [PATCH v6 2/5] irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS interrupts Grzegorz Jaszczyk
2020-09-15 15:19 ` Marc Zyngier
2020-09-16 15:13 ` Grzegorz Jaszczyk [this message]
2020-09-15 11:00 ` [PATCH v6 3/5] irqchip/irq-pruss-intc: Add logic for handling reserved interrupts Grzegorz Jaszczyk
2020-09-15 11:00 ` [PATCH v6 4/5] irqchip/irq-pruss-intc: Implement irq_{get,set}_irqchip_state ops Grzegorz Jaszczyk
2020-09-15 11:00 ` [PATCH v6 5/5] irqchip/irq-pruss-intc: Add support for ICSSG INTC on K3 SoCs Grzegorz Jaszczyk
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='CAMxfBF6=eu+o8ZLOra16X8M=Yv4QmSgr1umk2hVO7jANEfN9Jw@mail.gmail.com' \
--to=grzegorz.jaszczyk@linaro.org \
--cc=david@lechnology.com \
--cc=devicetree@vger.kernel.org \
--cc=jason@lakedaemon.net \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=maz@kernel.org \
--cc=praneeth@ti.com \
--cc=robh+dt@kernel.org \
--cc=rogerq@ti.com \
--cc=s-anna@ti.com \
--cc=tglx@linutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).