Re: [PATCH 12/16] irqchip/eip201-aic: Add support for Safexcel EIP-201 AIC

Thomas Gleixner
2026-03-28T13:10:50Z

On Fri, Mar 27 2026 at 21:09, Miquel Raynal wrote:
> +config SAFEXCEL_EIP201_AIC
> + tristate "Safexcel EIP201 AIC"

TAB, not spaces please

> + select IRQ_DOMAIN
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2026 Schneider Electric
> + * Authored by Miquel Raynal <miquel.raynal@bootlin.com>
> + * Based on the work from Mathieu Hadjimegrian <mathieu.hadjimegrian@non.se.com>
> + */
> +
> +#include "linux/irq.h"
> +#include "linux/stddef.h"

That's not a standard include format.

> +
> +struct eip201_aic {
> + struct device *dev;
> + void __iomem *regs;
> + struct irq_domain *domain;
> + struct irq_chip_generic *gc;
> + u32 type;
> + u32 pol;
> +};

Please follow:

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers

> +
> + /* Type register indicates:

See 'comment style' in the same document.

> + * - '1' for edge interrupts
> + * - '0' for level interrupts
> + */
> + if (*out_type & IRQ_TYPE_LEVEL_MASK &&
> + EIP201_AIC_INT(aic->type, *out_hwirq))

No line break required. You have 100 characters.

> +static int eip201_aic_probe(struct platform_device *pdev)
> +{
> + struct eip201_aic *aic;
> + struct clk *clk;
> + u32 rev;
> + int irq;
> + int ret;

See 'variable declarations' in the same document.

> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;

Leaks the chip and the domain.

> +static struct platform_driver eip201_aic_driver = {
> + .probe = eip201_aic_probe,
> + .remove = eip201_aic_remove,
> + .driver = {
> + .name = "safexcel-eip201-aic",
> + .of_match_table = eip201_aic_of_match,

See above.

Thanks,

tglx