[PATCH v3 0/4] spi: add support for Amlogic A9

[PATCH v3 0/4] spi: add support for Amlogic A9

[Xianwei Zhao via B4 Relay]

Add bindings for A9 with some features, and driver for A9 base on A4.

Fix the incorrect keep_ss of the last descriptor.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Changes in v3:
- Re-wrap binding commit msg and drop "amlogic,word-gap" property.
- Handle the hardware word gap through the existing word_delay mechanism in the transfer path instead.
- Link to v2: https://lore.kernel.org/r/20260723-a9-spisg-v2-0-9fc39106b7e2@amlogic.com

Changes in v2:
- Move the added properties defined to top-level and then reestrict them  in bindings.
- Fix some pre-exiting bug reported by Sashiko AI review that as a separate submission.
- Limit the value that will be writed to CFG_CS_HOLD.
- Link to v1: https://lore.kernel.org/r/20260717-a9-spisg-v1-0-2eeea77be19f@amlogic.com

---
Sunny Luo (1):
      spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor

Xianwei Zhao (3):
      spi: dt-bindings: amlogic: spisg: Document A9-specific properties
      spi: amlogic: spisg: Fix transfer size limit and timeout handling
      spi: amlogic: spisg: Add support for A9 controller features

 .../devicetree/bindings/spi/amlogic,a4-spisg.yaml  | 30 ++++++-
 drivers/spi/spi-amlogic-spisg.c                    | 99 +++++++++++++++++++---
 2 files changed, 111 insertions(+), 18 deletions(-)
---
base-commit: d0b5dd16226814e5c73ba8daf88d3d91c990a38f
change-id: 20260715-a9-spisg-f19ce0403a50

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>

[PATCH v3 1/4] spi: dt-bindings: amlogic: spisg: Document A9-specific properties

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Document the Amlogic A9 SPISG controller and its additional Device Tree
properties.

Add the "amlogic,a9-spisg" compatible string and define the A9-specific
"amlogic,mo-idle-output" property, which configure the MOSI idle output
level.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 .../devicetree/bindings/spi/amlogic,a4-spisg.yaml  | 30 +++++++++++++++++++---
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/amlogic,a4-spisg.yaml b/Documentation/devicetree/bindings/spi/amlogic,a4-spisg.yaml
index 9bfb8089f7ea..427d8883009c 100644
--- a/Documentation/devicetree/bindings/spi/amlogic,a4-spisg.yaml
+++ b/Documentation/devicetree/bindings/spi/amlogic,a4-spisg.yaml
@@ -11,12 +11,11 @@ maintainers:
   - Xianwei Zhao <xianwei.zhao@amlogic.com>
   - Sunny Luo <sunny.luo@amlogic.com>
 
-allOf:
-  - $ref: spi-controller.yaml#
-
 properties:
   compatible:
-    const: amlogic,a4-spisg
+    enum:
+      - amlogic,a4-spisg
+      - amlogic,a9-spisg
 
   reg:
     maxItems: 1
@@ -35,6 +34,16 @@ properties:
   resets:
     maxItems: 1
 
+  amlogic,mo-idle-output:
+    description: |
+      Controls the MOSI output level when the controller is idle.
+      0 - Drive MOSI low
+      1 - Drive MOSI high
+      2 - Follow the last transmitted bit
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2]
+    default: 0
+
 required:
   - compatible
   - reg
@@ -42,6 +51,19 @@ required:
   - clocks
   - clock-names
 
+allOf:
+  - $ref: spi-controller.yaml#
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - amlogic,a9-spisg
+    then:
+      properties:
+        amlogic,mo-idle-output: false
+
 unevaluatedProperties: false
 
 examples:

-- 
2.52.0

Re: [PATCH v3 1/4] spi: dt-bindings: amlogic: spisg: Document A9-specific properties

[Mark Brown]

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

