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

[Konrad Dybcio]

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.

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>
>>>>
>>>
>>>
> 
> 
>