On Sun, 26 Jun 2022 01:43:26 +0100,
Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote:
>
> The Renesas RZ/Five SoC has a RISC-V AX45MP AndesCore with NCEPLIC100. The
> NCEPLIC100 supports both edge-triggered and level-triggered interrupts. In
> case of edge-triggered interrupts NCEPLIC100 ignores the next interrupt
> edge until the previous completion message has been received and
> NCEPLIC100 doesn't support pending interrupt counter, hence losing the
> interrupts if not acknowledged in time.
>
> So the workaround for edge-triggered interrupts to be handled correctly
> and without losing is that it needs to be acknowledged first and then
> handler must be run so that we don't miss on the next edge-triggered
> interrupt.
>
> This patch adds a new compatible string for Renesas RZ/Five SoC and adds
> support to change interrupt flow based on the interrupt type. It also
> implements irq_ack and irq_set_type callbacks.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> v1->v2:
> * Implemented IRQ flow as suggested by Marc
>
> RFC-->v1:
> * Fixed review comments pointed by Geert
> * Dropped handle_fasteoi_ack_irq support as for the PLIC we need to
> claim the interrupt by reading the register and then acknowledge it.
> * Add a new chained handler for RZ/Five SoC.
> ---
> drivers/irqchip/Kconfig | 1 +
> drivers/irqchip/irq-sifive-plic.c | 73 ++++++++++++++++++++++++++++++-
> 2 files changed, 72 insertions(+), 2 deletions(-)
[...]
>
> +static int plic_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct plic_handler *handler = this_cpu_ptr(&plic_handlers);
> +
> + if (handler->priv->of_data != RENESAS_R9A07G043_PLIC)
> + return 0;
> +
> + switch (type) {
> + case IRQ_TYPE_LEVEL_HIGH:
> + irq_set_chip_handler_name_locked(d, &renesas_rzfive_edge_plic_chip,
> + handle_fasteoi_ack_irq,
> + "Edge");
> + break;
> +
> + case IRQ_TYPE_EDGE_RISING:
> + irq_set_chip_handler_name_locked(d, &plic_chip,
> + handle_fasteoi_irq,
> + "Level");
> + break;
Really? Have you even tested this?
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
> irq_hw_number_t hwirq)
> {
> @@ -198,6 +248,19 @@ static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq,
> return 0;
> }
>
> +static int plic_irq_domain_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + struct plic_priv *priv = d->host_data;
> +
> + if (priv->of_data == RENESAS_R9A07G043_PLIC)
> + return irq_domain_translate_twocell(d, fwspec, hwirq, type);
> +
> + return irq_domain_translate_onecell(d, fwspec, hwirq, type);
> +}
> +
> static int plic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> unsigned int nr_irqs, void *arg)
> {
> @@ -206,7 +269,7 @@ static int plic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> unsigned int type;
> struct irq_fwspec *fwspec = arg;
>
> - ret = irq_domain_translate_onecell(domain, fwspec, &hwirq, &type);
> + ret = plic_irq_domain_translate(domain, fwspec, &hwirq, &type);
> if (ret)
> return ret;
>
> @@ -220,7 +283,7 @@ static int plic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> }
>
> static const struct irq_domain_ops plic_irqdomain_ops = {
> - .translate = irq_domain_translate_onecell,
> + .translate = plic_irq_domain_translate,
> .alloc = plic_irq_domain_alloc,
> .free = irq_domain_free_irqs_top,
> };
> @@ -293,6 +356,11 @@ static int __init plic_init(struct device_node *node,
> if (!priv)
> return -ENOMEM;
>
> + if (of_device_is_compatible(node, "renesas,r9a07g043-plic")) {
> + priv->of_data = RENESAS_R9A07G043_PLIC;
> + plic_chip.name = "Renesas RZ/Five PLIC";
NAK. The irq_chip structure isn't the place for platform marketing.
This is way too long anyway (and same for the edge version), and you
even sent me a patch to make that structure const...
M.
--
Without deviation from the norm, progress is not possible.
