Enable vreg_l6p which is used as a pull-up for the CCI busses, to make
sure I2C communication works as expected.
Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
---
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index e115b6a52b299ef663ccfb614785f8f89091f39d..2dd2c452592aa6b0ac826f19eb9cb1a8b90cee47 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -749,6 +749,8 @@ vreg_l6p: ldo6 {
regulator-name = "vreg_l6p";
regulator-min-microvolt = <1700000>;
regulator-max-microvolt = <1904000>;
+ /* Pull-up for CCI I2C busses */
+ regulator-always-on;
};
vreg_l7p: ldo7 {
--
2.43.0On 10/2/25 12:15 PM, Griffin Kroah-Hartman wrote:
> Enable vreg_l6p which is used as a pull-up for the CCI busses, to make
> sure I2C communication works as expected.
>
> Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
> ---
Makes me wonder if we should maybe extend the CCI definition
(or maybe the common i2c-bus binding?) to accept an external
pull-up supply, as this is a common problem.. (+Bryan, Wolfram)
We could then shut down the regulator when cameras are not
in use, preserving some trace amounts of power.
Or maybe L6P is already used as a pull-up supply for more things
onboard and should be always-on either way? Could you please
check that, Griffin?
Konrad
> arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> index e115b6a52b299ef663ccfb614785f8f89091f39d..2dd2c452592aa6b0ac826f19eb9cb1a8b90cee47 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -749,6 +749,8 @@ vreg_l6p: ldo6 {
> regulator-name = "vreg_l6p";
> regulator-min-microvolt = <1700000>;
> regulator-max-microvolt = <1904000>;
> + /* Pull-up for CCI I2C busses */
> + regulator-always-on;
> };
>
> vreg_l7p: ldo7 {
>
Quoting Konrad Dybcio (2025-10-02 13:45:49)
> On 10/2/25 12:15 PM, Griffin Kroah-Hartman wrote:
> > Enable vreg_l6p which is used as a pull-up for the CCI busses, to make
> > sure I2C communication works as expected.
> >
> > Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
> > ---
>
> Makes me wonder if we should maybe extend the CCI definition
> (or maybe the common i2c-bus binding?) to accept an external
> pull-up supply, as this is a common problem.. (+Bryan, Wolfram)
I'm a little confused about terminology here. To me CCI is the
communiation protocol (how to write the registers on the i2c bus). But
here' we're talking about 'pulling up' a cci bus ?
Is this actually impacting the bus - or is it more that it's /powering/
the camera and VCM both simultaneously (which is what happens on the RPi
cameras)
My curiosity lies in the fact that indeed we somehow need to be able to
coordinate the power relationship between multiple devices which ...
while independent for configuration - they do impact each other. I.e. if
you power on the camera and it simultaneously powers on the VCM - you
get the VCM jumping position if it's not also configured, so I
anticipate various bits of complexities here if they are all powered by
the same line.
I don't think a camera module should always be powered on for a phone
running on a battery - perhaps on this device the sensors have a
separate power down control ?
--
Kieran
> We could then shut down the regulator when cameras are not
> in use, preserving some trace amounts of power.
>
> Or maybe L6P is already used as a pull-up supply for more things
> onboard and should be always-on either way? Could you please
> check that, Griffin?
>
> Konrad
>
> > arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > index e115b6a52b299ef663ccfb614785f8f89091f39d..2dd2c452592aa6b0ac826f19eb9cb1a8b90cee47 100644
> > --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> > @@ -749,6 +749,8 @@ vreg_l6p: ldo6 {
> > regulator-name = "vreg_l6p";
> > regulator-min-microvolt = <1700000>;
> > regulator-max-microvolt = <1904000>;
> > + /* Pull-up for CCI I2C busses */
> > + regulator-always-on;
> > };
> >
> > vreg_l7p: ldo7 {
> >
On 10/3/25 12:35 PM, Kieran Bingham wrote:
> Quoting Konrad Dybcio (2025-10-02 13:45:49)
>> On 10/2/25 12:15 PM, Griffin Kroah-Hartman wrote:
>>> Enable vreg_l6p which is used as a pull-up for the CCI busses, to make
>>> sure I2C communication works as expected.
>>>
>>> Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
>>> ---
>>
>> Makes me wonder if we should maybe extend the CCI definition
>> (or maybe the common i2c-bus binding?) to accept an external
>> pull-up supply, as this is a common problem.. (+Bryan, Wolfram)
>
> I'm a little confused about terminology here. To me CCI is the
> communiation protocol (how to write the registers on the i2c bus). But
> here' we're talking about 'pulling up' a cci bus ?
CCI is unfortunately also the name of the I2C controller housed within
the camera subsystem on qc platforms and we're talking about pulling
up sda/scl lanes
Konrad
>
> Is this actually impacting the bus - or is it more that it's /powering/
> the camera and VCM both simultaneously (which is what happens on the RPi
> cameras)
>
> My curiosity lies in the fact that indeed we somehow need to be able to
> coordinate the power relationship between multiple devices which ...
> while independent for configuration - they do impact each other. I.e. if
> you power on the camera and it simultaneously powers on the VCM - you
> get the VCM jumping position if it's not also configured, so I
> anticipate various bits of complexities here if they are all powered by
> the same line.
>
> I don't think a camera module should always be powered on for a phone
> running on a battery - perhaps on this device the sensors have a
> separate power down control ?
>
> --
> Kieran
>
>> We could then shut down the regulator when cameras are not
>> in use, preserving some trace amounts of power.
>>
>> Or maybe L6P is already used as a pull-up supply for more things
>> onboard and should be always-on either way? Could you please
>> check that, Griffin?
>>
>> Konrad
>>
>>> arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>> index e115b6a52b299ef663ccfb614785f8f89091f39d..2dd2c452592aa6b0ac826f19eb9cb1a8b90cee47 100644
>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>> @@ -749,6 +749,8 @@ vreg_l6p: ldo6 {
>>> regulator-name = "vreg_l6p";
>>> regulator-min-microvolt = <1700000>;
>>> regulator-max-microvolt = <1904000>;
>>> + /* Pull-up for CCI I2C busses */
>>> + regulator-always-on;
>>> };
>>>
>>> vreg_l7p: ldo7 {
>>>
Hi Konrad,
Answering on behalf of Griffin :)
On Thu Oct 2, 2025 at 2:45 PM CEST, Konrad Dybcio wrote:
> On 10/2/25 12:15 PM, Griffin Kroah-Hartman wrote:
>> Enable vreg_l6p which is used as a pull-up for the CCI busses, to make
>> sure I2C communication works as expected.
>>
>> Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
>> ---
>
> Makes me wonder if we should maybe extend the CCI definition
> (or maybe the common i2c-bus binding?) to accept an external
> pull-up supply, as this is a common problem.. (+Bryan, Wolfram)
Yes!
Unfortunately though this effort has stalled some years ago. There is
"struct regulator *bus_regulator;" in "struct i2c_adapter" already and
vbus-supply is documented in i2c-mt65xx but afaik this not functional
because some code was ripped out ago because of some AMDGPU regressions.
> We could then shut down the regulator when cameras are not
> in use, preserving some trace amounts of power.
>
> Or maybe L6P is already used as a pull-up supply for more things
> onboard and should be always-on either way? Could you please
> check that, Griffin?
L6P is only used as pull-up reg for cci0, cci1 and cci3; and as power
for stuff in the camera modules. So if everything camera-related is off,
turning it off is actually a good idea.
Regards
Luca
>
> Konrad
>
>> arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> index e115b6a52b299ef663ccfb614785f8f89091f39d..2dd2c452592aa6b0ac826f19eb9cb1a8b90cee47 100644
>> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>> @@ -749,6 +749,8 @@ vreg_l6p: ldo6 {
>> regulator-name = "vreg_l6p";
>> regulator-min-microvolt = <1700000>;
>> regulator-max-microvolt = <1904000>;
>> + /* Pull-up for CCI I2C busses */
>> + regulator-always-on;
>> };
>>
>> vreg_l7p: ldo7 {
>>
> Unfortunately though this effort has stalled some years ago. There is > "struct regulator *bus_regulator;" in "struct i2c_adapter" already and > vbus-supply is documented in i2c-mt65xx but afaik this not functional > because some code was ripped out ago because of some AMDGPU regressions. Thanks for mentioning this. It all sounded familiar to me but I couldn't put my finger exactly.
On 02/10/2025 13:45, Konrad Dybcio wrote: > On 10/2/25 12:15 PM, Griffin Kroah-Hartman wrote: >> Enable vreg_l6p which is used as a pull-up for the CCI busses, to make >> sure I2C communication works as expected. "vreg_l6p is the voltage source for the pull-up resistor" >> >> Signed-off-by: Griffin Kroah-Hartman<griffin.kroah@fairphone.com> >> --- > Makes me wonder if we should maybe extend the CCI definition > (or maybe the common i2c-bus binding?) to accept an external > pull-up supply, as this is a common problem.. (+Bryan, Wolfram) always-on is up to the platform - i.e. do you care about the floating-state of the i2c bus in power-collapse anyway ? Feels like something up to the platform designers. > We could then shut down the regulator when cameras are not > in use, preserving some trace amounts of power.> Or maybe L6P is already used as a pull-up supply for more things > onboard and should be always-on either way? Could you please > check that, Griffin? If we drop always-on and introduce a "pullup-supply" to the CCI bindings then, it would be up the CCI driver to enable/disable the bus pullups and then we can optionally do the power-rail disable when entering power-collapse. That'd be nice. As-is though I personally am fine with the patch as-is with the updated commit text above - consider the CCI change as extra homework ;) Assuming the commit log is tweaked. Reviewed-by: Bryan O'Donoghue <bod@kernel.org>
© 2016 - 2025 Red Hat, Inc.