diff mbox series

[v4,2/2] spi: dw: Add dma controller capability checks

Message ID 20230327043405.881645-3-joychakr@google.com
State New
Headers
Series spi: dw: DW SPI DMA Driver updates |

Commit Message

Joy Chakraborty March 27, 2023, 4:34 a.m. UTC 
  Check capabilities of DMA controller during init to make sure it is
capable of handling MEM2DEV for tx channel, DEV2MEM for rx channel
and store addr_width capabilities to check per transfer to make sure the
bits/word requirement can be met for that transfer.

Signed-off-by: Joy Chakraborty <joychakr@google.com>
---
 drivers/spi/spi-dw-dma.c | 54 ++++++++++++++++++++++++++++++++--------
 drivers/spi/spi-dw.h     |  1 +
 2 files changed, 44 insertions(+), 11 deletions(-)

Comments

Serge Semin March 28, 2023, 6:03 p.m. UTC | #1 
On Mon, Mar 27, 2023 at 04:34:05AM +0000, Joy Chakraborty wrote:
> Check capabilities of DMA controller during init to make sure it is
> capable of handling MEM2DEV for tx channel, DEV2MEM for rx channel
> and store addr_width capabilities to check per transfer to make sure the
> bits/word requirement can be met for that transfer.
> 
> Signed-off-by: Joy Chakraborty <joychakr@google.com>
> ---
>  drivers/spi/spi-dw-dma.c | 54 ++++++++++++++++++++++++++++++++--------
>  drivers/spi/spi-dw.h     |  1 +
>  2 files changed, 44 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> index b3a88bb75907..f47483ec369f 100644
> --- a/drivers/spi/spi-dw-dma.c
> +++ b/drivers/spi/spi-dw-dma.c
> @@ -23,6 +23,8 @@
>  #define DW_SPI_TX_BUSY		1
>  #define DW_SPI_TX_BURST_LEVEL	16
>  
> +static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes);
> +
>  static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
>  {
>  	struct dw_dma_slave *s = param;
> @@ -72,12 +74,15 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
>  	dw_writel(dws, DW_SPI_DMATDLR, dws->txburst);
>  }
>  
> -static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
> +static int dw_spi_dma_caps_init(struct dw_spi *dws)
>  {

> +	int ret;
>  	struct dma_slave_caps tx = {0}, rx = {0};

1. Preserve the reverse xmas tree order please (driver local convention).
2. The zero-initialization can be dropped since the function will halt
on further procedures if any of the dma_get_slave_caps() calls fail.
Meanwhile if all of them are successfully executed the capability
structures will be sanely initialized.

>  
> -	dma_get_slave_caps(dws->txchan, &tx);
> -	dma_get_slave_caps(dws->rxchan, &rx);

> +	ret = dma_get_slave_caps(dws->txchan, &tx);
	if (ret)
		return ret;
< newline
> +	ret |= dma_get_slave_caps(dws->rxchan, &rx);
> +	if (ret)
> +		return ret;

No OR-ing the errnos please. They aren't bitfields.

>  
>  	if (tx.max_sg_burst > 0 && rx.max_sg_burst > 0)
>  		dws->dma_sg_burst = min(tx.max_sg_burst, rx.max_sg_burst);
> @@ -87,6 +92,18 @@ static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
>  		dws->dma_sg_burst = rx.max_sg_burst;
>  	else
>  		dws->dma_sg_burst = 0;
> +
> +	/*
> +	 * Assuming both channels belong to the same DMA controller hence the
> +	 * address width capabilities most likely would be the same.
> +	 */
> +	dws->dma_addr_widths = tx.dst_addr_widths & rx.src_addr_widths;
> +

> +	if (!(tx.directions & BIT(DMA_MEM_TO_DEV) &&
> +	      rx.directions & BIT(DMA_DEV_TO_MEM)))
> +		return -ENXIO;

Please move this to be right after the capabilities are retrieved.
There is no point in the SG-burst and addr-width data initializations
if a DMA-controller isn't suitable for the Tx/Rx DMAs.

-Serge(y)

> +
> +	return 0;
>  }
>  
>  static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> @@ -95,6 +112,7 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
>  	struct dw_dma_slave dma_rx = { .src_id = 0 }, *rx = &dma_rx;
>  	struct pci_dev *dma_dev;
>  	dma_cap_mask_t mask;
> +	int ret = -EBUSY;
>  
>  	/*
>  	 * Get pci device for DMA controller, currently it could only
> @@ -124,20 +142,24 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
>  
>  	init_completion(&dws->dma_completion);
>  
> -	dw_spi_dma_maxburst_init(dws);
> +	ret = dw_spi_dma_caps_init(dws);
> +	if (ret)
> +		goto free_txchan;
>  
> -	dw_spi_dma_sg_burst_init(dws);
> +	dw_spi_dma_maxburst_init(dws);
>  
>  	pci_dev_put(dma_dev);
>  
>  	return 0;
> -
> +free_txchan:
> +	dma_release_channel(dws->txchan);
> +	dws->txchan = NULL;
>  free_rxchan:
>  	dma_release_channel(dws->rxchan);
>  	dws->rxchan = NULL;
>  err_exit:
>  	pci_dev_put(dma_dev);
> -	return -EBUSY;
> +	return ret;
>  }
>  
>  static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> @@ -163,12 +185,16 @@ static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
>  
>  	init_completion(&dws->dma_completion);
>  
> -	dw_spi_dma_maxburst_init(dws);
> +	ret = dw_spi_dma_caps_init(dws);
> +	if (ret)
> +		goto free_txchan;
>  
> -	dw_spi_dma_sg_burst_init(dws);
> +	dw_spi_dma_maxburst_init(dws);
>  
>  	return 0;
> -
> +free_txchan:
> +	dma_release_channel(dws->txchan);
> +	dws->txchan = NULL;
>  free_rxchan:
>  	dma_release_channel(dws->rxchan);
>  	dws->rxchan = NULL;
> @@ -202,8 +228,14 @@ static bool dw_spi_can_dma(struct spi_controller *master,
>  			   struct spi_device *spi, struct spi_transfer *xfer)
>  {
>  	struct dw_spi *dws = spi_controller_get_devdata(master);
> +	enum dma_slave_buswidth dma_bus_width;
> +
> +	if (xfer->len <= dws->fifo_len)
> +		return false;
> +
> +	dma_bus_width = dw_spi_dma_convert_width(dws->n_bytes);
>  
> -	return xfer->len > dws->fifo_len;
> +	return dws->dma_addr_widths & BIT(dma_bus_width);
>  }
>  
>  static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 9e8eb2b52d5c..3962e6dcf880 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -190,6 +190,7 @@ struct dw_spi {
>  	struct dma_chan		*rxchan;
>  	u32			rxburst;
>  	u32			dma_sg_burst;
> +	u32			dma_addr_widths;
>  	unsigned long		dma_chan_busy;
>  	dma_addr_t		dma_addr; /* phy address of the Data register */
>  	const struct dw_spi_dma_ops *dma_ops;
> -- 
> 2.40.0.348.gf938b09366-goog
>
Joy Chakraborty March 29, 2023, 3:56 a.m. UTC | #2 
Hi Serge(y),

