Re: [PATCH v3 2/3] arm64: dts: qcom: sdm845-mtp: Add RPMh VRM/XOB regulators
On Wed 22 Aug 10:36 PDT 2018, Douglas Anderson wrote: > diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts > b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts [..] > + > + vdd_qusb_hs0: > + vdda_hp_pcie_core: > + vdda_mipi_csi0_0p9: > + vdda_mipi_csi1_0p9: > + vdda_mipi_csi2_0p9: > + vdda_mipi_dsi0_pll: > + vdda_mipi_dsi1_pll: > + vdda_qlink_lv: > + vdda_qlink_lv_ck: > + vdda_qrefs_0p875: > + vdda_pcie_core: > + vdda_pll_cc_ebi01: > + vdda_pll_cc_ebi23: > + vdda_sp_sensor: > + vdda_ufs1_core: > + vdda_ufs2_core: > + vdda_usb1_ss_core: > + vdda_usb2_ss_core: While I agree with Doug that it would be connect to use the input pin name to describe the supplies of the various block I don't like the level of indirection it results in when reading the dts. So I would prefer that any of these just reference vreg_l1a_0p875 directly. That said, the patch looks good. Reviewed-by: Bjorn Andersson Regards, Bjorn
Re: [PATCH v3 2/3] arm64: dts: qcom: sdm845-mtp: Add RPMh VRM/XOB regulators
On Wed 22 Aug 10:36 PDT 2018, Douglas Anderson wrote: > diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts > b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts [..] > + > + vdd_qusb_hs0: > + vdda_hp_pcie_core: > + vdda_mipi_csi0_0p9: > + vdda_mipi_csi1_0p9: > + vdda_mipi_csi2_0p9: > + vdda_mipi_dsi0_pll: > + vdda_mipi_dsi1_pll: > + vdda_qlink_lv: > + vdda_qlink_lv_ck: > + vdda_qrefs_0p875: > + vdda_pcie_core: > + vdda_pll_cc_ebi01: > + vdda_pll_cc_ebi23: > + vdda_sp_sensor: > + vdda_ufs1_core: > + vdda_ufs2_core: > + vdda_usb1_ss_core: > + vdda_usb2_ss_core: While I agree with Doug that it would be connect to use the input pin name to describe the supplies of the various block I don't like the level of indirection it results in when reading the dts. So I would prefer that any of these just reference vreg_l1a_0p875 directly. That said, the patch looks good. Reviewed-by: Bjorn Andersson Regards, Bjorn
Re: [PATCH v3 2/3] arm64: dts: qcom: sdm845-mtp: Add RPMh VRM/XOB regulators
Quoting Douglas Anderson (2018-08-22 10:36:28) > Add regulator devices for PMIC regulators managed via VRM and XOB > RPMh accelerators. > > A few notes here: > - Regulators are added directly to the board file. While it's true > that this will mean a bunch of copy/pasting for other boards that > are very similar, this is probably the right call since boards > could make changes to the way these regulators are hooked up and > trying to find a way to avoid duplication will result in some > confusing node overrides. > - Regulators that are hooked up to supply pins on the SoC are given > an alias matching the name of that pin (pin name comes from the > Qualcomm SoC "device specification" doc). > - Other regulator labels are based on the schematic. If there is > more than one logical name on the schematic for the same rail the > secondary names are also listed and should be referred to as > appropriate. > - Regulators all default to HPM mode w/ no ability to switch modes. > Future patches can switch things to LPM and possibly add > dynamic load switching if we have determined there's a benefit. > This should only be done for rails where we'll actually be able > to take advantage of the lower power modes so we don't need to > churn with lots of patches adding regulator_set_load() calls > to drivers. > > NOTE: This patch is loosely based on one originally shared to me by > David Collins. > > Signed-off-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3 2/3] arm64: dts: qcom: sdm845-mtp: Add RPMh VRM/XOB regulators
Quoting Douglas Anderson (2018-08-22 10:36:28) > Add regulator devices for PMIC regulators managed via VRM and XOB > RPMh accelerators. > > A few notes here: > - Regulators are added directly to the board file. While it's true > that this will mean a bunch of copy/pasting for other boards that > are very similar, this is probably the right call since boards > could make changes to the way these regulators are hooked up and > trying to find a way to avoid duplication will result in some > confusing node overrides. > - Regulators that are hooked up to supply pins on the SoC are given > an alias matching the name of that pin (pin name comes from the > Qualcomm SoC "device specification" doc). > - Other regulator labels are based on the schematic. If there is > more than one logical name on the schematic for the same rail the > secondary names are also listed and should be referred to as > appropriate. > - Regulators all default to HPM mode w/ no ability to switch modes. > Future patches can switch things to LPM and possibly add > dynamic load switching if we have determined there's a benefit. > This should only be done for rails where we'll actually be able > to take advantage of the lower power modes so we don't need to > churn with lots of patches adding regulator_set_load() calls > to drivers. > > NOTE: This patch is loosely based on one originally shared to me by > David Collins. > > Signed-off-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd
