linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] soi: Don't call DMA sync API when not needed
@ 2024-05-22 17:09 Andy Shevchenko
  2024-05-22 17:09 ` [PATCH v1 1/2] spi: Don't mark message DMA mapped when no transfer in it is Andy Shevchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Andy Shevchenko @ 2024-05-22 17:09 UTC (permalink / raw)
  To: Mark Brown, linux-spi, linux-kernel; +Cc: Andy Shevchenko

A couple of fixes to avoid calling DMA sync API when it's not needed.
This doesn't stop from discussing if IOMMU code is doing the right thing,
i.e. dereferences SG list when orig_nents == 0, but this is a separate
story.

Andy Shevchenko (2):
  spi: Don't mark message DMA mapped when no transfer in it is
  spi: Check if transfer is mapped before calling DMA sync APIs

 drivers/spi/spi.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/2] spi: Don't mark message DMA mapped when no transfer in it is
  2024-05-22 17:09 [PATCH v1 0/2] soi: Don't call DMA sync API when not needed Andy Shevchenko
@ 2024-05-22 17:09 ` Andy Shevchenko
  2024-05-22 17:09 ` [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs Andy Shevchenko
  2024-05-23 15:56 ` [PATCH v1 0/2] soi: Don't call DMA sync API when not needed Mark Brown
  2 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2024-05-22 17:09 UTC (permalink / raw)
  To: Mark Brown, linux-spi, linux-kernel; +Cc: Andy Shevchenko

There is no need to set the DMA mapped flag of the message if it has
no mapped transfers. Moreover, it may give the code a chance to take
the wrong paths, i.e. to exercise DMA related APIs on unmapped data.
Make __spi_map_msg() to bail earlier on the above mentioned cases.

Fixes: 99adef310f68 ("spi: Provide core support for DMA mapping transfers")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b2efd4964f7c..51811f04e463 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1243,6 +1243,7 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 	else
 		rx_dev = ctlr->dev.parent;
 
+	ret = -ENOMSG;
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		/* The sync is done before each transfer. */
 		unsigned long attrs = DMA_ATTR_SKIP_CPU_SYNC;
@@ -1272,6 +1273,9 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 			}
 		}
 	}
+	/* No transfer has been mapped, bail out with success */
+	if (ret)
+		return 0;
 
 	ctlr->cur_rx_dma_dev = rx_dev;
 	ctlr->cur_tx_dma_dev = tx_dev;
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs
  2024-05-22 17:09 [PATCH v1 0/2] soi: Don't call DMA sync API when not needed Andy Shevchenko
  2024-05-22 17:09 ` [PATCH v1 1/2] spi: Don't mark message DMA mapped when no transfer in it is Andy Shevchenko
@ 2024-05-22 17:09 ` Andy Shevchenko
  2024-05-22 18:41   ` Nícolas F. R. A. Prado
  2024-05-23 15:56 ` [PATCH v1 0/2] soi: Don't call DMA sync API when not needed Mark Brown
  2 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2024-05-22 17:09 UTC (permalink / raw)
  To: Mark Brown, linux-spi, linux-kernel
  Cc: Andy Shevchenko, Nícolas F . R . A . Prado, Neil Armstrong

The resent update to remove the orig_nents checks revealed
that not all DMA sync backends can cope with the unallocated
SG list, while supplying orig_nents == 0 (the commit 861370f49ce4
("iommu/dma: force bouncing if the size is not cacheline-aligned"),
for example, makes that happen for the IOMMU case). It means
we have to check if the buffers are DMA mapped before trying
to sync them. Re-introduce that check in a form of calling
->can_dma() in the same way as it's done in the DMA mapping loop
for the SPI transfers.

Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reported-by: Neil Armstrong <neil.armstrong@linaro.org>
Closes: https://lore.kernel.org/r/8ae675b5-fcf9-4c9b-b06a-4462f70e1322@linaro.org
Closes: https://lore.kernel.org/all/d3679496-2e4e-4a7c-97ed-f193bd53af1d@notapiano
Fixes: 8cc3bad9d9d6 ("spi: Remove unneded check for orig_nents")
Suggested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 51811f04e463..cc8bb7d5ba1a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1311,7 +1311,7 @@ static int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message *msg)
 	return 0;
 }
 
