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

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

On 9/12/2026 6:34 PM, Jyothi Kumar Seerapu wrote:
> Implement I2C High-Speed (HS) mode for the Qualcomm GENI I2C controller.
> Detect HS mode requests based on 3.4 MHz frequency and configure the
> hardware accordingly. When HS mode is active, set the source clock to
> 100 MHz and program timing parameters (TCYCLE=28, TLOW=38).
> 
> For FIFO/SE-DMA transfers, use HS-specific opcodes I2C_HS_WRITE (0xa)
> and I2C_HS_READ (0xb). Transmit the master code at Fast Mode Plus
> timing (1 MHz) before switching to 3.4 MHz for data transfer.
> 
> For GPI DMA, extend the configuration with a CONFIG1 TRE to pass HS
> timing parameters to the DMA engine. Add gpi_i2c_config1 with
> tcycle_cnt and tlow_cnt fields (defaults: 28 and 38).
> Use the set_config1 flag in gpi_i2c_config to send CONFIG1 TRE before
> the GO TRE in HS mode.
> 
> Signed-off-by: Jyothi Kumar Seerapu <jyothi.seerapu@oss.qualcomm.com>
> ---
>   drivers/i2c/busses/i2c-qcom-geni.c | 127 +++++++++++++++++++++++++++++++++----
>   1 file changed, 113 insertions(+), 14 deletions(-)

[...]

>   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 */
why ? can you mention reason too ?
And also instead of 2, can we use the macro now for each speed ?> +	 
gi2c->clk_fld = &itr[2];
> +		return 0;
> +	}
> +
> +	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 +257,12 @@ static int qcom_geni_i2c_conf(struct geni_se *se, unsigned long freq)

[...]

> @@ -836,6 +886,13 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>   
>   	gi2c->num_msgs = num;
>   	gi2c->is_tx_multi_desc_xfer = false;
> +	gi2c->config1_sent = false;
> +
> +	/* Initialize config1 TRE settings for HS mode */
> +	if (gi2c->is_hs_mode) {
> +		peripheral.config1.tcycle_cnt = I2C_HS_TCYCLE_CNT;
> +		peripheral.config1.tlow_cnt = I2C_HS_TLOW_CNT;
> +	}
>   
>   	tx_multi_xfer = &gi2c->i2c_multi_desc_config;
>   	memset(tx_multi_xfer, 0, sizeof(struct geni_i2c_gpi_multi_desc_xfer));
> @@ -883,14 +940,25 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>   		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
>   			peripheral.multi_msg = false;
>   
> -		ret =  geni_i2c_gpi(gi2c, msgs, &config,
> -				    &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c);
> +		/* Set config1 TRE only for HS mode */
> +		if (gi2c->is_hs_mode && !gi2c->config1_sent) {
> +			peripheral.set_config1 = 1;
> +			gi2c->config1_sent = true;
> +		} else {
> +			peripheral.set_config1 = 0;
I guess default should be already 0, isn't it ?> +		}
> +
> +		ret =  geni_i2c_gpi(gi2c, msgs, &config, &tx_addr, &tx_buf,
> +				    gi2c->is_hs_mode ? I2C_HS_WRITE : I2C_WRITE, gi2c->tx_c);
>   		if (ret)
>   			goto err;
>   
> +		/* CONFIG1 TRE is only for the TX channel; clear before RX call */
> +		peripheral.set_config1 = 0;
> +
>   		if (msgs[i].flags & I2C_M_RD) {
> -			ret =  geni_i2c_gpi(gi2c, msgs, &config,
> -					    &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c);
> +			ret =  geni_i2c_gpi(gi2c, msgs, &config, &rx_addr, &rx_buf,
> +					    gi2c->is_hs_mode ? I2C_HS_READ : I2C_READ, gi2c->rx_c);
>   			if (ret)
>   				goto err;
>   
> @@ -1037,6 +1105,7 @@ static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
>   static int geni_i2c_init(struct geni_i2c_dev *gi2c)
>   {
>   	u32 proto, tx_depth;
> +	unsigned long freq_out;
>   	bool fifo_disable;
>   	int ret;
>   
> @@ -1046,6 +1115,36 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
>   		return ret;
>   	}
>   
> +	/* HS mode requires QUPv3 version >= 4.3 and source clock=100 MHz */
Supported for Version >= 4.3 (instead of requires)> +	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",
I don't think we need to say required version, just reporting supported 
or not, should be good enough IMO.> +				major, minor,
> +				QUP_I2C_HS_MIN_MAJOR, QUP_I2C_HS_MIN_MINOR);
> +			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);
>