On Tue, Mar 28, 2023 at 11:33 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> On Mon, Mar 27, 2023 at 04:34:05AM +0000, Joy Chakraborty wrote:
> > Check capabilities of DMA controller during init to make sure it is
> > capable of handling MEM2DEV for tx channel, DEV2MEM for rx channel
> > and store addr_width capabilities to check per transfer to make sure the
> > bits/word requirement can be met for that transfer.
> >
> > Signed-off-by: Joy Chakraborty <joychakr@google.com>
> > ---
> >  drivers/spi/spi-dw-dma.c | 54 ++++++++++++++++++++++++++++++++--------
> >  drivers/spi/spi-dw.h     |  1 +
> >  2 files changed, 44 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> > index b3a88bb75907..f47483ec369f 100644
> > --- a/drivers/spi/spi-dw-dma.c
> > +++ b/drivers/spi/spi-dw-dma.c
> > @@ -23,6 +23,8 @@
> >  #define DW_SPI_TX_BUSY               1
> >  #define DW_SPI_TX_BURST_LEVEL        16
> >
> > +static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes);
> > +
> >  static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
> >  {
> >       struct dw_dma_slave *s = param;
> > @@ -72,12 +74,15 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> >       dw_writel(dws, DW_SPI_DMATDLR, dws->txburst);
> >  }
> >
> > -static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
> > +static int dw_spi_dma_caps_init(struct dw_spi *dws)
> >  {
>
> > +     int ret;
> >       struct dma_slave_caps tx = {0}, rx = {0};
>
> 1. Preserve the reverse xmas tree order please (driver local convention).
> 2. The zero-initialization can be dropped since the function will halt
> on further procedures if any of the dma_get_slave_caps() calls fail.
> Meanwhile if all of them are successfully executed the capability
> structures will be sanely initialized.
>

Sure, will update

> >
> > -     dma_get_slave_caps(dws->txchan, &tx);
> > -     dma_get_slave_caps(dws->rxchan, &rx);
>
> > +     ret = dma_get_slave_caps(dws->txchan, &tx);
>         if (ret)
>                 return ret;
> < newline
> > +     ret |= dma_get_slave_caps(dws->rxchan, &rx);
> > +     if (ret)
> > +             return ret;
>
> No OR-ing the errnos please. They aren't bitfields.

Sure, I will update this.
In this case do you think we need an error print here to differentiate
which slave caps failed ?

>
> >
> >       if (tx.max_sg_burst > 0 && rx.max_sg_burst > 0)
> >               dws->dma_sg_burst = min(tx.max_sg_burst, rx.max_sg_burst);
> > @@ -87,6 +92,18 @@ static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
> >               dws->dma_sg_burst = rx.max_sg_burst;
> >       else
> >               dws->dma_sg_burst = 0;
> > +
> > +     /*
> > +      * Assuming both channels belong to the same DMA controller hence the
> > +      * address width capabilities most likely would be the same.
> > +      */
> > +     dws->dma_addr_widths = tx.dst_addr_widths & rx.src_addr_widths;
> > +
>
> > +     if (!(tx.directions & BIT(DMA_MEM_TO_DEV) &&
> > +           rx.directions & BIT(DMA_DEV_TO_MEM)))
> > +             return -ENXIO;
>
> Please move this to be right after the capabilities are retrieved.
> There is no point in the SG-burst and addr-width data initializations
> if a DMA-controller isn't suitable for the Tx/Rx DMAs.

On second thought I see that dma_get_slave_caps already checks if
direction exists in dmaengine.c:
...
int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
...
    if (!device->directions)
        return -ENXIO;
...
But it does not check the capability w.r.t the type of channel i.e.
tx/rx , so we can keep this check as well.
Either Way, in case we keep it I shall move this as you suggested.

>
> -Serge(y)
>
> > +
> > +     return 0;
> >  }
> >
> >  static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> > @@ -95,6 +112,7 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> >       struct dw_dma_slave dma_rx = { .src_id = 0 }, *rx = &dma_rx;
> >       struct pci_dev *dma_dev;
> >       dma_cap_mask_t mask;
> > +     int ret = -EBUSY;
> >
> >       /*
> >        * Get pci device for DMA controller, currently it could only
> > @@ -124,20 +142,24 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> >
> >       init_completion(&dws->dma_completion);
> >
> > -     dw_spi_dma_maxburst_init(dws);
> > +     ret = dw_spi_dma_caps_init(dws);
> > +     if (ret)
> > +             goto free_txchan;
> >
> > -     dw_spi_dma_sg_burst_init(dws);
> > +     dw_spi_dma_maxburst_init(dws);
> >
> >       pci_dev_put(dma_dev);
> >
> >       return 0;
> > -
> > +free_txchan:
> > +     dma_release_channel(dws->txchan);
> > +     dws->txchan = NULL;
> >  free_rxchan:
> >       dma_release_channel(dws->rxchan);
> >       dws->rxchan = NULL;
> >  err_exit:
> >       pci_dev_put(dma_dev);
> > -     return -EBUSY;
> > +     return ret;
> >  }
> >
> >  static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> > @@ -163,12 +185,16 @@ static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> >
> >       init_completion(&dws->dma_completion);
> >
> > -     dw_spi_dma_maxburst_init(dws);
> > +     ret = dw_spi_dma_caps_init(dws);
> > +     if (ret)
> > +             goto free_txchan;
> >
> > -     dw_spi_dma_sg_burst_init(dws);
> > +     dw_spi_dma_maxburst_init(dws);
> >
> >       return 0;
> > -
> > +free_txchan:
> > +     dma_release_channel(dws->txchan);
> > +     dws->txchan = NULL;
> >  free_rxchan:
> >       dma_release_channel(dws->rxchan);
> >       dws->rxchan = NULL;
> > @@ -202,8 +228,14 @@ static bool dw_spi_can_dma(struct spi_controller *master,
> >                          struct spi_device *spi, struct spi_transfer *xfer)
> >  {
> >       struct dw_spi *dws = spi_controller_get_devdata(master);
> > +     enum dma_slave_buswidth dma_bus_width;
> > +
> > +     if (xfer->len <= dws->fifo_len)
> > +             return false;
> > +
> > +     dma_bus_width = dw_spi_dma_convert_width(dws->n_bytes);
> >
> > -     return xfer->len > dws->fifo_len;
> > +     return dws->dma_addr_widths & BIT(dma_bus_width);
> >  }
> >
> >  static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
> > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> > index 9e8eb2b52d5c..3962e6dcf880 100644
> > --- a/drivers/spi/spi-dw.h
> > +++ b/drivers/spi/spi-dw.h
> > @@ -190,6 +190,7 @@ struct dw_spi {
> >       struct dma_chan         *rxchan;
> >       u32                     rxburst;
> >       u32                     dma_sg_burst;
> > +     u32                     dma_addr_widths;
> >       unsigned long           dma_chan_busy;
> >       dma_addr_t              dma_addr; /* phy address of the Data register */
> >       const struct dw_spi_dma_ops *dma_ops;
> > --
> > 2.40.0.348.gf938b09366-goog
> >