-static void spi_dma_sync_for_device(struct spi_controller *ctlr,
+static void spi_dma_sync_for_device(struct spi_controller *ctlr, struct spi_message *msg,
 				    struct spi_transfer *xfer)
 {
 	struct device *rx_dev = ctlr->cur_rx_dma_dev;
@@ -1320,11 +1320,14 @@ static void spi_dma_sync_for_device(struct spi_controller *ctlr,
 	if (!ctlr->cur_msg_mapped)
 		return;
 
+	if (!ctlr->can_dma(ctlr, msg->spi, xfer))
+		return;
+
 	dma_sync_sgtable_for_device(tx_dev, &xfer->tx_sg, DMA_TO_DEVICE);
 	dma_sync_sgtable_for_device(rx_dev, &xfer->rx_sg, DMA_FROM_DEVICE);
 }
 
-static void spi_dma_sync_for_cpu(struct spi_controller *ctlr,
+static void spi_dma_sync_for_cpu(struct spi_controller *ctlr, struct spi_message *msg,
 				 struct spi_transfer *xfer)
 {
 	struct device *rx_dev = ctlr->cur_rx_dma_dev;
@@ -1333,6 +1336,9 @@ static void spi_dma_sync_for_cpu(struct spi_controller *ctlr,
 	if (!ctlr->cur_msg_mapped)
 		return;
 
+	if (!ctlr->can_dma(ctlr, msg->spi, xfer))
+		return;
+
 	dma_sync_sgtable_for_cpu(rx_dev, &xfer->rx_sg, DMA_FROM_DEVICE);
 	dma_sync_sgtable_for_cpu(tx_dev, &xfer->tx_sg, DMA_TO_DEVICE);
 }
@@ -1350,11 +1356,13 @@ static inline int __spi_unmap_msg(struct spi_controller *ctlr,
 }
 
 static void spi_dma_sync_for_device(struct spi_controller *ctrl,
+				    struct spi_message *msg,
 				    struct spi_transfer *xfer)
 {
 }
 
 static void spi_dma_sync_for_cpu(struct spi_controller *ctrl,
+				 struct spi_message *msg,
 				 struct spi_transfer *xfer)
 {
 }
@@ -1626,10 +1634,10 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 			reinit_completion(&ctlr->xfer_completion);
 
 fallback_pio:
-			spi_dma_sync_for_device(ctlr, xfer);
+			spi_dma_sync_for_device(ctlr, msg, xfer);
 			ret = ctlr->transfer_one(ctlr, msg->spi, xfer);
 			if (ret < 0) {
-				spi_dma_sync_for_cpu(ctlr, xfer);
+				spi_dma_sync_for_cpu(ctlr, msg, xfer);
 
 				if (ctlr->cur_msg_mapped &&
 				   (xfer->error & SPI_TRANS_FAIL_NO_START)) {
@@ -1654,7 +1662,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 					msg->status = ret;
 			}
 
-			spi_dma_sync_for_cpu(ctlr, xfer);
+			spi_dma_sync_for_cpu(ctlr, msg, xfer);
 		} else {
 			if (xfer->len)
 				dev_err(&msg->spi->dev,
-- 
2.43.0.rc1.1336.g36b5255a03ac


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs
  2024-05-22 17:09 ` [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs Andy Shevchenko
@ 2024-05-22 18:41   ` Nícolas F. R. A. Prado
  2024-05-23 12:01     ` Mark Brown
  2024-05-29 12:35     ` Nícolas F. R. A. Prado
  0 siblings, 2 replies; 9+ messages in thread
From: Nícolas F. R. A. Prado @ 2024-05-22 18:41 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Mark Brown, linux-spi, linux-kernel, Neil Armstrong

On Wed, May 22, 2024 at 08:09:50PM +0300, Andy Shevchenko wrote:
> The resent update to remove the orig_nents checks revealed
> that not all DMA sync backends can cope with the unallocated
> SG list, while supplying orig_nents == 0 (the commit 861370f49ce4
> ("iommu/dma: force bouncing if the size is not cacheline-aligned"),
> for example, makes that happen for the IOMMU case). It means
> we have to check if the buffers are DMA mapped before trying
> to sync them. Re-introduce that check in a form of calling
> ->can_dma() in the same way as it's done in the DMA mapping loop
> for the SPI transfers.
> 
> Reported-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Reported-by: Neil Armstrong <neil.armstrong@linaro.org>
> Closes: https://lore.kernel.org/r/8ae675b5-fcf9-4c9b-b06a-4462f70e1322@linaro.org
> Closes: https://lore.kernel.org/all/d3679496-2e4e-4a7c-97ed-f193bd53af1d@notapiano
> Fixes: 8cc3bad9d9d6 ("spi: Remove unneded check for orig_nents")
> Suggested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Hi Andy,

sorry I keep bothering you.

I tested this series and I still get the oops (attached at the end for
reference). When I tried this change originally, I added it on top of the
patches you had supplied. And as it turns out one of them was necessary.
Specifically, if I add 

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index f94420858c22..9bc9fd10d538 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1220,6 +1220,11 @@ void spi_unmap_buf(struct spi_controller *ctlr, struct device *dev,
        spi_unmap_buf_attrs(ctlr, dev, sgt, dir, 0);
 }

+/* Dummy SG for unidirect transfers */
+static struct scatterlist dummy_sg = {
+       .page_link = SG_END,
+};
+
 static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 {
        struct device *tx_dev, *rx_dev;
@@ -1258,6 +1263,8 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
                                                attrs);
                        if (ret != 0)
                                return ret;
+               } else {
+                       xfer->tx_sg.sgl = &dummy_sg;
                }

                if (xfer->rx_buf != NULL) {
@@ -1271,6 +1278,8 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)

                                return ret;
                        }
+               } else {
+                       xfer->rx_sg.sgl = &dummy_sg;
                }
        }
        /* No transfer has been mapped, bail out with success */

on top of this series, then I don't get any oops. (The memset doesn't seem to be
required, or at least in my test it didn't trigger any issue).

I guess this is needed because the can_dma "flag" is shared between rx and tx,
but either one of them might not have a buffer assigned (for unidirectional
transfers).

Sorry about the confusion.

Thanks,
Nícolas

Oops:

[    3.085228] Unable to handle kernel NULL pointer dereference at virtual address 000000000000001c
[    3.096144] Unable to handle kernel NULL pointer dereference at virtual address 000000000000001c
[    3.101468] Mem abort info:
[    3.110363] Mem abort info:
[    3.113239]   ESR = 0x0000000096000004
[    3.116114]   ESR = 0x0000000096000004
[    3.119969]   EC = 0x25: DABT (current EL), IL = 32 bits
[    3.123827]   EC = 0x25: DABT (current EL), IL = 32 bits
[    3.129284]   SET = 0, FnV = 0
[    3.134744]   SET = 0, FnV = 0
[    3.137881]   EA = 0, S1PTW = 0
[    3.141022]   EA = 0, S1PTW = 0
[    3.144247]   FSC = 0x04: level 0 translation fault
[    3.147475]   FSC = 0x04: level 0 translation fault
[    3.152491] Data abort info:
[    3.157505] Data abort info:
[    3.160468]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[    3.163434]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[    3.169065]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    3.169069]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    3.169073] [000000000000001c] user address but active_mm is swapper
[    3.169078] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[    3.174711]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    3.179896] Modules linked in:
[    3.179903] CPU: 6 PID: 68 Comm: kworker/u32:2 Not tainted 6.9.0-next-20240515-00006-g6c6aba391be0 #427
[    3.185352]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    3.191869] Hardware name: Google Kingoftown (DT)
[    3.191872] Workqueue: events_unbound deferred_probe_work_func
[    3.198309] [000000000000001c] user address but active_mm is swapper
[    3.203495]
[    3.203497] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.247739] pc : iommu_dma_sync_sg_for_device+0x28/0x100
[    3.253204] lr : __dma_sync_sg_for_device+0x28/0x4c
[    3.258220] sp : ffff800080942dd0
[    3.261624] x29: ffff800080942dd0 x28: ffff1a58c1012010 x27: ffff1a58c1012010
[    3.268953] x26: ffff1a58c31a0800 x25: ffff1a58c31a0c80 x24: 00000000000186a0
[    3.276285] x23: ffff1a58c1012010 x22: 0000000000000001 x21: 0000000000000000
[    3.283615] x20: ffffc3561c10c718 x19: 0000000000000000 x18: ffffc3561cde8948
[    3.290943] x17: 0000000000010108 x16: 0000000000000000 x15: 0000000000000002
[    3.298274] x14: ffff1a58c09156c0 x13: 0000000000000001 x12: 0000000000000000
[    3.305602] x11: 071c71c71c71c71c x10: 0000000000000aa0 x9 : ffff800080942c90
[    3.312932] x8 : ffff1a5a36da4800 x7 : 4000000000000000 x6 : ffff1a5a36da4828
[    3.320265] x5 : ffffc3561b51a780 x4 : ffffc3561a704acc x3 : 0000000000000001
[    3.327596] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff1a58c1012010
[    3.334927] Call trace:
[    3.337442]  iommu_dma_sync_sg_for_device+0x28/0x100
[    3.342540]  __dma_sync_sg_for_device+0x28/0x4c
[    3.347203]  spi_transfer_one_message+0x3a8/0x700
[    3.352042]  __spi_pump_transfer_message+0x198/0x4dc
[    3.357143]  __spi_sync+0x2a0/0x3c4
[    3.360738]  spi_sync+0x30/0x54
[    3.363971]  spi_mem_exec_op+0x26c/0x41c
[    3.368008]  spi_nor_spimem_read_data+0x148/0x158
[    3.372848]  spi_nor_read_data+0x30/0x3c
[    3.376881]  spi_nor_read_sfdp+0x74/0xe4
[    3.380916]  spi_nor_parse_sfdp+0x120/0x11d0
[    3.385314]  spi_nor_sfdp_init_params_deprecated+0x3c/0x8c
[    3.390951]  spi_nor_scan+0x7ac/0xef8
[    3.394721]  spi_nor_probe+0x94/0x2ec
[    3.398490]  spi_mem_probe+0x6c/0xac
[    3.402171]  spi_probe+0x84/0xe4
[    3.405491]  really_probe+0xbc/0x2a0
[    3.409173]  __driver_probe_device+0x78/0x12c
[    3.413654]  driver_probe_device+0x40/0x160
[    3.417961]  __device_attach_driver+0xb8/0x134
[    3.422533]  bus_for_each_drv+0x84/0xe0
[    3.426478]  __device_attach+0xa8/0x1b0
[    3.430423]  device_initial_probe+0x14/0x20
[    3.434720]  bus_probe_device+0xa8/0xac
[    3.438664]  device_add+0x590/0x750
[    3.442257]  __spi_add_device+0x138/0x208
[    3.446378]  of_register_spi_device+0x394/0x57c
[    3.451037]  spi_register_controller+0x394/0x760
[    3.455787]  qcom_qspi_probe+0x328/0x390
[    3.459826]  platform_probe+0x68/0xd8
[    3.463595]  really_probe+0xbc/0x2a0
[    3.467277]  __driver_probe_device+0x78/0x12c
[    3.471760]  driver_probe_device+0x40/0x160
[    3.476068]  __device_attach_driver+0xb8/0x134
[    3.480641]  bus_for_each_drv+0x84/0xe0
[    3.484585]  __device_attach+0xa8/0x1b0
[    3.488530]  device_initial_probe+0x14/0x20
[    3.492838]  bus_probe_device+0xa8/0xac
[    3.496784]  deferred_probe_work_func+0x88/0xc0
[    3.501442]  process_one_work+0x154/0x298
[    3.505568]  worker_thread+0x304/0x408
[    3.509425]  kthread+0x118/0x11c
[    3.512745]  ret_from_fork+0x10/0x20
[    3.516431] Code: 2a0203f5 2a0303f6 a90363f7 aa0003f7 (b9401c20)
[    3.522686] ---[ end trace 0000000000000000 ]---

