Re: [PATCH] i2c: qcom-geni: Fix hardcoded clock index in SE_GENI_CLK_SEL

[Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>]

On 8/28/2026 3:59 PM, Viken Dadhaniya wrote:

> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 658636c1ee0e..a83297b5fb0a 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -82,6 +82,9 @@ enum geni_i2c_err_code {
>   #define XFER_TIMEOUT		HZ
>   #define RST_TIMEOUT		HZ
>   
> +#define GENI_SE_CLK_32MHZ	(32 * HZ_PER_MHZ)
> +#define GENI_SE_CLK_19P2MHZ	19200000UL
> +
>   struct geni_i2c_desc {
>   	bool no_dma_support;
>   	unsigned int tx_fifo_depth;
> @@ -127,6 +130,7 @@ struct geni_i2c_dev {
>   	spinlock_t lock;
>   	u32 clk_freq_out;
>   	const struct geni_i2c_clk_fld *clk_fld;
> +	u32 clk_idx;
>   	void *dma_buf;
>   	size_t xfer_len;
>   	dma_addr_t dma_addr;
> @@ -197,19 +201,42 @@ static const struct geni_i2c_clk_fld geni_i2c_clk_map_32mhz[] = {
>   static int geni_i2c_clk_map_idx(struct geni_i2c_dev *gi2c)
>   {
>   	const struct geni_i2c_clk_fld *itr;
> +	unsigned long res_freq;
>   
> -	if (clk_get_rate(gi2c->se.clk) == 32 * HZ_PER_MHZ)
> +	/*
> +	 * Counter tables are calibrated for a specific source clock frequency
Minor: Frequency counter tables> +	 * and are not valid for any multiple 
of it (e.g. 64 MHz, 128 MHz).
> +	 * Use exact=true and verify res_freq matches req_freq literally to
> +	 * reject harmonics: a 64 MHz clock that divides evenly to 32 MHz
> +	 * would pass exact matching but produce double the intended I2C
> +	 * frequency with these counter values.
> +	 */
> +	if (!geni_se_clk_freq_match(&gi2c->se, GENI_SE_CLK_32MHZ,
> +				    &gi2c->clk_idx, &res_freq, true) &&
> +	    res_freq == GENI_SE_CLK_32MHZ) {
>   		itr = geni_i2c_clk_map_32mhz;
> -	else
> +	} else if (!geni_se_clk_freq_match(&gi2c->se, GENI_SE_CLK_19P2MHZ,
> +					   &gi2c->clk_idx, &res_freq, true) &&
> +		   res_freq == GENI_SE_CLK_19P2MHZ) {
>   		itr = geni_i2c_clk_map_19p2mhz;
> +	} else {
> +		dev_err(gi2c->se.dev,
> +			"Unsupported SE source clock: must be exactly 32 MHz or 19.2 MHz\n");
> +		return -EINVAL;
> +	}
>   
>   	while (itr->clk_freq_out != 0) {
>   		if (itr->clk_freq_out == gi2c->clk_freq_out) {
>   			gi2c->clk_fld = itr;
> +			dev_dbg(gi2c->se.dev,
> +				"I2C clk selected: freq: %u Hz, clk_idx: %u\n",
> +				gi2c->clk_freq_out, gi2c->clk_idx);
>   			return 0;
>   		}
>   		itr++;
>   	}
> +
> +	dev_err(gi2c->se.dev, "Unsupported I2C output frequency %u Hz\n", gi2c->clk_freq_out);

add a line space before return>   	return -EINVAL;
>   }
>   

With above taken cared, rest all looks good.