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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 24 Jan 2024 10:53:53 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Changhuang Liang <changhuang.liang@...rfivetech.com>, Thomas Gleixner
 <tglx@...utronix.de>, Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>
Cc: Ley Foon Tan <leyfoon.tan@...rfivetech.com>, Jack Zhu
	 <jack.zhu@...rfivetech.com>, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH v1 2/2] irqchip: Add StarFive external interrupt
 controller

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