lkml.org 
[lkml]   [2022]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] spi: dw: Add deferred DMA-channels setup support
On Fri, Jun 10, 2022 at 02:22:28PM +0300, Andy Shevchenko wrote:
> 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@gmail.com>

Thanks.

>
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.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...

Right, but it makes this method looking alike dw_spi_dma_init_mfld().
I even used the same label names. It makes the code a little bit easier
to read.

>
> > + }
> > +
> > + 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.

ditto

-Sergey

>
> > + return ret;
> > }
> >
> > static void dw_spi_dma_exit(struct dw_spi *dws)
> > --
> > 2.35.1
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

\
 
 \ /
  Last update: 2022-06-10 22:32    [W:0.096 / U:0.836 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site