* [PATCH v3 0/4] spi: add support for Amlogic A9
@ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay
0 siblings, 0 replies; 21+ messages in thread
From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw)
To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao,
Krzysztof Kozlowski
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>
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH v3 0/4] spi: add support for Amlogic A9 @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao, Krzysztof Kozlowski 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> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/4] spi: add support for Amlogic A9 @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao, Krzysztof Kozlowski 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> _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/4] spi: dt-bindings: amlogic: spisg: Document A9-specific properties 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay (?) @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay -1 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao, Krzysztof Kozlowski 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 1/4] spi: dt-bindings: amlogic: spisg: Document A9-specific properties @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao, Krzysztof Kozlowski 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 1/4] spi: dt-bindings: amlogic: spisg: Document A9-specific properties @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao, Krzysztof Kozlowski 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay (?) @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay -1 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay @ 2026-07-31 9:22 ` sashiko-bot -1 siblings, 0 replies; 21+ messages in thread From: sashiko-bot @ 2026-07-31 9:22 UTC (permalink / raw) To: Xianwei Zhao; +Cc: neil.armstrong, devicetree, linux-amlogic, robh, conor+dt 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling @ 2026-07-31 9:22 ` sashiko-bot 0 siblings, 0 replies; 21+ messages in thread From: sashiko-bot @ 2026-07-31 9:22 UTC (permalink / raw) To: Xianwei Zhao; +Cc: neil.armstrong, devicetree, linux-amlogic, robh, conor+dt 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay (?) @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay -1 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay @ 2026-07-31 9:20 ` sashiko-bot -1 siblings, 0 replies; 21+ messages in thread From: sashiko-bot @ 2026-07-31 9:20 UTC (permalink / raw) To: Xianwei Zhao; +Cc: conor+dt, linux-amlogic, neil.armstrong, devicetree, robh 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor @ 2026-07-31 9:20 ` sashiko-bot 0 siblings, 0 replies; 21+ messages in thread From: sashiko-bot @ 2026-07-31 9:20 UTC (permalink / raw) To: Xianwei Zhao; +Cc: conor+dt, linux-amlogic, neil.armstrong, devicetree, robh 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay (?) @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay -1 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features @ 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 0 siblings, 0 replies; 21+ messages in thread From: Xianwei Zhao via B4 Relay @ 2026-07-31 9:08 UTC (permalink / raw) To: Sunny Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-amlogic, linux-spi, devicetree, linux-kernel, Xianwei Zhao 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay @ 2026-07-31 9:19 ` sashiko-bot -1 siblings, 0 replies; 21+ messages in thread From: sashiko-bot @ 2026-07-31 9:19 UTC (permalink / raw) To: Xianwei Zhao; +Cc: linux-amlogic, conor+dt, robh, devicetree, neil.armstrong 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features @ 2026-07-31 9:19 ` sashiko-bot 0 siblings, 0 replies; 21+ messages in thread From: sashiko-bot @ 2026-07-31 9:19 UTC (permalink / raw) To: Xianwei Zhao; +Cc: linux-amlogic, conor+dt, robh, devicetree, neil.armstrong 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2026-07-31 9:22 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-31 9:08 [PATCH v3 0/4] spi: add support for Amlogic A9 Xianwei Zhao via B4 Relay 2026-07-31 9:08 ` Xianwei Zhao 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 2026-07-31 9:08 ` [PATCH v3 1/4] spi: dt-bindings: amlogic: spisg: Document A9-specific properties Xianwei Zhao via B4 Relay 2026-07-31 9:08 ` Xianwei Zhao 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 2026-07-31 9:08 ` [PATCH v3 2/4] spi: amlogic: spisg: Fix transfer size limit and timeout handling Xianwei Zhao via B4 Relay 2026-07-31 9:08 ` Xianwei Zhao 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 2026-07-31 9:22 ` sashiko-bot 2026-07-31 9:22 ` sashiko-bot 2026-07-31 9:08 ` [PATCH v3 3/4] spi: amlogic: spisg: Fix the incorrect keep_ss of the last descriptor Xianwei Zhao via B4 Relay 2026-07-31 9:08 ` Xianwei Zhao 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 2026-07-31 9:20 ` sashiko-bot 2026-07-31 9:20 ` sashiko-bot 2026-07-31 9:08 ` [PATCH v3 4/4] spi: amlogic: spisg: Add support for A9 controller features Xianwei Zhao via B4 Relay 2026-07-31 9:08 ` Xianwei Zhao 2026-07-31 9:08 ` Xianwei Zhao via B4 Relay 2026-07-31 9:19 ` sashiko-bot 2026-07-31 9:19 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.