On Fri, Dec 11, 2020 at 01:15:58PM -0800, Sowjanya Komatineni wrote: > Tegra SoC has a Quad SPI controller starting from Tegra210. The probe/remove hooks LGTM now. Just two tiny nits that caught my eye: > + master = devm_spi_alloc_master(&pdev->dev, sizeof(*tqspi)); > + if (!master) { > + dev_err(&pdev->dev, "failed to allocate spi_master\n"); > + return -ENOMEM; > + } The memory allocater already emits an error message if it can't satisfy a request. Emitting an additional message here isn't really necessary. > +exit_free_irq: > + free_irq(qspi_irq, tqspi); > +exit_pm_disable: > + pm_runtime_disable(&pdev->dev); > + tegra_qspi_deinit_dma(tqspi); > + return ret; > +} > + > +static int tegra_qspi_remove(struct platform_device *pdev) > +{ > + struct spi_master *master = platform_get_drvdata(pdev); > + struct tegra_qspi *tqspi = spi_master_get_devdata(master); > + > + spi_unregister_master(master); > + free_irq(tqspi->irq, tqspi); > + tegra_qspi_deinit_dma(tqspi); > + pm_runtime_disable(&pdev->dev); The order of tegra_qspi_deinit_dma() and pm_runtime_disable() is reversed in the remove hook vis-a-vis the probe error path. It's nicer to use the same order as in the probe error path. Thanks, Lukas