mbox series

[v6,0/5] Add TI PRUSS Local Interrupt Controller IRQChip driver

Message ID 1600167651-20851-1-git-send-email-grzegorz.jaszczyk@linaro.org
Headers show
Series Add TI PRUSS Local Interrupt Controller IRQChip driver | expand

Message

Grzegorz Jaszczyk Sept. 15, 2020, 11 a.m. UTC
Hi All,

The following is a v6 version of the series [1-5] that adds an IRQChip
driver for the local interrupt controller present within a Programmable
Real-Time Unit and Industrial Communication Subsystem (PRU-ICSS) present on a
number of TI SoCs including OMAP architecture based AM335x, AM437x, AM57xx SoCs,
Keystone 2 architecture based 66AK2G SoCs, Davinci architecture based
OMAP-L138/DA850 SoCs and the latest K3 architecture based AM65x and J721E SoCs.
Please see the v1 cover-letter [1] for details about the features of this
interrupt controller.  More details can be found in any of the supported SoC
TRMs.  Eg: Chapter 30.1.6 of AM5728 TRM [6]

Please see the individual patches for exact changes in each patch, following are
the main changes from v5:
 - Improve patch #2 with regards to various Marc Zyngier comments.
 - Drop example from commit description of patch #4.

[1] https://patchwork.kernel.org/cover/11034561/
[2] https://patchwork.kernel.org/cover/11069749/
[3] https://patchwork.kernel.org/cover/11639055/
[4] https://patchwork.kernel.org/cover/11688727/
[5] https://patchwork.kernel.org/cover/11746463/
[6] http://www.ti.com/lit/pdf/spruhz6

Best regards
Grzegorz

David Lechner (1):
  irqchip/irq-pruss-intc: Implement irq_{get,set}_irqchip_state ops

Grzegorz Jaszczyk (1):
  irqchip/irq-pruss-intc: Add a PRUSS irqchip driver for PRUSS
    interrupts

Suman Anna (3):
  dt-bindings: irqchip: Add PRU-ICSS interrupt controller bindings
  irqchip/irq-pruss-intc: Add logic for handling reserved interrupts
  irqchip/irq-pruss-intc: Add support for ICSSG INTC on K3 SoCs

 .../interrupt-controller/ti,pruss-intc.yaml        | 158 +++++
 drivers/irqchip/Kconfig                            |  10 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-pruss-intc.c                   | 664 +++++++++++++++++++++
 4 files changed, 833 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml
 create mode 100644 drivers/irqchip/irq-pruss-intc.c

-- 
2.7.4

Comments

Grzegorz Jaszczyk Sept. 16, 2020, 3:13 p.m. UTC | #1
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