On Fri, Jul 31, 2026 at 09:08:25AM +0000, Xianwei Zhao via B4 Relay wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> Document the Amlogic A9 SPISG controller and its additional Device Tree
> properties.
> 
> Add the "amlogic,a9-spisg" compatible string and define the A9-specific
> "amlogic,mo-idle-output" property, which configure the MOSI idle output
> level.

> +  amlogic,mo-idle-output:
> +    description: |
> +      Controls the MOSI output level when the controller is idle.
> +      0 - Drive MOSI low
> +      1 - Drive MOSI high
> +      2 - Follow the last transmitted bit
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2]
> +    default: 0

This looks like the standard SPI_MOSI_IDLE_{LOW,HIGH} we have in the
kernel, plus one new option.  Usually the client driver would control
this.  Why do something different here?

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

[PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

The CFG_BLOCK_NUM field can encode at most 20 bits, so reduce
SPISG_BLOCK_MAX to 0xfffff to avoid programming a zero-length transfer.

Perform the delay calculation in 64-bit arithmetic to avoid overflow
when converting nanoseconds to SPI clock cycles.

Stop the controller on transfer timeout by clearing the descriptor list
register before returning an error.

Fixes: cef9991e04ae ("spi: Add Amlogic SPISG driver")
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/spi/spi-amlogic-spisg.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
index afc8af04638d..a6123975a37c 100644
--- a/drivers/spi/spi-amlogic-spisg.c
+++ b/drivers/spi/spi-amlogic-spisg.c
@@ -95,7 +95,7 @@
 
 #define SPISG_MAX_REG			0x40
 
-#define SPISG_BLOCK_MAX			0x100000
+#define SPISG_BLOCK_MAX			0xFFFFF
 
 #define SPISG_OP_MODE_WRITE_CMD		0
 #define SPISG_OP_MODE_READ_STS		1
@@ -175,7 +175,7 @@ static int spi_delay_to_sclk(u32 slck_speed_hz, struct spi_delay *delay)
 	if (ns < 0)
 		return 0;
 
-	return DIV_ROUND_UP_ULL(slck_speed_hz * ns, NSEC_PER_SEC);
+	return DIV_ROUND_UP_ULL((u64)slck_speed_hz * ns, NSEC_PER_SEC);
 }
 
 static inline u32 aml_spisg_sem_down_read(struct spisg_device *spisg)
@@ -565,10 +565,13 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 	aml_spisg_pending(spisg, descs_paddr, false, true);
 	if (wait_for_completion_timeout(&spisg->completion,
 					spi_controller_is_target(spisg->controller) ?
-					MAX_SCHEDULE_TIMEOUT : msecs_to_jiffies(ms)))
+					MAX_SCHEDULE_TIMEOUT : msecs_to_jiffies(ms))) {
 		ret = spisg->status ? -EIO : 0;
-	else
+	} else {
+		/* stop transfer */
+		regmap_write(spisg->map, SPISG_REG_DESC_LIST_H, 0);
 		ret = -ETIMEDOUT;
+	}
 
 	dma_unmap_single(dev, descs_paddr, descs_len, DMA_TO_DEVICE);
 end:

-- 
2.52.0

