Re: [PATCH] arm64: dts: qcom: qrb2210-rb1: Add regulators

From: Dmitry Baryshkov
Date: Tue Jul 11 2023 - 06:56:58 EST

On Tue, 11 Jul 2023 at 13:48, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
>
> On 11.07.2023 12:44, Dmitry Baryshkov wrote:
> > On Tue, 11 Jul 2023 at 13:28, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
> >>
> >> Add and assign RPM regulators coming from PM2250.
> >>
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> >> ---
> [...]
>
> >> + pm2250_l5: l5 {
> >> + /* CSI/DSI */
> >> + regulator-min-microvolts = <1232000>;
> >> + regulator-max-microvolts = <1232000>;
> >> + regulator-allow-set-load;
> >> + regulator-boot-on;
> >
> > why?
> >
> To answer all of the questions in one go:
>
> "it's because the regulator is enabled at boot"

I was more concerned about the following part:
If the bootloader didn't leave it on then OS should turn it on at boot ...

Please remind me, are we declaring them as boot-on so that the
regulator core can turn them off later if they are unused?

>
> Konrad
> >> + };
> >> +
> >> + pm2250_l6: l6 {
> >> + /* DRAM PLL */
> >> + regulator-min-microvolts = <928000>;
> >> + regulator-max-microvolts = <928000>;
> >> + regulator-always-on;
> >> + regulator-boot-on;
> >> + };
> >> +
> >> + pm2250_l7: l7 {
> >> + /* Wi-Fi CX/MX */
> >> + regulator-min-microvolts = <664000>;
> >> + regulator-max-microvolts = <664000>;
> >> + };
> >> +
> >> + /*
> >> + * L8 - VDD_LPI_CX
> >> + * L9 - VDD_LPI_MX
> >> + */
> >> +
> >> + pm2250_l10: l10 {
> >> + /* Wi-Fi RFA */
> >> + regulator-min-microvolts = <1300000>;
> >> + regulator-max-microvolts = <1300000>;
> >> + };
> >> +
> >> + pm2250_l11: l11 {
> >> + /* GPS RF1 */
> >> + regulator-min-microvolts = <1000000>;
> >> + regulator-max-microvolts = <1000000>;
> >> + regulator-boot-on;
> >
> > Shouldn't it be handled by the modem on its own?
> >
> >> + };
> >> +
> >> + pm2250_l12: l12 {
> >> + /* USB PHYs */
> >> + regulator-min-microvolts = <928000>;
> >> + regulator-max-microvolts = <928000>;
> >> + regulator-allow-set-load;
> >> + regulator-boot-on;
> >
> > You guess the question (and further on)
> >
> >> + };
> >> +
> >> + pm2250_l13: l13 {
> >> + /* USB/QFPROM/PLLs */
> >> + regulator-min-microvolts = <1800000>;
> >> + regulator-max-microvolts = <1800000>;
> >> + regulator-allow-set-load;
> >> + regulator-boot-on;
> >> + };
> >> +
> >> + pm2250_l14: l14 {
> >> + /* SDHCI1 VQMMC */
> >> + regulator-min-microvolts = <1800000>;
> >> + regulator-max-microvolts = <1800000>;
> >> + regulator-allow-set-load;
> >> + /* Broken hardware, never turn it off! */
> >> + regulator-always-on;
> >> + };
> >> +
> >> + pm2250_l15: l15 {
> >> + /* WCD/DSI/BT VDDIO */
> >> + regulator-min-microvolts = <1800000>;
> >> + regulator-max-microvolts = <1800000>;
> >> + regulator-allow-set-load;
> >> + regulator-always-on;
> >> + regulator-boot-on;
> >> + };
> >> +
> >> + pm2250_l16: l16 {
> >> + /* GPS RF2 */
> >> + regulator-min-microvolts = <1800000>;
> >> + regulator-max-microvolts = <1800000>;
> >> + regulator-boot-on;
> >> + };
> >> +
> >> + pm2250_l17: l17 {
> >> + regulator-min-microvolts = <3000000>;
> >> + regulator-max-microvolts = <3000000>;
> >> + };
> >> +
> >> + pm2250_l18: l18 {
> >> + /* VDD_PXn */
> >> + regulator-min-microvolts = <1800000>;
> >> + regulator-max-microvolts = <1800000>;
> >> + };
> >> +
> >> + pm2250_l19: l19 {
> >> + /* VDD_PXn */
> >> + regulator-min-microvolts = <1800000>;
> >> + regulator-max-microvolts = <1800000>;
> >> + };
> >> +
> >> + pm2250_l20: l20 {
> >> + /* SDHCI1 VMMC */
> >> + regulator-min-microvolts = <2856000>;
> >> + regulator-max-microvolts = <2856000>;
> >> + regulator-allow-set-load;
> >> + };
> >> +
> >> + pm2250_l21: l21 {
> >> + /* SDHCI2 VMMC */
> >> + regulator-min-microvolts = <2960000>;
> >> + regulator-max-microvolts = <3300000>;
> >> + regulator-allow-set-load;
> >> + regulator-boot-on;
> >> + };
> >> +
> >> + pm2250_l22: l22 {
> >> + /* Wi-Fi */
> >> + regulator-min-microvolts = <3312000>;
> >> + regulator-max-microvolts = <3312000>;
> >> + };
> >> + };
> >> +};
> >> +
> >> &sdhc_1 {
> >> + vmmc-supply = <&pm2250_l20>;
> >> + vqmmc-supply = <&pm2250_l14>;
> >> pinctrl-0 = <&sdc1_state_on>;
> >> pinctrl-1 = <&sdc1_state_off>;
> >> pinctrl-names = "default", "sleep";
> >> @@ -61,6 +322,8 @@ &sdhc_1 {
> >> };
> >>
> >> &sdhc_2 {
> >> + vmmc-supply = <&pm2250_l21>;
> >> + vqmmc-supply = <&pm2250_l4>;
> >> cd-gpios = <&tlmm 88 GPIO_ACTIVE_LOW>;
> >> pinctrl-0 = <&sdc2_state_on &sd_det_in_on>;
> >> pinctrl-1 = <&sdc2_state_off &sd_det_in_off>;
> >> @@ -104,6 +367,9 @@ &usb {
> >> };
> >>
> >> &usb_hsphy {
> >> + vdd-supply = <&pm2250_l12>;
> >> + vdda-pll-supply = <&pm2250_l13>;
> >> + vdda-phy-dpdm-supply = <&pm2250_l21>;
> >> status = "okay";
> >> };
> >>
> >>
> >> ---
> >> base-commit: 8e4b7f2f3d6071665b1dfd70786229c8a5d6c256
> >> change-id: 20230711-topic-rb1_regulator-44c4ade93246
> >>
> >> Best regards,
> >> --
> >> Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> >>
> >
> >



--
With best wishes
Dmitry