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

[Praveen Talari]

Hi

On 2/17/2026 5:20 PM, Konrad Dybcio wrote:
> 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

Sure, will do it in next patch

> [...]
>
> >   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

Yes, GPIOs are managed by Firmware, not by Linux.

> assume not since there's a TLMM node in [0])

I don't know the exact reason for this node in DT.

Thanks,
Praveen Talari
> Konrad
>
> [0] https://lore.kernel.org/linux-arm-msm/20250422231249.871995-1-quic_djaggi@xxxxxxxxxxx/