Re: [PATCH v2 2/2] i2c: qcom-geni: add I2C frequency table for 32 MHz firmware-based SEs

[Praveen Talari <praveen.talari@oss.qualcomm.com>]

Hi Kathiravan,

On 12-08-2026 14:00, Kathiravan Thirumoorthy wrote:
> On IPQ5424, the I2C controller can operate on both RTL-hardened SEs
> and firmware-based SEs. As documented in commit 506bb2ab0075
> ("i2c: qcom-geni: Support systems with 32MHz serial engine clock"),
> in IPQ5424 SEs use a 32 MHz source clock.
>
> The existing frequency table does not provide the required SCL timing
> parameters for firmware-based SEs. Add a dedicated frequency table
> using the SCL counter values recommended by the hardware design team
> and select it for firmware-based SEs to achieve the target bus
> frequencies.
>
> Move geni_i2c_clk_map_idx() from geni_i2c_resources_init() to
> geni_i2c_init() to ensure SE_HW_PARAM_2 is accessed only after the
> required clocks have been enabled.
>
> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
> ---
>   drivers/i2c/busses/i2c-qcom-geni.c | 25 ++++++++++++++++++-------
>   include/linux/soc/qcom/geni-se.h   |  1 +
>   2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 00013b41a6f5..15835f10b062 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -187,19 +187,28 @@ static const struct geni_i2c_clk_fld geni_i2c_clk_map_19p2mhz[] = {
>   };
>   
>   /* source_clock = 32 MHz */
> -static const struct geni_i2c_clk_fld geni_i2c_clk_map_32mhz[] = {
> +static const struct geni_i2c_clk_fld geni_i2c_clk_map_32mhz_minicore[] = {
>   	{ I2C_MAX_STANDARD_MODE_FREQ, 12, 9, 10, 26 },
>   	{ I2C_MAX_FAST_MODE_FREQ, 4, 3, 9, 19 },
>   	{ I2C_MAX_FAST_MODE_PLUS_FREQ, 2, 3, 5, 15 },
>   	{}
>   };
>   
> +static const struct geni_i2c_clk_fld geni_i2c_clk_map_32mhz_fwcore[] = {
> +	{ I2C_MAX_STANDARD_MODE_FREQ, 12, 9, 10, 25 },
> +	{ I2C_MAX_FAST_MODE_FREQ, 4, 3, 9, 18 },
> +	{ I2C_MAX_FAST_MODE_PLUS_FREQ, 1, 6, 15, 29 },
> +	{}
> +};
> +
>   static int geni_i2c_clk_map_idx(struct geni_i2c_dev *gi2c)
>   {
>   	const struct geni_i2c_clk_fld *itr;
> +	bool is_minicore = FIELD_GET(GEN_USE_MINICORES,
> +				     readl_relaxed(gi2c->se.base + SE_HW_PARAM_2));
>   
>   	if (clk_get_rate(gi2c->se.clk) == 32 * HZ_PER_MHZ)
> -		itr = geni_i2c_clk_map_32mhz;
> +		itr = is_minicore ? geni_i2c_clk_map_32mhz_minicore : geni_i2c_clk_map_32mhz_fwcore;
The source selection remains unchanged when using 32 MHz:
writel_relaxed(0, gi2c->se.base + SE_GENI_CLK_SEL);

However, what about the voltage requirements associated with the higher 
frequency?

Are they automatically managed by the underlying provider, or is 
additional handling needed in the driver?

Reference of konrad change for CCI I2C driver for 37.5MHz with voltage 
requirement.

https://lore.kernel.org/linux-arm-msm/20250904-topic-cci_updates-v1-4-d38559692703@oss.qualcomm.com/


Thanks,

Praveen Talari

>   	else
>   		itr = geni_i2c_clk_map_19p2mhz;
>   
> @@ -1046,6 +1055,13 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
>   		return ret;
>   	}
>   
> +	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);
> +		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);
> @@ -1109,11 +1125,6 @@ static int geni_i2c_resources_init(struct geni_se *se)
>   	if (ret)
>   		return ret;
>   
> -	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 geni_icc_set_bw_ab(&gi2c->se, GENI_DEFAULT_BW, GENI_DEFAULT_BW,
>   				  Bps_to_icc(gi2c->clk_freq_out));
>   }
> diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
> index 29a53bbc0dd4..7e8c0bb6d674 100644
> --- a/include/linux/soc/qcom/geni-se.h
> +++ b/include/linux/soc/qcom/geni-se.h
> @@ -331,6 +331,7 @@ struct geni_se {
>   #define RX_FIFO_DEPTH_SHFT		16
>   
>   /* SE_HW_PARAM_2 fields */
> +#define GEN_USE_MINICORES		BIT(12)
>   #define PROG_RAM_DEPTH_MSK		GENMASK(10, 0)
>   
>   #define HW_VER_MAJOR_MASK		GENMASK(31, 28)
>