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

[sashiko-bot@kernel.org]

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