From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The CCI clock has voltage requirements, which need to be described
through an OPP table.
The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz
(which is a value common across all SoCs), since it's not possible to
reach the required timings with the default 19.2 MHz rate.
Address both issues by introducing an OPP table and using it to vote
for the faster rate.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-qcom-cci.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
index 74fedfdec3ae4e034ec4d946179e963c783b5923..d6192e2a5e3bc4d908cba594d1910a41f3a41e9c 100644
--- a/drivers/i2c/busses/i2c-qcom-cci.c
+++ b/drivers/i2c/busses/i2c-qcom-cci.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
#include <linux/pm_runtime.h>
#define CCI_HW_VERSION 0x0
@@ -121,6 +122,7 @@ struct cci_data {
struct i2c_adapter_quirks quirks;
u16 queue_size[NUM_QUEUES];
struct hw_params params[3];
+ bool fast_mode_plus_supported;
};
struct cci {
@@ -466,9 +468,22 @@ static const struct i2c_algorithm cci_algo = {
.functionality = cci_func,
};
+static unsigned long cci_desired_clk_rate(struct cci *cci)
+{
+ if (cci->data->fast_mode_plus_supported)
+ return 37500000ULL;
+
+ return 19200000ULL;
+}
+
static int __maybe_unused cci_suspend_runtime(struct device *dev)
{
struct cci *cci = dev_get_drvdata(dev);
+ int ret;
+
+ ret = dev_pm_opp_set_rate(dev, 0);
+ if (ret)
+ return ret;
clk_bulk_disable_unprepare(cci->nclocks, cci->clocks);
@@ -484,6 +499,10 @@ static int __maybe_unused cci_resume_runtime(struct device *dev)
if (ret)
return ret;
+ ret = dev_pm_opp_set_rate(dev, cci_desired_clk_rate(cci));
+ if (ret)
+ return ret;
+
cci_init(cci);
return 0;
@@ -588,6 +607,19 @@ static int cci_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
+ ret = devm_pm_opp_set_clkname(dev, "cci");
+ if (ret)
+ return ret;
+
+ /* OPP table is optional */
+ ret = devm_pm_opp_of_add_table(dev);
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
+
+ ret = dev_pm_opp_set_rate(dev, cci_desired_clk_rate(cci));
+ if (ret)
+ return ret;
+
/* Interrupt */
ret = platform_get_irq(pdev, 0);
@@ -775,6 +807,7 @@ static const struct cci_data cci_v2_data = {
.trdhld = 3,
.tsp = 3
},
+ .fast_mode_plus_supported = true,
};
static const struct of_device_id cci_dt_match[] = {
--
2.51.0On 9/4/2025 8:01 PM, Konrad Dybcio wrote: > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > > The CCI clock has voltage requirements, which need to be described > through an OPP table. > > The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz Typo: 37.5 MHz> (which is a value common across all SoCs), since it's not possible to > reach the required timings with the default 19.2 MHz rate. > > Address both issues by introducing an OPP table and using it to vote > for the faster rate. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > --- [...]
On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > > The CCI clock has voltage requirements, which need to be described > through an OPP table. > > The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz > (which is a value common across all SoCs), since it's not possible to > reach the required timings with the default 19.2 MHz rate. > > Address both issues by introducing an OPP table and using it to vote > for the faster rate. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Using an OPP table for a single static rate that remains the same over the whole lifetime of the driver feels like overkill to me. Couldn't you just put the "required-opps" directly into the device node so that it is automatically applied when the device goes in/out of runtime suspend? And since you need to make DT additions anyway, couldn't you just use "assigned-clock-rates" to avoid the need for a driver patch entirely? We use that for e.g. USB clocks as well. Thanks, Stephan
On 9/8/25 10:36 AM, Stephan Gerhold wrote: > On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >> >> The CCI clock has voltage requirements, which need to be described >> through an OPP table. >> >> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz >> (which is a value common across all SoCs), since it's not possible to >> reach the required timings with the default 19.2 MHz rate. >> >> Address both issues by introducing an OPP table and using it to vote >> for the faster rate. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > > Using an OPP table for a single static rate that remains the same over > the whole lifetime of the driver feels like overkill to me. Couldn't you > just put the "required-opps" directly into the device node so that it is > automatically applied when the device goes in/out of runtime suspend? > > And since you need to make DT additions anyway, couldn't you just use > "assigned-clock-rates" to avoid the need for a driver patch entirely? We > use that for e.g. USB clocks as well. This is futureproofing, in case someone invents FastMode++ with a higher dvfs requirement or for when the driver adds presets for a 19.2 MHz CCI clock which would (marginally) decrease power consumption Konrad
On Mon, Sep 08, 2025 at 10:43:50AM +0200, Konrad Dybcio wrote: > On 9/8/25 10:36 AM, Stephan Gerhold wrote: > > On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: > >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >> > >> The CCI clock has voltage requirements, which need to be described > >> through an OPP table. > >> > >> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz > >> (which is a value common across all SoCs), since it's not possible to > >> reach the required timings with the default 19.2 MHz rate. > >> > >> Address both issues by introducing an OPP table and using it to vote > >> for the faster rate. > >> > >> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > > > > Using an OPP table for a single static rate that remains the same over > > the whole lifetime of the driver feels like overkill to me. Couldn't you > > just put the "required-opps" directly into the device node so that it is > > automatically applied when the device goes in/out of runtime suspend? > > > > And since you need to make DT additions anyway, couldn't you just use > > "assigned-clock-rates" to avoid the need for a driver patch entirely? We > > use that for e.g. USB clocks as well. > > This is futureproofing, in case someone invents FastMode++ with a higher > dvfs requirement or for when the driver adds presets for a 19.2 MHz CCI > clock which would (marginally) decrease power consumption > If 19.2 MHz CCI clock is feasible and has lower voltage requirements, then I would expect a separate entry for 19.2 MHz in the OPP table of PATCH 5/5? The DT is unrelated to what functionality you implement in the driver, and that would make the OPP table look less useless. :-) Thanks, Stephan
On 9/8/25 10:46 AM, Stephan Gerhold wrote: > On Mon, Sep 08, 2025 at 10:43:50AM +0200, Konrad Dybcio wrote: >> On 9/8/25 10:36 AM, Stephan Gerhold wrote: >>> On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: >>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>> >>>> The CCI clock has voltage requirements, which need to be described >>>> through an OPP table. >>>> >>>> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz >>>> (which is a value common across all SoCs), since it's not possible to >>>> reach the required timings with the default 19.2 MHz rate. >>>> >>>> Address both issues by introducing an OPP table and using it to vote >>>> for the faster rate. >>>> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>> >>> Using an OPP table for a single static rate that remains the same over >>> the whole lifetime of the driver feels like overkill to me. Couldn't you >>> just put the "required-opps" directly into the device node so that it is >>> automatically applied when the device goes in/out of runtime suspend? >>> >>> And since you need to make DT additions anyway, couldn't you just use >>> "assigned-clock-rates" to avoid the need for a driver patch entirely? We >>> use that for e.g. USB clocks as well. >> >> This is futureproofing, in case someone invents FastMode++ with a higher >> dvfs requirement or for when the driver adds presets for a 19.2 MHz CCI >> clock which would (marginally) decrease power consumption >> > > If 19.2 MHz CCI clock is feasible and has lower voltage requirements, > then I would expect a separate entry for 19.2 MHz in the OPP table of > PATCH 5/5? The DT is unrelated to what functionality you implement in > the driver, and that would make the OPP table look less useless. :-) The frequency plan for 8280 does not recommend any rate != 37.5 MHz For x1e80100 however, the lovsvs_d1 corner is recommended to be 30 (yes, thirty) MHz, sourced from CAM_PLL8 for $reasons Konrad
On Mon, Sep 08, 2025 at 11:49:52AM +0200, Konrad Dybcio wrote: > On 9/8/25 10:46 AM, Stephan Gerhold wrote: > > On Mon, Sep 08, 2025 at 10:43:50AM +0200, Konrad Dybcio wrote: > >> On 9/8/25 10:36 AM, Stephan Gerhold wrote: > >>> On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: > >>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >>>> > >>>> The CCI clock has voltage requirements, which need to be described > >>>> through an OPP table. > >>>> > >>>> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz > >>>> (which is a value common across all SoCs), since it's not possible to > >>>> reach the required timings with the default 19.2 MHz rate. > >>>> > >>>> Address both issues by introducing an OPP table and using it to vote > >>>> for the faster rate. > >>>> > >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >>> > >>> Using an OPP table for a single static rate that remains the same over > >>> the whole lifetime of the driver feels like overkill to me. Couldn't you > >>> just put the "required-opps" directly into the device node so that it is > >>> automatically applied when the device goes in/out of runtime suspend? > >>> > >>> And since you need to make DT additions anyway, couldn't you just use > >>> "assigned-clock-rates" to avoid the need for a driver patch entirely? We > >>> use that for e.g. USB clocks as well. > >> > >> This is futureproofing, in case someone invents FastMode++ with a higher > >> dvfs requirement or for when the driver adds presets for a 19.2 MHz CCI > >> clock which would (marginally) decrease power consumption > >> > > > > If 19.2 MHz CCI clock is feasible and has lower voltage requirements, > > then I would expect a separate entry for 19.2 MHz in the OPP table of > > PATCH 5/5? The DT is unrelated to what functionality you implement in > > the driver, and that would make the OPP table look less useless. :-) > > The frequency plan for 8280 does not recommend any rate != 37.5 MHz > > For x1e80100 however, the lovsvs_d1 corner is recommended to be 30 > (yes, thirty) MHz, sourced from CAM_PLL8 for $reasons > The 37.5 MHz rate still exists on X1E I presume, or are you saying we need more changes to support those odd 30 MHz? Personally, I'm not fully convinced there is ever going to be a use case of someone using a "non-standard" frequency. Even if "FastMode++" is invented most devices will probably want to use it. And the voltage requirements we're currently talking about here like "low svs" during camera use cases are kind of negligible compared to others too. But I'm fine with either solution, just wanted to mention it. :D Thanks, Stephan
On 9/8/25 11:57 AM, Stephan Gerhold wrote: > On Mon, Sep 08, 2025 at 11:49:52AM +0200, Konrad Dybcio wrote: >> On 9/8/25 10:46 AM, Stephan Gerhold wrote: >>> On Mon, Sep 08, 2025 at 10:43:50AM +0200, Konrad Dybcio wrote: >>>> On 9/8/25 10:36 AM, Stephan Gerhold wrote: >>>>> On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: >>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>>> >>>>>> The CCI clock has voltage requirements, which need to be described >>>>>> through an OPP table. >>>>>> >>>>>> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz >>>>>> (which is a value common across all SoCs), since it's not possible to >>>>>> reach the required timings with the default 19.2 MHz rate. >>>>>> >>>>>> Address both issues by introducing an OPP table and using it to vote >>>>>> for the faster rate. >>>>>> >>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>> >>>>> Using an OPP table for a single static rate that remains the same over >>>>> the whole lifetime of the driver feels like overkill to me. Couldn't you >>>>> just put the "required-opps" directly into the device node so that it is >>>>> automatically applied when the device goes in/out of runtime suspend? >>>>> >>>>> And since you need to make DT additions anyway, couldn't you just use >>>>> "assigned-clock-rates" to avoid the need for a driver patch entirely? We >>>>> use that for e.g. USB clocks as well. >>>> >>>> This is futureproofing, in case someone invents FastMode++ with a higher >>>> dvfs requirement or for when the driver adds presets for a 19.2 MHz CCI >>>> clock which would (marginally) decrease power consumption >>>> >>> >>> If 19.2 MHz CCI clock is feasible and has lower voltage requirements, >>> then I would expect a separate entry for 19.2 MHz in the OPP table of >>> PATCH 5/5? The DT is unrelated to what functionality you implement in >>> the driver, and that would make the OPP table look less useless. :-) >> >> The frequency plan for 8280 does not recommend any rate != 37.5 MHz >> >> For x1e80100 however, the lovsvs_d1 corner is recommended to be 30 >> (yes, thirty) MHz, sourced from CAM_PLL8 for $reasons >> > > The 37.5 MHz rate still exists on X1E I presume, or are you saying we > need more changes to support those odd 30 MHz? Yes, any corner over lowsvs_d1 is 37.5, sourced from cam_pll0 > Personally, I'm not fully convinced there is ever going to be a use case > of someone using a "non-standard" frequency. Even if "FastMode++" is > invented most devices will probably want to use it. Not really, there's no reason to make your i2c bus go fastfastfast if the devices on the other end can't cope with it > And the voltage > requirements we're currently talking about here like "low svs" during > camera use cases are kind of negligible compared to others too. Again, this is an I2C controller that seems to be associated with cameras.. No image data has to actually be processed for the communications to take place and you can attach any odd device Konrad > > But I'm fine with either solution, just wanted to mention it. :D > > Thanks, > Stephan
On Mon, Sep 08, 2025 at 12:00:13PM +0200, Konrad Dybcio wrote: > On 9/8/25 11:57 AM, Stephan Gerhold wrote: > > On Mon, Sep 08, 2025 at 11:49:52AM +0200, Konrad Dybcio wrote: > >> On 9/8/25 10:46 AM, Stephan Gerhold wrote: > >>> On Mon, Sep 08, 2025 at 10:43:50AM +0200, Konrad Dybcio wrote: > >>>> On 9/8/25 10:36 AM, Stephan Gerhold wrote: > >>>>> On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: > >>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >>>>>> > >>>>>> The CCI clock has voltage requirements, which need to be described > >>>>>> through an OPP table. > >>>>>> > >>>>>> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz > >>>>>> (which is a value common across all SoCs), since it's not possible to > >>>>>> reach the required timings with the default 19.2 MHz rate. > >>>>>> > >>>>>> Address both issues by introducing an OPP table and using it to vote > >>>>>> for the faster rate. > >>>>>> > >>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > >>>>> > >>>>> Using an OPP table for a single static rate that remains the same over > >>>>> the whole lifetime of the driver feels like overkill to me. Couldn't you > >>>>> just put the "required-opps" directly into the device node so that it is > >>>>> automatically applied when the device goes in/out of runtime suspend? > >>>>> > >>>>> And since you need to make DT additions anyway, couldn't you just use > >>>>> "assigned-clock-rates" to avoid the need for a driver patch entirely? We > >>>>> use that for e.g. USB clocks as well. > >>>> > >>>> This is futureproofing, in case someone invents FastMode++ with a higher > >>>> dvfs requirement or for when the driver adds presets for a 19.2 MHz CCI > >>>> clock which would (marginally) decrease power consumption > >>>> > >>> > >>> If 19.2 MHz CCI clock is feasible and has lower voltage requirements, > >>> then I would expect a separate entry for 19.2 MHz in the OPP table of > >>> PATCH 5/5? The DT is unrelated to what functionality you implement in > >>> the driver, and that would make the OPP table look less useless. :-) > >> > >> The frequency plan for 8280 does not recommend any rate != 37.5 MHz > >> > >> For x1e80100 however, the lovsvs_d1 corner is recommended to be 30 > >> (yes, thirty) MHz, sourced from CAM_PLL8 for $reasons > >> > > > > The 37.5 MHz rate still exists on X1E I presume, or are you saying we > > need more changes to support those odd 30 MHz? > > Yes, any corner over lowsvs_d1 is 37.5, sourced from cam_pll0 > > > Personally, I'm not fully convinced there is ever going to be a use case > > of someone using a "non-standard" frequency. Even if "FastMode++" is > > invented most devices will probably want to use it. > > Not really, there's no reason to make your i2c bus go fastfastfast if > the devices on the other end can't cope with it > > > And the voltage > > requirements we're currently talking about here like "low svs" during > > camera use cases are kind of negligible compared to others too. > > Again, this is an I2C controller that seems to be associated with > cameras.. No image data has to actually be processed for the > communications to take place and you can attach any odd device > My point is: In the unlikely case that support for faster I2C speeds is added in newer SoCs, I think you'd just get a new "standard" base clock frequency, add a new cci_data struct with adjusted timings and everyone will use that (even for the lower I2C speeds). I doubt anyone will bother adjusting and validating this for just one "corner"/voltage level less. There are much more effective targets for power optimization than the few bytes of I2C communication. :-) Thanks, Stephan
On 9/8/25 12:09 PM, Stephan Gerhold wrote: > On Mon, Sep 08, 2025 at 12:00:13PM +0200, Konrad Dybcio wrote: >> On 9/8/25 11:57 AM, Stephan Gerhold wrote: >>> On Mon, Sep 08, 2025 at 11:49:52AM +0200, Konrad Dybcio wrote: >>>> On 9/8/25 10:46 AM, Stephan Gerhold wrote: >>>>> On Mon, Sep 08, 2025 at 10:43:50AM +0200, Konrad Dybcio wrote: >>>>>> On 9/8/25 10:36 AM, Stephan Gerhold wrote: >>>>>>> On Thu, Sep 04, 2025 at 04:31:23PM +0200, Konrad Dybcio wrote: >>>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>>>>> >>>>>>>> The CCI clock has voltage requirements, which need to be described >>>>>>>> through an OPP table. >>>>>>>> >>>>>>>> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz >>>>>>>> (which is a value common across all SoCs), since it's not possible to >>>>>>>> reach the required timings with the default 19.2 MHz rate. >>>>>>>> >>>>>>>> Address both issues by introducing an OPP table and using it to vote >>>>>>>> for the faster rate. >>>>>>>> >>>>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >>>>>>> >>>>>>> Using an OPP table for a single static rate that remains the same over >>>>>>> the whole lifetime of the driver feels like overkill to me. Couldn't you >>>>>>> just put the "required-opps" directly into the device node so that it is >>>>>>> automatically applied when the device goes in/out of runtime suspend? >>>>>>> >>>>>>> And since you need to make DT additions anyway, couldn't you just use >>>>>>> "assigned-clock-rates" to avoid the need for a driver patch entirely? We >>>>>>> use that for e.g. USB clocks as well. >>>>>> >>>>>> This is futureproofing, in case someone invents FastMode++ with a higher >>>>>> dvfs requirement or for when the driver adds presets for a 19.2 MHz CCI >>>>>> clock which would (marginally) decrease power consumption >>>>>> >>>>> >>>>> If 19.2 MHz CCI clock is feasible and has lower voltage requirements, >>>>> then I would expect a separate entry for 19.2 MHz in the OPP table of >>>>> PATCH 5/5? The DT is unrelated to what functionality you implement in >>>>> the driver, and that would make the OPP table look less useless. :-) >>>> >>>> The frequency plan for 8280 does not recommend any rate != 37.5 MHz >>>> >>>> For x1e80100 however, the lovsvs_d1 corner is recommended to be 30 >>>> (yes, thirty) MHz, sourced from CAM_PLL8 for $reasons >>>> >>> >>> The 37.5 MHz rate still exists on X1E I presume, or are you saying we >>> need more changes to support those odd 30 MHz? >> >> Yes, any corner over lowsvs_d1 is 37.5, sourced from cam_pll0 >> >>> Personally, I'm not fully convinced there is ever going to be a use case >>> of someone using a "non-standard" frequency. Even if "FastMode++" is >>> invented most devices will probably want to use it. >> >> Not really, there's no reason to make your i2c bus go fastfastfast if >> the devices on the other end can't cope with it >> >>> And the voltage >>> requirements we're currently talking about here like "low svs" during >>> camera use cases are kind of negligible compared to others too. >> >> Again, this is an I2C controller that seems to be associated with >> cameras.. No image data has to actually be processed for the >> communications to take place and you can attach any odd device >> > > My point is: In the unlikely case that support for faster I2C speeds is > added in newer SoCs, I think you'd just get a new "standard" base clock > frequency, add a new cci_data struct with adjusted timings and everyone > will use that (even for the lower I2C speeds). I doubt anyone will > bother adjusting and validating this for just one "corner"/voltage level > less. There are much more effective targets for power optimization than > the few bytes of I2C communication. :-) There are OEMs that customize some of the timings (e.g. Sony) and I would expect SV efforts to at least cover the recommended frequency plan (which like for x1e sometimes contains >1 frequency).. That said, I do agree with you it's a rather niche/improbable corner of the SoC to worry about.. But using required-opps in dt-bindings for non-trivial hardware (CCI is a little bit more "fun" than the today's state of the upstream driver makes it seem) simply feels like asking for trouble (i.e. a ""real need"" for an opp table will probably come around one day, so I don't think the additional 10 lines of code to support it are that much of an issue either). Konrad
On 04/09/2025 15:31, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The CCI clock has voltage requirements, which need to be described
> through an OPP table.
>
> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz
> (which is a value common across all SoCs), since it's not possible to
> reach the required timings with the default 19.2 MHz rate.
>
> Address both issues by introducing an OPP table and using it to vote
> for the faster rate.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/i2c/busses/i2c-qcom-cci.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index 74fedfdec3ae4e034ec4d946179e963c783b5923..d6192e2a5e3bc4d908cba594d1910a41f3a41e9c 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -10,6 +10,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
>
> #define CCI_HW_VERSION 0x0
> @@ -121,6 +122,7 @@ struct cci_data {
> struct i2c_adapter_quirks quirks;
> u16 queue_size[NUM_QUEUES];
> struct hw_params params[3];
> + bool fast_mode_plus_supported;
that is a very long name for a flag
> };
>
> struct cci {
> @@ -466,9 +468,22 @@ static const struct i2c_algorithm cci_algo = {
> .functionality = cci_func,
> };
>
> +static unsigned long cci_desired_clk_rate(struct cci *cci)
> +{
> + if (cci->data->fast_mode_plus_supported)
> + return 37500000ULL;
> +
> + return 19200000ULL;
what's 32 bits between friends ?
> +}
> +
> static int __maybe_unused cci_suspend_runtime(struct device *dev)
> {
> struct cci *cci = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = dev_pm_opp_set_rate(dev, 0);
> + if (ret)
> + return ret;
>
> clk_bulk_disable_unprepare(cci->nclocks, cci->clocks);
>
> @@ -484,6 +499,10 @@ static int __maybe_unused cci_resume_runtime(struct device *dev)
> if (ret)
> return ret;
>
> + ret = dev_pm_opp_set_rate(dev, cci_desired_clk_rate(cci));
> + if (ret)
> + return ret;
> +
> cci_init(cci);
>
> return 0;
> @@ -588,6 +607,19 @@ static int cci_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> + ret = devm_pm_opp_set_clkname(dev, "cci");
> + if (ret)
> + return ret;
> +
> + /* OPP table is optional */
> + ret = devm_pm_opp_of_add_table(dev);
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
> +
> + ret = dev_pm_opp_set_rate(dev, cci_desired_clk_rate(cci));
> + if (ret)
> + return ret;
> +
> /* Interrupt */
>
> ret = platform_get_irq(pdev, 0);
> @@ -775,6 +807,7 @@ static const struct cci_data cci_v2_data = {
> .trdhld = 3,
> .tsp = 3
> },
> + .fast_mode_plus_supported = true,
> };
>
> static const struct of_device_id cci_dt_match[] = {
>
LGTM
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 9/4/25 4:31 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The CCI clock has voltage requirements, which need to be described
> through an OPP table.
>
> The 1 MHz FAST_PLUS mode requires the CCI core clock runs at 37,5 MHz
> (which is a value common across all SoCs), since it's not possible to
> reach the required timings with the default 19.2 MHz rate.
>
> Address both issues by introducing an OPP table and using it to vote
> for the faster rate.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/i2c/busses/i2c-qcom-cci.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index 74fedfdec3ae4e034ec4d946179e963c783b5923..d6192e2a5e3bc4d908cba594d1910a41f3a41e9c 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -10,6 +10,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> #include <linux/pm_runtime.h>
>
> #define CCI_HW_VERSION 0x0
> @@ -121,6 +122,7 @@ struct cci_data {
> struct i2c_adapter_quirks quirks;
> u16 queue_size[NUM_QUEUES];
> struct hw_params params[3];
> + bool fast_mode_plus_supported;
> };
>
> struct cci {
> @@ -466,9 +468,22 @@ static const struct i2c_algorithm cci_algo = {
> .functionality = cci_func,
> };
>
> +static unsigned long cci_desired_clk_rate(struct cci *cci)
> +{
> + if (cci->data->fast_mode_plus_supported)
> + return 37500000ULL;
> +
> + return 19200000ULL;
Well this is embarrassing
ULL -> UL
Konrad
© 2016 - 2025 Red Hat, Inc.