Re: [PATCH v2 2/4] spi: sg2044-nor: Honor SPI clock limits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] spi: sg2044-nor: severe lock contention due to clk_get_rate() in fast path
--

commit d5eb2cd92142b8e5a4297344fc25807495e0c3fc
Author: Han Gao <gaohan@iscas.ac.cn>
spi: sg2044-nor: Honor SPI clock limits

This commit dynamically calculates the clock divider before each transfer to
ensure the SCK does not exceed device or operation limits. It also advertises
the controller maximum speed to the SPI core to supply a default when omitted.

> diff --git a/drivers/spi/spi-sg2044-nor.c b/drivers/spi/spi-sg2044-nor.c
> index ff026ca0f7663..bc9a8ee9bce44 100644
> --- a/drivers/spi/spi-sg2044-nor.c
> +++ b/drivers/spi/spi-sg2044-nor.c

[ ... ]

> @@ -385,6 +385,29 @@ static ssize_t sg2044_spifmc_trans_reg(struct sg2044_spifmc *spifmc,
>  	return 0;
>  }
>  
> +static int sg2044_spifmc_set_speed(struct sg2044_spifmc *spifmc,
> +				   unsigned int speed_hz)
> +{
> +	unsigned long rate = clk_get_rate(spifmc->clk);

[Severity: High]
Does this introduce severe lock contention by acquiring the clock
framework's global mutex on every SPI transfer?

During every SPI memory operation, the fast path executes:
sg2044_spifmc_exec_op() -> sg2044_spifmc_set_speed() -> clk_get_rate()

Because clk_get_rate() takes a global mutex, doing this redundantly on
every transfer can drastically impact the performance of the system and
delay other subsystems interacting with clocks.

Could the clock rate be cached in the device struct during probe() instead
of being queried per-operation?

> +	u64 div;
> +	u32 reg;
> +
> +	if (!rate)
> +		return -EINVAL;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001-fix-sg204xspi-v2-0-014dbaf22b5e@iscas.ac.cn?part=2