This Patch series adds support for 32 bits per word trasfers using DMA
and some defensive checks around dma controller capabilities.
---
V1 Changes : Add support for AxSize=4 bytes to support 32bits/word.
---
V1->V2 Changes : Add dma capability check to make sure address widths
are supported.
---
V2->V3 Changes : Split changes , add DMA direction check and other
cosmetic chnages.
---
V3->V4 Changes : Fix Sparce Warning
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
---
V4->V5 Changes : Preserve reverse xmas Tree order, move direction
check before initalisation of further capabilities, remove zero
initialisations, remove error OR'ing.
---
V5->V6 Changes :
-Remove case of n_bytes=3 using 4_bytes buswidth
-Avoid forward decaration
-Break capability check patch into 2
-round n_bytes to power of 2 ( Bug Fix)
-Add more explanation in commit text.
---
V6->V7 Changes : Remove extra spaces, refer to functions in commit as
func()
---
V7->V8 Changes : Minor commment updates in patch 4/5
---
V8->V9 Changes : Minor formatting changes in patch 5/5
---
Joy Chakraborty (5):
spi: dw: Add 32 bpw support to SPI DW DMA driver
spi: dw: Move dw_spi_can_dma()
spi: dw: Add DMA directional capability check
spi: dw: Add DMA address widths capability check
spi: dw: Round of n_bytes to power of 2
drivers/spi/spi-dw-core.c | 5 ++-
drivers/spi/spi-dw-dma.c | 76 +++++++++++++++++++++++++++++----------
drivers/spi/spi-dw.h | 1 +
3 files changed, 63 insertions(+), 19 deletions(-)
--
2.40.1.495.gc816e09b53d-goog
Check capabilities of DMA controller during init to make sure it is
capable of handling MEM2DEV for tx channel, DEV2MEM for rx channel.
Current DW DMA driver requires both tx and rx channel to be configured
and functional for any kind of transfers to take effect including
half duplex. Hence, check for both tx and rx direction and fail on
unavailbility of either.
Signed-off-by: Joy Chakraborty <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Tested-by: Serge Semin <[email protected]>
* tested on Baikal-T1 based system with DW SPI-looped back interface
transferring a chunk of data with DFS:8,12,16.
---
drivers/spi/spi-dw-dma.c | 39 ++++++++++++++++++++++++++++++---------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
index f19c092920a1..22d0727a3789 100644
--- a/drivers/spi/spi-dw-dma.c
+++ b/drivers/spi/spi-dw-dma.c
@@ -72,12 +72,22 @@ 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)
{
- struct dma_slave_caps tx = {0}, rx = {0};
+ struct dma_slave_caps tx, rx;
+ int ret;
+
+ ret = dma_get_slave_caps(dws->txchan, &tx);
+ if (ret)
+ return ret;
+
+ ret = dma_get_slave_caps(dws->rxchan, &rx);
+ if (ret)
+ return ret;
- dma_get_slave_caps(dws->txchan, &tx);
- dma_get_slave_caps(dws->rxchan, &rx);
+ if (!(tx.directions & BIT(DMA_MEM_TO_DEV) &&
+ rx.directions & BIT(DMA_DEV_TO_MEM)))
+ return -ENXIO;
if (tx.max_sg_burst > 0 && rx.max_sg_burst > 0)
dws->dma_sg_burst = min(tx.max_sg_burst, rx.max_sg_burst);
@@ -95,6 +105,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 +135,25 @@ 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 +179,17 @@ 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;
--
2.40.1.495.gc816e09b53d-goog
Move dw_spi_can_dma() implementation below dw_spi_dma_convert_width()
for handing compile dependency in future patches.
Signed-off-by: Joy Chakraborty <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Tested-by: Serge Semin <[email protected]>
* tested on Baikal-T1 based system with DW SPI-looped back interface
transferring a chunk of data with DFS:8,12,16.
---
drivers/spi/spi-dw-dma.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
index c1b42cb59965..f19c092920a1 100644
--- a/drivers/spi/spi-dw-dma.c
+++ b/drivers/spi/spi-dw-dma.c
@@ -198,14 +198,6 @@ static irqreturn_t dw_spi_dma_transfer_handler(struct dw_spi *dws)
return IRQ_HANDLED;
}
-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);
-
- return xfer->len > dws->fifo_len;
-}
-
static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
{
switch (n_bytes) {
@@ -220,6 +212,14 @@ static enum dma_slave_buswidth dw_spi_dma_convert_width(u8 n_bytes)
}
}
+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);
+
+ return xfer->len > dws->fifo_len;
+}
+
static int dw_spi_dma_wait(struct dw_spi *dws, unsigned int len, u32 speed)
{
unsigned long long ms;
--
2.40.1.495.gc816e09b53d-goog
n_bytes variable in the driver represents the number of bytes per word
that needs to be sent/copied to fifo. Bits/word can be between 8 and 32
bits from the client but in memory they are a power of 2, same is mentioned
in spi.h header:
"
* @bits_per_word: Data transfers involve one or more words; word sizes
* like eight or 12 bits are common. In-memory wordsizes are
* powers of two bytes (e.g. 20 bit samples use 32 bits).
* This may be changed by the device's driver, or left at the
* default (0) indicating protocol words are eight bit bytes.
* The spi_transfer.bits_per_word can override this for each transfer.
"
Hence, round of n_bytes to a power of 2 to avoid values like 3 which
would generate unalligned/odd accesses to memory/fifo.
Fixes: a51acc2400d4 ("spi: dw: Add support for 32-bits max xfer size")
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Joy Chakraborty <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Tested-by: Serge Semin <[email protected]>
* tested on Baikal-T1 based system with DW SPI-looped back interface
transferring a chunk of data with DFS:8,12,16.
---
drivers/spi/spi-dw-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index c3bfb6c84cab..4976e3b8923e 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -426,7 +426,10 @@ static int dw_spi_transfer_one(struct spi_controller *master,
int ret;
dws->dma_mapped = 0;
- dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
+ dws->n_bytes =
+ roundup_pow_of_two(DIV_ROUND_UP(transfer->bits_per_word,
+ BITS_PER_BYTE));
+
dws->tx = (void *)transfer->tx_buf;
dws->tx_len = transfer->len / dws->n_bytes;
dws->rx = transfer->rx_buf;
--
2.40.1.495.gc816e09b53d-goog
Store address width capabilities of DMA controller during init and check
the same per transfer to make sure the bits/word requirement can be met.
Current DW DMA driver requires both tx and rx channel to be configured
and functional hence a subset of both tx and rx channel address width
capability is checked with the width requirement(n_bytes) for a
transfer.
Signed-off-by: Joy Chakraborty <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Tested-by: Serge Semin <[email protected]>
* tested on Baikal-T1 based system with DW SPI-looped back interface
transferring a chunk of data with DFS:8,12,16.
---
drivers/spi/spi-dw-dma.c | 17 ++++++++++++++++-
drivers/spi/spi-dw.h | 1 +
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
index 22d0727a3789..df819652901a 100644
--- a/drivers/spi/spi-dw-dma.c
+++ b/drivers/spi/spi-dw-dma.c
@@ -97,6 +97,15 @@ static int dw_spi_dma_caps_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
+ * peripheral side address width capabilities most likely would be
+ * the same.
+ */
+ dws->dma_addr_widths = tx.dst_addr_widths & rx.src_addr_widths;
+
+ return 0;
}
static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
@@ -237,8 +246,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 int dw_spi_dma_wait(struct dw_spi *dws, unsigned int len, u32 speed)
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.1.495.gc816e09b53d-goog
@Mark, @Andy
On Thu, Apr 27, 2023 at 12:33:09PM +0000, Joy Chakraborty wrote:
> This Patch series adds support for 32 bits per word trasfers using DMA
> and some defensive checks around dma controller capabilities.
I've done with reviewing and testing the series. My tags are already
added to the patch logs. @Andy do you still have any comments about
the updated patchset? If none, @Mark please merge it in if you are ok
with the changes.
-Serge(y)
> ---
> V1 Changes : Add support for AxSize=4 bytes to support 32bits/word.
> ---
> V1->V2 Changes : Add dma capability check to make sure address widths
> are supported.
> ---
> V2->V3 Changes : Split changes , add DMA direction check and other
> cosmetic chnages.
> ---
> V3->V4 Changes : Fix Sparce Warning
> | Reported-by: kernel test robot <[email protected]>
> | Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> ---
> V4->V5 Changes : Preserve reverse xmas Tree order, move direction
> check before initalisation of further capabilities, remove zero
> initialisations, remove error OR'ing.
> ---
> V5->V6 Changes :
> -Remove case of n_bytes=3 using 4_bytes buswidth
> -Avoid forward decaration
> -Break capability check patch into 2
> -round n_bytes to power of 2 ( Bug Fix)
> -Add more explanation in commit text.
> ---
> V6->V7 Changes : Remove extra spaces, refer to functions in commit as
> func()
> ---
> V7->V8 Changes : Minor commment updates in patch 4/5
> ---
> V8->V9 Changes : Minor formatting changes in patch 5/5
> ---
>
> Joy Chakraborty (5):
> spi: dw: Add 32 bpw support to SPI DW DMA driver
> spi: dw: Move dw_spi_can_dma()
> spi: dw: Add DMA directional capability check
> spi: dw: Add DMA address widths capability check
> spi: dw: Round of n_bytes to power of 2
>
> drivers/spi/spi-dw-core.c | 5 ++-
> drivers/spi/spi-dw-dma.c | 76 +++++++++++++++++++++++++++++----------
> drivers/spi/spi-dw.h | 1 +
> 3 files changed, 63 insertions(+), 19 deletions(-)
>
> --
> 2.40.1.495.gc816e09b53d-goog
>
On Mon, May 01, 2023 at 08:14:09PM +0300, Serge Semin wrote:
> @Mark, @Andy
Just as a note please don't add random characters to the start of the
name, for whatever reason it really annoys me.
> I've done with reviewing and testing the series. My tags are already
> added to the patch logs. @Andy do you still have any comments about
> the updated patchset? If none, @Mark please merge it in if you are ok
> with the changes.
We're still in the merge window right now.
On Tue, May 02, 2023 at 08:58:33AM +0900, Mark Brown wrote:
> On Mon, May 01, 2023 at 08:14:09PM +0300, Serge Semin wrote:
> > @Mark, @Andy
>
> Just as a note please don't add random characters to the start of the
> name, for whatever reason it really annoys me.
Ok.
>
> > I've done with reviewing and testing the series. My tags are already
> > added to the patch logs. @Andy do you still have any comments about
> > the updated patchset? If none, @Mark please merge it in if you are ok
> > with the changes.
>
> We're still in the merge window right now.
Ah, right. I absolutely forgot about that. Sorry for bothering. Let's
wait for the merge window to be closed then.
-Serge(y)
On Thu, Apr 27, 2023 at 12:33:13PM +0000, Joy Chakraborty wrote:
> Store address width capabilities of DMA controller during init and check
> the same per transfer to make sure the bits/word requirement can be met.
>
> Current DW DMA driver requires both tx and rx channel to be configured
> and functional hence a subset of both tx and rx channel address width
> capability is checked with the width requirement(n_bytes) for a
> transfer.
This breaks an x86 allmodconfig build:
/build/stage/linux/drivers/spi/spi-dw-dma.c: In function ‘dw_spi_dma_caps_init’:
/build/stage/linux/drivers/spi/spi-dw-dma.c:100:1: error: control reaches end of non-void function [-Werror=return-type]
100 | }
| ^
cc1: some warnings being treated as errors
On Mon, May 8, 2023 at 6:46 PM Mark Brown <[email protected]> wrote:
>
> On Thu, Apr 27, 2023 at 12:33:13PM +0000, Joy Chakraborty wrote:
> > Store address width capabilities of DMA controller during init and check
> > the same per transfer to make sure the bits/word requirement can be met.
> >
> > Current DW DMA driver requires both tx and rx channel to be configured
> > and functional hence a subset of both tx and rx channel address width
> > capability is checked with the width requirement(n_bytes) for a
> > transfer.
>
> This breaks an x86 allmodconfig build:
>
> /build/stage/linux/drivers/spi/spi-dw-dma.c: In function ‘dw_spi_dma_caps_init’:
> /build/stage/linux/drivers/spi/spi-dw-dma.c:100:1: error: control reaches end of non-void function [-Werror=return-type]
> 100 | }
> | ^
> cc1: some warnings being treated as errors
Moving "return 0" at the end of dw_spi_dma_caps_init() from patch
[4/5] to patch [3/5] to solve this and sending a V10 Patch.
Thanks
Joy