[    3.527428] Internal error: Oops: 0000000096000004 [#2] PREEMPT SMP
[    3.533868] Modules linked in:
[    3.537013] CPU: 2 PID: 102 Comm: cros_ec_spi_hig Tainted: G      D            6.9.0-next-20240515-00006-g6c6aba391be0 #427
[    3.548431] Hardware name: Google Kingoftown (DT)
[    3.553267] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.560412] pc : iommu_dma_sync_sg_for_device+0x28/0x100
[    3.565877] lr : __dma_sync_sg_for_device+0x28/0x4c
[    3.570899] sp : ffff8000809fb990
[    3.574312] x29: ffff8000809fb990 x28: ffff1a58c0ff8010 x27: ffff1a58c0ff8010
[    3.581644] x26: ffff1a58c4d39800 x25: ffff1a58c4d39c80 x24: 00000000000186a0
[    3.588976] x23: ffff1a58c0ff8010 x22: 0000000000000001 x21: 0000000000000000
[    3.596307] x20: ffffc3561c10c3d8 x19: 0000000000000000 x18: 0000000000000000
[    3.603639] x17: 000000040044ffff x16: 005000f2b5503510 x15: 0000000000000002
[    3.610972] x14: 0000000000000001 x13: 0000000000162b7a x12: 0000000000000001
[    3.618304] x11: ffff8000809fb8a0 x10: ffff1a58c1f93ff8 x9 : ffff1a58c4d39c69
[    3.625630] x8 : ffff1a58c102db04 x7 : 0000000000000000 x6 : 0000000000000001
[    3.632962] x5 : ffffc3561b51a780 x4 : ffffc3561a704acc x3 : 0000000000000001
[    3.640291] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff1a58c0ff8010
[    3.647623] Call trace:
[    3.650148]  iommu_dma_sync_sg_for_device+0x28/0x100
[    3.655249]  __dma_sync_sg_for_device+0x28/0x4c
[    3.659903]  spi_transfer_one_message+0x3a8/0x700
[    3.664746]  __spi_pump_transfer_message+0x198/0x4dc
[    3.669853]  __spi_sync+0x2a0/0x3c4
[    3.673441]  spi_sync_locked+0x10/0x1c
[    3.677299]  receive_n_bytes+0xc0/0x118
[    3.681248]  do_cros_ec_pkt_xfer_spi+0x3c0/0x530
[    3.685997]  cros_ec_xfer_high_pri_work+0x20/0x34
[    3.690835]  kthread_worker_fn+0xcc/0x184
[    3.694960]  kthread+0x118/0x11c
[    3.698282]  ret_from_fork+0x10/0x20
[    3.701964] Code: 2a0203f5 2a0303f6 a90363f7 aa0003f7 (b9401c20)
[    3.708221] ---[ end trace 0000000000000000 ]---

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs
  2024-05-22 18:41   ` Nícolas F. R. A. Prado
@ 2024-05-23 12:01     ` Mark Brown
  2024-05-29 12:35     ` Nícolas F. R. A. Prado
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2024-05-23 12:01 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado
  Cc: Andy Shevchenko, linux-spi, linux-kernel, Neil Armstrong

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

On Wed, May 22, 2024 at 02:41:48PM -0400, Nícolas F. R. A. Prado wrote:

> I tested this series and I still get the oops (attached at the end for
> reference). When I tried this change originally, I added it on top of the
> patches you had supplied. And as it turns out one of them was necessary.
> Specifically, if I add 
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index f94420858c22..9bc9fd10d538 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1220,6 +1220,11 @@ void spi_unmap_buf(struct spi_controller *ctlr, struct device *dev,
>         spi_unmap_buf_attrs(ctlr, dev, sgt, dir, 0);

The other two patches are already queued, could you send this one
incrementally please Andy?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/2] soi: Don't call DMA sync API when not needed
  2024-05-22 17:09 [PATCH v1 0/2] soi: Don't call DMA sync API when not needed Andy Shevchenko
  2024-05-22 17:09 ` [PATCH v1 1/2] spi: Don't mark message DMA mapped when no transfer in it is Andy Shevchenko
  2024-05-22 17:09 ` [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs Andy Shevchenko
@ 2024-05-23 15:56 ` Mark Brown
  2 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2024-05-23 15:56 UTC (permalink / raw)
  To: linux-spi, linux-kernel, Andy Shevchenko

