[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YqMpdEitU/84oUWV@smile.fi.intel.com>
Date: Fri, 10 Jun 2022 14:22:28 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc: Serge Semin <fancer.lancer@...il.com>,
Mark Brown <broonie@...nel.org>,
Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi: dw: Add deferred DMA-channels setup support
On Fri, Jun 10, 2022 at 10:50:06AM +0300, Serge Semin wrote:
> Currently if the source DMA device isn't ready to provide the channels
> capable of the SPI DMA transfers, the DW SSI controller will be registered
> with no DMA support. It isn't right since all what the driver needs to do
> is to postpone the probe procedure until the DMA device is ready. Let's
> fix that in the framework of the DWC SSI generic DMA implementation. First
> we need to use the dma_request_chan() method instead of the
> dma_request_slave_channel() function, because the later one is deprecated
> and most importantly doesn't return the failure cause but the
> NULL-pointer. Second we need to stop the DW SSI controller probe procedure
> if the -EPROBE_DEFER error is returned on the DMA initialization. The
> procedure will resume later when the channels are ready to be requested.
One nit-pick below
Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> ---
> drivers/spi/spi-dw-core.c | 5 ++++-
> drivers/spi/spi-dw-dma.c | 25 ++++++++++++++++++-------
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index ecea471ff42c..911ea9bddbee 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -942,7 +942,9 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>
> if (dws->dma_ops && dws->dma_ops->dma_init) {
> ret = dws->dma_ops->dma_init(dev, dws);
> - if (ret) {
> + if (ret == -EPROBE_DEFER) {
> + goto err_free_irq;
> + } else if (ret) {
> dev_warn(dev, "DMA init failed\n");
> } else {
> master->can_dma = dws->dma_ops->can_dma;
> @@ -963,6 +965,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
> if (dws->dma_ops && dws->dma_ops->dma_exit)
> dws->dma_ops->dma_exit(dws);
> dw_spi_enable_chip(dws, 0);
> +err_free_irq:
> free_irq(dws->irq, master);
> err_free_master:
> spi_controller_put(master);
> diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> index 63e5260100ec..1322b8cce5b7 100644
> --- a/drivers/spi/spi-dw-dma.c
> +++ b/drivers/spi/spi-dw-dma.c
> @@ -139,15 +139,20 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
>
> static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> {
> - dws->rxchan = dma_request_slave_channel(dev, "rx");
> - if (!dws->rxchan)
> - return -ENODEV;
> + int ret;
>
> - dws->txchan = dma_request_slave_channel(dev, "tx");
> - if (!dws->txchan) {
> - dma_release_channel(dws->rxchan);
> + dws->rxchan = dma_request_chan(dev, "rx");
> + if (IS_ERR(dws->rxchan)) {
> + ret = PTR_ERR(dws->rxchan);
> dws->rxchan = NULL;
> - return -ENODEV;
> + goto err_exit;
This change doesn't bring anything...
> + }
> +
> + dws->txchan = dma_request_chan(dev, "tx");
> + if (IS_ERR(dws->txchan)) {
> + ret = PTR_ERR(dws->txchan);
> + dws->txchan = NULL;
> + goto free_rxchan;
> }
>
> dws->master->dma_rx = dws->rxchan;
> @@ -160,6 +165,12 @@ static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> dw_spi_dma_sg_burst_init(dws);
>
> return 0;
> +
> +free_rxchan:
> + dma_release_channel(dws->rxchan);
> + dws->rxchan = NULL;
> +err_exit:
...and this too.
> + return ret;
> }
>
> static void dw_spi_dma_exit(struct dw_spi *dws)
> --
> 2.35.1
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists