Use devm_pm_runtime_enable(), devm_request_irq() and
devm_spi_register_controller() to simplify code.

And also register a callback spi_geni_release_dma_chan() with
devm_add_action_or_reset(), to release dma channel in both error
and device detach path, which can make sure the release sequence is
consistent with the original one.

1. Unregister spi controller.
2. Free the IRQ.
3. Free DMA chans
4. Disable runtime PM.

So the remove function can also be removed.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Suggested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v5:
- Do devm_add_action_or_reset() straight in spi_geni_grab_gpi_chan().
- Return devm_spi_register_controller() instead of return ret or 0.
- Add reviewed-by.
v4:
- Correct the "data" of devm_add_action_or_reset().
v3:
- Land the rest of the cleanups afterwards.
---
 drivers/spi/spi-geni-qcom.c | 63 ++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 6f4057330444..f6e40f90418f 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -604,6 +604,21 @@ static int spi_geni_prepare_message(struct spi_controller *spi,
 	return -EINVAL;
 }
 
+static void spi_geni_release_dma_chan(void *data)
+{
+	struct spi_geni_master *mas = data;
+
+	if (mas->rx) {
+		dma_release_channel(mas->rx);
+		mas->rx = NULL;
+	}
+
+	if (mas->tx) {
+		dma_release_channel(mas->tx);
+		mas->tx = NULL;
+	}
+}
+
 static int spi_geni_grab_gpi_chan(struct spi_geni_master *mas)
 {
 	int ret;
@@ -622,6 +637,12 @@ static int spi_geni_grab_gpi_chan(struct spi_geni_master *mas)
 		goto err_rx;
 	}
 
+	ret = devm_add_action_or_reset(mas->dev, spi_geni_release_dma_chan, mas);
+	if (ret) {
+		dev_err(mas->dev, "Unable to add action.\n");
+		return ret;
+	}
+
 	return 0;
 
 err_rx:
@@ -632,19 +653,6 @@ static int spi_geni_grab_gpi_chan(struct spi_geni_master *mas)
 	return ret;
 }
 
-static void spi_geni_release_dma_chan(struct spi_geni_master *mas)
-{
-	if (mas->rx) {
-		dma_release_channel(mas->rx);
-		mas->rx = NULL;
-	}
-
-	if (mas->tx) {
-		dma_release_channel(mas->tx);
-		mas->tx = NULL;
-	}
-}
-
 static int spi_geni_init(struct spi_geni_master *mas)
 {
 	struct spi_controller *spi = dev_get_drvdata(mas->dev);
@@ -1146,33 +1154,11 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (mas->cur_xfer_mode == GENI_GPI_DMA)
 		spi->flags = SPI_CONTROLLER_MUST_TX;
 
-	ret = request_irq(mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
-	if (ret)
-		goto spi_geni_release_dma;
-
-	ret = spi_register_controller(spi);
+	ret = devm_request_irq(dev, mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
 	if (ret)
-		goto spi_geni_probe_free_irq;
-
-	return 0;
-spi_geni_probe_free_irq:
-	free_irq(mas->irq, spi);
-spi_geni_release_dma:
-	spi_geni_release_dma_chan(mas);
-	return ret;
-}
-
-static void spi_geni_remove(struct platform_device *pdev)
-{
-	struct spi_controller *spi = platform_get_drvdata(pdev);
-	struct spi_geni_master *mas = spi_controller_get_devdata(spi);
-
-	/* Unregister _before_ disabling pm_runtime() so we stop transfers */
-	spi_unregister_controller(spi);
-
-	free_irq(mas->irq, spi);
+		return ret;
 
-	spi_geni_release_dma_chan(mas);
+	return devm_spi_register_controller(dev, spi);
 }
 
 static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
@@ -1254,7 +1240,6 @@ MODULE_DEVICE_TABLE(of, spi_geni_dt_match);
 
 static struct platform_driver spi_geni_driver = {
 	.probe  = spi_geni_probe,
-	.remove_new = spi_geni_remove,
 	.driver = {
 		.name = "geni_spi",
 		.pm = &spi_geni_pm_ops,
-- 
2.34.1

On Thu, 12 Sep 2024 17:17:01 +0800, Jinjie Ruan wrote:
> Use devm_pm_runtime_enable(), devm_request_irq() and
> devm_spi_register_controller() to simplify code.
> 
> And also register a callback spi_geni_release_dma_chan() with
> devm_add_action_or_reset(), to release dma channel in both error
> and device detach path, which can make sure the release sequence is
> consistent with the original one.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: geni-qcom: Use devm functions to simplify code
      commit: 07f1eb718db281c3e0cdb068ea7d73c30921a81c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark