lkml.org 
[lkml]   [2019]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5 06/10] irqchip: ti-sci-intr: Add support for Interrupt Router driver
From
Date
Hi Marc,

On 18/02/19 9:22 PM, Marc Zyngier wrote:
> On Tue, 12 Feb 2019 13:12:33 +0530
> Lokesh Vutla <lokeshvutla@ti.com> wrote:
>
>> Texas Instruments' K3 generation SoCs has an IP Interrupt Router
>> that does allows for redirection of input interrupts to host
>> interrupt controller. Interrupt Router inputs are either from a
>> peripheral or from an Interrupt Aggregator which is another
>> interrupt controller.
>>
>> Configuration of the interrupt router registers can only be done by
>> a system co-processor and the driver needs to send a message to this
>> co processor over TISCI protocol.
>>
>> Add support for Interrupt Router driver over TISCI protocol.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> Changes since v4:
>> - Moved the ti_sci irq resource handling to ti-sci-common.c from ti_sci.c.
>> Firmware maintainer rejected the idea of having this in firmware
>> driver as the resource handling is specific to the client.
>> - Obtain the number of peripheral interrupts attached to INTR by
>> parsing DT. Using this information store the max irqs that can be
>> allocated to INTA. This is done for pre allocating the INTA
>> irqs(vints) during inta driver probe.
>> This will not work for cases where there are more that 1 INTAs
>> attached to INTR, but wanted to show this approach as suggested by
>> Marc.
>
> Or not.
>
>>
>> MAINTAINERS | 3 +
>> drivers/irqchip/Kconfig | 11 +
>> drivers/irqchip/Makefile | 1 +
>> drivers/irqchip/irq-ti-sci-common.c | 131 ++++++++++++
>> drivers/irqchip/irq-ti-sci-common.h | 59 ++++++
>> drivers/irqchip/irq-ti-sci-intr.c | 315
>> ++++++++++++++++++++++++++++ 6 files changed, 520 insertions(+)
>> create mode 100644 drivers/irqchip/irq-ti-sci-common.c
>> create mode 100644 drivers/irqchip/irq-ti-sci-common.h
>> create mode 100644 drivers/irqchip/irq-ti-sci-intr.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c918d9b2ee18..823eeb672cf0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -15065,6 +15065,9 @@ F:
>> Documentation/devicetree/bindings/clock/ti,sci-clk.txt F:
>> drivers/clk/keystone/sci-clk.c F: drivers/reset/reset-ti-sci.c
>> F:
>> Documentation/devicetree/bindings/interrupt-controller/ti,sci-intr.txt
>> +F: drivers/irqchip/irq-ti-sci-intr.c +F:
>> drivers/irqchip/irq-ti-sci-common.c +F:
>> drivers/irqchip/irq-ti-sci-common.h
>> Texas Instruments ASoC drivers
>> M: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index 3d1e60779078..a8d9bed0254b 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -406,6 +406,17 @@ config IMX_IRQSTEER
>> help
>> Support for the i.MX IRQSTEER interrupt
>> multiplexer/remapper.
>> +config TI_SCI_INTR_IRQCHIP
>> + bool
>> + depends on TI_SCI_PROTOCOL && ARCH_K3
>> + select IRQ_DOMAIN
>> + select IRQ_DOMAIN_HIERARCHY
>> + help
>> + This enables the irqchip driver support for K3 Interrupt
>> router
>> + over TI System Control Interface available on some new
>> TI's SoCs.
>> + If you wish to use interrupt router irq resources managed
>> by the
>> + TI System Controller, say Y here. Otherwise, say N.
>> +
>> endmenu
>>
>> config SIFIVE_PLIC
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index c93713d24b86..0499fae148a9 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -94,3 +94,4 @@ obj-$(CONFIG_CSKY_APB_INTC) +=
>> irq-csky-apb-intc.o obj-$(CONFIG_SIFIVE_PLIC) +=
>> irq-sifive-plic.o obj-$(CONFIG_IMX_IRQSTEER) +=
>> irq-imx-irqsteer.o obj-$(CONFIG_MADERA_IRQ) +=
>> irq-madera.o +obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) +=
>> irq-ti-sci-intr.o irq-ti-sci-common.o diff --git
>> a/drivers/irqchip/irq-ti-sci-common.c
>> b/drivers/irqchip/irq-ti-sci-common.c new file mode 100644 index
>> 000000000000..79d9c4e8ea14 --- /dev/null
>> +++ b/drivers/irqchip/irq-ti-sci-common.c
>> @@ -0,0 +1,131 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Common Code for TISCI IRQCHIP drivers
>> + *
>> + * Copyright (C) 2019 Texas Instruments Incorporated -
>> http://www.ti.com/
>> + * Lokesh Vutla <lokeshvutla@ti.com>
>> + */
>> +
>> +#include <linux/of.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/device.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +#include "irq-ti-sci-common.h"
>> +
>> +/**
>> + * ti_sci_get_free_resource() - Get a free resource from TISCI
>> resource.
>> + * @res: Pointer to the TISCI resource
>> + *
>> + * Return: resource num if all went ok else TI_SCI_RESOURCE_NULL.
>> + */
>> +u16 ti_sci_get_free_resource(struct ti_sci_resource *res)
>> +{
>> + u16 set, free_bit;
>> +
>> + mutex_lock(&res->request_mutex);
>> + for (set = 0; set < res->sets; set++) {
>> + free_bit =
>> find_first_zero_bit(res->desc[set].res_map,
>> + res->desc[set].num);
>> + if (free_bit != res->desc[set].num) {
>> + set_bit(free_bit, res->desc[set].res_map);
>> + mutex_unlock(&res->request_mutex);
>> + return res->desc[set].start + free_bit;
>> + }
>> + }
>> + mutex_unlock(&res->request_mutex);
>> +
>> + return TI_SCI_RESOURCE_NULL;
>> +}
>> +
>> +/**
>> + * ti_sci_release_resource() - Release a resource from TISCI
>> resource.
>> + * @res: Pointer to the TISCI resource
>> + * @id: Resource id to be released.
>> + */
>> +void ti_sci_release_resource(struct ti_sci_resource *res, u16 id)
>> +{
>> + u16 set;
>> +
>> + mutex_lock(&res->request_mutex);
>> + for (set = 0; set < res->sets; set++) {
>> + if (res->desc[set].start <= id &&
>> + (res->desc[set].num + res->desc[set].start) > id)
>> + clear_bit(id - res->desc[set].start,
>> + res->desc[set].res_map);
>> + }
>> + mutex_unlock(&res->request_mutex);
>> +}
>> +
>> +u32 ti_sci_get_num_resources(struct ti_sci_resource *res)
>> +{
>> + u32 count = 0, set;
>> +
>> + for (set = 0; set < res->sets; set++)
>> + count += res->desc[set].num;
>> +
>> + return count;
>> +}
>> +
>> +/**
>> + * devm_ti_sci_get_of_resource() - Get a TISCI resource assigned to
>> a device
>> + * @handle: TISCI handle
>> + * @dev: Device pointer to which the resource is assigned
>> + * @of_prop: property name by which the resource are
>> represented
>> + *
>> + * Return: Pointer to ti_sci_resource if all went well else
>> appropriate
>> + * error pointer.
>> + */
>> +struct ti_sci_resource *
>> +devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
>> + struct device *dev, u32 dev_id, char
>> *of_prop) +{
>> + struct ti_sci_resource *res;
>> + u32 resource_subtype;
>> + int i, ret;
>> +
>> + res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
>> + if (!res)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + res->sets =
>> of_property_count_elems_of_size(dev_of_node(dev), of_prop,
>> + sizeof(u32));
>> + if (res->sets < 0) {
>> + dev_err(dev, "%s resource type ids not available\n",
>> of_prop);
>> + return ERR_PTR(res->sets);
>> + }
>> +
>> + res->desc = devm_kcalloc(dev, res->sets, sizeof(*res->desc),
>> + GFP_KERNEL);
>> + if (!res->desc)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + for (i = 0; i < res->sets; i++) {
>> + ret = of_property_read_u32_index(dev_of_node(dev),
>> of_prop, i,
>> + &resource_subtype);
>> + if (ret)
>> + return ERR_PTR(-EINVAL);
>> +
>> + ret = handle->ops.rm_core_ops.get_range(handle,
>> dev_id,
>> +
>> resource_subtype,
>> +
>> &res->desc[i].start,
>> +
>> &res->desc[i].num);
>> + if (ret) {
>> + dev_err(dev, "dev = %d subtype %d not
>> allocated for this host\n",
>> + dev_id, resource_subtype);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + dev_dbg(dev, "dev = %d, subtype = %d, start = %d,
>> num = %d\n",
>> + dev_id, resource_subtype, res->desc[i].start,
>> + res->desc[i].num);
>> +
>> + res->desc[i].res_map =
>> + devm_kzalloc(dev,
>> BITS_TO_LONGS(res->desc[i].num) *
>> + sizeof(*res->desc[i].res_map),
>> GFP_KERNEL);
>> + if (!res->desc[i].res_map)
>> + return ERR_PTR(-ENOMEM);
>> + }
>> + mutex_init(&res->request_mutex);
>> +
>> + return res;
>> +}
>> diff --git a/drivers/irqchip/irq-ti-sci-common.h
>> b/drivers/irqchip/irq-ti-sci-common.h new file mode 100644
>> index 000000000000..60c4b28bebab
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-ti-sci-common.h
>> @@ -0,0 +1,59 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Common header file for TISCI IRQCHIP drivers
>> + *
>> + * Copyright (C) 2019 Texas Instruments Incorporated -
>> http://www.ti.com/
>> + * Lokesh Vutla <lokeshvutla@ti.com>
>> + */
>> +
>> +#ifndef __TI_SCI_COMMON_IRQCHIP_H
>> +#define __TI_SCI_COMMON_IRQCHIP_H
>> +
>> +#include <linux/mutex.h>
>> +
>> +#define TI_SCI_RESOURCE_NULL 0xffff
>> +#define TI_SCI_DEV_ID_MASK 0xffff
>> +#define TI_SCI_DEV_ID_SHIFT 16
>> +#define TI_SCI_IRQ_ID_MASK 0xffff
>> +#define TI_SCI_IRQ_ID_SHIFT 0
>> +#define TI_SCI_VINT_IRQ BIT(0)
>> +#define HWIRQ_TO_DEVID(hwirq) (((hwirq) >>
>> (TI_SCI_DEV_ID_SHIFT)) & \
>> + (TI_SCI_DEV_ID_MASK))
>> +#define HWIRQ_TO_IRQID(hwirq) ((hwirq) & (TI_SCI_IRQ_ID_MASK))
>> +#define TO_HWIRQ(dev, index) ((((dev) & TI_SCI_DEV_ID_MASK)
>> << \
>> + TI_SCI_DEV_ID_SHIFT) | \
>> + ((index) & TI_SCI_IRQ_ID_MASK))
>> +
>> +/**
>> + * struct ti_sci_resource_desc - Description of TI SCI resource
>> instance range.
>> + * @start: Start index of the resource.
>> + * @num: Number of resources.
>> + * @res_map: Bitmap to manage the allocation of these
>> resources.
>> + */
>> +struct ti_sci_resource_desc {
>> + u16 start;
>> + u16 num;
>> + unsigned long *res_map;
>> +};
>> +
>> +/**
>> + * struct ti_sci_resource - Structure representing a resource
>> assigned
>> + * to a device.
>> + * @sets: Number of sets available from this resource
>> type
>> + * @request_mutex: mutex to protect request and release of
>> resources.
>> + * @desc: Array of resource descriptors.
>> + */
>> +struct ti_sci_resource {
>> + u16 sets;
>> + /* Mutex to protect request and release of resources */
>> + struct mutex request_mutex;
>> + struct ti_sci_resource_desc *desc;
>> +};
>> +
>> +u16 ti_sci_get_free_resource(struct ti_sci_resource *res);
>> +void ti_sci_release_resource(struct ti_sci_resource *res, u16 id);
>> +u32 ti_sci_get_num_resources(struct ti_sci_resource *res);
>> +struct ti_sci_resource *
>> +devm_ti_sci_get_of_resource(const struct ti_sci_handle *handle,
>> + struct device *dev, u32 dev_id, char
>> *of_prop); +#endif /*__TI_SCI_COMMON_IRQCHIP_H */
>
> Most the above "common" should be a separate patch, really. Also, why

