[PATCH v2 0/5] spi: atmel-quadspi: add support for LAN969x

[PATCH v2 0/5] spi: atmel-quadspi: add support for LAN969x

[Robert Marko]

This series adds QSPI support for the Microchip LAN969x which has two QSPI
controllers based on SAMA7G5.

It requires pad calibration, supports DMA, and supports 100 MHz operation.

It requires a different init sequence to SAMA7G5 so support for custom
.init per controller caps is added, and existing controllers relying on
checking gclk are converted to it.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>

Changes in v2:
* Make the DTS nodes apply on top of next-20260708, as SDMMC is not yet upstream

Robert Marko (5):
  dt-bindings: spi: Document LAN969x QSPI
  spi: atmel-quadspi: add controller init callback
  spi: atmel-quadspi: use init callback for gclk variants
  spi: atmel-quadspi: add LAN969x QSPI support
  arm64: dts: microchip: lan969x: add QSPI nodes

 .../bindings/spi/atmel,quadspi.yaml           |  1 +
 arch/arm64/boot/dts/microchip/lan9691.dtsi    | 33 ++++++++
 drivers/spi/Kconfig                           |  2 +-
 drivers/spi/atmel-quadspi.c                   | 78 ++++++++++++++++++-
 4 files changed, 110 insertions(+), 4 deletions(-)

-- 
2.55.0

[PATCH v2 1/5] dt-bindings: spi: Document LAN969x QSPI

[Robert Marko]

LAN969x has 2 QSPI instances, so document their compatible.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 Documentation/devicetree/bindings/spi/atmel,quadspi.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
index 30ab42c95c08..36c698ced99b 100644
--- a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
+++ b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
@@ -22,6 +22,7 @@ properties:
       - microchip,sama7d65-ospi
       - microchip,sama7g5-qspi
       - microchip,sama7g5-ospi
+      - microchip,lan9691-qspi
 
   reg:
     items:
-- 
2.55.0

[PATCH v2 2/5] spi: atmel-quadspi: add controller init callback

[Robert Marko]

Allow controller variants to provide a custom initialization callback
through their capability data.

This prepares the driver for variants which require a different hardware
initialization sequence without adding SoC checks to the common path.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/spi/atmel-quadspi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index aaf7f4c46b22..62ea84d234d0 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -256,8 +256,11 @@ static const struct atmel_qspi_pcal pcal[ATMEL_QSPI_PCAL_ARRAY_SIZE] = {
 	{200000000, 7},
 };
 
+struct atmel_qspi;
+
 struct atmel_qspi_caps {
 	u32 max_speed_hz;
+	int (*init)(struct atmel_qspi *aq);
 	bool has_qspick;
 	bool has_gclk;
 	bool has_ricr;
@@ -1156,6 +1159,9 @@ static int atmel_qspi_sama7g5_setup(struct spi_device *spi)
 	/* The controller can communicate with a single peripheral device (target). */
 	aq->target_max_speed_hz = spi->max_speed_hz;
 
+	if (aq->caps->init)
+		return aq->caps->init(aq);
+
 	return atmel_qspi_sama7g5_init(aq);
 }
 
@@ -1570,6 +1576,9 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev)
 		return ret;
 	}
 
+	if (aq->caps->init)
+		return aq->caps->init(aq);
+
 	if (aq->caps->has_gclk)
 		return atmel_qspi_sama7g5_init(aq);
 
-- 
2.55.0

[PATCH v2 3/5] spi: atmel-quadspi: use init callback for gclk variants

[Robert Marko]

Assign the existing SAMA7G5 initialization routine to every generic
clock variant and dispatch initialization exclusively through the
capability callback.

This keeps hardware capabilities separate from initialization selection
and lets variants override the sequence explicitly.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/spi/atmel-quadspi.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 62ea84d234d0..d1aec14e3978 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -1159,10 +1159,7 @@ static int atmel_qspi_sama7g5_setup(struct spi_device *spi)
 	/* The controller can communicate with a single peripheral device (target). */
 	aq->target_max_speed_hz = spi->max_speed_hz;
 
-	if (aq->caps->init)
-		return aq->caps->init(aq);
-
-	return atmel_qspi_sama7g5_init(aq);
+	return aq->caps->init(aq);
 }
 
 static int atmel_qspi_setup(struct spi_device *spi)
@@ -1576,11 +1573,8 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev)
 		return ret;
 	}
 
-	if (aq->caps->init)
-		return aq->caps->init(aq);
-
 	if (aq->caps->has_gclk)
