Re: [PATCH v1 2/2] irqchip: Add StarFive external interrupt controller

[Philipp Zabel]

On Mi, 2024-01-10 at 18:32 -0800, Changhuang Liang wrote:
[...]
> diff --git a/drivers/irqchip/irq-starfive-jh8100-intc.c b/drivers/irqchip/irq-starfive-jh8100-intc.c
> new file mode 100644
> index 000000000000..be8224111cf2
> --- /dev/null
> +++ b/drivers/irqchip/irq-starfive-jh8100-intc.c
> @@ -0,0 +1,180 @@
[...]
> +struct starfive_irq_chip {
> +	void __iomem *base;
> +	struct irq_domain *root_domain;
> +	struct clk *clk;
> +	struct reset_control *rst;

Since rst is never used outside of starfive_intc_init, there is no need
to store it here.

[...]
> +static int __init starfive_intc_init(struct device_node *intc,
> +				     struct device_node *parent)
> +{
> +	struct starfive_irq_chip *irqc;
> +	int ret;
> +	int parent_irq;
> +
> +	irqc = kzalloc(sizeof(*irqc), GFP_KERNEL);
> +	if (!irqc)
> +		return -ENOMEM;
> +
> +	irqc->base = of_iomap(intc, 0);
> +	if (!irqc->base) {
> +		pr_err("Unable to map IC registers\n");
> +		ret = -ENXIO;
> +		goto err_free;
> +	}
> +
> +	irqc->rst = of_reset_control_get_by_index(intc, 0);

Please use of_reset_control_get_exclusive(intc, NULL) instead.

> +	if (IS_ERR(irqc->rst)) {
> +		pr_err("Unable to get reset control\n");
> +		ret = PTR_ERR(irqc->rst);

Consider printing the error code via %pe.


regards
Philipp