Re: [PATCH 03/13] ARM: dts: qcom: add missing rpm regulators and cells for ipq8064

[Christian Marangi]

On Wed, Jul 06, 2022 at 03:02:44PM +0200, Konrad Dybcio wrote:
> 
> 
> On 5.07.2022 15:39, Christian Marangi wrote:
> > Add cells definition for rpm node and add missing regulators for the 4
> > regulator present on ipq8064. There regulators are controlled by rpm and
> > to correctly works gsbi4_i2c require to be NEVER disabled or rpm will
> > reject any regulator change request.
> That sounds.. very weird for a RPM regulator..
> 
>

I know but it's like that. In theory the smb208 can be controlled via
the gsbi4_i2c and bypass rpm completely but we have no ducmentation of
smb208 so rpm is the only way to scale voltage on this SoC.

If gsbi4_i2c is disabled any RPM voltage request fails as RPM in it's
firmware use the same line and assume it's always enabled.

> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
> > Tested-by: Jonathan McDowell <noodles@xxxxxxxx>
> > ---
> >  arch/arm/boot/dts/qcom-ipq8064.dtsi | 36 +++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> > index 1b4b72723ead..c0b05d2a2d6d 100644
> > --- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
> > +++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> > @@ -844,10 +844,46 @@ rpm: rpm@108000 {
> >  			clocks = <&gcc RPM_MSG_RAM_H_CLK>;
> >  			clock-names = "ram";
> >  
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> >  			rpmcc: clock-controller {
> >  				compatible = "qcom,rpmcc-ipq806x", "qcom,rpmcc";
> >  				#clock-cells = <1>;
> >  			};
> > +
> > +			smb208_regulators: regulators {
> Are you sure it is used on all ipq8064 boards? And with the same
> voltage settings?
> 

Out of 28 device we have on openwrt only the one present upstream
doesn't use the smb208 regulator and use it's own way.

The voltage are all the same, it does change with other revision of the
SoC (ipq8065) but they will pushed in another series later.

> > +				compatible = "qcom,rpm-smb208-regulators";
> > +				status = "okay";
> They are enabled by default, as you are defining them here and
> the status property is not overwritten anywhere else.
> 
> Konrad
> > +
> > +				smb208_s1a: s1a {
> > +					regulator-min-microvolt = <1050000>;
> > +					regulator-max-microvolt = <1150000>;
> > +
> > +					qcom,switch-mode-frequency = <1200000>;
> > +				};
> > +
> > +				smb208_s1b: s1b {
> > +					regulator-min-microvolt = <1050000>;
> > +					regulator-max-microvolt = <1150000>;
> > +
> > +					qcom,switch-mode-frequency = <1200000>;
> > +				};
> > +
> > +				smb208_s2a: s2a {
> > +					regulator-min-microvolt = < 800000>;
> > +					regulator-max-microvolt = <1250000>;
> > +
> > +					qcom,switch-mode-frequency = <1200000>;
> > +				};
> > +
> > +				smb208_s2b: s2b {
> > +					regulator-min-microvolt = < 800000>;
> > +					regulator-max-microvolt = <1250000>;
> > +
> > +					qcom,switch-mode-frequency = <1200000>;
> > +				};
> > +			};
> >  		};
> >  
> >  		tcsr: syscon@1a400000 {

-- 
	Ansuel