Thanks
Joy
Serge Semin March 29, 2023, 1:35 p.m. UTC | #3 
On Wed, Mar 29, 2023 at 09:26:47AM +0530, Joy Chakraborty wrote:
> Hi Serge(y),
> 
> On Tue, Mar 28, 2023 at 11:33 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> >
> > On Mon, Mar 27, 2023 at 04:34:05AM +0000, Joy Chakraborty wrote:
> > > Check capabilities of DMA controller during init to make sure it is
> > > capable of handling MEM2DEV for tx channel, DEV2MEM for rx channel
> > > and store addr_width capabilities to check per transfer to make sure the
> > > bits/word requirement can be met for that transfer.
> > >
> > > Signed-off-by: Joy Chakraborty <joychakr@google.com>
> > > ---
> > >  drivers/spi/spi-dw-dma.c | 54 ++++++++++++++++++++++++++++++++--------
> > >  drivers/spi/spi-dw.h     |  1 +
> > >  2 files changed, 44 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> > > index b3a88bb75907..f47483ec369f 100644
> > > --- a/drivers/spi/spi-dw-dma.c
> > > +++ b/drivers/spi/spi-dw-dma.c
> > > @@ -23,6 +23,8 @@
> > >  #define DW_SPI_TX_BUSY               1
> > >  #define DW_SPI_TX_BURST_LEVEL        16
> > >
> > > +static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes);
> > > +
> > >  static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
> > >  {
> > >       struct dw_dma_slave *s = param;
> > > @@ -72,12 +74,15 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> > >       dw_writel(dws, DW_SPI_DMATDLR, dws->txburst);
> > >  }
> > >
> > > -static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
> > > +static int dw_spi_dma_caps_init(struct dw_spi *dws)
> > >  {
> >
> > > +     int ret;
> > >       struct dma_slave_caps tx = {0}, rx = {0};
> >
> > 1. Preserve the reverse xmas tree order please (driver local convention).
> > 2. The zero-initialization can be dropped since the function will halt
> > on further procedures if any of the dma_get_slave_caps() calls fail.
> > Meanwhile if all of them are successfully executed the capability
> > structures will be sanely initialized.
> >
> 
> Sure, will update
> 
> > >
> > > -     dma_get_slave_caps(dws->txchan, &tx);
> > > -     dma_get_slave_caps(dws->rxchan, &rx);
> >
> > > +     ret = dma_get_slave_caps(dws->txchan, &tx);
> >         if (ret)
> >                 return ret;
> > < newline
> > > +     ret |= dma_get_slave_caps(dws->rxchan, &rx);
> > > +     if (ret)
> > > +             return ret;
> >
> > No OR-ing the errnos please. They aren't bitfields.
> 
> Sure, I will update this.

> In this case do you think we need an error print here to differentiate
> which slave caps failed ?

No. dw_spi_add_host() will inform (dev_warn()) about the
DMA-initialization failures. Printing something besides of that
won't help much since in any case an additional debugging would be
necessary.

> 
> >
> > >
> > >       if (tx.max_sg_burst > 0 && rx.max_sg_burst > 0)
> > >               dws->dma_sg_burst = min(tx.max_sg_burst, rx.max_sg_burst);
> > > @@ -87,6 +92,18 @@ static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
> > >               dws->dma_sg_burst = rx.max_sg_burst;
> > >       else
> > >               dws->dma_sg_burst = 0;
> > > +
> > > +     /*
> > > +      * Assuming both channels belong to the same DMA controller hence the
> > > +      * address width capabilities most likely would be the same.
> > > +      */
> > > +     dws->dma_addr_widths = tx.dst_addr_widths & rx.src_addr_widths;
> > > +
> >
> > > +     if (!(tx.directions & BIT(DMA_MEM_TO_DEV) &&
> > > +           rx.directions & BIT(DMA_DEV_TO_MEM)))
> > > +             return -ENXIO;
> >
> > Please move this to be right after the capabilities are retrieved.
> > There is no point in the SG-burst and addr-width data initializations
> > if a DMA-controller isn't suitable for the Tx/Rx DMAs.
> 
> On second thought I see that dma_get_slave_caps already checks if
> direction exists in dmaengine.c:
> ...
> int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps)
> ...
>     if (!device->directions)
>         return -ENXIO;
> ...
> But it does not check the capability w.r.t the type of channel i.e.
> tx/rx , so we can keep this check as well.

