Re: [PATCH 2/3] gpio: axiado: add SGPIO controller support

[Krzysztof Kozlowski]

On 14/04/2026 15:48, Petar Stepanovic wrote:
> +
> +	for (i = 0; i < sgpio->max_offset_regs; i++) {
> +		sgpio->slices[2].reg_ss[i] = 0;
> +		dout_value = be32_to_cpu(prop[i]);
> +
> +		for (dout_reverse = 0; dout_reverse < 32; ++dout_reverse) {
> +			sgpio->slices[2].reg_ss[i] <<= 1;
> +			sgpio->slices[2].reg_ss[i] |= (dout_value & 1);
> +			dout_value >>= 1;
> +		}
> +	}
> +
> +	sgpio_hw_init(sgpio);
> +
> +	irq = platform_get_irq(pdev, 0);
> +

Odd style

> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get parent IRQ: %d\n", irq);
> +		return irq;
> +	}
> +	/* Store parent IRQ for cleanup */
> +	sgpio->parent_irq = irq;
> +
> +	rc = devm_request_threaded_irq(&pdev->dev, irq, NULL, sgpio_irq_handler,
> +				       IRQF_ONESHOT, "axiado-sgpio", sgpio);
> +
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Failed to request threaded IRQ %d: %d\n",
> +			irq, rc);

Nope

> +		return rc;
> +	}
> +
> +	sgpio->chip.parent = &pdev->dev;
> +	sgpio->chip.ngpio = sgpio->ngpios * 2;
> +	sgpio->chip.owner = THIS_MODULE;
> +	sgpio->chip.direction_input = ax3000_sgpio_dir_in;
> +	sgpio->chip.direction_output = ax3000_sgpio_dir_out;
> +	sgpio->chip.get = ax3000_sgpio_get;
> +	sgpio->chip.set = ax3000_sgpio_set;
> +	sgpio->chip.label = dev_name(&pdev->dev);
> +	sgpio->chip.base = -1;
> +
> +	girq = &sgpio->chip.irq;
> +
> +	girq->chip = &axiado_sgpio_irqchip;
> +	girq->handler = handle_edge_irq;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->num_parents = 1;
> +	girq->parents =
> +		devm_kcalloc(&pdev->dev, 1, sizeof(*girq->parents), GFP_KERNEL);
> +	if (!girq->parents) {
> +		dev_err(&pdev->dev, "Failed to allocate parents array\n");
> +		return -ENOMEM;

Ykes...

> +	}



> +
> +static struct platform_driver sgpio_driver = {
> +	.driver = {
> +		.name = "sgpio",
> +		.owner = THIS_MODULE,

Uh, that's 13 year old code. Please drop everything and write from
scratch using latest reviewed drivers as your base. No point to repeat
same review and fix the same issues we already fixed during last 13 years...

> +		.of_match_table = ax_sgpio_match,
> +	},
> +	.probe = sgpio_probe,
> +	.remove = sgpio_remove,
> +};
> +
> +static int __init ax_sgpio_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&sgpio_driver);
> +	if (ret < 0) {
> +		pr_err("Failed to register SGPIO driver\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit ax_sgpio_exit(void)
> +{
> +	platform_driver_unregister(&sgpio_driver);
> +}
> +
> +module_init(ax_sgpio_init);
> +module_exit(ax_sgpio_exit);

And that's one more.

module_platform_driver, no?

> +
> +MODULE_DESCRIPTION("Axiado Serial GPIO Driver");
> +MODULE_AUTHOR("Axiado Corporation");
> +MODULE_LICENSE("GPL");
> 


Best regards,
Krzysztof