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

[Thomas Gleixner]

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@xxxxxxxxxxx>
> + * Based on the work from Mathieu Hadjimegrian <mathieu.hadjimegrian@xxxxxxxxxx>
> + */
> +
> +#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