drivers/i2c/busses/i2c-qcom-geni.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)
qcom_geni_i2c_conf() writes a hardcoded 0 to SE_GENI_CLK_SEL, which
selects an index from the hardware clock performance table. This always
picks the first table entry regardless of the actual source clock
configuration. On platforms where the matching entry is not at index 0,
the wrong source clock divider is active and the I2C bus runs at an
incorrect frequency.
Use geni_se_clk_freq_match() in geni_i2c_clk_map_idx() to find the
performance table index for the source clock (32 MHz or 19.2 MHz). Store
the resolved index in a new clk_idx field in geni_i2c_dev and write it
to SE_GENI_CLK_SEL instead of the hardcoded 0.
Fixes: 37692de5d523 ("i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller")
Cc: stable@vger.kernel.org
Signed-off-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-qcom-geni.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
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
+ * 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);
return -EINVAL;
}
@@ -219,7 +246,7 @@ 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);
+ writel_relaxed(gi2c->clk_idx, 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);
@@ -1111,8 +1138,7 @@ static int geni_i2c_resources_init(struct geni_se *se)
ret = geni_i2c_clk_map_idx(gi2c);
if (ret)
- return dev_err_probe(gi2c->se.dev, ret, "Invalid clk frequency %d Hz\n",
- gi2c->clk_freq_out);
+ return ret;
return geni_icc_set_bw_ab(&gi2c->se, GENI_DEFAULT_BW, GENI_DEFAULT_BW,
Bps_to_icc(gi2c->clk_freq_out));
---
base-commit: 3d83758432b5e6ed9507500a57efb0f3af41ee7d
change-id: 20260807-i2c-fix-se-clk-conf-89f6f92d373a
Best regards,
--
Viken Dadhaniya <viken.dadhaniya@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.
On 9/21/2026 2:44 PM, Mukesh Savaliya wrote:
>
>
> 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).
Updated in v2.
>> + * 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;
Added in v2.
>> }
>>
>
> With above taken cared, rest all looks good.
>
© 2016 - 2026 Red Hat, Inc.