Re: [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling

[sashiko-bot]

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

New issues:
- [High] The newly added timeout abort path stops the controller but fails to synchronize or disable pending interrupts, causing a race condition where a late interrupt can falsely complete a subsequent transfer.

Pre-existing issues:
- [High] Transfers exceeding SPISG_BLOCK_MAX blocks are silently truncated by the driver because it caps the block count but fails to implement the max_transfer_size controller callback.
- [High] The driver incorrectly mixes `devm_clk_get_enabled()` with manual `clk_disable_unprepare()` calls, leading to unbalanced clock reference counting.
--

--- Patch [2]: [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling ---
commit f3597777f9b3ab326fe08698fcd9389c455e53d4
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>

spi: amlogic: spisg: Fix transfer size limit and timeout handling

The commit reduces SPISG_BLOCK_MAX to avoid zero-length transfers and fixes
a 64-bit delay calculation. It also clears the descriptor list register on a
transfer timeout to stop the controller.

> diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
> --- a/drivers/spi/spi-amlogic-spisg.c
> +++ b/drivers/spi/spi-amlogic-spisg.c
> @@ -95,7 +95,7 @@
>  
>  #define SPISG_MAX_REG			0x40
>  
> -#define SPISG_BLOCK_MAX			0x100000
> +#define SPISG_BLOCK_MAX			0xFFFFF

[Severity: High]
This isn't a bug introduced by this patch, but does the driver silently
truncate large transfers?

In aml_spisg_probe(), the driver sets ctlr->max_dma_len = SPISG_BLOCK_MAX,
but it does not implement the max_transfer_size controller callback.

When a transfer is larger than SPISG_BLOCK_MAX, aml_spisg_setup_transfer()
silently caps it:

	blocks = min_t(int, blocks, SPISG_BLOCK_MAX);

Since the SPI core isn't aware of the maximum transfer size limit, it could
submit oversized transfers that get silently truncated. This could leave DMA
buffers partially uninitialized on RX. Should we implement the
ctlr->max_transfer_size callback to prevent this?

[ ... ]

> @@ -565,10 +565,13 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
>  	aml_spisg_pending(spisg, descs_paddr, false, true);
>  	if (wait_for_completion_timeout(&spisg->completion,
>  					spi_controller_is_target(spisg->controller) ?
> -					MAX_SCHEDULE_TIMEOUT : msecs_to_jiffies(ms)))
> +					MAX_SCHEDULE_TIMEOUT : msecs_to_jiffies(ms))) {
>  		ret = spisg->status ? -EIO : 0;
> -	else
> +	} else {
> +		/* stop transfer */
> +		regmap_write(spisg->map, SPISG_REG_DESC_LIST_H, 0);
>  		ret = -ETIMEDOUT;
> +	}

[Severity: High]
Could this timeout path race with a late interrupt?

By stopping the controller without synchronizing or masking the IRQ, a
delayed interrupt might still arrive. If it does, aml_spisg_irq()
unconditionally completes it:

	complete(&spisg->completion);

If the SPI core immediately enqueues the next message, could this late
interrupt prematurely complete the new transfer and prematurely unmap its
active DMA buffers?

[Severity: High]
This is a pre-existing issue, but does the driver have an unbalanced clock
reference count issue?

The driver acquires clocks using devm_clk_get_enabled(), but manually
disables them during error handling and removal. For example, in the error
path of aml_spisg_probe():

out_clk:
	if (spisg->core)
		clk_disable_unprepare(spisg->core);
	clk_disable_unprepare(spisg->pclk);

Since these are devm-managed clocks, the cleanup framework will also
disable them, which results in a double-disable. Should these manual
clk_disable_unprepare() calls be removed, or should the driver use the
non-devm clock getters?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-a9-spisg-v3-0-a15da3f70029@amlogic.com?part=2

[PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor

[Xianwei Zhao via B4 Relay]

From: Sunny Luo <sunny.luo@amlogic.com>

The driver currently unconditionally clears CFG_KEEP_SS on the last
descriptor, causing the last transfer's cs_change setting to be ignored.

Record the cs_change value of the last SPI transfer and use it to
program CFG_KEEP_SS on the final descriptor. When a null descriptor is
inserted to implement the cs-hold delay, keep CFG_KEEP_SS set on the
preceding transfer descriptor and apply the recorded value to the final
descriptor instead.

This ensures the controller handles chip select correctly for the last
transfer regardless of whether a cs-hold delay is required.

Fixes: cef9991e04ae ("spi: Add Amlogic SPISG driver")
Signed-off-by: Sunny Luo <sunny.luo@amlogic.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/spi/spi-amlogic-spisg.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
index a6123975a37c..2d66f00dccd4 100644
--- a/drivers/spi/spi-amlogic-spisg.c
+++ b/drivers/spi/spi-amlogic-spisg.c
@@ -489,6 +489,7 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 	struct spisg_descriptor_extra *exdescs, *exdesc;
 	dma_addr_t descs_paddr;
 	int desc_num = 1, descs_len;
+	bool last_xfer_keep_ss = false;
 	u32 cs_hold_in_sclk = 0;
 	int ret = -EIO;
 
@@ -529,9 +530,11 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 				spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup));
 
 		/* calculate cs-hold delay with the last xfer speed */
