| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <54e871a8-dfa4-f65b-5869-d1661144cbf0@ti.com>
Date: Tue, 16 Jul 2019 12:21:12 -0500
From: Suman Anna <s-anna@...com>
To: David Lechner <david@...hnology.com>,
Marc Zyngier <marc.zyngier@....com>,
Rob Herring <robh+dt@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Jason Cooper <jason@...edaemon.net>
CC: Tony Lindgren <tony@...mide.com>, "Andrew F. Davis" <afd@...com>,
Roger Quadros <rogerq@...com>,
Lokesh Vutla <lokeshvutla@...com>,
Grygorii Strashko <grygorii.strashko@...com>,
Sekhar Nori <nsekhar@...com>,
Murali Karicheri <m-karicheri2@...com>,
<devicetree@...r.kernel.org>, <linux-omap@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/6] irqchip/irq-pruss-intc: Add a PRUSS irqchip driver
for PRUSS interrupts
Hi David,
On 7/11/19 11:45 AM, David Lechner wrote:
> On 7/7/19 10:52 PM, Suman Anna wrote:
>> From: "Andrew F. Davis" <afd@...com>
>>
>> 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 host interrupts
>> relies on the mapping configuration provided either through the PRU
>> firmware blob or via the PRU application's device tree node. The
>> mappings will be programmed during the boot/shutdown of a PRU core.
>>
>> 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.
>
> The all of the explanations above are very helpful. Great work.
>
>>
>> Signed-off-by: Andrew F. Davis <afd@...com>
>> Signed-off-by: Suman Anna <s-anna@...com>
>> Signed-off-by: Roger Quadros <rogerq@...com>
>> ---
>> Prior version: https://patchwork.kernel.org/patch/10795761/
>>
>> drivers/irqchip/Kconfig | 10 +
>> drivers/irqchip/Makefile | 1 +
>> drivers/irqchip/irq-pruss-intc.c | 352 +++++++++++++++++++++++++++++++
>> 3 files changed, 363 insertions(+)
>> create mode 100644 drivers/irqchip/irq-pruss-intc.c
>>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index 659c5e0fb835..b0a9479d527c 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -447,6 +447,16 @@ config TI_SCI_INTA_IRQCHIP
>> If you wish to use interrupt aggregator irq resources managed
>> by the
>> TI System Controller, say Y here. Otherwise, say N.
>> +config TI_PRUSS_INTC
>> + tristate "TI PRU-ICSS Interrupt Controller"
>> + depends on ARCH_DAVINCI || SOC_AM33XX || SOC_AM437X || SOC_DRA7XX
>> || ARCH_KEYSTONE
>> + select IRQ_DOMAIN
>> + help
>> + This enables support for the PRU-ICSS Local Interrupt Controller
>> + present within a PRU-ICSS subsystem present on various TI SoCs.
>> + The PRUSS INTC enables various interrupts to be routed to
>> multiple
>> + different processors within the SoC.
>> +
>> endmenu
>> config SIFIVE_PLIC
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 606a003a0000..717f1d49e549 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -100,3 +100,4 @@ obj-$(CONFIG_MADERA_IRQ) += irq-madera.o
>> obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
>> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
>> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
>> +obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
>> diff --git a/drivers/irqchip/irq-pruss-intc.c
>> b/drivers/irqchip/irq-pruss-intc.c
>> new file mode 100644
>> index 000000000000..d62186ad1be4
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-pruss-intc.c
>> @@ -0,0 +1,352 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PRU-ICSS INTC IRQChip driver for various TI SoCs
>> + *
>> + * Copyright (C) 2016-2019 Texas Instruments Incorporated -
>> http://www.ti.com/
>> + * Andrew F. Davis <afd@...com>
>> + * Suman Anna <s-anna@...com>
>> + */
>> +
>> +#include <linux/irq.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +
>> +/*
>> + * Number of host interrupts reaching the main MPU sub-system. Note
>> that this
>> + * is not the same as the total number of host interrupts supported
>> by the PRUSS
>> + * INTC instance
>> + */
>> +#define MAX_NUM_HOST_IRQS 8
>> +
>> +/* minimum starting host interrupt number for MPU */
>> +#define MIN_PRU_HOST_INT 2
>> +
>> +/* maximum number of system events */
>> +#define MAX_PRU_SYS_EVENTS 64
>> +
>> +/* PRU_ICSS_INTC registers */
>> +#define PRU_INTC_REVID 0x0000
>> +#define PRU_INTC_CR 0x0004
>> +#define PRU_INTC_GER 0x0010
>> +#define PRU_INTC_GNLR 0x001c
>> +#define PRU_INTC_SISR 0x0020
>> +#define PRU_INTC_SICR 0x0024
>> +#define PRU_INTC_EISR 0x0028
>> +#define PRU_INTC_EICR 0x002c
>> +#define PRU_INTC_HIEISR 0x0034
>> +#define PRU_INTC_HIDISR 0x0038
>> +#define PRU_INTC_GPIR 0x0080
>> +#define PRU_INTC_SRSR0 0x0200
>> +#define PRU_INTC_SRSR1 0x0204
>> +#define PRU_INTC_SECR0 0x0280
>> +#define PRU_INTC_SECR1 0x0284
>> +#define PRU_INTC_ESR0 0x0300
>> +#define PRU_INTC_ESR1 0x0304
>> +#define PRU_INTC_ECR0 0x0380
>> +#define PRU_INTC_ECR1 0x0384
>> +#define PRU_INTC_CMR(x) (0x0400 + (x) * 4)
>> +#define PRU_INTC_HMR(x) (0x0800 + (x) * 4)
>> +#define PRU_INTC_HIPIR(x) (0x0900 + (x) * 4)
>> +#define PRU_INTC_SIPR0 0x0d00
>> +#define PRU_INTC_SIPR1 0x0d04
>> +#define PRU_INTC_SITR0 0x0d80
>> +#define PRU_INTC_SITR1 0x0d84
>> +#define PRU_INTC_HINLR(x) (0x1100 + (x) * 4)
>> +#define PRU_INTC_HIER 0x1500
>> +
>> +/* HIPIR register bit-fields */
>> +#define INTC_HIPIR_NONE_HINT 0x80000000
>
> Unused macro. See below.
Will use the macro in the code.
>
>> +
>> +/**
>> + * struct pruss_intc - PRUSS interrupt controller structure
>> + * @irqs: kernel irq numbers corresponding to PRUSS host interrupts
>> + * @base: base virtual address of INTC register space
>> + * @irqchip: irq chip for this interrupt controller
>> + * @domain: irq domain for this interrupt controller
>> + * @lock: mutex to serialize access to INTC
>> + * @host_mask: indicate which HOST IRQs are enabled
>> + */
>> +struct pruss_intc {
>> + unsigned int irqs[MAX_NUM_HOST_IRQS];
>> + void __iomem *base;
>> + struct irq_chip *irqchip;
>> + struct irq_domain *domain;
>> + struct mutex lock; /* PRUSS INTC lock */
>> + u32 host_mask;
This is also unused in this patch, will move this to patch 4.
>> +};
>> +
>> +static inline u32 pruss_intc_read_reg(struct pruss_intc *intc,
>> unsigned int reg)
>> +{
>> + return readl_relaxed(intc->base + reg);
>> +}
>> +
>> +static inline void pruss_intc_write_reg(struct pruss_intc *intc,
>> + unsigned int reg, u32 val)
>> +{
>> + writel_relaxed(val, intc->base + reg);
>> +}
>> +
>> +static int pruss_intc_check_write(struct pruss_intc *intc, unsigned
>> int reg,
>> + unsigned int sysevent)
>> +{
>> + if (!intc)
>> + return -EINVAL;
>> +
>> + if (sysevent >= MAX_PRU_SYS_EVENTS)
>> + return -EINVAL;
>> +
>> + pruss_intc_write_reg(intc, reg, sysevent);
>> +
>> + return 0;
>> +}
>> +
>> +static void pruss_intc_init(struct pruss_intc *intc)
>> +{
>> + int i;
>> +
>> + /* configure polarity to active high for all system interrupts */
>> + pruss_intc_write_reg(intc, PRU_INTC_SIPR0, 0xffffffff);
>> + pruss_intc_write_reg(intc, PRU_INTC_SIPR1, 0xffffffff);
>> +
>> + /* configure type to pulse interrupt for all system interrupts */
>> + pruss_intc_write_reg(intc, PRU_INTC_SITR0, 0);
>> + pruss_intc_write_reg(intc, PRU_INTC_SITR1, 0);
>> +
>> + /* clear all 16 interrupt channel map registers */
>> + for (i = 0; i < 16; i++)
>> + pruss_intc_write_reg(intc, PRU_INTC_CMR(i), 0);
>> +
>> + /* clear all 3 host interrupt map registers */
>> + for (i = 0; i < 3; i++)
>> + pruss_intc_write_reg(intc, PRU_INTC_HMR(i), 0);
>> +}
>> +
>> +static void pruss_intc_irq_ack(struct irq_data *data)
>> +{
>> + struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>> + unsigned int hwirq = data->hwirq;
>> +
>> + pruss_intc_check_write(intc, PRU_INTC_SICR, hwirq);
>> +}
>> +
>> +static void pruss_intc_irq_mask(struct irq_data *data)
>> +{
>> + struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>> + unsigned int hwirq = data->hwirq;
>> +
>> + pruss_intc_check_write(intc, PRU_INTC_EICR, hwirq);
>> +}
>> +
>> +static void pruss_intc_irq_unmask(struct irq_data *data)
>> +{
>> + struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>> + unsigned int hwirq = data->hwirq;
>> +
>> + pruss_intc_check_write(intc, PRU_INTC_EISR, hwirq);
>> +}
>> +
>> +static int pruss_intc_irq_retrigger(struct irq_data *data)
>> +{
>> + struct pruss_intc *intc = irq_data_get_irq_chip_data(data);
>> + unsigned int hwirq = data->hwirq;
>> +
>> + return pruss_intc_check_write(intc, PRU_INTC_SISR, hwirq);
>> +}
>> +
>> +static int pruss_intc_irq_reqres(struct irq_data *data)
>> +{
>> + if (!try_module_get(THIS_MODULE))
>> + return -ENODEV;
>> +
>> + return 0;
>> +}
>> +
>> +static void pruss_intc_irq_relres(struct irq_data *data)
>> +{
>> + module_put(THIS_MODULE);
>> +}
>> +
>> +static int pruss_intc_irq_domain_map(struct irq_domain *d, unsigned
>> int virq,
>> + irq_hw_number_t hw)
>> +{
>> + struct pruss_intc *intc = d->host_data;
>> +
>> + irq_set_chip_data(virq, intc);
>> + irq_set_chip_and_handler(virq, intc->irqchip, handle_level_irq);
>> +
>> + return 0;
>> +}
>> +
>> +static void pruss_intc_irq_domain_unmap(struct irq_domain *d,
>> unsigned int virq)
>> +{
>> + irq_set_chip_and_handler(virq, NULL, NULL);
>> + irq_set_chip_data(virq, NULL);
>> +}
>> +
>> +static const struct irq_domain_ops pruss_intc_irq_domain_ops = {
>> + .xlate = irq_domain_xlate_onecell,
>> + .map = pruss_intc_irq_domain_map,
>> + .unmap = pruss_intc_irq_domain_unmap,
>> +};
>> +
>> +static void pruss_intc_irq_handler(struct irq_desc *desc)
>> +{
>> + unsigned int irq = irq_desc_get_irq(desc);
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> + struct pruss_intc *intc = irq_get_handler_data(irq);
>> + u32 hipir;
>> + unsigned int virq;
>> + int i, hwirq;
>> +
>> + chained_irq_enter(chip, desc);
>> +
>> + /* find our host irq number */
>> + for (i = 0; i < MAX_NUM_HOST_IRQS; i++)
>> + if (intc->irqs[i] == irq)
>> + break;
>> + if (i == MAX_NUM_HOST_IRQS)
>> + goto err;
>> +
>> + i += MIN_PRU_HOST_INT;
>> +
>> + /* get highest priority pending PRUSS system event */
>> + hipir = pruss_intc_read_reg(intc, PRU_INTC_HIPIR(i));
>> + while (!(hipir & BIT(31))) {
>
> Is BIT(31) here supposed to be INTC_HIPIR_NONE_HINT?
Yes, will replace BIT(31) with the macro.
>
>> + hwirq = hipir & GENMASK(9, 0);
>> + virq = irq_linear_revmap(intc->domain, hwirq);
>> +
>> + /*
>> + * NOTE: manually ACK any system events that do not have a
>> + * handler mapped yet
>> + */
>> + if (unlikely(!virq))
>> + pruss_intc_check_write(intc, PRU_INTC_SICR, hwirq);
>> + else
>> + generic_handle_irq(virq);
>> +
>> + /* get next system event */
>> + hipir = pruss_intc_read_reg(intc, PRU_INTC_HIPIR(i));
>> + }
>> +err:
>> + chained_irq_exit(chip, desc);
>> +}
>> +
>> +static int pruss_intc_probe(struct platform_device *pdev)
>> +{
>> + static const char * const irq_names[] = {
>> + "host0", "host1", "host2", "host3",
>> + "host4", "host5", "host6", "host7", };
>> + struct device *dev = &pdev->dev;
>> + struct pruss_intc *intc;
>> + struct resource *res;
>> + struct irq_chip *irqchip;
>> + int i, irq;
>> +
>> + intc = devm_kzalloc(dev, sizeof(*intc), GFP_KERNEL);
>> + if (!intc)
>> + return -ENOMEM;
>> + platform_set_drvdata(pdev, intc);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + intc->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(intc->base)) {
>> + dev_err(dev, "failed to parse and map intc memory resource\n");
>> + return PTR_ERR(intc->base);
>> + }
>> +
>> + dev_dbg(dev, "intc memory: pa %pa size 0x%zx va %pK\n", &res->start,
>> + (size_t)resource_size(res), intc->base);
>> +
>> + mutex_init(&intc->lock);
>> +
>> + pruss_intc_init(intc);
>> +
>> + irqchip = devm_kzalloc(dev, sizeof(*irqchip), GFP_KERNEL);
>> + if (!irqchip)
>> + return -ENOMEM;
>> +
>> + irqchip->irq_ack = pruss_intc_irq_ack;
>> + irqchip->irq_mask = pruss_intc_irq_mask;
>> + irqchip->irq_unmask = pruss_intc_irq_unmask;
>> + irqchip->irq_retrigger = pruss_intc_irq_retrigger;
>> + irqchip->irq_request_resources = pruss_intc_irq_reqres;
>> + irqchip->irq_release_resources = pruss_intc_irq_relres;
>> + irqchip->name = dev_name(dev);
>
> Should we also set `irqchip->parent_device = dev;` here?
>
> I tried it and had to add pm runtime stuff as well, otherwise
> requesting irqs would fail.
I haven't seen any during my local testing. What sort of failure are you
seeing?
The clocking for the overall PRUSS module will be handled in either the
ti-sysc driver for OMAP SoCs or in the pruss platform driver.
>
>> + intc->irqchip = irqchip;
>> +
>> + /* always 64 events */
>> + intc->domain = irq_domain_add_linear(dev->of_node,
>> MAX_PRU_SYS_EVENTS,
>> + &pruss_intc_irq_domain_ops, intc);
>> + if (!intc->domain)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
>> + irq = platform_get_irq_byname(pdev, irq_names[i]);
>> + if (irq < 0) {
>> + dev_err(dev->parent, "platform_get_irq_byname failed for
>> %s : %d\n",
>
> Why dev->parent instead of just dev?
Yeah, will fix this. It is a vestige from previous code where we were
actually parsing the interrupts from the PRUSS node instead of the PRUSS
INTC node.
>
>> + irq_names[i], irq);
>> + goto fail_irq;
>> + }
>> +
>> + intc->irqs[i] = irq;
>> + irq_set_handler_data(irq, intc);
>> + irq_set_chained_handler(irq, pruss_intc_irq_handler);
>> + }
>> +
>> + return 0;
>> +
>> +fail_irq:
>> + while (--i >= 0) {
>> + if (intc->irqs[i])
>> + irq_set_chained_handler_and_data(intc->irqs[i], NULL,
>> + NULL);
>> + }
>> + irq_domain_remove(intc->domain);
>> + return irq;
>> +}
>> +
>> +static int pruss_intc_remove(struct platform_device *pdev)
>> +{
>> + struct pruss_intc *intc = platform_get_drvdata(pdev);
>> + unsigned int hwirq;
>> + int i;
>> +
>> + for (i = 0; i < MAX_NUM_HOST_IRQS; i++) {
>> + if (intc->irqs[i])
>> + irq_set_chained_handler_and_data(intc->irqs[i], NULL,
>> + NULL);
>> + }
>> +
>> + if (intc->domain) {
>
> Is it actuall possible for intc->domain to be NULL here?
Nope, will remove it.
Thank you for all the review comments.
regards
Suman
>
>> + for (hwirq = 0; hwirq < MAX_PRU_SYS_EVENTS; hwirq++)
>> + irq_dispose_mapping(irq_find_mapping(intc->domain,
>> + hwirq));
>> + irq_domain_remove(intc->domain);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id pruss_intc_of_match[] = {
>> + { .compatible = "ti,pruss-intc", },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, pruss_intc_of_match);
>> +
>> +static struct platform_driver pruss_intc_driver = {
>> + .driver = {
>> + .name = "pruss-intc",
>> + .of_match_table = pruss_intc_of_match,
>> + },
>> + .probe = pruss_intc_probe,
>> + .remove = pruss_intc_remove,
>> +};
>> +module_platform_driver(pruss_intc_driver);
>> +
>> +MODULE_AUTHOR("Andrew F. Davis <afd@...com>");
>> +MODULE_AUTHOR("Suman Anna <s-anna@...com>");
>> +MODULE_DESCRIPTION("TI PRU-ICSS INTC Driver");
>> +MODULE_LICENSE("GPL v2");
>>
>
Powered by blists - more mailing lists