ok will move this into a separate patch.

> isn't the whole resource management part of the firmware interface? I
> see that "Firmware maintainer rejected the idea", but I don't really
> buy the rational. I don't see anything irq specific to the whole
> devm_ti_sci_get_of_resource and co, to be honest.

I guess the concern is mostly about the way ids are represented from DT. Using
this every tisci client should mandate to have separate DT property for each of
its resource.

>
>> diff --git a/drivers/irqchip/irq-ti-sci-intr.c b/drivers/irqchip/irq-ti-sci-intr.c
>> new file mode 100644
>> index 000000000000..7e224552a735
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-ti-sci-intr.c
>> @@ -0,0 +1,315 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Texas Instruments' K3 Interrupt Router irqchip driver
>> + *
>> + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
>> + * Lokesh Vutla <lokeshvutla@ti.com>
>> + */

[..snip..]

>> + err = ti_sci_intr_irq_domain_translate(domain, fwspec, &hwirq, &type);
>> + if (err)
>> + return err;
>> +
>> + vint_irq = fwspec->param[3] & TI_SCI_VINT_IRQ;
>> +
>> + err = ti_sci_intr_alloc_gic_irq(domain, virq, hwirq, type, vint_irq);
>> + if (err)
>> + return err;
>> +
>> + err = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
>> + &ti_sci_intr_irq_chip,
>> + (void *)vint_irq);
>
> I think I see what you're trying to achieve. I suggest you look a
> uintptr_t instead of playing with unrelated data types whose semantic
> really doesn't apply here.

