spi: dw: Add 32 bpw support to DW DMA Controller

  If DW Controller is capable of 32 bits per word support then SW or DMA
controller has to write 32bit or 4byte data to the FIFO at a time.

This Patch adds support for AxSize = 4 bytes configuration from dw dma
driver if n_bytes i.e. number of bytes per write to fifo is 4.

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

Hello Joy.

On Mon, Mar 20, 2023 at 05:57:46AM +0000, Joy Chakraborty wrote:
> If DW Controller is capable of 32 bits per word support then SW or DMA
> controller has to write 32bit or 4byte data to the FIFO at a time.
> 
> This Patch adds support for AxSize = 4 bytes configuration from dw dma
> driver if n_bytes i.e. number of bytes per write to fifo is 4.
> 
> Signed-off-by: Joy Chakraborty <joychakr@google.com>

> ---
>  drivers/spi/spi-dw-dma.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> index ababb910b391..7d06ecfdebe1 100644
> --- a/drivers/spi/spi-dw-dma.c
> +++ b/drivers/spi/spi-dw-dma.c
> @@ -212,6 +212,8 @@ static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
>  		return DMA_SLAVE_BUSWIDTH_1_BYTE;
>  	else if (n_bytes == 2)
>  		return DMA_SLAVE_BUSWIDTH_2_BYTES;

> +	else if (n_bytes == 4)
> +		return DMA_SLAVE_BUSWIDTH_4_BYTES;

In case of the DFS-width being of 32-bits size n_bytes can be 4 and
theoretically _3_ (practically it's unluckily, but anyway). Here
it is:
...
if (dws->caps & DW_SPI_CAP_DFS32)
	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
...
dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
...

So what about converting the dw_spi_dma_convert_width() method to
having the switch-case statement and adding the adjacent "case 3:
case 4:" statement there?

* We could add the individual case-3 branch with DMA_SLAVE_BUSWIDTH_3_BYTES
* returned, but the DMA-engines with 3-bytes bus width capability are
* so rare. So is the case of having n_bytes == 3. Thus I guess it
* won't hurt to extend the bus up to four bytes even though there are
* only three bytes required.

Please also note. Currently the spi-dw-dma.o driver doesn't make sure
that the requested buswidth is actually supported by the DMA-engine
(see dma_slave_caps.{src,dst}_addr_widths fields semantics). It would
be nice to have some sanity check in there, but until then note DMA
may still fail even if you specify a correct buswidth.

-Serge(y)

>  
>  	return DMA_SLAVE_BUSWIDTH_UNDEFINED;
>  }
> -- 
> 2.40.0.rc1.284.g88254d51c5-goog
>
  

Hi Serge(y),

