| Message ID | 20230320055746.2070049-1-joychakr@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: dw: Add 32 bpw support to DW DMA Controller | expand |
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 > >
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; return DMA_SLAVE_BUSWIDTH_UNDEFINED; }
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(+)