Re: [PATCH 7/7] i2c: qcom-geni: Use geni_se_set_rate() for setting source clock frequency
Mukesh Savaliya <[email protected]>
| Newsgroups | org.kernel.vger.linux-pm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.arm-scmi,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-i2c,org.kernel.vger.linux-kernel,org.kernel.vger.linux-serial,org.kernel.vger.linux-spi |
|---|---|
| Message-ID | <[email protected]> |
On 8/25/2026 12:38 PM, Praveen Talari wrote:
[...]
>>> index a23554d101fd..4561e2d235d5 100644
>>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>>> @@ -213,11 +213,11 @@ static int geni_i2c_clk_map_idx(struct
>>> geni_i2c_dev *gi2c)
>>> return -EINVAL;
>>> }
>>> -static int qcom_geni_i2c_conf(struct geni_se *se, unsigned long freq)
>>> +static int qcom_geni_i2c_conf(struct geni_se *se)
>>> {
>>> struct geni_i2c_dev *gi2c = dev_get_drvdata(se->dev);
>>> const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
>>> - u32 val;
>>> + u32 val, ret;
>>> writel_relaxed(0, gi2c->se.base + SE_GENI_CLK_SEL);
>>> @@ -233,6 +233,10 @@ static int qcom_geni_i2c_conf(struct geni_se
>>> *se, unsigned long freq)
>>> itr->clk_div, itr->t_high_cnt,
>>> itr->t_low_cnt, itr->t_cycle_cnt);
>>> + ret = geni_se_set_rate(&gi2c->se, 19200000);
>>
>> hard code ?
>
> Good point. The 19.2 MHz value is not intended to be a permanent
> limitation.
>
> This matches the frequency currently used by the I2C driver and
> therefore preserves
>
> the existing behavior. If support for higher source clock frequencies
> (e.g. 32 MHz or others)
>
This is already added in driver for 32 MHz and also AFAIK, support is
added for 3.4 MHZ which considers 100Mhz. so make it something which is
generic. Hard code is not recommended here.
Can we store source freq selection and continue with that ?
> is added in the future, the requested OPP/performance state will need to
> be updated accordingly.
>
>
>> May work currently, but if higher frequency support added, source may
>> change.
>>
>> Also you have added in commit message, but add a comment also here.
>>
[...]