On Mon, Mar 20, 2023 at 7:34 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> Hello Joy.
>
> On Mon, Mar 20, 2023 at 05:57:46AM +0000, Joy Chakraborty wrote:
> > If DW Controller is capable of 32 bits per word support then SW or DMA
> > controller has to write 32bit or 4byte data to the FIFO at a time.
> >
> > This Patch adds support for AxSize = 4 bytes configuration from dw dma
> > driver if n_bytes i.e. number of bytes per write to fifo is 4.
> >
> > Signed-off-by: Joy Chakraborty <joychakr@google.com>
>
> > ---
> >  drivers/spi/spi-dw-dma.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> > index ababb910b391..7d06ecfdebe1 100644
> > --- a/drivers/spi/spi-dw-dma.c
> > +++ b/drivers/spi/spi-dw-dma.c
> > @@ -212,6 +212,8 @@ static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
> >               return DMA_SLAVE_BUSWIDTH_1_BYTE;
> >       else if (n_bytes == 2)
> >               return DMA_SLAVE_BUSWIDTH_2_BYTES;
>
> > +     else if (n_bytes == 4)
> > +             return DMA_SLAVE_BUSWIDTH_4_BYTES;
>
> In case of the DFS-width being of 32-bits size n_bytes can be 4 and
> theoretically _3_ (practically it's unluckily, but anyway). Here
> it is:
> ...
> if (dws->caps & DW_SPI_CAP_DFS32)
>         master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
> ...
> dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
> ...
>
> So what about converting the dw_spi_dma_convert_width() method to
> having the switch-case statement and adding the adjacent "case 3:
> case 4:" statement there?
>
> * We could add the individual case-3 branch with DMA_SLAVE_BUSWIDTH_3_BYTES
> * returned, but the DMA-engines with 3-bytes bus width capability are
> * so rare. So is the case of having n_bytes == 3. Thus I guess it
> * won't hurt to extend the bus up to four bytes even though there are
> * only three bytes required.
>
> Please also note. Currently the spi-dw-dma.o driver doesn't make sure
> that the requested buswidth is actually supported by the DMA-engine
> (see dma_slave_caps.{src,dst}_addr_widths fields semantics). It would
> be nice to have some sanity check in there, but until then note DMA
> may still fail even if you specify a correct buswidth.
>
> -Serge(y)

Will be creating a V2 Patch with the suggested changes.
For the case of n_bytes = 3 would be using DMA_SLAVE_BUSWIDTH_4_BYTES
since that is what
the FIFO driver is doing as well while writing to the FIFO register in
the core driver:
...
        if (dws->n_bytes == 1)
                txw = *(u8 *)(dws->tx);
        else if (dws->n_bytes == 2)
                txw = *(u16 *)(dws->tx);
        else
                txw = *(u32 *)(dws->tx);
...

For the case of sanity checking of dma capability, I am planning to
add it to can_dma function so that it
defaults to a non-dma mode if the DMA controller is not capable of
satisfying the bits/word requirement.

Thanks
Joy
>
> >
> >       return DMA_SLAVE_BUSWIDTH_UNDEFINED;
> >  }
> > --
> > 2.40.0.rc1.284.g88254d51c5-goog
> >


On Mon, Mar 20, 2023 at 7:34 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> Hello Joy.
>
> On Mon, Mar 20, 2023 at 05:57:46AM +0000, Joy Chakraborty wrote:
> > If DW Controller is capable of 32 bits per word support then SW or DMA
> > controller has to write 32bit or 4byte data to the FIFO at a time.
> >
> > This Patch adds support for AxSize = 4 bytes configuration from dw dma
> > driver if n_bytes i.e. number of bytes per write to fifo is 4.
> >
> > Signed-off-by: Joy Chakraborty <joychakr@google.com>
>
> > ---
> >  drivers/spi/spi-dw-dma.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> > index ababb910b391..7d06ecfdebe1 100644
> > --- a/drivers/spi/spi-dw-dma.c
> > +++ b/drivers/spi/spi-dw-dma.c
> > @@ -212,6 +212,8 @@ static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
> >               return DMA_SLAVE_BUSWIDTH_1_BYTE;
> >       else if (n_bytes == 2)
> >               return DMA_SLAVE_BUSWIDTH_2_BYTES;
>
> > +     else if (n_bytes == 4)
> > +             return DMA_SLAVE_BUSWIDTH_4_BYTES;
>
> In case of the DFS-width being of 32-bits size n_bytes can be 4 and
> theoretically _3_ (practically it's unluckily, but anyway). Here
> it is:
> ...
> if (dws->caps & DW_SPI_CAP_DFS32)
>         master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
> ...
> dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
> ...
>
> So what about converting the dw_spi_dma_convert_width() method to
> having the switch-case statement and adding the adjacent "case 3:
> case 4:" statement there?
>
> * We could add the individual case-3 branch with DMA_SLAVE_BUSWIDTH_3_BYTES
> * returned, but the DMA-engines with 3-bytes bus width capability are
> * so rare. So is the case of having n_bytes == 3. Thus I guess it
> * won't hurt to extend the bus up to four bytes even though there are
> * only three bytes required.
>
> Please also note. Currently the spi-dw-dma.o driver doesn't make sure
> that the requested buswidth is actually supported by the DMA-engine
> (see dma_slave_caps.{src,dst}_addr_widths fields semantics). It would
> be nice to have some sanity check in there, but until then note DMA
> may still fail even if you specify a correct buswidth.
>
> -Serge(y)
>
> >
> >       return DMA_SLAVE_BUSWIDTH_UNDEFINED;
> >  }
> > --
> > 2.40.0.rc1.284.g88254d51c5-goog
> >