Re: [PATCH 2/2] i2c: qcom-geni: Add support for I2C High-Speed mode

[Mukesh Savaliya]

On 8/24/2026 4:55 PM, Jyothi Kumar Seerapu wrote:
[...]>   struct geni_i2c_err_log {
> @@ -162,9 +180,14 @@ static const struct geni_i2c_err_log gi2c_log[] = {
>   struct geni_i2c_clk_fld {
>   	u32	clk_freq_out;
>   	u8	clk_div;
> -	u8	t_high_cnt;
> -	u8	t_low_cnt;
> -	u8	t_cycle_cnt;
> +	/*
> +	 * In normal mode, these counter values fit within 8 bits.
> +	 * In High-Speed mode, HS_TLOW_COUNT and HS_TCYCLE_COUNT are
> +	 * 10-bit fields, so u16 is required.
> +	 */
Now, normal mode doesn't matter once switched to 16 bit.
May be comment also doesn't matter once change is merged, you can add 
some information in commit log though. Will be taken as default size.> + 
u16	t_high_cnt;
> +	u16	t_low_cnt;
> +	u16	t_cycle_cnt;
>   };
>   
>   /*
> @@ -194,10 +217,30 @@ static const struct geni_i2c_clk_fld geni_i2c_clk_map_32mhz[] = {
>   	{}
>   };
>   
> +/* source_clock = 100 MHz */
> +static const struct geni_i2c_clk_fld geni_i2c_clk_map_100mhz[] = {
> +	{ I2C_MAX_STANDARD_MODE_FREQ, 1, 449, 548, 998 },
> +	{ I2C_MAX_FAST_MODE_FREQ, 1, 76, 167, 248 },
> +	{ I2C_MAX_FAST_MODE_PLUS_FREQ, 1, 23, 59, 98 },
> +	{}
> +};
> +
>   static int geni_i2c_clk_map_idx(struct geni_i2c_dev *gi2c)
>   {
>   	const struct geni_i2c_clk_fld *itr;
>   
> +	/* Check if HS mode is requested */
> +	if (gi2c->clk_freq_out == I2C_HS_MODE_FREQ) {
> +		gi2c->is_hs_mode = true;
> +		/* For HS mode, source clock should be 100 MHz */
> +		itr = geni_i2c_clk_map_100mhz;
> +		/* For HS mode, start with 1MHz for master code */
> +		gi2c->clk_fld = &itr[2];
> +		return 0;
> +	}
> +
else part ?> +	gi2c->is_hs_mode = false;
> +
>   	if (clk_get_rate(gi2c->se.clk) == 32 * HZ_PER_MHZ)
>   		itr = geni_i2c_clk_map_32mhz;
>   	else
> @@ -219,7 +262,12 @@ static int qcom_geni_i2c_conf(struct geni_se *se, unsigned long freq)
>   	const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
>   	u32 val;
>   
> -	writel_relaxed(0, gi2c->se.base + SE_GENI_CLK_SEL);
> +	if (gi2c->is_hs_mode) {
> +		writel_relaxed(I2C_HS_TCYCLE_CNT, gi2c->se.base + SE_I2C_HS_TCYCLE_REG);
> +		writel_relaxed(I2C_HS_TLOW_CNT, gi2c->se.base + SE_I2C_HS_TLOW_REG);
> +	}
> +
> +	writel_relaxed(gi2c->dfs_index, gi2c->se.base + SE_GENI_CLK_SEL);
>   
>   	val = (itr->clk_div << CLK_DIV_SHFT) | SER_CLK_EN;
>   	writel_relaxed(val, gi2c->se.base + GENI_SER_M_CLK_CFG);
> @@ -500,7 +548,11 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
>   		geni_se_select_mode(se, GENI_SE_FIFO);
>   
>   	writel_relaxed(len, se->base + SE_I2C_RX_TRANS_LEN);
> -	geni_se_setup_m_cmd(se, I2C_READ, m_param);
> +
> +	if (gi2c->is_hs_mode) 
gi2c->is_hs_mode ? i2c->op = I2C_HS_READ : I2C_READ;
geni_se_setup_m_cmd(se, i2c->op, m_param);> +		geni_se_setup_m_cmd(se, 
I2C_HS_READ, m_param);
> +	else
> +		geni_se_setup_m_cmd(se, I2C_READ, m_param);
>   
>   	if (dma_buf && geni_se_rx_dma_prep(se, dma_buf, len, &rx_dma)) {
>   		geni_se_select_mode(se, GENI_SE_FIFO);
> @@ -539,7 +591,11 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
>   		geni_se_select_mode(se, GENI_SE_FIFO);
>   
>   	writel_relaxed(len, se->base + SE_I2C_TX_TRANS_LEN);
> -	geni_se_setup_m_cmd(se, I2C_WRITE, m_param);
> +
> +	if (gi2c->is_hs_mode)
> +		geni_se_setup_m_cmd(se, I2C_HS_WRITE, m_param);
> +	else
> +		geni_se_setup_m_cmd(se, I2C_WRITE, m_param);
same as above>
>   	if (dma_buf && geni_se_tx_dma_prep(se, dma_buf, len, &tx_dma)) {
>   		geni_se_select_mode(se, GENI_SE_FIFO);
> @@ -700,7 +756,7 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
>   		goto out;
>   	}
>   

[...]

> @@ -1046,6 +1122,39 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
>   		return ret;
>   	}
>   
> +	/*
> +	 * For I2C High-Speed mode, first verify QUP HW version supports it
> +	 * (requires QUPv3 core >= 4.3 per HPG), then configure 100 MHz source clock.
> +	 */
/* HS mode requires QUPv3 version >= 4.3 and source clock=100 MHz */
> +	if (gi2c->is_hs_mode) {
> +		u32 hw_ver = geni_se_get_qup_hw_version(&gi2c->se);
> +		u32 major = GENI_SE_VERSION_MAJOR(hw_ver);
> +		u32 minor = GENI_SE_VERSION_MINOR(hw_ver);
> +
> +		if (major < QUP_I2C_HS_MIN_MAJOR ||
> +		    (major == QUP_I2C_HS_MIN_MAJOR && minor < QUP_I2C_HS_MIN_MINOR)) {
> +			dev_err(gi2c->se.dev,
> +				"QUP HW v%u.%u does not support I2C HS mode (requires >= %u.%u)\n",
> +				major, minor,
> +				QUP_I2C_HS_MIN_MAJOR, QUP_I2C_HS_MIN_MINOR);
then why it entered into is_hs_mode condition ? > +			ret = -EOPNOTSUPP;
> +			goto err;
> +		}
> +
> +		ret = geni_se_clk_freq_match(&gi2c->se, I2C_HS_SRC_CLK_FREQ,
> +					     &gi2c->dfs_index, &freq_out, false);
> +		if (ret) {
> +			dev_err(gi2c->se.dev, "Failed to get DFS index for HS mode: %d\n", ret);
> +			goto err;
> +		}
> +
> +		ret = clk_set_rate(gi2c->se.clk, freq_out);
> +		if (ret) {
> +			dev_err(gi2c->se.dev, "Failed to set HS mode clock rate: %d\n", ret);
> +			goto err;
> +		}
> +	}
> +
>   	proto = geni_se_read_proto(&gi2c->se);
>   	if (proto == GENI_SE_INVALID_PROTO) {
>   		ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);