On 03/08/17 12:15, Masahiro Yamada wrote:
> UniPhier SoCs contain AIDET (ARM Interrupt Detector). This is intended
> to provide additional features that are not covered by GIC. The main
> purpose is to provide logic inverter to support low level and falling
> edge trigger type for interrupt lines from on-board devices.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> ---
>
> .../socionext,uniphier-aidet.txt | 32 +++
> MAINTAINERS | 1 +
> drivers/irqchip/Kconfig | 5 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-uniphier-aidet.c | 252 +++++++++++++++++++++
> 5 files changed, 291 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/socionext,uniphier-aidet.txt
> create mode 100644 drivers/irqchip/irq-uniphier-aidet.c
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/socionext,uniphier-aidet.txt b/Documentation/devicetree/bindings/interrupt-controller/socionext,uniphier-aidet.txt
> new file mode 100644
> index 000000000000..af57fbccd234
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/socionext,uniphier-aidet.txt
> @@ -0,0 +1,32 @@
> +UniPhier AIDET
> +
> +UniPhier AIDET (ARM Interrupt Detector) is an add-on block for ARM GIC (Generic
> +Interrupt Controller). GIC itself can handle only high level and rising edge
> +interrupts. The AIDET provides logic inverter to support low level and falling
> +edge interrupts.
> +
> +Required properties:
> +- compatible: Should be one of the following:
> + "socionext,uniphier-ld4-aidet" - for LD4 SoC
> + "socionext,uniphier-pro4-aidet" - for Pro4 SoC
> + "socionext,uniphier-sld8-aidet" - for sLD8 SoC
> + "socionext,uniphier-pro5-aidet" - for Pro5 SoC
> + "socionext,uniphier-pxs2-aidet" - for PXs2/LD6b SoC
> + "socionext,uniphier-ld11-aidet" - for LD11 SoC
> + "socionext,uniphier-ld20-aidet" - for LD20 SoC
> + "socionext,uniphier-pxs3-aidet" - for PXs3 SoC
> +- reg: Specifies offset and length of the register set for the device.
> +- interrupt-controller: Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an interrupt
> + source. The value should be 2. The first cell defines the interrupt number.
> + The second cell specifies the trigger type as defined in interrupts.txt in
> + this directory.
"interrupt number" is a pretty vague concept. You probably want to
explain how it relates to the GIC (is that the INTID? or the SPI number?
What about PPIs?).
> +
> +Example:
> +
> + aidet: aidet@5fc20000 {
> + compatible = "socionext,uniphier-pro4-aidet";
> + reg = <0x5fc20000 0x200>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 567343b8ffaa..26b3d10b7344 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1993,6 +1993,7 @@ F: arch/arm64/boot/dts/socionext/
> F: drivers/bus/uniphier-system-bus.c
> F: drivers/clk/uniphier/
> F: drivers/i2c/busses/i2c-uniphier*
> +F: drivers/irqchip/irq-uniphier-aidet.c
> F: drivers/pinctrl/uniphier/
> F: drivers/reset/reset-uniphier.c
> F: drivers/tty/serial/8250/8250_uniphier.c
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index f1fd5f44d1d4..a0d7218a1677 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -306,3 +306,8 @@ config QCOM_IRQ_COMBINER
> help
> Say yes here to add support for the IRQ combiner devices embedded
> in Qualcomm Technologies chips.
> +
> +config IRQ_UNIPHIER_AIDET
> + def_bool y
> + depends on ARCH_UNIPHIER || COMPILE_TEST
> + select IRQ_DOMAIN_HIERARCHY
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e88d856cc09c..2c630574986f 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -78,3 +78,4 @@ obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
> obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o
> obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
> obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
> +obj-$(CONFIG_IRQ_UNIPHIER_AIDET) += irq-uniphier-aidet.o
> diff --git a/drivers/irqchip/irq-uniphier-aidet.c b/drivers/irqchip/irq-uniphier-aidet.c
> new file mode 100644
> index 000000000000..8ceed5070e77
> --- /dev/null
> +++ b/drivers/irqchip/irq-uniphier-aidet.c
> @@ -0,0 +1,252 @@
> +/*
> + * Driver for UniPhier AIDET (ARM Interrupt Detector)
> + *
> + * Copyright (C) 2017 Socionext Inc.
> + * Author: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +#define UNIPHIER_AIDET_NR_IRQS 256
> +
> +#define UNIPHIER_AIDET_DETCONF 0x00
> +
> +struct uniphier_aidet_priv {
> + struct irq_chip chip;
> + struct irq_domain *domain;
> + void __iomem *reg_base;
> + spinlock_t lock;
> + u32 saved_vals[UNIPHIER_AIDET_NR_IRQS / 32];
> +};
> +
> +static void uniphier_aidet_reg_update(struct uniphier_aidet_priv *priv,
> + unsigned int reg, u32 mask, u32 val)
> +{
> + unsigned long flags;
> + u32 tmp;
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + tmp = readl(priv->reg_base + reg);
> + tmp &= ~mask;
> + tmp |= mask & val;
> + writel(tmp, priv->reg_base + reg);
Given that these accesses do not seem to rely on anything making it into
memory before the access, consider using the _relaxed accessors (no need
for two DSBs here).
> + spin_unlock_irqrestore(&priv->lock, flags);
> +}
> +
> +static void uniphier_aidet_detconf_update(struct uniphier_aidet_priv *priv,
> + unsigned long index, unsigned int val)
> +{
> + unsigned int reg;
> + u32 mask;
> +
> + reg = UNIPHIER_AIDET_DETCONF + index / 32 * 4;
What is the purpose of UNIPHIER_AIDET_DETCONF here, which is always 0?
> + mask = BIT(index % 32);
> +
> + uniphier_aidet_reg_update(priv, reg, mask, val ? mask : 0);
> +}
> +
> +static int uniphier_aidet_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> + struct uniphier_aidet_priv *priv = data->chip_data;
> + unsigned int val = 0;
> +
> + /* enable inverter for active low triggers */
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + case IRQ_TYPE_LEVEL_HIGH:
Nit: consider moving the "val" assignment here so that the symmetry
between the two cases becomes obvious.
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + val = 1;
> + type = IRQ_TYPE_EDGE_RISING;
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + val = 1;
> + type = IRQ_TYPE_LEVEL_HIGH;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + uniphier_aidet_detconf_update(priv, data->hwirq, val);
> +
> + return irq_chip_set_type_parent(data, type);
> +}
> +
> +static int uniphier_aidet_domain_translate(struct irq_domain *domain,
> + struct irq_fwspec *fwspec,
> + unsigned long *out_hwirq,
> + unsigned int *out_type)
> +{
> + if (WARN_ON(fwspec->param_count < 2))
> + return -EINVAL;
> +
> + *out_hwirq = fwspec->param[0];
> + *out_type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
> +
> + return 0;
> +}
> +
> +static int uniphier_aidet_domain_alloc(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs,
> + void *arg)
> +{
> + struct uniphier_aidet_priv *priv = domain->host_data;
> + irq_hw_number_t hwirq;
> + unsigned int type;
> + int i, ret;
> +
> + ret = uniphier_aidet_domain_translate(domain, arg, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + if (hwirq >= UNIPHIER_AIDET_NR_IRQS)
> + return -ENXIO;
> +
> + for (i = 0; i < nr_irqs; i++) {
> + struct irq_fwspec parent_fwspec;
> +
> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> + &priv->chip, priv);
> + if (ret)
> + return ret;
> +
> + /* parent is GIC */
> + parent_fwspec.fwnode = domain->parent->fwnode;
> + parent_fwspec.param_count = 3;
> + parent_fwspec.param[0] = 0; /* SPI */
> + parent_fwspec.param[1] = hwirq - 32;
> + parent_fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
So this is SPI only? And your first line starts at 32? So what is in the
32bit register?
> +
> + ret = irq_domain_alloc_irqs_parent(domain, virq, 1,
> + &parent_fwspec);
> + if (ret)
> + return ret;
> +
> + virq++;
> + hwirq++;
> + }
Two things here: is there any case where you actually have nr_irqs not
being 1? As far as I know, this is only used for Multi-MSI, and nothing
else. And if you do need it, then you should probably have a slightly
better error handling (you're leaking interrupts on error here).
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops uniphier_aidet_domain_ops = {
> + .alloc = uniphier_aidet_domain_alloc,
> + .free = irq_domain_free_irqs_common,
> + .translate = uniphier_aidet_domain_translate,
> +};
> +
> +static int uniphier_aidet_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *parent_np;
> + struct irq_domain *parent_domain;
> + struct uniphier_aidet_priv *priv;
> + struct resource *res;
> +
> + parent_np = of_irq_find_parent(dev->of_node);
> + if (!parent_np)
> + return -ENXIO;
> +
> + parent_domain = irq_find_host(parent_np);
> + of_node_put(parent_np);
> + if (!parent_domain)
> + return -EPROBE_DEFER;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->reg_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(priv->reg_base))
> + return PTR_ERR(priv->reg_base);
> +
> + spin_lock_init(&priv->lock);
> +
> + priv->chip.name = dev_name(dev);
> + priv->chip.irq_mask = irq_chip_mask_parent;
> + priv->chip.irq_unmask = irq_chip_unmask_parent;
> + priv->chip.irq_eoi = irq_chip_eoi_parent;
> + priv->chip.irq_set_type = uniphier_aidet_irq_set_type;
> +
> + priv->domain = irq_domain_add_hierarchy(parent_domain, 0,
> + UNIPHIER_AIDET_NR_IRQS,
> + dev->of_node,
> + &uniphier_aidet_domain_ops,
> + priv);
You seem to be able to handle multiple AIDET devices, and yet your DT
description doesn't specify a base/span for the interrupt lines that are
covered by this device. Is that something you intend to support?
> + if (!priv->domain)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused uniphier_aidet_suspend(struct device *dev)
> +{
> + struct uniphier_aidet_priv *priv = dev_get_drvdata(dev);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(priv->saved_vals); i++)
> + priv->saved_vals[i] = readl(priv->reg_base +
readl_relaxed
> + UNIPHIER_AIDET_DETCONF + i * 4);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused uniphier_aidet_resume(struct device *dev)
> +{
> + struct uniphier_aidet_priv *priv = dev_get_drvdata(dev);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(priv->saved_vals); i++)
> + writel(priv->saved_vals[i],
writel_relaxed
> + priv->reg_base + UNIPHIER_AIDET_DETCONF + i * 4);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops uniphier_aidet_pm_ops = {
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(uniphier_aidet_suspend,
> + uniphier_aidet_resume)
> +};
> +
> +static const struct of_device_id uniphier_aidet_match[] = {
> + { .compatible = "socionext,uniphier-ld4-aidet" },
> + { .compatible = "socionext,uniphier-pro4-aidet" },
> + { .compatible = "socionext,uniphier-sld8-aidet" },
> + { .compatible = "socionext,uniphier-pro5-aidet" },
> + { .compatible = "socionext,uniphier-pxs2-aidet" },
> + { .compatible = "socionext,uniphier-ld11-aidet" },
> + { .compatible = "socionext,uniphier-ld20-aidet" },
> + { .compatible = "socionext,uniphier-pxs3-aidet" },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver uniphier_aidet_driver = {
> + .probe = uniphier_aidet_probe,
> + .driver = {
> + .name = "uniphier-aidet",
> + .of_match_table = uniphier_aidet_match,
> + .pm = &uniphier_aidet_pm_ops,
> + },
> +};
> +builtin_platform_driver(uniphier_aidet_driver);
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