As you commented on the dt-bindings patch to differentiate vints using the INTA
ids, this will not be needed anymore. Will completely drop this.

>
>> + if (err) {
>> + ti_sci_intr_irq_domain_free(domain, virq, 1);
>> + return err;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct irq_domain_ops ti_sci_intr_irq_domain_ops = {
>> + .alloc = ti_sci_intr_irq_domain_alloc,
>> + .free = ti_sci_intr_irq_domain_free,
>> + .translate = ti_sci_intr_irq_domain_translate,
>> +};
>> +
>> +static int ti_sci_intr_irq_domain_probe(struct platform_device *pdev)
>> +{
>> + struct irq_domain *parent_domain, *domain;
>> + struct device_node *parent_node, *dn;
>> + struct ti_sci_intr_irq_domain *intr;
>> + struct device *dev = &pdev->dev;
>> + struct of_phandle_iterator it;
>> + int ret, count, intsize;
>> +
>> + parent_node = of_irq_find_parent(dev_of_node(dev));
>> + if (!parent_node) {
>> + dev_err(dev, "Failed to get IRQ parent node\n");
>> + return -ENODEV;
>> + }
>> +
>> + parent_domain = irq_find_host(parent_node);
>> + if (!parent_domain) {
>> + dev_err(dev, "Failed to find IRQ parent domain\n");
>> + return -ENODEV;
>> + }
>> +
>> + intr = devm_kzalloc(dev, sizeof(*intr), GFP_KERNEL);
>> + if (!intr)
>> + return -ENOMEM;
>> +
>> + intr->sci = devm_ti_sci_get_by_phandle(dev, "ti,sci");
>> + if (IS_ERR(intr->sci)) {
>> + ret = PTR_ERR(intr->sci);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "ti,sci read fail %d\n", ret);
>> + intr->sci = NULL;
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(dev_of_node(dev), "ti,sci-dst-id",
>> + (u32 *)&intr->dst_id);
>> + if (ret) {
>> + dev_err(dev, "missing 'ti,sci-dst-id' property\n");
>> + return -EINVAL;
>> + }
>> +
>> + intr->dst_irq = devm_ti_sci_get_of_resource(intr->sci, dev,
>> + intr->dst_id,
>> + "ti,sci-rm-range-girq");
>> + if (IS_ERR(intr->dst_irq)) {
>> + dev_err(dev, "Destination irq resource allocation failed\n");
>> + return PTR_ERR(intr->dst_irq);
>> + }
>> +
>> + domain = irq_domain_add_hierarchy(parent_domain, 0, 0, dev_of_node(dev),
>> + &ti_sci_intr_irq_domain_ops, intr);
>> + if (!domain) {
>> + dev_err(dev, "Failed to allocate IRQ domain\n");
>> + return -ENOMEM;
>> + }
>> +
>> + if (of_property_read_u32(dev_of_node(dev), "#interrupt-cells",
>> + &intsize))
>> + return -EINVAL;
>> +
>> + count = 0;
>> + for_each_node_with_property(dn, "interrupts") {
>> + if (of_irq_find_parent(dn) != dev_of_node(dev))
>> + continue;
>> + count += of_property_count_elems_of_size(dn, "interrupts",
>> + sizeof(u32) * intsize);
>> + }
>> +
>> + for_each_node_with_property(dn, "interrupts-extended") {
>> + of_for_each_phandle(&it, ret, dn, "interrupts-extended",
>> + "#interrupt-cells", 0) {
>> + if (it.node == dev_of_node(dev))
>> + count++;
>> + }
>> + }
>
> What is this trying to do? Counting the number of interrupt descriptors
> in the whole system? What does this even mean?

I am just trying for max utilization of GIC interrupts available for this. Since
inta vints are not represented in DT, total gic available - whatever is in DT
gives the max interrupts that can be used for the child INTA.

>
> What I suggested was to allocate the required number of interrupts
> for a given device, based on the requirements of that device. I also

okay, then Ill get the number of interrupts needed for INTA from DT.

> suggested to investigate the x86 two phase allocation mechanism for the
> INTA driver.

Sorry, Ill try to explore this path. Any pointers to the doc/code will be really
helpful :)

Thanks and regards,
Lokesh

>
> I never suggested that you'd parse the DT to guess how many interrupts
> you'd need to allocate...
>
> M.
>
>> +
>> + intr->vint_irqs = ti_sci_get_num_resources(intr->dst_irq) - count;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id ti_sci_intr_irq_domain_of_match[] = {
>> + { .compatible = "ti,sci-intr", },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ti_sci_intr_irq_domain_of_match);
>> +
>> +static struct platform_driver ti_sci_intr_irq_domain_driver = {
>> + .probe = ti_sci_intr_irq_domain_probe,
>> + .driver = {
>> + .name = "ti-sci-intr",
>> + .of_match_table = ti_sci_intr_irq_domain_of_match,
>> + },
>> +};
>> +module_platform_driver(ti_sci_intr_irq_domain_driver);
>> +
>> +MODULE_AUTHOR("Lokesh Vutla <lokeshvutla@ticom>");
>> +MODULE_DESCRIPTION("K3 Interrupt Router driver over TI SCI protocol");
>> +MODULE_LICENSE("GPL v2");
>
>
>

\
 
 \ /
  Last update: 2019-02-20 14:18    [W:0.150 / U:0.624 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site