-		if (list_is_last(&xfer->transfer_list, &msg->transfers))
+		if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
 			cs_hold_in_sclk =
 				spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_hold);
+			last_xfer_keep_ss = xfer->cs_change;
+		}
 
 		desc++;
 		exdesc++;
@@ -539,13 +542,17 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 				       xfer->effective_speed_hz);
 	}
 
-	if (cs_hold_in_sclk)
+	if (cs_hold_in_sclk) {
 		/* additional null-descriptor to achieve the cs-hold delay */
 		aml_spisg_setup_null_desc(spisg, desc, cs_hold_in_sclk);
-	else
 		desc--;
+		desc->cfg_bus |= FIELD_PREP(CFG_KEEP_SS, 1);
+		desc++;
+	} else {
+		desc--;
+	}
 
-	desc->cfg_bus |= FIELD_PREP(CFG_KEEP_SS, 0);
+	FIELD_MODIFY(CFG_KEEP_SS, &desc->cfg_bus, last_xfer_keep_ss);
 	desc->cfg_start |= FIELD_PREP(CFG_EOC, 1);
 
 	/* some tolerances */

-- 
2.52.0

Re: [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor

[sashiko-bot]

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

Pre-existing issues:
- [High] The DMA descriptors are defined using native integer types and lack endianness conversions.
--

--- Patch [3]: [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor ---
commit b9d6f07e79dcd1e9c5b73de8be6904945ef9d97b
Author: Sunny Luo <sunny.luo@amlogic.com>

spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor

This commit fixes an issue where CFG_KEEP_SS was unconditionally
cleared on the last descriptor, causing the cs_change setting to be
ignored for the final transfer. It records the cs_change value and
applies it correctly, even when a null descriptor is inserted for
cs-hold delay.

> diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
> index a6123975a37cf..2d66f00dccd45 100644
> --- a/drivers/spi/spi-amlogic-spisg.c
> +++ b/drivers/spi/spi-amlogic-spisg.c
[ ... ]
> @@ -539,13 +542,17 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
>  				       xfer->effective_speed_hz);
>  	}
>  
> -	if (cs_hold_in_sclk)
> +	if (cs_hold_in_sclk) {
>  		/* additional null-descriptor to achieve the cs-hold delay */
>  		aml_spisg_setup_null_desc(spisg, desc, cs_hold_in_sclk);
> -	else
>  		desc--;
> +		desc->cfg_bus |= FIELD_PREP(CFG_KEEP_SS, 1);
> +		desc++;
> +	} else {
> +		desc--;
> +	}
>  
> -	desc->cfg_bus |= FIELD_PREP(CFG_KEEP_SS, 0);
> +	FIELD_MODIFY(CFG_KEEP_SS, &desc->cfg_bus, last_xfer_keep_ss);

[Severity: High]
This is a pre-existing issue, but since this code modifies a hardware-facing
DMA descriptor in aml_spisg_transfer_one_message(), will these native integer
assignments cause failures on big-endian architectures?

Looking at drivers/spi/spi-amlogic-spisg.c, struct spisg_descriptor defines
these fields using native integer types:

struct spisg_descriptor {
	u32				cfg_start;
	u32				cfg_bus;
	u64				tx_paddr;
	u64				rx_paddr;
};

