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.