Right. dma_get_slave_caps() doesn't check the particular channel
dir capability which is essential for us in this case. So please keep the
directions check here.

> Either Way, in case we keep it I shall move this as you suggested.

Ok. Thanks.

-Serge(y)

> 
> >
> > -Serge(y)
> >
> > > +
> > > +     return 0;
> > >  }
> > >
> > >  static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> > > @@ -95,6 +112,7 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> > >       struct dw_dma_slave dma_rx = { .src_id = 0 }, *rx = &dma_rx;
> > >       struct pci_dev *dma_dev;
> > >       dma_cap_mask_t mask;
> > > +     int ret = -EBUSY;
> > >
> > >       /*
> > >        * Get pci device for DMA controller, currently it could only
> > > @@ -124,20 +142,24 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> > >
> > >       init_completion(&dws->dma_completion);
> > >
> > > -     dw_spi_dma_maxburst_init(dws);
> > > +     ret = dw_spi_dma_caps_init(dws);
> > > +     if (ret)
> > > +             goto free_txchan;
> > >
> > > -     dw_spi_dma_sg_burst_init(dws);
> > > +     dw_spi_dma_maxburst_init(dws);
> > >
> > >       pci_dev_put(dma_dev);
> > >
> > >       return 0;
> > > -
> > > +free_txchan:
> > > +     dma_release_channel(dws->txchan);
> > > +     dws->txchan = NULL;
> > >  free_rxchan:
> > >       dma_release_channel(dws->rxchan);
> > >       dws->rxchan = NULL;
> > >  err_exit:
> > >       pci_dev_put(dma_dev);
> > > -     return -EBUSY;
> > > +     return ret;
> > >  }
> > >
> > >  static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> > > @@ -163,12 +185,16 @@ static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
> > >
> > >       init_completion(&dws->dma_completion);
> > >
> > > -     dw_spi_dma_maxburst_init(dws);
> > > +     ret = dw_spi_dma_caps_init(dws);
> > > +     if (ret)
> > > +             goto free_txchan;
> > >
> > > -     dw_spi_dma_sg_burst_init(dws);
> > > +     dw_spi_dma_maxburst_init(dws);
> > >
> > >       return 0;
> > > -
> > > +free_txchan:
> > > +     dma_release_channel(dws->txchan);
> > > +     dws->txchan = NULL;
> > >  free_rxchan:
> > >       dma_release_channel(dws->rxchan);
> > >       dws->rxchan = NULL;
> > > @@ -202,8 +228,14 @@ static bool dw_spi_can_dma(struct spi_controller *master,
> > >                          struct spi_device *spi, struct spi_transfer *xfer)
> > >  {
> > >       struct dw_spi *dws = spi_controller_get_devdata(master);
> > > +     enum dma_slave_buswidth dma_bus_width;
> > > +
> > > +     if (xfer->len <= dws->fifo_len)
> > > +             return false;
> > > +
> > > +     dma_bus_width = dw_spi_dma_convert_width(dws->n_bytes);
> > >
> > > -     return xfer->len > dws->fifo_len;
> > > +     return dws->dma_addr_widths & BIT(dma_bus_width);
> > >  }
> > >
> > >  static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
> > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> > > index 9e8eb2b52d5c..3962e6dcf880 100644
> > > --- a/drivers/spi/spi-dw.h
> > > +++ b/drivers/spi/spi-dw.h
> > > @@ -190,6 +190,7 @@ struct dw_spi {
> > >       struct dma_chan         *rxchan;
> > >       u32                     rxburst;
> > >       u32                     dma_sg_burst;
> > > +     u32                     dma_addr_widths;
> > >       unsigned long           dma_chan_busy;
> > >       dma_addr_t              dma_addr; /* phy address of the Data register */
> > >       const struct dw_spi_dma_ops *dma_ops;
> > > --
> > > 2.40.0.348.gf938b09366-goog
> > >
> 
> Thanks
> Joy
diff mbox series

