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

From: Jyothi Kumar Seerapu

Date: Mon Sep 07 2026 - 12:20:51 EST

On 8/28/2026 1:12 AM, Mukesh Savaliya wrote:

Hi Mukesh, Thanks for the review comments.> 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.>

Sure, will remove these comments in V2.+

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 ?

No, else is not required here, as the if condition for I2C HS mode case returns from the function. For non-HS mode case the below will execute.

+ 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>

Yes this can be set like below:
geni_se_setup_m_cmd(se, gi2c->is_hs_mode ? I2C_HS_WRITE : I2C_WRITE, m_param);

      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 */

Sure, will update in V2.>> + 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 ?

Need to check QUP version only for I2C HS mode requests, for other I2C modes no need of QUP version check. > + 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);