[PATCH v2 0/7] spi: Add support for panic mem writes

[PATCH v2 0/7] spi: Add support for panic mem writes

[Paul Cercueil]

Hi,

Here is the v2 of my patchset that updates the MTD spi-nor code as well
as the spi-mem code, to add support for panic mem writes to a memory
partition configured as pstore.

Changelog:

 - Old patch [4/5] ("spi: cadence-xspi: Don't use infinite timeout in
   register poll") was dropped. The V2's [6/7] updates that code so
   there was little point in keeping the patch, especially since it
   was fixing a never-taken code path.

 - [1/7]: 
        - Don't call spi_mem_access_start/end on panic. We don't need to
          flush the SPI queue at that point, and we don't need to lock the
          mutexes.
        - Check that the SPI controller's PM status is active, and bail out
          otherwise.

 - [2/7]: New patch.
          Updates the spi-nor code to use the iopoll macros, to make it
	  easier to convert it to atomic-context polling later.

 - [3/7]: (was v1's [2/5])
        - Update spi_nor_wait_till_ready_with_timeout() to use
          poll_timeout_us_atomic() on panic
        - Alloc temp buffer with GFP_ATOMIC in spi_nor_octal_dtr_write()
          on panic
        - Bail out if nor->rww.ongoing_pe is set in spi_nor_panic_write()

 - [4/7]: New patch.
          Address an issue found by Sashiko, where we poll for a bit in
          the IRQ status register but never clear the bit back to 0.

 - [5/7]: (was v1's [3/5])
        - Make sure we only enable interrupts if not using polling mode

 - [6/7]:
        - Don't enable interrupts if doing a panic write
        - Use a new cdns_readl_poll() macro that will resolve to calling
          readl_relaxed_poll_timeout_atomic() when doing a panic write, or
          readl_relaxed_poll_timeout() otherwise.
        - Drop cdns_xspi->panic_write. We don't actually need to check it
	  in cdns_xspi_is_dll_locked(), because this code is never called
	  in the kernel panic path.

 - [4/7]: New patch.
          Update the dt-bindings to make the IRQ line optional.

Cheers,
-Paul

Tanmay Jagdale (7):
  spi: spi-mem: Add support for panic mem writes
  mtd: spi-nor: Wait for NOR ready with poll_timeout_us()
  mtd: spi-nor: Add support for panic writes
  spi: cadence-xspi: Clear "SDMA ready" IRQ flag after polling
  spi: cadence-xspi: Add irq-less support
  spi: cadence-xspi: Add support for panic writes
  spi: dt-bindings: cadence: Make XSPI's IRQ line optional

 .../devicetree/bindings/spi/cdns,xspi.yaml    |   1 -
 drivers/mtd/spi-nor/core.c                    | 111 +++++++++++++----
 drivers/spi/spi-cadence-xspi.c                | 116 +++++++++++-------
 drivers/spi/spi-mem.c                         |  35 +++++-
 include/linux/spi/spi-mem.h                   |   5 +
 5 files changed, 194 insertions(+), 74 deletions(-)

-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 1/7] spi: spi-mem: Add support for panic mem writes

[Paul Cercueil]

From: Tanmay Jagdale <tanmay@marvell.com>

Add a 'panic_write' flag to the controller's capabilities struct.
When set, the SPI controller indicates that it will be able to send
commands to a connected NOR or NAND memory in the context of a kernel
panic, which means without sleeping and without relying on IRQs.

Whether or not a given memory operation is a panic mem write is given by
the spi_mem_op.panic_write flag.

Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Co-developed-by: Paul Cercueil <paul.cercueil@bootlin.com>
Signed-off-by: Paul Cercueil <paul.cercueil@bootlin.com>
---

Notes:
    v2: - Don't call spi_mem_access_start/end on panic. We don't need to
          flush the SPI queue at that point, and we don't need to lock the
          mutexes.
        - Check that the SPI controller's PM status is active, and bail out
          otherwise.

 drivers/spi/spi-mem.c       | 35 +++++++++++++++++++++++++++++++++--
 include/linux/spi/spi-mem.h |  5 +++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index b94680b60af9..3b6fa44e4550 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -398,6 +398,31 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	u8 *tmpbuf;
 	int ret;
 
+	if (op->panic_write) {
+		if (!spi_mem_controller_is_capable(ctlr, panic_write))
+			return -EOPNOTSUPP;
+
+		/*
+		 * Only support panic writes if we'll go through .exec_op().
+		 * The regular SPI path (spi_sync(), etc.) explicitly mentions
+		 * that it can only be used from a context that may sleep, which
+		 * is not the case on a kernel panic.
+		 * If this changes in the future, this could be reworked, but
+		 * for now this will have to do.
+		 */
+		if (WARN_ON_ONCE(!ctlr->mem_ops || !ctlr->mem_ops->exec_op))
+			return -EOPNOTSUPP;
+
+		if (spi_get_csgpiod(mem->spi, 0))
+			return -EPERM;
+
+		/* On panic IRQs are disabled so we cannot wait on locks */
+		if (mutex_is_locked(&ctlr->bus_lock_mutex) ||
+		    mutex_is_locked(&ctlr->io_mutex)) {
+			return -EPERM;
+		}
+	}
+
 	/* Make sure the operation frequency is correct before going futher */
 	spi_mem_adjust_op_freq(mem, (struct spi_mem_op *)op);
 
@@ -420,7 +445,10 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 		return -EOPNOTSUPP;
 
 	if (ctlr->mem_ops && ctlr->mem_ops->exec_op && !spi_get_csgpiod(mem->spi, 0)) {
-		ret = spi_mem_access_start(mem);
+		if (!op->panic_write)
+			ret = spi_mem_access_start(mem);
+		else if (ctlr->auto_runtime_pm)
+			ret = pm_runtime_get_if_active(ctlr->dev.parent);
 		if (ret)
 			return ret;
 
@@ -428,7 +456,10 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 		ret = ctlr->mem_ops->exec_op(mem, op);
 		trace_spi_mem_stop_op(mem, op);
 
-		spi_mem_access_end(mem);
+		if (!op->panic_write)
+			spi_mem_access_end(mem);
+		else if (ctlr->auto_runtime_pm)
+			pm_runtime_put_noidle(ctlr->dev.parent);
 
 		/*
 		 * Some controllers only optimize specific paths (typically the
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 0cce6b57242a..2633307bfad5 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -169,6 +169,7 @@ enum spi_mem_data_dir {
  *		 operation does not involve transferring data
  * @data.buf.in: input buffer (must be DMA-able)
  * @data.buf.out: output buffer (must be DMA-able)
+ * @panic_write: The command is executed in the context of a kernel panic
  * @max_freq: frequency limitation wrt this operation. 0 means there is no
  *	      specific constraint and the highest achievable frequency can be
  *	      attempted.
@@ -211,6 +212,7 @@ struct spi_mem_op {
 		} buf;
 	} data;
 
+	bool panic_write;
 	unsigned int max_freq;
 };
 
@@ -392,6 +394,8 @@ struct spi_controller_mem_ops {
  *                   is a pause in the transfer (eg. internal bus contention or
  *                   DMA arbitration on an interconnect). Features such as NAND
  *                   continuous reads shall not be leveraged.
+ * @panic_write: Set to true if the controller driver supports writing to a
+ *               connected NAND/NOR memory in a kernel panic context
  */
 struct spi_controller_mem_caps {
 	bool dtr;
@@ -400,6 +404,7 @@ struct spi_controller_mem_caps {
 	bool per_op_freq;
 	bool secondary_op_tmpl;
 	bool no_cs_assertion;
+	bool panic_write;
 };
 
 #define spi_mem_controller_is_capable(ctlr, cap)	\
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 2/7] mtd: spi-nor: Wait for NOR ready with poll_timeout_us()

[Paul Cercueil]

From: Tanmay Jagdale <tanmay@marvell.com>

Instead of polling for the spi_nor_ready() with a custom loop, use
poll_timeout_us(). The sleep period between two polls is set to 100 us
which I think should be fine.

This will make it simpler to support waiting for the SPI NOR in atomic
context later.

Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Co-developed-by: Paul Cercueil <paul.cercueil@bootlin.com>
Signed-off-by: Paul Cercueil <paul.cercueil@bootlin.com>
---

Notes:
    v2: New patch

 drivers/mtd/spi-nor/core.c | 39 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 8bc117b46e02..2509fb983d4f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -12,6 +12,7 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/errno.h>
+#include <linux/iopoll.h>
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/mtd/mtd.h>
@@ -32,13 +33,13 @@
  * For everything but full-chip erase; probably could be much smaller, but kept
  * around for safety for now
  */
-#define DEFAULT_READY_WAIT_JIFFIES		(40UL * HZ)
+#define DEFAULT_READY_TIMEOUT_MS		40000UL
 
 /*
  * For full-chip erase, calibrated to a 2MB flash (M25P16); should be scaled up
  * for larger flash
  */
-#define CHIP_ERASE_2MB_READY_WAIT_JIFFIES	(40UL * HZ)
+#define CHIP_ERASE_2MB_READY_TIMEOUT_MS		40000UL
 
 #define SPI_NOR_MAX_ADDR_NBYTES	4
 
@@ -712,29 +713,21 @@ static int spi_nor_ready(struct spi_nor *nor)
  * Return: 0 on success, -errno otherwise.
  */
 static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
-						unsigned long timeout_jiffies)
+						unsigned long timeout_ms)
 {
-	unsigned long deadline;
-	int timeout = 0, ret;
+	int ret, status;
 
-	deadline = jiffies + timeout_jiffies;
-
-	while (!timeout) {
-		if (time_after_eq(jiffies, deadline))
-			timeout = 1;
-
-		ret = spi_nor_ready(nor);
-		if (ret < 0)
-			return ret;
-		if (ret)
-			return 0;
-
-		cond_resched();
+	ret = poll_timeout_us(status = spi_nor_ready(nor), status,
+			      100, timeout_ms * 1000, false);
+	if (ret < 0) {
+		dev_dbg(nor->dev, "flash operation timed out\n");
+		return ret;
 	}
 
-	dev_dbg(nor->dev, "flash operation timed out\n");
+	if (status < 0)
+		return status;
 
-	return -ETIMEDOUT;
+	return 0;
 }
 
 /**
@@ -747,7 +740,7 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 int spi_nor_wait_till_ready(struct spi_nor *nor)
 {
 	return spi_nor_wait_till_ready_with_timeout(nor,
-						    DEFAULT_READY_WAIT_JIFFIES);
+						    DEFAULT_READY_TIMEOUT_MS);
 }
 
 /**
@@ -1781,8 +1774,8 @@ static int spi_nor_erase_dice(struct spi_nor *nor, loff_t addr,
 	 * pull these from CFI/SFDP, but these values should be good
 	 * enough for now.
 	 */
-	timeout = max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES,
-		      CHIP_ERASE_2MB_READY_WAIT_JIFFIES *
+	timeout = max(CHIP_ERASE_2MB_READY_TIMEOUT_MS,
+		      CHIP_ERASE_2MB_READY_TIMEOUT_MS *
 		      (unsigned long)(nor->mtd.size / SZ_2M));
 
 	do {
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 3/7] mtd: spi-nor: Add support for panic writes

[Paul Cercueil]

From: Tanmay Jagdale <tanmay@marvell.com>

If the SPI controller supports it, provide the functionality of doing a
panic write.

Update the spi-nor core code and controllers to make sure they won't try
to grab locked mutexes or sleep when doing a panic write, as preemption
and IRQs are disabled.

The information is carried down to the SPI controller driver through the
spi_mem_op structure. If the corresponding flag is set there, and the
SPI driver does support panic writes, it then has to respect the same
rules.

Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Co-developed-by: Paul Cercueil <paul.cercueil@bootlin.com>
Signed-off-by: Paul Cercueil <paul.cercueil@bootlin.com>
---

Notes:
    v2: - Update spi_nor_wait_till_ready_with_timeout() to use
          poll_timeout_us_atomic() on panic
        - Alloc temp buffer with GFP_ATOMIC in spi_nor_octal_dtr_write()
          on panic
        - Bail out if nor->rww.ongoing_pe is set in spi_nor_panic_write()

 drivers/mtd/spi-nor/core.c | 78 +++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 2509fb983d4f..281acc54b324 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -97,6 +97,9 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
 	if (op->data.nbytes)
 		op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
 
+	if (nor->mtd.oops_panic_write)
+		op->panic_write = true;
+
 	if (spi_nor_protocol_is_dtr(proto)) {
 		/*
 		 * SPIMEM supports mixed DTR modes, but right now we can only
@@ -282,7 +285,7 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
 	if (spi_nor_spimem_bounce(nor, &op))
 		memcpy(nor->bouncebuf, buf, op.data.nbytes);
 
-	if (nor->dirmap.wdesc) {
+	if (nor->dirmap.wdesc && !nor->mtd.oops_panic_write) {
 		nbytes = spi_mem_dirmap_write(nor->dirmap.wdesc, op.addr.val,
 					      op.data.nbytes, op.data.buf.out);
 	} else {
@@ -665,6 +668,9 @@ static void spi_nor_rww_end_rdst(struct spi_nor *nor)
 
 static int spi_nor_lock_rdst(struct spi_nor *nor)
 {
+	if (nor->mtd.oops_panic_write)
+		return 0;
+
 	if (spi_nor_use_parallel_locking(nor))
 		return spi_nor_rww_start_rdst(nor);
 
@@ -673,6 +679,9 @@ static int spi_nor_lock_rdst(struct spi_nor *nor)
 
 static void spi_nor_unlock_rdst(struct spi_nor *nor)
 {
+	if (nor->mtd.oops_panic_write)
+		return;
+
 	if (spi_nor_use_parallel_locking(nor)) {
 		spi_nor_rww_end_rdst(nor);
 		wake_up(&nor->rww.wait);
@@ -717,8 +726,14 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 {
 	int ret, status;
 
-	ret = poll_timeout_us(status = spi_nor_ready(nor), status,
-			      100, timeout_ms * 1000, false);
+	if (nor->mtd.oops_panic_write) {
+		ret = poll_timeout_us_atomic(status = spi_nor_ready(nor), status,
+					     100, timeout_ms * 1000, false);
+	} else {
+		ret = poll_timeout_us(status = spi_nor_ready(nor), status,
+				      100, timeout_ms * 1000, false);
+	}
+
 	if (ret < 0) {
 		dev_dbg(nor->dev, "flash operation timed out\n");
 		return ret;
@@ -1284,6 +1299,9 @@ static void spi_nor_rww_end_io(struct spi_nor *nor)
 
 static int spi_nor_lock_device(struct spi_nor *nor)
 {
+	if (nor->mtd.oops_panic_write)
+		return 0;
+
 	if (!spi_nor_use_parallel_locking(nor))
 		return 0;
 
@@ -1292,6 +1310,9 @@ static int spi_nor_lock_device(struct spi_nor *nor)
 
 static void spi_nor_unlock_device(struct spi_nor *nor)
 {
+	if (nor->mtd.oops_panic_write)
+		return;
+
 	if (spi_nor_use_parallel_locking(nor)) {
 		spi_nor_rww_end_io(nor);
 		wake_up(&nor->rww.wait);
@@ -1329,6 +1350,9 @@ int spi_nor_prep_and_lock(struct spi_nor *nor)
 {
 	int ret;
 
+	if (nor->mtd.oops_panic_write)
+		return 0;
+
 	ret = spi_nor_prep(nor);
 	if (ret)
 		return ret;
@@ -1344,6 +1368,9 @@ int spi_nor_prep_and_lock(struct spi_nor *nor)
 
 void spi_nor_unlock_and_unprep(struct spi_nor *nor)
 {
+	if (nor->mtd.oops_panic_write)
+		return;
+
 	if (!spi_nor_use_parallel_locking(nor)) {
 		mutex_unlock(&nor->lock);
 	} else {
@@ -1400,6 +1427,9 @@ static int spi_nor_prep_and_lock_pe(struct spi_nor *nor, loff_t start, size_t le
 {
 	int ret;
 
+	if (nor->mtd.oops_panic_write)
+		return 0;
+
 	ret = spi_nor_prep(nor);
 	if (ret)
 		return ret;
@@ -1415,6 +1445,9 @@ static int spi_nor_prep_and_lock_pe(struct spi_nor *nor, loff_t start, size_t le
 
 static void spi_nor_unlock_and_unprep_pe(struct spi_nor *nor, loff_t start, size_t len)
 {
+	if (nor->mtd.oops_panic_write)
+		return;
+
 	if (!spi_nor_use_parallel_locking(nor)) {
 		mutex_unlock(&nor->lock);
 	} else {
@@ -1473,6 +1506,9 @@ static int spi_nor_prep_and_lock_rd(struct spi_nor *nor, loff_t start, size_t le
 {
 	int ret;
 
+	if (nor->mtd.oops_panic_write)
+		return 0;
+
 	ret = spi_nor_prep(nor);
 	if (ret)
 		return ret;
@@ -1488,6 +1524,9 @@ static int spi_nor_prep_and_lock_rd(struct spi_nor *nor, loff_t start, size_t le
 
 static void spi_nor_unlock_and_unprep_rd(struct spi_nor *nor, loff_t start, size_t len)
 {
+	if (nor->mtd.oops_panic_write)
+		return;
+
 	if (!spi_nor_use_parallel_locking(nor)) {
 		mutex_unlock(&nor->lock);
 	} else {
@@ -2196,6 +2235,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
 static int spi_nor_octal_dtr_write(struct spi_nor *nor, loff_t to, size_t len,
 				   const u8 *buf)
 {
+	gfp_t mm_flags = GFP_KERNEL;
 	u8 *tmp_buf;
 	size_t bytes_written;
 	loff_t start, end;
@@ -2204,7 +2244,10 @@ static int spi_nor_octal_dtr_write(struct spi_nor *nor, loff_t to, size_t len,
 	if (IS_ALIGNED(to, 2) && IS_ALIGNED(len, 2))
 		return spi_nor_write_data(nor, to, len, buf);
 
-	tmp_buf = kmalloc(nor->params->page_size, GFP_KERNEL);
+	if (nor->mtd.oops_panic_write)
+		mm_flags |= GFP_ATOMIC;
+
+	tmp_buf = kmalloc(nor->params->page_size, mm_flags);
 	if (!tmp_buf)
 		return -ENOMEM;
 
@@ -3386,6 +3429,26 @@ static void spi_nor_soft_reset(struct spi_nor *nor)
 	usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
 }
 
+static int spi_nor_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
+			       size_t *retlen, const u_char *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+
+	/*
+	 * At this point preemption and local interrupts are disabled, so we
+	 * can't get the lock if it's taken.
+	 */
+	if (mutex_is_locked(&nor->lock))
+		return -EPERM;
+
+	if (spi_nor_use_parallel_locking(nor) &&
+	    (nor->rww.ongoing_io || nor->rww.ongoing_rd || nor->rww.ongoing_pe)) {
+		return -EPERM;
+	}
+
+	return spi_nor_write(mtd, to, len, retlen, buf);
+}
+
 /* mtd suspend handler */
 static int spi_nor_suspend(struct mtd_info *mtd)
 {
@@ -3589,8 +3652,13 @@ static int spi_nor_set_mtd_info(struct spi_nor *nor)
 	mtd->size = nor->params->size;
 	mtd->_read = spi_nor_read;
 	/* Might be already set by some SST flashes. */
-	if (!mtd->_write)
+	if (!mtd->_write) {
 		mtd->_write = spi_nor_write;
+		if (nor->spimem &&
+		    spi_mem_controller_is_capable(nor->spimem->spi->controller, panic_write)) {
+			mtd->_panic_write = spi_nor_panic_write;
+		}
+	}
 	mtd->_suspend = spi_nor_suspend;
 	mtd->_resume = spi_nor_resume;
 	mtd->_get_device = spi_nor_get_device;
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 4/7] spi: cadence-xspi: Clear "SDMA ready" IRQ flag after polling

[Paul Cercueil]

From: Tanmay Jagdale <tanmay@marvell.com>

When polling for the "SDMA ready" event, once the corresponding bit is
set in the CDNS_XSPI_SDMA_TRIGGER we should reset it to zero. Otherwise,
any subsequent call to cdns_xspi_is_sdma_ready() will just return early.

Fixes: 931e389ded0f ("spi: cadence: Add Marvell xfer operation support")
Cc: stable@vger.kernel.org
Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Co-developed-by: Paul Cercueil <paul.cercueil@bootlin.com>
Signed-off-by: Paul Cercueil <paul.cercueil@bootlin.com>
---

Notes:
    v2: New patch

 drivers/spi/spi-cadence-xspi.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 39c868a5b171..095bf640b379 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -1068,12 +1068,20 @@ static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
 {
 	u32 ctrl_stat;
 
-	return !readl_relaxed_poll_timeout
-		(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
-		ctrl_stat,
-		(ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
-		sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
-		sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0);
+	if (readl_relaxed_poll_timeout
+	    (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
+	     ctrl_stat,
+	     (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
+	     sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
+	     sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0)) {
+		return false;
+	}
+
+	/* Clear the flag */
+	writel(CDNS_XSPI_SDMA_TRIGGER,
+	       cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
+
+	return true;
 }
 
 static int cdns_xspi_transfer_one_message_b0(struct spi_controller *controller,
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 5/7] spi: cadence-xspi: Add irq-less support

[Paul Cercueil]

From: Tanmay Jagdale <tanmay@marvell.com>

Make the interrupt line optional. If not provided, the driver will
instead resort to polling registers.

This can be useful in itself, but will be used later to support write on
kernel panic.

Note the cdns_xspi_is_{sdma,stig}_ready() seem to have moved for no
reason; they were actually moved out of a conditional compilation block
(#ifdef CONFIG_64BIT), as they are needed in conf-independent paths now.

Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Co-developed-by: Paul Cercueil <paul.cercueil@bootlin.com>
Signed-off-by: Paul Cercueil <paul.cercueil@bootlin.com>
---

Notes:
    v2: Make sure we only enable interrupts if not using polling mode

 drivers/spi/spi-cadence-xspi.c | 105 ++++++++++++++++++---------------
 1 file changed, 59 insertions(+), 46 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 095bf640b379..185dffda54bb 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -365,6 +365,39 @@ static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
 					  100, 1000);
 }
 
+static bool cdns_xspi_is_stig_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
+{
+	u32 ctrl_stat;
+
+	return !readl_relaxed_poll_timeout
+		(cdns_xspi->iobase + CDNS_XSPI_CTRL_STATUS_REG,
+		ctrl_stat,
+		((ctrl_stat & BIT(3)) == 0),
+		sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
+		sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0);
+}
+
+static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
+{
+	u32 ctrl_stat;
+
+	if (readl_relaxed_poll_timeout
+	    (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
+	     ctrl_stat,
+	     (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
+	     sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
+	     sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0)) {
+		return false;
+	}
+
+	/* Clear the flag */
+	writel(CDNS_XSPI_SDMA_TRIGGER,
+	       cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
+
+	return true;
+}
+
+
 static void cdns_xspi_trigger_command(struct cdns_xspi_dev *cdns_xspi,
 				      u32 cmd_regs[6])
 {
@@ -532,7 +565,8 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
 	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
 
-	cdns_xspi->set_interrupts_handler(cdns_xspi, true);
+	if (cdns_xspi->irq >= 0)
+		cdns_xspi->set_interrupts_handler(cdns_xspi, true);
 	cdns_xspi->sdma_error = false;
 
 	memset(cmd_regs, 0, sizeof(cmd_regs));
@@ -562,16 +596,25 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 
 		cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
 
-		wait_for_completion(&cdns_xspi->sdma_complete);
-		if (cdns_xspi->sdma_error) {
-			cdns_xspi->set_interrupts_handler(cdns_xspi, false);
+		if (cdns_xspi->irq >= 0) {
+			wait_for_completion(&cdns_xspi->sdma_complete);
+			if (cdns_xspi->sdma_error) {
+				cdns_xspi->set_interrupts_handler(cdns_xspi, false);
+				return -EIO;
+			}
+		} else if (!cdns_xspi_is_sdma_ready(cdns_xspi, true)) {
 			return -EIO;
 		}
+
 		cdns_xspi->sdma_handler(cdns_xspi);
 	}
 
-	wait_for_completion(&cdns_xspi->cmd_complete);
-	cdns_xspi->set_interrupts_handler(cdns_xspi, false);
+	if (cdns_xspi->irq >= 0) {
+		wait_for_completion(&cdns_xspi->cmd_complete);
+		cdns_xspi->set_interrupts_handler(cdns_xspi, false);
+	} else if (!cdns_xspi_is_stig_ready(cdns_xspi, true)) {
+		return -EIO;
+	}
 
 	cmd_status = cdns_xspi_check_command_status(cdns_xspi);
 	if (cmd_status)
@@ -1052,38 +1095,6 @@ static int cdns_xspi_prepare_transfer(int cs, int dir, int len, u32 *cmd_regs)
 	return 0;
 }
 
-static bool cdns_xspi_is_stig_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
-{
-	u32 ctrl_stat;
-
-	return !readl_relaxed_poll_timeout
-		(cdns_xspi->iobase + CDNS_XSPI_CTRL_STATUS_REG,
-		ctrl_stat,
-		((ctrl_stat & BIT(3)) == 0),
-		sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
-		sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0);
-}
-
-static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
-{
-	u32 ctrl_stat;
-
-	if (readl_relaxed_poll_timeout
-	    (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
-	     ctrl_stat,
-	     (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
-	     sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
-	     sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0)) {
-		return false;
-	}
-
-	/* Clear the flag */
-	writel(CDNS_XSPI_SDMA_TRIGGER,
-	       cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
-
-	return true;
-}
-
 static int cdns_xspi_transfer_one_message_b0(struct spi_controller *controller,
 					   struct spi_message *m)
 {
@@ -1276,15 +1287,17 @@ static int cdns_xspi_probe(struct platform_device *pdev)
 	}
 #endif
 
-	cdns_xspi->irq = platform_get_irq(pdev, 0);
-	if (cdns_xspi->irq < 0)
-		return -ENXIO;
+	cdns_xspi->irq = platform_get_irq_optional(pdev, 0);
+	if (cdns_xspi->irq < 0 && cdns_xspi->irq != -ENXIO)
+		return cdns_xspi->irq;
 
-	ret = devm_request_irq(dev, cdns_xspi->irq, cdns_xspi_irq_handler,
-			       IRQF_SHARED, pdev->name, cdns_xspi);
-	if (ret) {
-		dev_err(dev, "Failed to request IRQ: %d\n", cdns_xspi->irq);
-		return ret;
+	if (cdns_xspi->irq >= 0) {
+		ret = devm_request_irq(dev, cdns_xspi->irq, cdns_xspi_irq_handler,
+				       IRQF_SHARED, pdev->name, cdns_xspi);
+		if (ret) {
+			dev_err(dev, "Failed to request IRQ: %d\n", cdns_xspi->irq);
+			return ret;
+		}
 	}
 
 #ifdef CONFIG_64BIT
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 6/7] spi: cadence-xspi: Add support for panic writes

[Paul Cercueil]

From: Tanmay Jagdale <tanmay@marvell.com>

Advertise support for panic writes. When a memory op is received with
the 'panic' flag set, the driver will then work without relying on
interrupts and without sleeping.

Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Co-developed-by: Paul Cercueil <paul.cercueil@bootlin.com>
Signed-off-by: Paul Cercueil <paul.cercueil@bootlin.com>
---

Notes:
    v2: - Don't enable interrupts if doing a panic write
        - Use a new cdns_readl_poll() macro that will resolve to calling
          readl_relaxed_poll_timeout_atomic() when doing a panic write, or
          readl_relaxed_poll_timeout() otherwise.
        - Drop cdns_xspi->panic_write. We don't actually need to check it in
          cdns_xspi_is_dll_locked(), because this code is never called in
          the kernel panic path.

 drivers/spi/spi-cadence-xspi.c | 53 ++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 185dffda54bb..6310d0aa2d21 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -287,6 +287,11 @@
 #define GENERIC_CMD_REG_3_NEEDED(len)		(len > 7)
 #define GENERIC_CMD_REG_2_NEEDED(len)		(len > 3)
 
+
+#define cdns_readl_poll(addr, val, cond, delay_us, timeout_us, can_sleep) \
+	((can_sleep) ? readl_relaxed_poll_timeout(addr, val, cond, delay_us, timeout_us) \
+		     : readl_relaxed_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us))
+
 enum cdns_xspi_stig_instr_type {
 	CDNS_XSPI_STIG_INSTR_TYPE_0,
 	CDNS_XSPI_STIG_INSTR_TYPE_1,
@@ -353,40 +358,33 @@ struct cdns_xspi_dev {
 	int current_xfer_qword;
 };
 
-static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
+static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi, bool sleep)
 {
 	u32 ctrl_stat;
 
-	return readl_relaxed_poll_timeout(cdns_xspi->iobase +
-					  CDNS_XSPI_CTRL_STATUS_REG,
-					  ctrl_stat,
-					  ((ctrl_stat &
-					    CDNS_XSPI_CTRL_BUSY) == 0),
-					  100, 1000);
+	return cdns_readl_poll(cdns_xspi->iobase + CDNS_XSPI_CTRL_STATUS_REG,
+			       ctrl_stat, !(ctrl_stat & CDNS_XSPI_CTRL_BUSY),
+			       100, 1000, sleep);
 }
 
 static bool cdns_xspi_is_stig_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
 {
 	u32 ctrl_stat;
 
-	return !readl_relaxed_poll_timeout
-		(cdns_xspi->iobase + CDNS_XSPI_CTRL_STATUS_REG,
-		ctrl_stat,
-		((ctrl_stat & BIT(3)) == 0),
-		sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
-		sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0);
+	return !cdns_readl_poll(cdns_xspi->iobase + CDNS_XSPI_CTRL_STATUS_REG,
+				ctrl_stat, !(ctrl_stat & BIT(3)),
+				MRVL_XSPI_POLL_DELAY_US,
+				MRVL_XSPI_POLL_TIMEOUT_US, sleep);
 }
 
 static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
 {
 	u32 ctrl_stat;
 
-	if (readl_relaxed_poll_timeout
-	    (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
-	     ctrl_stat,
-	     (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
-	     sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
-	     sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0)) {
+	if (cdns_readl_poll(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
+			    ctrl_stat, (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
+			    MRVL_XSPI_POLL_DELAY_US, MRVL_XSPI_POLL_TIMEOUT_US,
+			    sleep)) {
 		return false;
 	}
 
@@ -558,14 +556,14 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 	int ret;
 	int dummybytes = op->dummy.nbytes;
 
-	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
+	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi, !op->panic_write);
 	if (ret < 0)
 		return -EIO;
 
 	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
 	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
 
-	if (cdns_xspi->irq >= 0)
+	if (cdns_xspi->irq >= 0 && !op->panic_write)
 		cdns_xspi->set_interrupts_handler(cdns_xspi, true);
 	cdns_xspi->sdma_error = false;
 
@@ -596,23 +594,23 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
 
 		cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
 
-		if (cdns_xspi->irq >= 0) {
+		if (cdns_xspi->irq >= 0 && !op->panic_write) {
 			wait_for_completion(&cdns_xspi->sdma_complete);
 			if (cdns_xspi->sdma_error) {
 				cdns_xspi->set_interrupts_handler(cdns_xspi, false);
 				return -EIO;
 			}
-		} else if (!cdns_xspi_is_sdma_ready(cdns_xspi, true)) {
+		} else if (!cdns_xspi_is_sdma_ready(cdns_xspi, !op->panic_write)) {
 			return -EIO;
 		}
 
 		cdns_xspi->sdma_handler(cdns_xspi);
 	}
 
-	if (cdns_xspi->irq >= 0) {
+	if (cdns_xspi->irq >= 0 && !op->panic_write) {
 		wait_for_completion(&cdns_xspi->cmd_complete);
 		cdns_xspi->set_interrupts_handler(cdns_xspi, false);
-	} else if (!cdns_xspi_is_stig_ready(cdns_xspi, true)) {
+	} else if (!cdns_xspi_is_stig_ready(cdns_xspi, !op->panic_write)) {
 		return -EIO;
 	}
 
@@ -1196,6 +1194,10 @@ static int cdns_xspi_transfer_one_message_b0(struct spi_controller *controller,
 }
 #endif
 
+static const struct spi_controller_mem_caps cdns_xspi_mem_caps = {
+	.panic_write = true,
+};
+
 static int cdns_xspi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1316,6 +1318,7 @@ static int cdns_xspi_probe(struct platform_device *pdev)
 	}
 
 	host->num_chipselect = 1 << cdns_xspi->hw_num_banks;
+	host->mem_caps = &cdns_xspi_mem_caps;
 
 	ret = devm_spi_register_controller(dev, host);
 	if (ret) {
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 7/7] spi: dt-bindings: cadence: Make XSPI's IRQ line optional

[Paul Cercueil]

From: Tanmay Jagdale <tanmay@marvell.com>

The spi-cadence-xspi driver will happily work without an IRQ line
registered, which means that this IRQ line is optional.

Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Co-developed-by: Paul Cercueil <paul.cercueil@bootlin.com>
Signed-off-by: Paul Cercueil <paul.cercueil@bootlin.com>
---

Notes:
    v2: New patch

 Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
index 38a5795589de..dd8d03c07e68 100644
--- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -43,7 +43,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
 
 allOf:
   - $ref: spi-controller.yaml#
-- 
2.47.3


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 7/7] spi: dt-bindings: cadence: Make XSPI's IRQ line optional

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: justifying optional hardware property with driver capability
--

commit d3552de358b2091602e8175c29c60b182ad1aa2a
Author: Tanmay Jagdale <tanmay@marvell.com>

spi: dt-bindings: cadence: Make XSPI's IRQ line optional

The commit makes the 'interrupts' property optional in the Cadence XSPI
binding because the driver can operate in polling mode.

> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index 38a5795589de2..dd8d03c07e688 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -43,7 +43,6 @@ properties:
>  required:
>    - compatible
>    - reg
> -  - interrupts

[Severity: Low]
Device tree bindings are intended to describe hardware rather than software
driver capabilities.

The commit message justifies removing this constraint because the driver
"will happily work without an IRQ line registered". Does removing this
constraint based on software capability lead to an inaccurate hardware
description if the interrupt line is always present in the hardware?

Should the justification for making this property optional be based on
hardware instead, such as whether the IRQ is physically omitted or
disconnected on certain boards?

>  
>  allOf:
>    - $ref: spi-controller.yaml#

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=7

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/7] spi: spi-mem: Add support for panic mem writes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] spi: spi-mem: inverted pm_runtime_get_if_active() return check
- [High] spi: spi-mem: fallback path deadlocks during panic_write on -EOPNOTSUPP
- [High] spi: deadlock in panic context from pm_runtime_get_if_active()
--

commit 3d801ffb98468bd4b61d20480140ce8dbbcf4ae2
Author: Tanmay Jagdale <tanmay@marvell.com>

spi: spi-mem: Add support for panic mem writes

Add a 'panic_write' flag to the controller's capabilities struct.
When set, the SPI controller indicates that it will be able to send
commands to a connected NOR or NAND memory in the context of a kernel
panic, which means without sleeping and without relying on IRQs.

> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index b94680b60af95..3b6fa44e45504 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c

[ ... ]

> @@ -420,7 +445,10 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  		return -EOPNOTSUPP;
>  
>  	if (ctlr->mem_ops && ctlr->mem_ops->exec_op && !spi_get_csgpiod(mem->spi, 0)) {
> -		ret = spi_mem_access_start(mem);
> +		if (!op->panic_write)
> +			ret = spi_mem_access_start(mem);
> +		else if (ctlr->auto_runtime_pm)
> +			ret = pm_runtime_get_if_active(ctlr->dev.parent);
>  		if (ret)
>  			return ret;

[Severity: High]
Is it safe to call pm_runtime_get_if_active() here during a kernel panic?

pm_runtime_get_if_active() relies on pm_runtime_get_conditional(), which
unconditionally acquires dev->power.lock via spin_lock_irqsave(). In a panic
context with NMIs or IRQs disabled, trying to acquire a spinlock that may be
held by a halted CPU causes an infinite spin. On PREEMPT_RT, spin_lock_irqsave()
maps to a sleeping rt_mutex, which is strictly forbidden in atomic context.

[Severity: High]
Does this logic correctly handle the return value of pm_runtime_get_if_active()?

The API pm_runtime_get_if_active() returns 1 if the device is active (and its
usage count is incremented), 0 if inactive, and a negative errno on failure. By
evaluating if (ret), the intended logic is inverted in two ways:

1. If the device is active (returns 1), the condition evaluates to true, causing
the function to prematurely return 1. The panic write is aborted, and because
execution exits early, the pm_runtime_put_noidle() clean-up is skipped, leaking
the PM usage counter permanently.

2. If the device is suspended (returns 0), the condition evaluates to false.
Execution erroneously proceeds to perform SPI operations on powered-down
hardware.

> @@ -428,7 +456,10 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  		ret = ctlr->mem_ops->exec_op(mem, op);
>  		trace_spi_mem_stop_op(mem, op);
>  
> -		spi_mem_access_end(mem);
> +		if (!op->panic_write)
> +			spi_mem_access_end(mem);
> +		else if (ctlr->auto_runtime_pm)
> +			pm_runtime_put_noidle(ctlr->dev.parent);

[Severity: High]
Following up on the return value logic above, if pm_runtime_get_if_active()
returned 0 (device suspended), does this incorrectly drop a reference?

Since the function proceeded on 0, this pm_runtime_put_noidle() drops a PM
reference that was never actually acquired, resulting in an unbalanced decrement
and an underflow of the usage counter.

>  		/*
>  		 * Some controllers only optimize specific paths (typically the
>  		 * read path) and expect the core to use the regular SPI
>  		 * interface in other cases.
>  		 */
>  		if (!ret || (ret != -ENOTSUPP && ret != -EOPNOTSUPP)) {
>  			spi_mem_add_op_stats(ctlr->pcpu_statistics, op, ret);
>  			spi_mem_add_op_stats(mem->spi->pcpu_statistics, op, ret);
>  
>  			return ret;
>  		}
>  	}

[Severity: High]
If the controller's exec_op implementation dynamically returns -EOPNOTSUPP
during a panic write, could this fallback path cause a deadlock?

Operations executed during a kernel panic must not sleep or wait on completions.
If this check fails, execution bypasses the return and falls through to the
generic SPI message path:

tmpbuf = kzalloc(tmpbufsize, GFP_KERNEL | GFP_DMA);
[ ... ]
ret = spi_sync(mem->spi, &msg);

This fallback path unconditionally allocates memory with GFP_KERNEL and executes
spi_sync(), which waits on completions with interrupts disabled. Hitting this
fallthrough path during a panic will cause a hard deadlock and prevent the panic
dump from completing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 3/7] mtd: spi-nor: Add support for panic writes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] pm: spi-mem: mishandled pm_runtime_get_if_active() silently breaks panic write
- [High] mm: spi-nor: panic write allocation uses sleeping GFP mask
- [High] spi: spi-mem: panic write fallback path sleeps on ENOTSUPP
--

commit a1f9f053470ff10fa10c133dc5aeb0827bc367a1
Author: Tanmay Jagdale <tanmay@marvell.com>

    mtd: spi-nor: Add support for panic writes
    
    If the SPI controller supports it, provide the functionality of doing a
    panic write.
    
    Update the spi-nor core code and controllers to make sure they won't try
    to grab locked mutexes or sleep when doing a panic write, as preemption
    and IRQs are disabled.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 2509fb983d4f2..281acc54b3245 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c

[ ... ]

> @@ -2196,6 +2235,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>  static int spi_nor_octal_dtr_write(struct spi_nor *nor, loff_t to, size_t len,
>  				   const u8 *buf)
>  {
> +	gfp_t mm_flags = GFP_KERNEL;
>  	u8 *tmp_buf;
>  	size_t bytes_written;
>  	loff_t start, end;
> @@ -2204,7 +2244,10 @@ static int spi_nor_octal_dtr_write(struct spi_nor *nor, loff_t to, size_t len,
>  	if (IS_ALIGNED(to, 2) && IS_ALIGNED(len, 2))
>  		return spi_nor_write_data(nor, to, len, buf);
>  
> -	tmp_buf = kmalloc(nor->params->page_size, GFP_KERNEL);
> +	if (nor->mtd.oops_panic_write)
> +		mm_flags |= GFP_ATOMIC;
> +
> +	tmp_buf = kmalloc(nor->params->page_size, mm_flags);

[Severity: High]
Does this bitwise OR successfully remove the sleepable __GFP_DIRECT_RECLAIM
flag?

Because mm_flags is initialized to GFP_KERNEL (which includes
__GFP_DIRECT_RECLAIM), bitwise ORing GFP_ATOMIC into the mask does not
clear the __GFP_DIRECT_RECLAIM bit. This means the resulting mask still
instructs the page allocator that it is allowed to sleep and enter direct
reclaim.

Since panic writes execute in a strictly atomic context with preemption and
local interrupts disabled, could this trigger a bug or deadlock and prevent
the oops log from being saved?

[ ... ]

> @@ -3589,8 +3652,13 @@ static int spi_nor_set_mtd_info(struct spi_nor *nor)
>  	mtd->size = nor->params->size;
>  	mtd->_read = spi_nor_read;
>  	/* Might be already set by some SST flashes. */
> -	if (!mtd->_write)
> +	if (!mtd->_write) {
>  		mtd->_write = spi_nor_write;
> +		if (nor->spimem &&
> +		    spi_mem_controller_is_capable(nor->spimem->spi->controller, panic_write)) {
> +			mtd->_panic_write = spi_nor_panic_write;
> +		}
> +	}

[Severity: Critical]
By hooking up panic write here, this delegates to spi_mem_exec_op() in
drivers/spi/spi-mem.c, which seems to mishandle the runtime pm reference
for panic writes.

In spi_mem_exec_op(), the panic write path does:

    else if (ctlr->auto_runtime_pm)
        ret = pm_runtime_get_if_active(ctlr->dev.parent);
    if (ret)
        return ret;

Since pm_runtime_get_if_active() returns 1 if the device is active, this
catches the success case (1) as an error and returns immediately, skipping
ctlr->mem_ops->exec_op() and leaking the pm reference.

Conversely, if the device was suspended, it returns 0, execution continues
on the suspended controller, and the subsequent pm_runtime_put_noidle() will
underflow the usage count.

Does this silently break the panic write execution?

[Severity: High]
Additionally, if the controller's exec_op() method dynamically returns
-ENOTSUPP or -EOPNOTSUPP, spi_mem_exec_op() falls back to a path that
cannot execute in atomic context:

    tmpbuf = kzalloc(tmpbufsize, GFP_KERNEL | GFP_DMA);
    ...
    ret = spi_sync(mem->spi, &msg);

Both kzalloc(GFP_KERNEL) and spi_sync() can sleep. If this fallback is
triggered during a panic write, will it violate atomic context constraints
and deadlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=3

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 6/7] spi: cadence-xspi: Add support for panic writes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] spi: cadence-xspi: Data corruption in panic writes due to stale interrupt status
--

commit f18a015e23b878b52027e1103a55a1cf4327cf51
Author: Tanmay Jagdale <tanmay@marvell.com>

spi: cadence-xspi: Add support for panic writes

Advertise support for panic writes. When a memory op is received with
the 'panic' flag set, the driver will then work without relying on
interrupts and without sleeping.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 185dffda54bbd..6310d0aa2d211 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c
[ ... ]
> @@ -353,40 +358,33 @@ struct cdns_xspi_dev {
>  	int current_xfer_qword;
>  };
[ ... ]
>  static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
>  {
>  	u32 ctrl_stat;
>  
> -	if (readl_relaxed_poll_timeout
> -	    (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
> -	     ctrl_stat,
> -	     (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
> -	     sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
> -	     sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0)) {
> +	if (cdns_readl_poll(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
> +			    ctrl_stat, (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
> +			    MRVL_XSPI_POLL_DELAY_US, MRVL_XSPI_POLL_TIMEOUT_US,
> +			    sleep)) {
>  		return false;
>  	}
[ ... ]
> @@ -558,14 +556,14 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
>  	int ret;
>  	int dummybytes = op->dummy.nbytes;
>  
> -	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
> +	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi, !op->panic_write);
>  	if (ret < 0)
>  		return -EIO;
>  
>  	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
>  	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
>  
> -	if (cdns_xspi->irq >= 0)
> +	if (cdns_xspi->irq >= 0 && !op->panic_write)
>  		cdns_xspi->set_interrupts_handler(cdns_xspi, true);

[Severity: High]
Does the driver need to clear the sticky status bits in
CDNS_XSPI_INTR_STATUS_REG when bypassing the interrupt handler for panic
writes?

If a kernel panic occurs immediately after a previous hardware operation
finishes but before the interrupt handler can execute, bits like
CDNS_XSPI_SDMA_TRIGGER could remain set in the hardware status register.

Because the interrupt handler is bypassed here, would the polling loop in
cdns_xspi_is_sdma_ready() immediately read the stale CDNS_XSPI_SDMA_TRIGGER
bit as true and proceed prematurely before the hardware has prepared the new
transfer?

>  	cdns_xspi->sdma_error = false;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=6

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/7] mtd: spi-nor: Wait for NOR ready with poll_timeout_us()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: spi-nor: 32-bit integer overflow in timeout_ms * 1000
--

commit d88f329aca4b57db5e4ba0a0fc748ee4d7440aba
Author: Tanmay Jagdale <tanmay@marvell.com>

mtd: spi-nor: Wait for NOR ready with poll_timeout_us()

Instead of polling for the spi_nor_ready() with a custom loop, use
poll_timeout_us(). The sleep period between two polls is set to 100 us
which I think should be fine.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 8bc117b46e028..2509fb983d4f2 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
[ ... ]
> @@ -712,29 +713,21 @@ static int spi_nor_ready(struct spi_nor *nor)
>   * Return: 0 on success, -errno otherwise.
>   */
>  static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
> -						unsigned long timeout_jiffies)
> +						unsigned long timeout_ms)
>  {
> -	unsigned long deadline;
> -	int timeout = 0, ret;
> +	int ret, status;
>  
> -	deadline = jiffies + timeout_jiffies;
> -
> -	while (!timeout) {
> -		if (time_after_eq(jiffies, deadline))
> -			timeout = 1;
> -
> -		ret = spi_nor_ready(nor);
> -		if (ret < 0)
> -			return ret;
> -		if (ret)
> -			return 0;
> -
> -		cond_resched();
> +	ret = poll_timeout_us(status = spi_nor_ready(nor), status,
> +			      100, timeout_ms * 1000, false);
                                   ^^^^^^^^^^^^^^^^^^^
[Severity: High]
Can this multiplication overflow on 32-bit systems?

In spi_nor_erase_dice(), the timeout scales with the flash size. For a 256MB
flash, timeout_ms evaluates to 40000UL * 128 = 5,120,000.

Because both operands are 32-bit types on 32-bit platforms, the arithmetic
could overflow ULONG_MAX (4,294,967,295), wrapping to 825,032,704 before being
implicitly promoted to u64 inside the poll_timeout_us macro.

This truncates the wait timeout from ~85 minutes to ~13.7 minutes, which could
cause legitimately slow erase operations on large flashes to prematurely abort
with -ETIMEDOUT.

> +	if (ret < 0) {
> -	dev_dbg(nor->dev, "flash operation timed out\n");
> +		dev_dbg(nor->dev, "flash operation timed out\n");
> +		return ret;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=2

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 5/7] spi: cadence-xspi: Add irq-less support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] spi: cadence-xspi: 1ms polling timeout is too short for large STIG transfers
- [High] spi: cadence-xspi: stale completion state after early abort or double-completion
- [High] spi: cadence-xspi: missing SDMA error check in polling mode
- [High] spi: cadence-xspi: spurious completions from uncleared polling status
--

commit 16ebab91423ce939f852c48607b518f29634b538
Author: Tanmay Jagdale <tanmay@marvell.com>

spi: cadence-xspi: Add irq-less support

Make the interrupt line optional. If not provided, the driver will
instead resort to polling registers.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 095bf640b379a..185dffda54bbd 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c
> @@ -365,6 +365,39 @@ static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
>  					  100, 1000);
>  }
>  
> +static bool cdns_xspi_is_stig_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
> +{
> +	u32 ctrl_stat;
> +
> +	return !readl_relaxed_poll_timeout
> +		(cdns_xspi->iobase + CDNS_XSPI_CTRL_STATUS_REG,
> +		ctrl_stat,
> +		((ctrl_stat & BIT(3)) == 0),
> +		sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
> +		sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0);

[Severity: High]
Does this polling loop leave the CDNS_XSPI_STIG_DONE bit uncleared in
INTR_STATUS_REG?

If a shared IRQ fires later, cdns_xspi_irq_handler() will see the uncleared
CDNS_XSPI_STIG_DONE bit and call complete(&cdns_xspi->cmd_complete). This can
corrupt the completion counter and cause subsequent commands to prematurely
return from wait_for_completion().

> +}
> +
> +static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
> +{
> +	u32 ctrl_stat;
> +
> +	if (readl_relaxed_poll_timeout
> +	    (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
> +	     ctrl_stat,
> +	     (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
> +	     sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
> +	     sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0)) {

[Severity: High]
Does this polling loop need to check for CDNS_XSPI_SDMA_ERROR?

If a hardware SDMA error occurs and the trigger bit is never set, this will
loop unnecessarily until the timeout. Worse, if both the error and trigger bits
are set, this returns true and fails to clear the error bit, which could lead
to executing the DMA handler on corrupted data.

[Severity: High]
This hardcodes a 1ms polling timeout (MRVL_XSPI_POLL_TIMEOUT_US).

In cdns_xspi_send_stig_command(), this timeout is applied to STIG transfers
that can be up to sdmasize (often 4096 bytes or more). At typical SPI
frequencies, will large transfers take longer than 1ms and spuriously fail with
-EIO?

> +		return false;
> +	}
> +
> +	/* Clear the flag */
> +	writel(CDNS_XSPI_SDMA_TRIGGER,
> +	       cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
> +
> +	return true;
> +}
> +
> +

[ ... ]

> @@ -562,16 +596,25 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
>  
>  		cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
>  
> -		wait_for_completion(&cdns_xspi->sdma_complete);
> -		if (cdns_xspi->sdma_error) {
> -			cdns_xspi->set_interrupts_handler(cdns_xspi, false);
> +		if (cdns_xspi->irq >= 0) {
> +			wait_for_completion(&cdns_xspi->sdma_complete);
> +			if (cdns_xspi->sdma_error) {
> +				cdns_xspi->set_interrupts_handler(cdns_xspi, false);
> +				return -EIO;
> +			}

[Severity: High]
If an sdma_error occurs, this path returns -EIO and abandons the wait on
cmd_complete.

Because set_interrupts_handler(..., false) disables interrupts without clearing
the hardware status, the hardware command may still finish and set the
CDNS_XSPI_STIG_DONE status.

When the next transfer re-enables interrupts, the stale status will immediately
trigger the IRQ handler and signal cmd_complete. Without reinitializing
cmd_complete, won't the new transfer falsely succeed before its hardware
operation actually finishes?

Also, if SDMA_ERROR and SDMA_TRIGGER both occur, won't the IRQ handler call
complete(&cdns_xspi->sdma_complete) twice, leaving another stale completion?

> +		} else if (!cdns_xspi_is_sdma_ready(cdns_xspi, true)) {
>  			return -EIO;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=5

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/7] mtd: spi-nor: Wait for NOR ready with poll_timeout_us()

[Paul Cercueil]

Hi Sashiko,

Le mardi 13 octobre 2026 à 14:30 +0000, sashiko-bot@kernel.org a écrit :
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [High] mtd: spi-nor: 32-bit integer overflow in timeout_ms * 1000
> --
> 
> commit d88f329aca4b57db5e4ba0a0fc748ee4d7440aba
> Author: Tanmay Jagdale <tanmay@marvell.com>
> 
> mtd: spi-nor: Wait for NOR ready with poll_timeout_us()
> 
> Instead of polling for the spi_nor_ready() with a custom loop, use
> poll_timeout_us(). The sleep period between two polls is set to 100 us
> which I think should be fine.
> 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 8bc117b46e028..2509fb983d4f2 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> [ ... ]
> > @@ -712,29 +713,21 @@ static int spi_nor_ready(struct spi_nor *nor)
> >   * Return: 0 on success, -errno otherwise.
> >   */
> >  static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
> > -						unsigned long
> > timeout_jiffies)
> > +						unsigned long timeout_ms)
> >  {
> > -	unsigned long deadline;
> > -	int timeout = 0, ret;
> > +	int ret, status;
> >  
> > -	deadline = jiffies + timeout_jiffies;
> > -
> > -	while (!timeout) {
> > -		if (time_after_eq(jiffies, deadline))
> > -			timeout = 1;
> > -
> > -		ret = spi_nor_ready(nor);
> > -		if (ret < 0)
> > -			return ret;
> > -		if (ret)
> > -			return 0;
> > -
> > -		cond_resched();
> > +	ret = poll_timeout_us(status = spi_nor_ready(nor), status,
> > +			      100, timeout_ms * 1000, false);
>                                    ^^^^^^^^^^^^^^^^^^^
> [Severity: High]
> Can this multiplication overflow on 32-bit systems?
> 
> In spi_nor_erase_dice(), the timeout scales with the flash size. For a 256MB
> flash, timeout_ms evaluates to 40000UL * 128 = 5,120,000.
> 
> Because both operands are 32-bit types on 32-bit platforms, the arithmetic
> could overflow ULONG_MAX (4,294,967,295), wrapping to 825,032,704 before being
> implicitly promoted to u64 inside the poll_timeout_us macro.
> 
> This truncates the wait timeout from ~85 minutes to ~13.7 minutes, which could
> cause legitimately slow erase operations on large flashes to prematurely abort
> with -ETIMEDOUT.
> 
> > +	if (ret < 0) {
> > -	dev_dbg(nor->dev, "flash operation timed out\n");
> > +		dev_dbg(nor->dev, "flash operation timed out\n");
> > +		return ret;
> > +	}

Technically you're not wrong, but to overflow the multiplication here, you'd
need a NOR size of at least (UINT32_MAX / 40000 * 2MiB) ~= 210 GiB, good luck
finding one of these.

I guess I need a v3 anyway, so I'll switch 'timeout_ms' to a u64 there.

Cheers,
-Paul

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/7] spi: spi-mem: Add support for panic mem writes

[Paul Cercueil]

Hi Sashiko,

Le mardi 13 octobre 2026 à 14:23 +0000, sashiko-bot@kernel.org a écrit :
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s)
> to consider:
> - [High] spi: spi-mem: inverted pm_runtime_get_if_active() return check
> - [High] spi: spi-mem: fallback path deadlocks during panic_write on -
> EOPNOTSUPP
> - [High] spi: deadlock in panic context from pm_runtime_get_if_active()
> --
> 
> commit 3d801ffb98468bd4b61d20480140ce8dbbcf4ae2
> Author: Tanmay Jagdale <tanmay@marvell.com>
> 
> spi: spi-mem: Add support for panic mem writes
> 
> Add a 'panic_write' flag to the controller's capabilities struct.
> When set, the SPI controller indicates that it will be able to send
> commands to a connected NOR or NAND memory in the context of a kernel
> panic, which means without sleeping and without relying on IRQs.
> 
> > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > index b94680b60af95..3b6fa44e45504 100644
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> 
> [ ... ]
> 
> > @@ -420,7 +445,10 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct
> > spi_mem_op *op)
> >  		return -EOPNOTSUPP;
> >  
> >  	if (ctlr->mem_ops && ctlr->mem_ops->exec_op &&
> > !spi_get_csgpiod(mem->spi, 0)) {
> > -		ret = spi_mem_access_start(mem);
> > +		if (!op->panic_write)
> > +			ret = spi_mem_access_start(mem);
> > +		else if (ctlr->auto_runtime_pm)
> > +			ret = pm_runtime_get_if_active(ctlr->dev.parent);
> >  		if (ret)
> >  			return ret;
> 
> [Severity: High]
> Is it safe to call pm_runtime_get_if_active() here during a kernel panic?
> 
> pm_runtime_get_if_active() relies on pm_runtime_get_conditional(), which
> unconditionally acquires dev->power.lock via spin_lock_irqsave(). In a panic
> context with NMIs or IRQs disabled, trying to acquire a spinlock that may be
> held by a halted CPU causes an infinite spin. On PREEMPT_RT,
> spin_lock_irqsave()
> maps to a sleeping rt_mutex, which is strictly forbidden in atomic context.

ACK, I'll just drop pm_runtime_get_if_active() here, and restore the return -
EOPNOTSUPP that was in v1 when ctlr->auto_runtime_pm is set.

> [Severity: High]
> If the controller's exec_op implementation dynamically returns -EOPNOTSUPP
> during a panic write, could this fallback path cause a deadlock?

Good point, dear bot. We do not want to go to the fallback code.

Cheers,
-Paul

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 3/7] mtd: spi-nor: Add support for panic writes

[Paul Cercueil]

Hi Sashiko,

Le mardi 13 octobre 2026 à 14:27 +0000, sashiko-bot@kernel.org a écrit :
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s)
> to consider:
> - [Critical] pm: spi-mem: mishandled pm_runtime_get_if_active() silently
> breaks panic write
> - [High] mm: spi-nor: panic write allocation uses sleeping GFP mask
> - [High] spi: spi-mem: panic write fallback path sleeps on ENOTSUPP
> --
> 
> commit a1f9f053470ff10fa10c133dc5aeb0827bc367a1
> Author: Tanmay Jagdale <tanmay@marvell.com>
> 
>     mtd: spi-nor: Add support for panic writes
>     
>     If the SPI controller supports it, provide the functionality of doing a
>     panic write.
>     
>     Update the spi-nor core code and controllers to make sure they won't try
>     to grab locked mutexes or sleep when doing a panic write, as preemption
>     and IRQs are disabled.
> 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 2509fb983d4f2..281acc54b3245 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> 
> [ ... ]
> 
> > @@ -2196,6 +2235,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t
> > from, size_t len,
> >  static int spi_nor_octal_dtr_write(struct spi_nor *nor, loff_t to, size_t
> > len,
> >  				   const u8 *buf)
> >  {
> > +	gfp_t mm_flags = GFP_KERNEL;
> >  	u8 *tmp_buf;
> >  	size_t bytes_written;
> >  	loff_t start, end;
> > @@ -2204,7 +2244,10 @@ static int spi_nor_octal_dtr_write(struct spi_nor
> > *nor, loff_t to, size_t len,
> >  	if (IS_ALIGNED(to, 2) && IS_ALIGNED(len, 2))
> >  		return spi_nor_write_data(nor, to, len, buf);
> >  
> > -	tmp_buf = kmalloc(nor->params->page_size, GFP_KERNEL);
> > +	if (nor->mtd.oops_panic_write)
> > +		mm_flags |= GFP_ATOMIC;
> > +
> > +	tmp_buf = kmalloc(nor->params->page_size, mm_flags);
> 
> [Severity: High]
> Does this bitwise OR successfully remove the sleepable __GFP_DIRECT_RECLAIM
> flag?
> 
> Because mm_flags is initialized to GFP_KERNEL (which includes
> __GFP_DIRECT_RECLAIM), bitwise ORing GFP_ATOMIC into the mask does not
> clear the __GFP_DIRECT_RECLAIM bit. This means the resulting mask still
> instructs the page allocator that it is allowed to sleep and enter direct
> reclaim.

My bad, it should be GFP_KERNEL *or* GFP_ATOMIC. Good catch.

Cheers,
-Paul

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/