Re: [PATCH v5 03/17] spi: parse spi-max-post-config-frequency-hz into post_config_max_speed_hz

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The newly added `post_config_max_speed_hz` property is not copied when cloning `spi_device` structures for ancillary or muxed devices.
- [Medium] `spi->post_config_max_speed_hz` is not clamped to the SPI controller's maximum supported frequency.
--

--- Patch [3]: [PATCH v5 03/17] spi: parse spi-max-post-config-frequency-hz into post_config_max_speed_hz ---
commit 96b305610a39094bde249080c339e160beaa3e60
Author: Santhosh Kumar K <s-k6@ti.com>

spi: parse spi-max-post-config-frequency-hz into post_config_max_speed_hz

Add post_config_max_speed_hz to struct spi_device and parse it from
the spi-max-post-config-frequency-hz DT property in of_spi_parse_dt().

> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index d9e6b4b87c899..7e7393d25810d 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2602,6 +2602,8 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>  	/* Device speed */
>  	if (!of_property_read_u32(nc, "spi-max-frequency", &value))
>  		spi->max_speed_hz = value;
> +	if (!of_property_read_u32(nc, "spi-max-post-config-frequency-hz", &value))
> +		spi->post_config_max_speed_hz = value;

[Severity: Medium]
Does this code need to clamp post_config_max_speed_hz to the controller's
maximum supported frequency?

During device setup in __spi_setup(), max_speed_hz is safely clamped:

drivers/spi/spi.c:__spi_setup() {
    ...
    if (spi->controller->max_speed_hz &&
        (!spi->max_speed_hz ||
         spi->max_speed_hz > spi->controller->max_speed_hz))
        spi->max_speed_hz = spi->controller->max_speed_hz;
    ...
}

Will controller drivers directly reading spi->post_config_max_speed_hz see an
unvalidated value if the DT property exceeds the controller's max speed?

[ ... ]

> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 4c285d3ede1d8..aa7b1b34a594f 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h

[ ... ]

> @@ -193,6 +195,7 @@ struct spi_device {
>  	struct device		dev;
>  	struct spi_controller	*controller;
>  	u32			max_speed_hz;
> +	u32			post_config_max_speed_hz;

[Severity: Medium]
Should this new property be copied when cloning spi_device structures for
ancillary or muxed devices?

In spi_new_ancillary_device(), the parent device's speed and mode constraints
are copied to the ancillary device:

drivers/spi/spi.c:spi_new_ancillary_device() {
    ...
    ancillary->max_speed_hz = spi->max_speed_hz;
    ancillary->mode = spi->mode;
    ...
}

A similar copy happens in spi_mux_select():

drivers/spi/spi-mux.c:spi_mux_select() {
    ...
    priv->spi->max_speed_hz = spi->max_speed_hz;
    ...
}

Will ancillary or multiplexed SPI devices fail to inherit the
post_config_max_speed_hz property, resulting in a value of 0?

>  	u8			bits_per_word;
>  	bool			rt;
>  #define SPI_NO_TX		BIT(31)		/* No transmit wire */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717185116.2065505-1-s-k6@ti.com?part=3