On Wed, 22 May 2024 20:09:48 +0300, Andy Shevchenko wrote:
> A couple of fixes to avoid calling DMA sync API when it's not needed.
> This doesn't stop from discussing if IOMMU code is doing the right thing,
> i.e. dereferences SG list when orig_nents == 0, but this is a separate
> story.
> 
> Andy Shevchenko (2):
>   spi: Don't mark message DMA mapped when no transfer in it is
>   spi: Check if transfer is mapped before calling DMA sync APIs
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/2] spi: Don't mark message DMA mapped when no transfer in it is
      commit: 9f788ba457b45b0ce422943fcec9fa35c4587764
[2/2] spi: Check if transfer is mapped before calling DMA sync APIs
      commit: da560097c05612f8d360f86528f6213629b9c395

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs
  2024-05-22 18:41   ` Nícolas F. R. A. Prado
  2024-05-23 12:01     ` Mark Brown
@ 2024-05-29 12:35     ` Nícolas F. R. A. Prado
  2024-05-29 12:48       ` Andy Shevchenko
  1 sibling, 1 reply; 9+ messages in thread
From: Nícolas F. R. A. Prado @ 2024-05-29 12:35 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Mark Brown, linux-spi, linux-kernel, Neil Armstrong

On Wed, May 22, 2024 at 02:41:51PM -0400, Nícolas F. R. A. Prado wrote:
> On Wed, May 22, 2024 at 08:09:50PM +0300, Andy Shevchenko wrote:
[..]
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index f94420858c22..9bc9fd10d538 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1220,6 +1220,11 @@ void spi_unmap_buf(struct spi_controller *ctlr, struct device *dev,
>         spi_unmap_buf_attrs(ctlr, dev, sgt, dir, 0);
>  }
> 
> +/* Dummy SG for unidirect transfers */
> +static struct scatterlist dummy_sg = {
> +       .page_link = SG_END,
> +};
> +
>  static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
>  {
>         struct device *tx_dev, *rx_dev;
> @@ -1258,6 +1263,8 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
>                                                 attrs);
>                         if (ret != 0)
>                                 return ret;
> +               } else {
> +                       xfer->tx_sg.sgl = &dummy_sg;
>                 }
> 
>                 if (xfer->rx_buf != NULL) {
> @@ -1271,6 +1278,8 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
> 
>                                 return ret;
>                         }
> +               } else {
> +                       xfer->rx_sg.sgl = &dummy_sg;
>                 }
>         }
>         /* No transfer has been mapped, bail out with success */

