Linux I2C development
 help / color / mirror / Atom feed
From: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
To: Jyothi Kumar Seerapu <jyothi.seerapu@oss.qualcomm.com>,
	Vinod Koul <vkoul@kernel.org>, Frank Li <Frank.Li@kernel.org>,
	Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>,
	Andi Shyti <andi.shyti@kernel.org>
Cc: linux-arm-msm@vger.kernel.org, dmaengine@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
Subject: Re: [PATCH 2/2] i2c: qcom-geni: Add support for I2C High-Speed mode
Date: Fri, 28 Aug 2026 01:12:09 +0530	[thread overview]
Message-ID: <419902d5-cbb6-4d8f-acb6-d9640adb56d9@oss.qualcomm.com> (raw)
In-Reply-To: <20260824-i2c-hs-v1-2-8dd94e5c6113@oss.qualcomm.com>



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);
> 


  reply	other threads:[~2026-08-27 19:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24 11:25 [PATCH 0/2] i2c: Add I2C High-Speed mode support for qcom-geni Jyothi Kumar Seerapu
2026-08-24 11:25 ` [PATCH 1/2] dmaengine: qcom-gpi: Add I2C High-Speed mode configuration support Jyothi Kumar Seerapu
2026-08-27 19:26   ` Mukesh Savaliya
2026-08-24 11:25 ` [PATCH 2/2] i2c: qcom-geni: Add support for I2C High-Speed mode Jyothi Kumar Seerapu
2026-08-27 19:42   ` Mukesh Savaliya [this message]
2026-08-27 19:26 ` [PATCH 0/2] i2c: Add I2C High-Speed mode support for qcom-geni Mukesh Savaliya

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=419902d5-cbb6-4d8f-acb6-d9640adb56d9@oss.qualcomm.com \
    --to=mukesh.savaliya@oss.qualcomm.com \
    --cc=Frank.Li@kernel.org \
    --cc=andi.shyti@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=jyothi.seerapu@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viken.dadhaniya@oss.qualcomm.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox