Re: [PATCH 2/3] i2c: qcom-cci: Add missing cci_clk_rate for msm8953

[Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>]

Hi Loic, Luca,

On 7/21/26 17:58, Loic Poulain wrote:
> The msm8953 CCI data was added after cci_clk_rate was removed from the
> driver, so it never got a clock rate entry. The DT assigns 19.2 MHz to
> GCC_CAMSS_CCI_CLK and the hw_params values match those of v1/v1.5 which
> were also calibrated for 19.2 MHz.
> 
> Fixes: d202341d9b0c ("i2c: qcom-cci: Add msm8953 compatible")
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   drivers/i2c/busses/i2c-qcom-cci.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index 19e4719f13b29b2cf60e113565b1b63f19d0e669..5f74edde8558382e63e2c3d0fbe19489f325e8ce 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -783,6 +783,7 @@ static const struct cci_data cci_msm8953_data = {
>   		.max_write_len = 11,
>   		.max_read_len = 12,
>   	},
> +	.cci_clk_rate = 19200000,
>   	.params[I2C_MODE_STANDARD] = {
>   		.thigh = 78,
>   		.tlow = 114,
> 

this simple change is wrong, but commit d202341d9b0c ("i2c: qcom-cci: Add
msm8953 compatible") causes it, but please note that the commit d202341d9b0c
has no explicit issues per se (if proper frequencies are set in dt), but
this particular commit 2/3 breaks the whole picture.

Let's see, MSM8953 I2C_MODE_STANDARD/I2C_MODE_FAST settings repeat the ones
for v1/v1.5 and 19.2MHz supply clock frequency, but I2C_MODE_FAST_PLUS
speed setting very close to v2 parameters and assumes 37.MHz supply clock
frequency.

So, at least one clock rate setting for all modes is invalid in this case.
Automatically it means the reverted commit 1/3 in the series does not
directly lead to the wanted and well-managed data, and the logic in 3/3
becomes invalid.

I would suggest to grasp the statement above carefully, design a bit
better solution, meanwhile postpone applying any commits from the
series, especially if a breaking change holds a Fixes tag.

-- 
Best wishes,
Vladimir