Patch

diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
index b3a88bb75907..f47483ec369f 100644
--- a/drivers/spi/spi-dw-dma.c
+++ b/drivers/spi/spi-dw-dma.c
@@ -23,6 +23,8 @@ 
 #define DW_SPI_TX_BUSY		1
 #define DW_SPI_TX_BURST_LEVEL	16
 
+static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes);
+
 static bool dw_spi_dma_chan_filter(struct dma_chan *chan, void *param)
 {
 	struct dw_dma_slave *s = param;
@@ -72,12 +74,15 @@  static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
 	dw_writel(dws, DW_SPI_DMATDLR, dws->txburst);
 }
 
-static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
+static int dw_spi_dma_caps_init(struct dw_spi *dws)
 {
+	int ret;
 	struct dma_slave_caps tx = {0}, rx = {0};
 
-	dma_get_slave_caps(dws->txchan, &tx);
-	dma_get_slave_caps(dws->rxchan, &rx);
+	ret = dma_get_slave_caps(dws->txchan, &tx);
+	ret |= dma_get_slave_caps(dws->rxchan, &rx);
+	if (ret)
+		return ret;
 
 	if (tx.max_sg_burst > 0 && rx.max_sg_burst > 0)
 		dws->dma_sg_burst = min(tx.max_sg_burst, rx.max_sg_burst);
@@ -87,6 +92,18 @@  static void dw_spi_dma_sg_burst_init(struct dw_spi *dws)
 		dws->dma_sg_burst = rx.max_sg_burst;
 	else
 		dws->dma_sg_burst = 0;
+
+	/*
+	 * Assuming both channels belong to the same DMA controller hence the
+	 * address width capabilities most likely would be the same.
+	 */
+	dws->dma_addr_widths = tx.dst_addr_widths & rx.src_addr_widths;
+
+	if (!(tx.directions & BIT(DMA_MEM_TO_DEV) &&
+	      rx.directions & BIT(DMA_DEV_TO_MEM)))
+		return -ENXIO;
+
+	return 0;
 }
 
 static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