And native values are directly assigned to them without endianness conversion,
such as desc->tx_paddr = paddr in aml_spisg_setup_transfer(), or via
FIELD_PREP() and FIELD_MODIFY() here.

On big-endian CPU configurations, won't the hardware receive byte-swapped
values for physical addresses and configuration fields, potentially leading
to DMA faults or memory corruption? Should these hardware descriptors be
defined with explicit endian types (like __le32 and __le64) and updated
using conversions like cpu_to_le32()?

>  	desc->cfg_start |= FIELD_PREP(CFG_EOC, 1);
>  
>  	/* some tolerances */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-a9-spisg-v3-0-a15da3f70029@amlogic.com?part=3

[PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

The Amlogic A9 SPISG controller extends the A4 controller with additional
configuration options, including:

- Extended CS setup timing
- Hardware-controlled CS hold timing
- MOSI idle output configuration
- Configurable word delay

Add SoC-specific capability data and configure these features when they
are supported by the underlying hardware while keeping compatibility with
existing A4 controllers.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/spi/spi-amlogic-spisg.c | 75 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
index 2d66f00dccd4..729da11bc539 100644
--- a/drivers/spi/spi-amlogic-spisg.c
+++ b/drivers/spi/spi-amlogic-spisg.c
@@ -37,6 +37,12 @@
 #define CFG_HW_POS			BIT(6)
 /* start on vsync falling */
 #define CFG_HW_NEG			BIT(7)
+#define CFG_WORD_GAP			GENMASK(9, 8)
+#define CFG_MO_IDLE_OUTPUT		GENMASK(11, 10)
+/* cs hold time in pclk */
+#define CFG_CS_HOLD			GENMASK(26, 12)
+/* high 4 bits of cs setup time in sclk */
+#define CFG_CS_SETUP_EXTEND		GENMASK(30, 27)
 
 #define SPISG_REG_CFG_START		0x08
 #define CFG_BLOCK_NUM			GENMASK(19, 0)
@@ -143,6 +149,13 @@ struct spisg_descriptor_extra {
 	int				rx_ccsg_len;
 };
 
+struct aml_spisg_data {
+	bool				mo_idle_output_ctrl;
+	bool				word_gap_ctrl;
+	bool				cs_hold_ctrl;
+	bool				cs_setup_extend_ctrl;
+};
+
 struct spisg_device {
 	struct spi_controller		*controller;
 	struct platform_device		*pdev;
@@ -152,6 +165,7 @@ struct spisg_device {
 	struct clk			*sclk;
 	struct clk_div_table		*tbl;
 	struct completion		completion;
+	const struct aml_spisg_data	*data;
 	u32				status;
 	u32				speed_hz;
 	u32				effective_speed_hz;
@@ -424,7 +438,8 @@ static void aml_spisg_setup_null_desc(struct spisg_device *spisg,
 static void aml_spisg_pending(struct spisg_device *spisg,
 			      dma_addr_t desc_paddr,
 			      bool trig,
-			      bool irq_en)
+			      bool irq_en,
+			      u32 delay)
 {
 	u32 desc_l, desc_h, cfg_spi, irq_enable;
 
@@ -437,6 +452,13 @@ static void aml_spisg_pending(struct spisg_device *spisg,
 #endif
 
 	cfg_spi = spisg->cfg_spi;
+
+	if (spisg->data && spisg->data->word_gap_ctrl) {
+		if (delay > 3)
+			delay = 3;
+		cfg_spi |=  FIELD_PREP(CFG_WORD_GAP, delay);
+	}
+
 	if (trig)
 		cfg_spi |= CFG_HW_POS;
 	else
@@ -483,6 +505,7 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 {
 	struct spisg_device *spisg = spi_controller_get_devdata(ctlr);
 	struct device *dev = &spisg->pdev->dev;
+	const struct aml_spisg_data *data = spisg->data;
 	unsigned long long ms = 0;
 	struct spi_transfer *xfer;
 	struct spisg_descriptor *descs, *desc;
@@ -491,6 +514,7 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 	int desc_num = 1, descs_len;
 	bool last_xfer_keep_ss = false;
 	u32 cs_hold_in_sclk = 0;
+	u32 val, delay = 0;
 	int ret = -EIO;
 
 	if (!aml_spisg_sem_down_read(spisg)) {
@@ -524,10 +548,20 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 			goto end;
 		}
 
-		/* calculate cs-setup delay with the first xfer speed */
-		if (list_is_first(&xfer->transfer_list, &msg->transfers))
-			desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP,
-				spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup));
+		/* calculate cs-setup delay with the first xfer speed  and word dealy*/
+		if (list_is_first(&xfer->transfer_list, &msg->transfers)) {
+			val = spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup);
+			if (data && data->cs_setup_extend_ctrl) {
+				val = min_t(u32, 0xFF, val);
+				desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP, val & 0xF);
+				FIELD_MODIFY(CFG_CS_SETUP_EXTEND, &spisg->cfg_spi, val >> 4);
+			} else {
+				val = min_t(u32, 0xF, val);
+				desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP, val);
+			}
+
+			delay = spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->word_delay);
+		}
 
 		/* calculate cs-hold delay with the last xfer speed */
 		if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
