[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d94c79bf-04c4-4e87-bd7e-a8755508ac89@suswa.mountain>
Date: Wed, 29 Nov 2023 11:21:06 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: James Tai <james.tai@...ltek.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
kernel test robot <lkp@...el.com>,
Dan Carpenter <error27@...il.com>
Subject: Re: [PATCH v3 2/6] irqchip: Add interrupt controller support for
Realtek DHC SoCs
On Wed, Nov 29, 2023 at 01:43:35PM +0800, James Tai wrote:
> +static int realtek_intc_subset(struct device_node *node, struct realtek_intc_data *data, int index)
> +{
> + struct realtek_intc_subset_data *subset_data = &data->subset_data[index];
> + const struct realtek_intc_subset_cfg *cfg = &data->info->cfg[index];
> + int irq;
> +
> + irq = irq_of_parse_and_map(node, index);
> + if (irq <= 0)
> + return irq;
I don't think irq_of_parse_and_map() can return negatives. Only zero
on error. Returning zero on error is a historical artifact with IRQ
functions and a constant source of bugs. But here returning zero is
success. See my blog for more details:
https://staticthinking.wordpress.com/2023/08/07/writing-a-check-for-zero-irq-error-codes/
So this should probably be:
irq = irq_of_parse_and_map(node, index);
if (!irq)
return -ENODEV;
I should create a static checker warning for this... Possibly I should
create a static checker warning any time someone does:
if (foo <= 0)
return foo;
> +
> + subset_data->common = data;
> + subset_data->cfg = cfg;
> + subset_data->parent_irq = irq;
> + irq_set_chained_handler_and_data(irq, realtek_intc_handler, subset_data);
> +
> + return 0;
> +}
> +
> +int realtek_intc_probe(struct platform_device *pdev, const struct realtek_intc_info *info)
> +{
> + struct realtek_intc_data *data;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> + int ret, i;
> +
> + data = devm_kzalloc(dev, struct_size(data, subset_data, info->cfg_num), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->base = of_iomap(node, 0);
> + if (!data->base) {
> + ret = -ENOMEM;
> + goto out_cleanup;
devm_ allocations are cleaned up automatically so there is no need to
call devm_kfree() before returning.
regards,
dan carpenter
> + }
> +
> + data->info = info;
> +
> + raw_spin_lock_init(&data->lock);
> +
> + data->domain = irq_domain_add_linear(node, 32, &realtek_intc_domain_ops, data);
> + if (!data->domain) {
> + ret = -ENOMEM;
> + goto out_cleanup;
> + }
> +
> + data->subset_data_num = info->cfg_num;
> + for (i = 0; i < info->cfg_num; i++) {
> + ret = realtek_intc_subset(node, data, i);
> + if (ret) {
> + WARN(ret, "failed to init subset %d: %d", i, ret);
> + ret = -ENOMEM;
> + goto out_cleanup;
> + }
> + }
> +
> + platform_set_drvdata(pdev, data);
> +
> + return 0;
> +
> +out_cleanup:
> +
> + if (data->base)
> + iounmap(data->base);
> +
> + devm_kfree(dev, data);
> +
> + return ret;
> +}
Powered by blists - more mailing lists