@@ -95,6 +112,7 @@  static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
 	struct dw_dma_slave dma_rx = { .src_id = 0 }, *rx = &dma_rx;
 	struct pci_dev *dma_dev;
 	dma_cap_mask_t mask;
+	int ret = -EBUSY;
 
 	/*
 	 * Get pci device for DMA controller, currently it could only
@@ -124,20 +142,24 @@  static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
 
 	init_completion(&dws->dma_completion);
 
-	dw_spi_dma_maxburst_init(dws);
+	ret = dw_spi_dma_caps_init(dws);
+	if (ret)
+		goto free_txchan;
 
-	dw_spi_dma_sg_burst_init(dws);
+	dw_spi_dma_maxburst_init(dws);
 
 	pci_dev_put(dma_dev);
 
 	return 0;
-
+free_txchan:
+	dma_release_channel(dws->txchan);
+	dws->txchan = NULL;
 free_rxchan:
 	dma_release_channel(dws->rxchan);
 	dws->rxchan = NULL;
 err_exit:
 	pci_dev_put(dma_dev);
-	return -EBUSY;
+	return ret;
 }
 
 static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
@@ -163,12 +185,16 @@  static int dw_spi_dma_init_generic(struct device *dev, struct dw_spi *dws)
 
 	init_completion(&dws->dma_completion);
 
-	dw_spi_dma_maxburst_init(dws);
+	ret = dw_spi_dma_caps_init(dws);
+	if (ret)
+		goto free_txchan;
 
-	dw_spi_dma_sg_burst_init(dws);
+	dw_spi_dma_maxburst_init(dws);
 
 	return 0;
-
+free_txchan:
+	dma_release_channel(dws->txchan);
+	dws->txchan = NULL;
 free_rxchan:
 	dma_release_channel(dws->rxchan);
 	dws->rxchan = NULL;
@@ -202,8 +228,14 @@  static bool dw_spi_can_dma(struct spi_controller *master,
 			   struct spi_device *spi, struct spi_transfer *xfer)
 {
 	struct dw_spi *dws = spi_controller_get_devdata(master);
+	enum dma_slave_buswidth dma_bus_width;
+
+	if (xfer->len <= dws->fifo_len)
+		return false;
+
+	dma_bus_width = dw_spi_dma_convert_width(dws->n_bytes);
 
-	return xfer->len > dws->fifo_len;
+	return dws->dma_addr_widths & BIT(dma_bus_width);
 }
 
 static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 9e8eb2b52d5c..3962e6dcf880 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -190,6 +190,7 @@  struct dw_spi {
 	struct dma_chan		*rxchan;
 	u32			rxburst;
 	u32			dma_sg_burst;
+	u32			dma_addr_widths;
 	unsigned long		dma_chan_busy;
 	dma_addr_t		dma_addr; /* phy address of the Data register */
 	const struct dw_spi_dma_ops *dma_ops;