-		return atmel_qspi_sama7g5_init(aq);
+		return aq->caps->init(aq);
 
 	ret = pm_runtime_force_resume(dev);
 	if (ret < 0)
@@ -1638,6 +1632,7 @@ static const struct atmel_qspi_caps atmel_sam9x60_qspi_caps = {
 
 static const struct atmel_qspi_caps atmel_sam9x7_ospi_caps = {
 	.max_speed_hz = SAM9X7_QSPI_MAX_SPEED_HZ,
+	.init = atmel_qspi_sama7g5_init,
 	.has_gclk = true,
 	.octal = true,
 	.has_dma = true,
@@ -1648,6 +1643,7 @@ static const struct atmel_qspi_caps atmel_sam9x7_ospi_caps = {
 
 static const struct atmel_qspi_caps atmel_sama7d65_ospi_caps = {
 	.max_speed_hz = SAMA7G5_QSPI0_MAX_SPEED_HZ,
+	.init = atmel_qspi_sama7g5_init,
 	.has_gclk = true,
 	.octal = true,
 	.has_dma = true,
@@ -1658,6 +1654,7 @@ static const struct atmel_qspi_caps atmel_sama7d65_ospi_caps = {
 
 static const struct atmel_qspi_caps atmel_sama7d65_qspi_caps = {
 	.max_speed_hz = SAMA7G5_QSPI1_SDR_MAX_SPEED_HZ,
+	.init = atmel_qspi_sama7g5_init,
 	.has_gclk = true,
 	.has_dma = true,
 	.has_2xgclk = true,
@@ -1666,6 +1663,7 @@ static const struct atmel_qspi_caps atmel_sama7d65_qspi_caps = {
 
 static const struct atmel_qspi_caps atmel_sama7g5_ospi_caps = {
 	.max_speed_hz = SAMA7G5_QSPI0_MAX_SPEED_HZ,
+	.init = atmel_qspi_sama7g5_init,
 	.has_gclk = true,
 	.octal = true,
 	.has_dma = true,
@@ -1675,6 +1673,7 @@ static const struct atmel_qspi_caps atmel_sama7g5_ospi_caps = {
 
 static const struct atmel_qspi_caps atmel_sama7g5_qspi_caps = {
 	.max_speed_hz = SAMA7G5_QSPI1_SDR_MAX_SPEED_HZ,
+	.init = atmel_qspi_sama7g5_init,
 	.has_gclk = true,
 	.has_dma = true,
 	.has_dllon = true,
-- 
2.55.0

[PATCH v2 4/5] spi: atmel-quadspi: add LAN969x QSPI support

[Robert Marko]

Microchip LAN969x has two QSPI controllers based on SAMA7G5 QSPI.

It requires pad calibration, supports DMA, and supports 100 MHz operation.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/spi/Kconfig         |  2 +-
 drivers/spi/atmel-quadspi.c | 66 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8782514bb89b..bb3773f99ad9 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -189,7 +189,7 @@ config SPI_AT91_USART
 
 config SPI_ATMEL_QUADSPI
 	tristate "Atmel Quad SPI Controller"
-	depends on ARCH_AT91 || COMPILE_TEST
+	depends on ARCH_MICROCHIP || COMPILE_TEST
 	depends on OF && HAS_IOMEM
 	help
 	  This enables support for the Quad SPI controller in master mode.
diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index d1aec14e3978..f05e2617fa89 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -1152,6 +1152,58 @@ static int atmel_qspi_sama7g5_init(struct atmel_qspi *aq)
 	return ret;
 }
 
+static int atmel_qspi_lan969x_init(struct atmel_qspi *aq)
+{
+	u32 val;
+	int ret;
+
+	atmel_qspi_write(QSPI_CR_DLLOFF, aq, QSPI_CR);
+	ret = readl_poll_timeout(aq->regs + QSPI_SR2, val,
+				 !(val & QSPI_SR2_DLOCK), 40,
+				 ATMEL_QSPI_TIMEOUT);
+	if (ret)
+		return ret;
+
+	ret = atmel_qspi_set_gclk(aq);
+	if (ret)
+		return ret;
+
+	/* Start the DLL before resetting the controller. */
+	atmel_qspi_write(QSPI_CR_DLLON | QSPI_CR_STPCAL, aq, QSPI_CR);
+	ret = readl_poll_timeout(aq->regs + QSPI_SR2, val,
+				 (val & QSPI_SR2_DLOCK) &&
+				 !(val & QSPI_SR2_CALBSY), 40,
+				 ATMEL_QSPI_TIMEOUT);
+	if (ret)
+		return ret;
+
+	atmel_qspi_write(QSPI_CR_QSPIDIS, aq, QSPI_CR);
+	ret = atmel_qspi_reg_sync(aq);
+	if (ret)
+		return ret;
+
+	atmel_qspi_write(QSPI_CR_SWRST, aq, QSPI_CR);
+	ret = atmel_qspi_reg_sync(aq);
+	if (ret)
+		return ret;
+
+	ret = atmel_qspi_set_pad_calibration(aq);
+	if (ret)
+		return ret;
+
+	aq->mr = 0;
+	aq->scr = 0;
+
+	ret = atmel_qspi_set_serial_memory_mode(aq);
+	if (ret)
+		return ret;
+
+	atmel_qspi_write(QSPI_CR_QSPIEN, aq, QSPI_CR);
+	return readl_poll_timeout(aq->regs + QSPI_SR2, val,
+				  (val & QSPI_SR2_QSPIENS), 40,
+				  ATMEL_QSPI_TIMEOUT);
+}
+
 static int atmel_qspi_sama7g5_setup(struct spi_device *spi)
 {
 	struct atmel_qspi *aq = spi_controller_get_devdata(spi->controller);
@@ -1679,6 +1731,15 @@ static const struct atmel_qspi_caps atmel_sama7g5_qspi_caps = {
 	.has_dllon = true,
 };
 
+static const struct atmel_qspi_caps atmel_lan969x_qspi_caps = {
+	.max_speed_hz = SAM9X7_QSPI_MAX_SPEED_HZ,
+	.init = atmel_qspi_lan969x_init,
+	.has_gclk = true,
+	.has_dma = true,
+	.has_padcalib = true,
+	.has_dllon = true,
+};
+
 static const struct of_device_id atmel_qspi_dt_ids[] = {
 	{
 		.compatible = "atmel,sama5d2-qspi",
@@ -1708,7 +1769,10 @@ static const struct of_device_id atmel_qspi_dt_ids[] = {
 		.compatible = "microchip,sama7d65-qspi",
 		.data = &atmel_sama7d65_qspi_caps,
 	},
-
+	{
+		.compatible = "microchip,lan9691-qspi",
+		.data = &atmel_lan969x_qspi_caps,
+	},
 
 	{ /* sentinel */ }
 };
-- 
2.55.0

[PATCH v2 5/5] arm64: dts: microchip: lan969x: add QSPI nodes

[Robert Marko]

Add the required DT nodes for both QSPI controllers.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
Changes in v2:
* Make it apply on top of next-20260708, as SDMMC is not yet upstream

 arch/arm64/boot/dts/microchip/lan9691.dtsi | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/lan9691.dtsi b/arch/arm64/boot/dts/microchip/lan9691.dtsi
index ed997d87dd09..a8541c6a098d 100644
--- a/arch/arm64/boot/dts/microchip/lan9691.dtsi
+++ b/arch/arm64/boot/dts/microchip/lan9691.dtsi
@@ -396,6 +396,39 @@ clks: clock-controller@e00c00b4 {
 			clock-names = "cpu", "ddr", "sys";
 		};
 
+		qspi0: spi@e0804000 {
+			compatible = "microchip,lan9691-qspi";
+			reg = <0xe0804000 0x00000100>,
+			      <0x20000000 0x08000000>;
+			reg-names = "qspi_base", "qspi_mmap";
+			interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&fabric_clk>, <&clks GCK_ID_QSPI0>;
+			clock-names = "pclk", "gclk";
+			assigned-clocks = <&clks GCK_ID_QSPI0>;
+			assigned-clock-rates = <100000000>;
+			dmas = <&dma AT91_XDMAC_DT_PERID(1)>,
+			       <&dma AT91_XDMAC_DT_PERID(0)>;
+			dma-names = "tx", "rx";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		qspi2: spi@e0834000 {
+			compatible = "microchip,lan9691-qspi";
+			reg = <0xe0834000 0x00000100>,
+			      <0x30000000 0x04000000>;
+			reg-names = "qspi_base", "qspi_mmap";
+			interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&fabric_clk>, <&clks GCK_ID_QSPI2>;
+			clock-names = "pclk", "gclk";
+			assigned-clocks = <&clks GCK_ID_QSPI2>;
+			assigned-clock-rates = <100000000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		reset: reset-controller@e201000c {
 			compatible = "microchip,lan9691-switch-reset",
 				     "microchip,lan966x-switch-reset";
-- 
2.55.0

Re: [PATCH v2 4/5] spi: atmel-quadspi: add LAN969x QSPI support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [High] Loss of SPI Chip Select custom timings due to explicitly clearing shadow registers in lan969x init.
- [High] Missing clk_disable_unprepare() on error paths in atmel_qspi_lan969x_init() causes clock reference leak.

Pre-existing issues:
- [High] Missing PM runtime wakeup in has_gclk device setup path causes system hang.
- [High] Silent loss of QSPI_MR configuration across suspend/resume because shadow register check prevents hardware update.
- [High] Clock enable count leak in atmel_qspi_set_gclk() on repeated SPI setup.
- [Medium] System resume timeout if no SPI child devices are attached to the controller.
--

--- Patch [4]: [PATCH v2 4/5] spi: atmel-quadspi: add LAN969x QSPI support ---
commit e7c8f2e53e20c016b424a0d36952bf174920cb87
Author: Robert Marko <robert.marko@sartura.hr>

spi: atmel-quadspi: add LAN969x QSPI support

Microchip LAN969x has two QSPI controllers based on SAMA7G5 QSPI.
It requires pad calibration, supports DMA, and supports 100 MHz operation.

> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index d1aec14e39789..f05e2617fa89e 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
[ ... ]
> @@ -1152,6 +1152,58 @@ static int atmel_qspi_sama7g5_init(struct atmel_qspi *aq)
>  	return ret;
>  }
>  
> +static int atmel_qspi_lan969x_init(struct atmel_qspi *aq)
> +{
> +	u32 val;
> +	int ret;
> +
> +	atmel_qspi_write(QSPI_CR_DLLOFF, aq, QSPI_CR);
> +	ret = readl_poll_timeout(aq->regs + QSPI_SR2, val,
> +				 !(val & QSPI_SR2_DLOCK), 40,
> +				 ATMEL_QSPI_TIMEOUT);
> +	if (ret)
> +		return ret;
> +
> +	ret = atmel_qspi_set_gclk(aq);
> +	if (ret)
> +		return ret;
> +
> +	/* Start the DLL before resetting the controller. */
> +	atmel_qspi_write(QSPI_CR_DLLON | QSPI_CR_STPCAL, aq, QSPI_CR);
> +	ret = readl_poll_timeout(aq->regs + QSPI_SR2, val,
> +				 (val & QSPI_SR2_DLOCK) &&
> +				 !(val & QSPI_SR2_CALBSY), 40,
> +				 ATMEL_QSPI_TIMEOUT);
> +	if (ret)
> +		return ret;

[Severity: High]
Does this early return path leave the hardware generic clock running?

The function calls atmel_qspi_set_gclk() which prepares and enables aq->gclk.
If the subsequent readl_poll_timeout() operations fail (such as waiting for
the DLL to lock), the function directly returns the error code.

Without calling clk_disable_unprepare(aq->gclk), won't this result in a
permanent clock reference leak?

[ ... ]
> +	atmel_qspi_write(QSPI_CR_SWRST, aq, QSPI_CR);
> +	ret = atmel_qspi_reg_sync(aq);
> +	if (ret)
> +		return ret;
> +
> +	ret = atmel_qspi_set_pad_calibration(aq);
> +	if (ret)
> +		return ret;
> +
> +	aq->mr = 0;
> +	aq->scr = 0;

[Severity: High]
Does explicitly clearing these shadow registers permanently wipe the
cached hardware timings?

During system resume or dynamic reconfiguration via spi_setup(), 
atmel_qspi_resume() or atmel_qspi_setup() invokes aq->caps->init().
Zeroing out aq->mr and aq->scr clears the chip select timings (cs_setup,
cs_hold, cs_inactive) that were configured by atmel_qspi_set_cs_timing().

Will this cause data communication failures with SPI memory because the timings
are wiped from the cache and never written back, making the controller operate
with zero delays?

> +
> +	ret = atmel_qspi_set_serial_memory_mode(aq);
> +	if (ret)
> +		return ret;
> +
> +	atmel_qspi_write(QSPI_CR_QSPIEN, aq, QSPI_CR);
> +	return readl_poll_timeout(aq->regs + QSPI_SR2, val,
> +				  (val & QSPI_SR2_QSPIENS), 40,
> +				  ATMEL_QSPI_TIMEOUT);
> +}


Additionally, there are some pre-existing issues in this file that I noticed
while reviewing the code.

[Severity: High]
This isn't a bug introduced by this patch, but does the has_gclk device setup
path lack a PM runtime wakeup, potentially causing a system hang?

drivers/spi/atmel-quadspi.c:atmel_qspi_setup() {
    ...
	if (aq->caps->has_gclk)
		return atmel_qspi_sama7g5_setup(spi);
    ...
}

The atmel_qspi_setup() function delegates to atmel_qspi_sama7g5_setup() which
directly invokes hardware register writes via aq->caps->init(aq). Unlike the
fallback non-gclk path in the exact same function, this path fails to call
pm_runtime_resume_and_get().

If spi_setup() is called by a client driver while the SPI controller has
autosuspended, won't the driver touch registers with the power domain or
peripheral clock disabled, causing a system crash or bus fault?


[Severity: Medium]
This is a pre-existing issue, but does atmel_qspi_resume() unconditionally
attempt hardware initialization using uninitialized speed values?

drivers/spi/atmel-quadspi.c:atmel_qspi_resume() {
    ...
	if (aq->caps->has_gclk)
		return aq->caps->init(aq);
    ...
}

If no SPI child devices probe successfully, aq->target_max_speed_hz remains 0.
During system resume, atmel_qspi_resume() executes aq->caps->init(aq) for
has_gclk controllers. This invokes atmel_qspi_set_gclk(), setting the clock
to 0 Hz.

Won't the subsequent polling for the DLL lock (QSPI_SR2_DLOCK) inevitably
timeout since the clock is halted, causing readl_poll_timeout() to stall for
1000ms?


[Severity: High]
This is a pre-existing issue, but does the shadow register check block
hardware writes if the bit is already cached, breaking restoration on resume?

drivers/spi/atmel-quadspi.c:atmel_qspi_set_serial_memory_mode() {
    ...
	if (!(aq->mr & QSPI_MR_SMM)) {
		aq->mr |= QSPI_MR_SMM;
		atmel_qspi_write(aq->mr, aq, QSPI_MR);
    ...
}

During system resume, non-gclk controllers rely on atmel_qspi_init() to
restore hardware state after a software reset (QSPI_CR_SWRST). It calls
atmel_qspi_set_serial_memory_mode() to restore the Mode Register (QSPI_MR).

Because aq->mr retains its pre-suspend value with the SMM bit set, the
condition evaluates to false and the register write is silently skipped.
Could this leave the hardware at its reset state of 0 and break device
communication after resume?


[Severity: High]
This is a pre-existing issue, but does atmel_qspi_set_gclk() unconditionally
increment the clock enable count during every setup call?

drivers/spi/atmel-quadspi.c:atmel_qspi_set_gclk() {
    ...
	/* Enable the QSPI generic clock */
	ret = clk_prepare_enable(aq->gclk);
    ...
}

spi_setup() can be called multiple times during the lifecycle of a SPI device.
Every time it is called on a has_gclk controller, it invokes
aq->caps->init(aq), which sequentially calls atmel_qspi_set_gclk().

Since clk_disable_unprepare() is only ever called once during device suspend
or removal, won't this endlessly increment the clock reference count and leak
the reference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709112006.390742-1-robert.marko@sartura.hr?part=4

Re: [PATCH v2 3/5] spi: atmel-quadspi: use init callback for gclk variants

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The `.init` callback invocation in `atmel_qspi_resume` is strictly coupled to `has_gclk`, contradicting the commit message claim.

Pre-existing issues:
- [High] Runtime PM usage counter leak permanently prevents runtime suspend for gclk variants.
- [High] System resume accesses memory-mapped hardware registers with the peripheral clock disabled, causing a bus fault.
--

--- Patch [3]: [PATCH v2 3/5] spi: atmel-quadspi: use init callback for gclk variants ---
commit ce20548a48abc8b47ba02b69140b1f7ba51f52af
Author: Robert Marko <robert.marko@sartura.hr>

spi: atmel-quadspi: use init callback for gclk variants
    
Assign the existing SAMA7G5 initialization routine to every generic
clock variant and dispatch initialization exclusively through the
capability callback.

> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index 62ea84d234d02..d1aec14e39789 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c

[ ... ]

> @@ -1576,11 +1573,8 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev)
>  		return ret;
>  	}
>  
> -	if (aq->caps->init)
> -		return aq->caps->init(aq);
> -
>  	if (aq->caps->has_gclk)
> -		return atmel_qspi_sama7g5_init(aq);
> +		return aq->caps->init(aq);

[Severity: High]
This is a pre-existing issue, but does this early return cause a runtime PM
usage counter leak?

In atmel_qspi_suspend(), the usage counter is incremented but not balanced
for has_gclk variants:

    ret = pm_runtime_resume_and_get(dev);
    ...
    if (aq->caps->has_gclk) {
        ret = atmel_qspi_sama7g5_suspend(aq);
        clk_disable_unprepare(aq->pclk);
        return ret;
    }

By returning early here in atmel_qspi_resume(), the
pm_runtime_put_autosuspend(dev) call at the end of the function is completely
bypassed. Will this permanently inflate the usage counter on every system
suspend/resume cycle and prevent the device from entering runtime suspend?

[Severity: High]
This is also a pre-existing issue, but does this early return lead to accessing
hardware registers with the peripheral clock disabled?

During atmel_qspi_suspend(), the peripheral clock is explicitly disabled for
has_gclk variants:

    clk_disable_unprepare(aq->pclk);
    return ret;

In atmel_qspi_resume(), clk_prepare() is called, but clk_enable() is never
invoked before the early return:

    ret = clk_prepare(aq->pclk);
    ...
    if (aq->caps->has_gclk)
        return aq->caps->init(aq);

Since pm_runtime_force_resume() is bypassed by this early return, the runtime
resume callback is not triggered to enable the clock. Will this cause a bus
fault when aq->caps->init() performs register writes (e.g., to QSPI_CR)?

[Severity: Medium]
The commit message states that this change "keeps hardware capabilities
separate from initialization selection".

However, by removing the generic check for aq->caps->init and gating it
strictly behind aq->caps->has_gclk, doesn't this couple the init callback
invocation to the has_gclk capability?

If a non-has_gclk variant defined an init callback, wouldn't it now be
ignored during system resume?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709112006.390742-1-robert.marko@sartura.hr?part=3

Re: [PATCH v2 2/5] spi: atmel-quadspi: add controller init callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] The custom `.init` callback is evaluated unconditionally in `atmel_qspi_resume()`, bypassing required PM state restoration for non-gclk variants.
- [Medium] Unbalanced clock prepare operations on the error path of the newly added `.init()` callback in `atmel_qspi_resume()`.

Pre-existing issues:
- [High] Pre-existing bugs in the `has_gclk` suspend/resume paths cause a PM runtime usage counter leak and perform hardware register accesses with disabled clocks.
- [High] The generic clock (`gclk`) enable count leaks continuously because `clk_prepare_enable()` is invoked unconditionally during every SPI setup.
--

--- Patch [2]: [PATCH v2 2/5] spi: atmel-quadspi: add controller init callback ---
commit 6c4a6f692bd1bda4ee70cd3b53efc7320ebf6191
Author: Robert Marko <robert.marko@sartura.hr>

spi: atmel-quadspi: add controller init callback

Allow controller variants to provide a custom initialization callback
through their capability data. This prepares the driver for variants
which require a different hardware initialization sequence without
adding SoC checks to the common path.

> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index aaf7f4c46b22f..62ea84d234d02 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c

[ ... ]

> @@ -1156,6 +1159,9 @@ static int atmel_qspi_sama7g5_setup(struct spi_device *spi)
>  	/* The controller can communicate with a single peripheral device (target). */
>  	aq->target_max_speed_hz = spi->max_speed_hz;
>  
> +	if (aq->caps->init)
> +		return aq->caps->init(aq);
> +

[Severity: High]
This is a pre-existing issue, but does invoking initialization callbacks
from atmel_qspi_sama7g5_setup() cause a generic clock enable count leak?

The generic clock enable count appears to leak continuously because
atmel_qspi_set_gclk() unconditionally calls clk_prepare_enable().

Since spi_setup() can be called multiple times during the device lifecycle,
such as when the SPI core probes child devices or adjusts transfer speeds,
the clock's enable count repeatedly increments:

drivers/spi/atmel-quadspi.c:atmel_qspi_set_gclk() {
    ...
	ret = clk_prepare_enable(aq->gclk);
    ...
}

Because clk_disable_unprepare() is only called once during system
suspend, the clock becomes severely unbalanced and might never be fully
shut down.

>  	return atmel_qspi_sama7g5_init(aq);
>  }

[ ... ]

> @@ -1570,6 +1576,9 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	if (aq->caps->init)
> +		return aq->caps->init(aq);
> +

[Severity: Medium]
Will this unconditional early return bypass required power management state
restoration for non-gclk variants?

If a non-gclk variant defines an init callback, it will intercept the
system resume path and completely bypass pm_runtime_force_resume() and the
default non-gclk hardware restoration sequence that occurs further down in
atmel_qspi_resume().

[Severity: Medium]
Does this also create a clock prepare count leak on the error path?

If the newly added init callback fails and returns a non-zero error code
in atmel_qspi_resume(), the function returns immediately.

This skips the necessary clk_unprepare() calls for aq->pclk and aq->qspick
that were successfully prepared earlier in atmel_qspi_resume().

>  	if (aq->caps->has_gclk)
>  		return atmel_qspi_sama7g5_init(aq);

[Severity: High]
This is a pre-existing issue, but does returning early for has_gclk
devices leak a power management runtime usage counter and access hardware
with disabled clocks?

In atmel_qspi_suspend(), pm_runtime_resume_and_get() increments the power
management usage count. For has_gclk devices, it returns early, skipping
pm_runtime_force_suspend() and missing a balancing pm_runtime_put():

drivers/spi/atmel-quadspi.c:atmel_qspi_suspend() {
    ...
	if (aq->caps->has_gclk) {
		ret = atmel_qspi_sama7g5_suspend(aq);
		clk_disable_unprepare(aq->pclk);
		return ret;
	}
    ...
}

In atmel_qspi_resume(), it calls clk_prepare() for aq->pclk but misses
clk_enable(). It then returns early via atmel_qspi_sama7g5_init(),
completely skipping pm_runtime_put_autosuspend().

As a result, every system suspend/resume cycle permanently leaks a power
management usage count.

Furthermore, because clk_enable() is never called, the hardware
initialization callback accesses registers with a disabled peripheral clock,
which could cause an asynchronous external abort or bus fault on resume:

drivers/spi/atmel-quadspi.c:atmel_qspi_lan969x_init() {
    ...
	atmel_qspi_write(QSPI_CR_DLLOFF, aq, QSPI_CR);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709112006.390742-1-robert.marko@sartura.hr?part=2

Re: [PATCH v2 5/5] arm64: dts: microchip: lan969x: add QSPI nodes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `qspi2` node is missing `dmas` and `dma-names` properties (unlike `qspi0`), which degrades performance to PIO mode and causes the atmel-quadspi driver to log a spurious error during probe.
--

--- Patch [5]: [PATCH v2 5/5] arm64: dts: microchip: lan969x: add QSPI nodes ---
commit 297e92dccc7e525621a995eb1402956ad591b3bf
Author: Robert Marko <robert.marko@sartura.hr>

arm64: dts: microchip: lan969x: add QSPI nodes

This commit adds device tree nodes for both QSPI controllers on the Microchip
LAN969x SoC. It configures base addresses, interrupts, and clocks for the
controllers.

> diff --git a/arch/arm64/boot/dts/microchip/lan9691.dtsi b/arch/arm64/boot/dts/microchip/lan9691.dtsi
> index ed997d87dd09f..a8541c6a098d1 100644
> --- a/arch/arm64/boot/dts/microchip/lan9691.dtsi
> +++ b/arch/arm64/boot/dts/microchip/lan9691.dtsi

[ ... ]

> @@ -396,6 +396,39 @@ clks: clock-controller@e00c00b4 {
>  			clock-names = "cpu", "ddr", "sys";
>  		};
>  
> +		qspi0: spi@e0804000 {
> +			compatible = "microchip,lan9691-qspi";

[ ... ]

> +			dmas = <&dma AT91_XDMAC_DT_PERID(1)>,
> +			       <&dma AT91_XDMAC_DT_PERID(0)>;
> +			dma-names = "tx", "rx";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
> +		qspi2: spi@e0834000 {
> +			compatible = "microchip,lan9691-qspi";
> +			reg = <0xe0834000 0x00000100>,
> +			      <0x30000000 0x04000000>;
> +			reg-names = "qspi_base", "qspi_mmap";
> +			interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&fabric_clk>, <&clks GCK_ID_QSPI2>;
> +			clock-names = "pclk", "gclk";
> +			assigned-clocks = <&clks GCK_ID_QSPI2>;
> +			assigned-clock-rates = <100000000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};

[Severity: Medium]
Is it intentional that qspi2 is missing the dmas and dma-names properties?

During probe, the atmel-quadspi driver unconditionally checks for dma since
aq->caps->has_dma is true for LAN969x compatible strings, and then calls
atmel_qspi_dma_init().

Without the dmas property present here, devm_dma_request_chan() returns
-ENODEV. The driver handles this with dev_err_probe(), which emits a
kernel error log ('error -ENODEV: RX DMA channel is not available') because
it is not -EPROBE_DEFER.

The controller will then fall back to pio mode, degrading performance.

If the hardware supports dma for qspi2, should these properties be added?
If it does not, should the driver be updated to avoid unconditionally setting
has_dma to true for all instances?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709112006.390742-1-robert.marko@sartura.hr?part=5

Re: [PATCH v2 5/5] arm64: dts: microchip: lan969x: add QSPI nodes

[Robert Marko]

On Thu, Jul 9, 2026 at 1:50 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `qspi2` node is missing `dmas` and `dma-names` properties (unlike `qspi0`), which degrades performance to PIO mode and causes the atmel-quadspi driver to log a spurious error during probe.
> --
>
> --- Patch [5]: [PATCH v2 5/5] arm64: dts: microchip: lan969x: add QSPI nodes ---
> commit 297e92dccc7e525621a995eb1402956ad591b3bf
> Author: Robert Marko <robert.marko@sartura.hr>
>
> arm64: dts: microchip: lan969x: add QSPI nodes
>
> This commit adds device tree nodes for both QSPI controllers on the Microchip
> LAN969x SoC. It configures base addresses, interrupts, and clocks for the
> controllers.
>
> > diff --git a/arch/arm64/boot/dts/microchip/lan9691.dtsi b/arch/arm64/boot/dts/microchip/lan9691.dtsi
> > index ed997d87dd09f..a8541c6a098d1 100644
> > --- a/arch/arm64/boot/dts/microchip/lan9691.dtsi
> > +++ b/arch/arm64/boot/dts/microchip/lan9691.dtsi
>
> [ ... ]
>
> > @@ -396,6 +396,39 @@ clks: clock-controller@e00c00b4 {
> >                       clock-names = "cpu", "ddr", "sys";
> >               };
> >
> > +             qspi0: spi@e0804000 {
> > +                     compatible = "microchip,lan9691-qspi";
>
> [ ... ]
>
> > +                     dmas = <&dma AT91_XDMAC_DT_PERID(1)>,
> > +                            <&dma AT91_XDMAC_DT_PERID(0)>;
> > +                     dma-names = "tx", "rx";
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +                     status = "disabled";
> > +             };
> > +
> > +             qspi2: spi@e0834000 {
> > +                     compatible = "microchip,lan9691-qspi";
> > +                     reg = <0xe0834000 0x00000100>,
> > +                           <0x30000000 0x04000000>;
> > +                     reg-names = "qspi_base", "qspi_mmap";
> > +                     interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
> > +                     clocks = <&fabric_clk>, <&clks GCK_ID_QSPI2>;
> > +                     clock-names = "pclk", "gclk";
> > +                     assigned-clocks = <&clks GCK_ID_QSPI2>;
> > +                     assigned-clock-rates = <100000000>;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +                     status = "disabled";
> > +             };
>
> [Severity: Medium]
> Is it intentional that qspi2 is missing the dmas and dma-names properties?
>
> During probe, the atmel-quadspi driver unconditionally checks for dma since
> aq->caps->has_dma is true for LAN969x compatible strings, and then calls
> atmel_qspi_dma_init().
>
> Without the dmas property present here, devm_dma_request_chan() returns
> -ENODEV. The driver handles this with dev_err_probe(), which emits a
> kernel error log ('error -ENODEV: RX DMA channel is not available') because
> it is not -EPROBE_DEFER.
>
> The controller will then fall back to pio mode, degrading performance.
>
> If the hardware supports dma for qspi2, should these properties be added?
> If it does not, should the driver be updated to avoid unconditionally setting
> has_dma to true for all instances?

The datasheet is quite conflicting for QSPI2, as it sometimes
references QSPI0 and QSPI2,
but then sometimes references QPSI0 and QSPI1.

DMA lists channels 17 and 18 for QSPI2, but it also lists 15 and 16
for QSPI1, which does not exist.
So, I will drop QSPI2 node in v3 until its clear which DMA channels are correct.

Regards,
Robert
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709112006.390742-1-robert.marko@sartura.hr?part=5



-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr

Re: [PATCH v2 1/5] dt-bindings: spi: Document LAN969x QSPI

[Mark Brown]

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

On Thu, Jul 09, 2026 at 01:19:07PM +0200, Robert Marko wrote:
> LAN969x has 2 QSPI instances, so document their compatible.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

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

Re: [PATCH v2 1/5] dt-bindings: spi: Document LAN969x QSPI

[Conor Dooley]

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

On Thu, Jul 09, 2026 at 01:19:07PM +0200, Robert Marko wrote:
> LAN969x has 2 QSPI instances, so document their compatible.

Please include a note about why a fallback cannot be used.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: changes-requested

> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> ---
>  Documentation/devicetree/bindings/spi/atmel,quadspi.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> index 30ab42c95c08..36c698ced99b 100644
> --- a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> @@ -22,6 +22,7 @@ properties:
>        - microchip,sama7d65-ospi
>        - microchip,sama7g5-qspi
>        - microchip,sama7g5-ospi
> +      - microchip,lan9691-qspi
>  
>    reg:
>      items:
> -- 
> 2.55.0
> 

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