linux-kernel - Re: [PATCH 4/5] i2c: qcom-cci: Add OPP table support and enforce FAST

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1899862b-530b-4a75-93fa-c70c90d98016@oss.qualcomm.com>
Date: Mon, 8 Sep 2025 12:00:13 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
Cc: Konrad Dybcio <konradybcio@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Loic Poulain <loic.poulain@....qualcomm.com>,
        Robert Foss
 <rfoss@...nel.org>, Andi Shyti <andi.shyti@...nel.org>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH 4/5] i2c: qcom-cci: Add OPP table support and enforce
 FAST_PLUS requirements

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@....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@....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