Re: [PATCH v5 13/13] i2c: qcom-geni: Enable I2C on SA8255p Qualcomm platforms

From: Konrad Dybcio

Date: Tue Feb 17 2026 - 06:50:22 EST


On 2/6/26 6:41 PM, Praveen Talari wrote:
> The Qualcomm automotive SA8255p SoC relies on firmware to configure
> platform resources, including clocks, interconnects and TLMM.
> The driver requests resources operations over SCMI using power
> and performance protocols.
>
> The SCMI power protocol enables or disables resources like clocks,
> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> such as resume/suspend, to control power on/off.
>
> The SCMI performance protocol manages I2C frequency, with each
> frequency rate represented by a performance level. The driver uses
> geni_se_set_perf_opp() API to request the desired frequency rate..
>
> As part of geni_se_set_perf_opp(), the OPP for the requested frequency
> is obtained using dev_pm_opp_find_freq_floor() and the performance
> level is set using dev_pm_opp_set_opp().
>
> Acked-by: Viken Dadhaniya <viken.dadhaniya@xxxxxxxxxxxxxxxx>
> Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>
> ---

[...]

> @@ -215,6 +220,7 @@ static void qcom_geni_i2c_conf(struct geni_i2c_dev *gi2c)
> val |= itr->t_low_cnt << LOW_COUNTER_SHFT;
> val |= itr->t_cycle_cnt;
> writel_relaxed(val, gi2c->se.base + SE_I2C_SCL_COUNTERS);
> + return 0;

ultra nit: a \n before return statements is preferred

[...]

> static const struct geni_i2c_desc i2c_master_hub = {
> .no_dma_support = true,
> .tx_fifo_depth = 16,
> + .resources_init = geni_i2c_resources_init,
> + .set_rate = qcom_geni_i2c_conf,
> + .power_on = geni_se_resources_activate,
> + .power_off = geni_se_resources_deactivate,
> +};
> +
> +static const struct geni_i2c_desc sa8255p_geni_i2c = {
> + .resources_init = geni_se_domain_attach,
> + .set_rate = geni_se_set_perf_opp,

I noticed that because this lacks .power_on/off, the
pinctrl_pm_select_xxx_state() functions are never called. Are the GPIOs
managed through the power/perf domains of the QUP devices too? (I would
assume not since there's a TLMM node in [0])

Konrad

[0] https://lore.kernel.org/linux-arm-msm/20250422231249.871995-1-quic_djaggi@xxxxxxxxxxx/