Hi Andy,

I can send this patch to the list myself with your authorship, I just need your
SoB.

Thanks,
Nícolas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs
  2024-05-29 12:35     ` Nícolas F. R. A. Prado
@ 2024-05-29 12:48       ` Andy Shevchenko
  2024-05-29 12:56         ` Nícolas F. R. A. Prado
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2024-05-29 12:48 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado
  Cc: Mark Brown, linux-spi, linux-kernel, Neil Armstrong

On Wed, May 29, 2024 at 08:35:26AM -0400, Nícolas F. R. A. Prado wrote:
> On Wed, May 22, 2024 at 02:41:51PM -0400, Nícolas F. R. A. Prado wrote:
> > On Wed, May 22, 2024 at 08:09:50PM +0300, Andy Shevchenko wrote:

[..]

> I can send this patch to the list myself with your authorship, I just need
> your SoB.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

P.S. Sorry for the delay, I was and still is on a sick leave.


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs
  2024-05-29 12:48       ` Andy Shevchenko
@ 2024-05-29 12:56         ` Nícolas F. R. A. Prado
  0 siblings, 0 replies; 9+ messages in thread
From: Nícolas F. R. A. Prado @ 2024-05-29 12:56 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Mark Brown, linux-spi, linux-kernel, Neil Armstrong

On Wed, May 29, 2024 at 03:48:05PM +0300, Andy Shevchenko wrote:
> On Wed, May 29, 2024 at 08:35:26AM -0400, Nícolas F. R. A. Prado wrote:
> > On Wed, May 22, 2024 at 02:41:51PM -0400, Nícolas F. R. A. Prado wrote:
> > > On Wed, May 22, 2024 at 08:09:50PM +0300, Andy Shevchenko wrote:
> 
> [..]
> 
> > I can send this patch to the list myself with your authorship, I just need
> > your SoB.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> P.S. Sorry for the delay, I was and still is on a sick leave.

No need to be sorry, health comes first. Hope you get better soon! :)

Thanks,
Nícolas

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-05-29 12:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-22 17:09 [PATCH v1 0/2] soi: Don't call DMA sync API when not needed Andy Shevchenko
2024-05-22 17:09 ` [PATCH v1 1/2] spi: Don't mark message DMA mapped when no transfer in it is Andy Shevchenko
2024-05-22 17:09 ` [PATCH v1 2/2] spi: Check if transfer is mapped before calling DMA sync APIs Andy Shevchenko
2024-05-22 18:41   ` Nícolas F. R. A. Prado
2024-05-23 12:01     ` Mark Brown
2024-05-29 12:35     ` Nícolas F. R. A. Prado
2024-05-29 12:48       ` Andy Shevchenko
2024-05-29 12:56         ` Nícolas F. R. A. Prado
2024-05-23 15:56 ` [PATCH v1 0/2] soi: Don't call DMA sync API when not needed Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).