On Tue, 11 Jul 2023 at 14:41, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote: > > On 11.07.2023 12:56, Dmitry Baryshkov wrote: > > 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? > In the Linux implementation, yes. Ack. Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > Konrad > > > >> > >> 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