@@ -542,7 +576,13 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 				       xfer->effective_speed_hz);
 	}
 
-	if (cs_hold_in_sclk) {
+	if (data && data->cs_hold_ctrl) {
+		cs_hold_in_sclk = cs_hold_in_sclk ? : 1;
+		val = cs_hold_in_sclk * (FIELD_GET(CFG_CLK_DIV, spisg->cfg_bus) + 1);
+		val = min_t(u32, 0x7FFF, val);
+		FIELD_MODIFY(CFG_CS_HOLD, &spisg->cfg_spi, val);
+		desc--;
+	} else if (cs_hold_in_sclk) {
 		/* additional null-descriptor to achieve the cs-hold delay */
 		aml_spisg_setup_null_desc(spisg, desc, cs_hold_in_sclk);
 		desc--;
@@ -569,7 +609,7 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
 	}
 
 	reinit_completion(&spisg->completion);
-	aml_spisg_pending(spisg, descs_paddr, false, true);
+	aml_spisg_pending(spisg, descs_paddr, false, true, delay);
 	if (wait_for_completion_timeout(&spisg->completion,
 					spi_controller_is_target(spisg->controller) ?
 					MAX_SCHEDULE_TIMEOUT : msecs_to_jiffies(ms))) {
@@ -725,6 +765,7 @@ static int aml_spisg_probe(struct platform_device *pdev)
 	struct spisg_device *spisg;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
+	u32 val = 0;
 	int ret, irq;
 
 	const struct regmap_config aml_regmap_config = {
@@ -743,6 +784,7 @@ static int aml_spisg_probe(struct platform_device *pdev)
 
 	spisg = spi_controller_get_devdata(ctlr);
 	spisg->controller = ctlr;
+	spisg->data = (struct aml_spisg_data *)of_device_get_match_data(dev);
 
 	spisg->pdev = pdev;
 	platform_set_drvdata(pdev, spisg);
@@ -773,6 +815,14 @@ static int aml_spisg_probe(struct platform_device *pdev)
 
 	spisg->cfg_spi = FIELD_PREP(CFG_SFLASH_WP, 1) |
 			 FIELD_PREP(CFG_SFLASH_HD, 1);
+
+	if (spisg->data && spisg->data->mo_idle_output_ctrl) {
+		if (!of_property_read_u32(dev->of_node, "amlogic,mo-idle-output", &val))
+			spisg->cfg_spi |=  FIELD_PREP(CFG_MO_IDLE_OUTPUT, val);
+		else
+			spisg->cfg_spi |=  FIELD_PREP(CFG_MO_IDLE_OUTPUT, 0);
+	}
+
 	if (spi_controller_is_target(ctlr)) {
 		spisg->cfg_spi |= FIELD_PREP(CFG_SLAVE_EN, 1);
 		spisg->cfg_bus = FIELD_PREP(CFG_TX_TUNING, 0xf);
@@ -859,6 +909,13 @@ static int spisg_resume_runtime(struct device *dev)
 	return 0;
 }
 
+static const struct aml_spisg_data a9_spisg_data = {
+	.mo_idle_output_ctrl = true,
+	.word_gap_ctrl = true,
+	.cs_hold_ctrl = true,
+	.cs_setup_extend_ctrl = true,
+};
+
 static const struct dev_pm_ops amlogic_spisg_pm_ops = {
 	.runtime_suspend	= spisg_suspend_runtime,
 	.runtime_resume		= spisg_resume_runtime,
@@ -868,6 +925,10 @@ static const struct of_device_id amlogic_spisg_of_match[] = {
 	{
 		.compatible = "amlogic,a4-spisg",
 	},
+	{
+		.compatible = "amlogic,a9-spisg",
+		.data = &a9_spisg_data,
+	},
 
 	{ /* sentinel */ }
 };

-- 
2.52.0

Re: [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ignored transfer-specific word_delay causes hardware gap to be misconfigured by reading msg->spi->word_delay instead of xfer->word_delay.
--

--- Patch [4]: [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features ---
commit ecee00fa5a6dbd8a553a6c4d9ef9bf2089d1bdd9
Author: Xianwei Zhao <xianwei.zhao@amlogic.com>

spi: amlogic: spisg: Add support for A9 controller features

This commit extends the Amlogic A9 SPISG controller with additional
configuration options. It adds support for extended CS setup/hold
timing, MOSI idle output configuration, and configurable word delays.

> diff --git a/drivers/spi/spi-amlogic-spisg.c b/drivers/spi/spi-amlogic-spisg.c
> index 2d66f00dccd45..729da11bc539e 100644
> --- a/drivers/spi/spi-amlogic-spisg.c
> +++ b/drivers/spi/spi-amlogic-spisg.c

[ ... ]

> @@ -524,10 +548,20 @@ static int aml_spisg_transfer_one_message(struct spi_controller *ctlr,
>  			goto end;
>  		}
>  
> -		/* calculate cs-setup delay with the first xfer speed */
> -		if (list_is_first(&xfer->transfer_list, &msg->transfers))
> -			desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP,
> -				spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup));
> +		/* calculate cs-setup delay with the first xfer speed  and word dealy*/
> +		if (list_is_first(&xfer->transfer_list, &msg->transfers)) {
> +			val = spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->cs_setup);
> +			if (data && data->cs_setup_extend_ctrl) {
> +				val = min_t(u32, 0xFF, val);
> +				desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP, val & 0xF);
> +				FIELD_MODIFY(CFG_CS_SETUP_EXTEND, &spisg->cfg_spi, val >> 4);
> +			} else {
> +				val = min_t(u32, 0xF, val);
> +				desc->cfg_bus |= FIELD_PREP(CFG_CS_SETUP, val);
> +			}
> +
> +			delay = spi_delay_to_sclk(xfer->effective_speed_hz, &msg->spi->word_delay);

[Severity: High]
Should this be reading xfer->word_delay instead of msg->spi->word_delay?

When an SPI client driver submits a transfer with a specific word_delay set,
the SPI core evaluates and populates xfer->word_delay. By reading the
device-level default delay from msg->spi->word_delay here in
aml_spisg_transfer_one_message(), it appears any transfer-specific delay
overrides requested by the client will be ignored.

> +		}
>  
>  		/* calculate cs-hold delay with the last xfer speed */
>  		if (list_is_last(&xfer->transfer_list, &msg->transfers)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-a9-spisg-v3-0-a15da3f70029@amlogic.com?part=4