Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 2 Feb 2022 08:03:10 +0000
From:   Jonathan McDowell <noodles@earth.li>
To:     Ansuel Smith <ansuelsmth@gmail.com>
Cc:     Bjorn Andersson <bjorn.andersson@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 03/17] ARM: dts: qcom: add missing rpm regulators and
 cells for ipq8064
Message-ID: <Yfo6vgdTR4ZLGZZ4@earth.li>
References: <20220118012051.21691-1-ansuelsmth@gmail.com>
 <20220118012051.21691-4-ansuelsmth@gmail.com>
 <Yfhmum8BnB1JIALP@builder.lan>
 <YflGGM45F3TqERNj@earth.li>
 <YfmtHFfW00Qr2cLc@Ansuel-xps.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YfmtHFfW00Qr2cLc@Ansuel-xps.localdomain>
Precedence: bulk

On Tue, Feb 01, 2022 at 10:58:52PM +0100, Ansuel Smith wrote:
> On Tue, Feb 01, 2022 at 02:39:20PM +0000, Jonathan McDowell wrote:
> > On Mon, Jan 31, 2022 at 04:46:18PM -0600, Bjorn Andersson wrote:
> > > On Mon 17 Jan 19:20 CST 2022, Ansuel Smith 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.
> > > > 
> > > 
> > > Is the SMB208 mandatory on all ipq8064 designs, or should this be pushed
> > > out to the device dts?
> > 
> > It's not; the RB3011 uses a different regulator (a TPS563900).
> >
> 
> Fact is that that's a special case. We have 20 devices based on ipq806x
> and they all have smb208 regulators.

Indeed, it's another Mikrotik special unfortunately (I haven't managed
to get the SMEM driver to work on the platform either).

> Is the TPS563900 also controlled by rpm?

AFAICT it's CPU controlled via I2C. It looks like one output is shared
for the CPU cores etc and the other is for the NSS cores, rather than
the full control the smb208 offers.

> Anyway should we use a dedicated dtsi to declare the correct regulators?

I've got no problem with smb208 being the default, but please add any
appropriate disabling of it to the RB3011 DTS.

> > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > > ---
> > > >  arch/arm/boot/dts/qcom-ipq8064.dtsi | 35 +++++++++++++++++++++++++++++
> > > >  1 file changed, 35 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> > > > index 094125605bea..824cf13dd037 100644
> > > > --- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
> > > > +++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> > > > @@ -829,10 +829,45 @@ 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>;
> > > >  			};
> > > > +
> > > > +			regulators {
> > > > +				compatible = "qcom,rpm-smb208-regulators";
> > > > +
> > > > +				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 {
> > > > -- 
> > > > 2.33.1

J.

-- 
/-\                             | 101 things you can't have too much
|@/  Debian GNU/Linux Developer |         of : 13 - Holidays.
\-                              |