Re: [PATCH v3 5/6] irqchip/renesas-rzv2h: Add CA55 software interrupt support

[Thomas Gleixner <tglx@kernel.org>]

On Mon, Feb 09 2026 at 10:41, Prabhakar wrote:
>  static int rzv2h_icu_set_type(struct irq_data *d, unsigned int type)
>  {
> +	unsigned int gic_type = IRQ_TYPE_LEVEL_HIGH;
>  	unsigned int hw_irq = irqd_to_hwirq(d);
>  	int ret;
>  
> @@ -445,6 +475,11 @@ static int rzv2h_icu_set_type(struct irq_data *d, unsigned int type)
>  		/* TINT */
>  		ret = rzv2h_tint_set_type(d, type);
>  		break;
> +	case ICU_CA55_INT_START ... ICU_CA55_INT_LAST:
> +		/* CA55 Software Interrupts have EDGE_RISING type */
> +		gic_type = IRQ_TYPE_EDGE_RISING;

So this unconditionally selects EDGE_RISING independent of the type
provided by the caller. Interesting choice and compatible with the rest
of the code - _not_.

> +
> +static int rzv2h_icu_setup_irqs(struct platform_device *pdev,
> +				struct irq_domain *irq_domain)

I told you before that you have 100 characters. Get rid of these line breaks.

> +{
> +	bool irq_inject = IS_ENABLED(CONFIG_GENERIC_IRQ_INJECTION);
> +	static const char * const rzv2h_swint_names[] = {
> +		"int-ca55-0", "int-ca55-1",
> +		"int-ca55-2", "int-ca55-3",
> +	};
> +	static const u8 swint_idx[] = { 0, 1, 2, 3 };
> +	struct device *dev = &pdev->dev;
> +	struct irq_fwspec fwspec;
> +	unsigned int virq;
> +	unsigned int i;

Coalesce same types into a single line. See Documentation/....

> +	int ret;
> +
> +	for (i = 0; i < ICU_CA55_INT_COUNT && irq_inject; i++) {
> +		fwspec.fwnode = irq_domain->fwnode;
> +		fwspec.param_count = 2;
> +		fwspec.param[0] = ICU_CA55_INT_START + i;
> +		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +
> +		virq = irq_create_fwspec_mapping(&fwspec);
> +		if (!virq)
> +			return dev_err_probe(dev, -EINVAL, "failed to create IRQ mapping for %s\n",
> +					     rzv2h_swint_names[i]);

This lacks curly brackets on the if(). See Documentation/.....


> +		ret = devm_request_irq(dev, virq, rzv2h_icu_swint_irq, 0, dev_name(dev),
> +				       (void *)&swint_idx[i]);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to request %s IRQ\n",
> +					     rzv2h_swint_names[i]);

Ditto.

> +	}

Thanks,

        tglx