[PATCH 2/3] spi: axiado: Add driver for Axiado SPI DB controller

[Mark Brown]

On Mon, Sep 15, 2025 at 06:11:56AM -0700, Vladimir Moravcevic wrote:

> +	/*Calculate the maximum data payload that can fit into the FIFO. */
> +	if (fifo_total_bytes <= protocol_overhead_bytes) {
> +		max_transfer_payload_bytes = 0;
> +		dev_warn(&spi->dev, "SPI FIFO (%zu bytes) is too small for protocol overhead (%zu bytes)! Max data size forced to 0.\n",
> +			 fifo_total_bytes, protocol_overhead_bytes);

This might be a good fit for dev_warn_once(), I imagine if gets
triggered lots of whatever operation triggers it will happen and the
current code would spam the logs.

> +	ret = devm_request_irq(&pdev->dev, irq, ax_spi_irq,
> +			       0, pdev->name, ctlr);
> +	if (ret != 0) {
> +		ret = -ENXIO;
> +		dev_err(&pdev->dev, "request_irq failed\n");
> +		goto clk_dis_all;
> +	}

None of the other allocations are managed using devm, you most likely
have unsafe race conditions especially if the interrupt line is shared.

> +static void ax_spi_remove(struct platform_device *pdev)
> +{
> +	struct spi_controller *ctlr = platform_get_drvdata(pdev);
> +	struct ax_spi *xspi = spi_controller_get_devdata(ctlr);
> +
> +	clk_disable_unprepare(xspi->ref_clk);
> +	clk_disable_unprepare(xspi->pclk);
> +	pm_runtime_set_suspended(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
> +	spi_unregister_controller(ctlr);
> +}

This will do a bunch of teardown before unregistering the controller
meaning that new operations might be submitted after the clocks are
disabled which I imagine won't go well.  You should unregister from the
subsystem first, then tear down the other resources.

> +
> +static struct platform_driver ax_spi_driver = {
> +	.probe	= ax_spi_probe,
> +	.remove	= ax_spi_remove,
> +	.driver = {
> +		.name = AX_SPI_NAME,
> +		.of_match_table = ax_spi_of_match,
> +	},
> +};

There were a bunch of runtime PM calls but there are no PM operations
here at all.  That's not specifically a problem, for example power
domain level PM with full state retention would work here, but it seems
like at least stopping and starting the clocks would be a good idea.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20250915/4bbd3467/attachment.sig>