| Message ID | 20210607113840.15435-1-bhupesh.sharma@linaro.org |
|---|---|
| Headers | show |
| Series | arm64: dts: qcom: Add SA8155p-adp board DTS | expand |
Hi Vinod, Thanks for your review. On Mon, 7 Jun 2021 at 20:52, Vinod Koul <vkoul@kernel.org> wrote: > > On 07-06-21, 17:08, Bhupesh Sharma wrote: > > Add base DTS file for sa8155p-adp and enable boot to console, > > tlmm reserved range and also include pmic file(s). > > I see ufs added too, pls mention that as well Oops, missed that. Will fix it in v2. > --- a/arch/arm64/boot/dts/qcom/Makefile > > +++ b/arch/arm64/boot/dts/qcom/Makefile > > @@ -71,6 +71,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-xiaomi-beryllium.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm850-lenovo-yoga-c630.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sm8150-hdk.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sm8150-mtp.dtb > > +dtb-$(CONFIG_ARCH_QCOM) += sa8155p-adp.dtb > > I think this should go before sdm.. Oh, ok, I thought of keeping all boards based on sm8150 SoC together. But alphabetically, it makes more sense to put it earlier. > > + vdd_usb_hs_core: > > + vdda_pll_hv_cc_ebi01: > > + vdda_pll_hv_cc_ebi23: > > + vdda_ufs_2ln_core: > > + vdda_ufs_2ln_core: > > + vdda_usb_ss_core: > > + vdda_usb_ss_dp_core_1: > > + vdda_usb_ss_dp_core_2: > > + vdda_sp_sensor: > > + vdda_qlink_lv: > > + vdda_qlink_lv_ck: > > + vdda_qrefs_0p875_5: > > I didnt find these labels very useful, so maybe remove? > It helped me to understand that a regulator is vreg_l5a_0p88 as it > implies I am using l5a with 0p88V :) While a few labels like 'vdd_usb_hs_core' are used in this patch (for example) to denote 'vdda-pll-supply ' of 'usb_1_hsphy', the others would be required as we enable further on-boards peripherals in the dts. I will recheck and limit these further in v2. > > + vreg_l5a_0p88: ldo5 { > > + regulator-min-microvolt = <880000>; > > + regulator-max-microvolt = <880000>; > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > Pls do add regulator-name property, it helps in understanding which ldo > in logs/debugfs, otherwise ldo5 will comes for both pmics That's a good point. Will fix this in v2. Regards, Bhupesh > -- > ~Vinod
On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > Add base DTS file for sa8155p-adp and enable boot to console, Please spell out "sa8155-adp", i.e. "Add base DTS for SA8155p Automotive Development Platform." > tlmm reserved range and also include pmic file(s). > > SA8155p-adp board is based on sm8150 Qualcomm Snapdragon SoC. > It's not based on sm8150, it's based on sa8155p, so let's express this as "The SA8155p platform is similar to the SM8150, so use this as base for now", to document why we decided to do this. > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Andy Gross <agross@kernel.org> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-gpio@vger.kernel.org > Cc: bhupesh.linux@gmail.com This would go into the git history as "I specifically asked for input from these people", so please keep this list shorter (but for a change like this it's probably better to omit it completely) > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 363 +++++++++++++++++++++++ > 2 files changed, 364 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sa8155p-adp.dts > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index 456502aeee49..38d3a4728871 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -71,6 +71,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-xiaomi-beryllium.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm850-lenovo-yoga-c630.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8150-hdk.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8150-mtp.dtb > +dtb-$(CONFIG_ARCH_QCOM) += sa8155p-adp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8250-hdk.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8250-mtp.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm8350-hdk.dtb > diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts > new file mode 100644 > index 000000000000..470d740e060a > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts > @@ -0,0 +1,363 @@ > +// SPDX-License-Identifier: BSD-3-Clause > +/* > + * Copyright (c) 2021, Linaro Limited > + */ > + > +/dts-v1/; > + > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > +#include <dt-bindings/gpio/gpio.h> > +#include "sm8150.dtsi" > +#include "pmm8155au_1.dtsi" > +#include "pmm8155au_2.dtsi" > + > +/ { > + model = "Qualcomm Technologies, Inc. SA8155P ADP"; > + compatible = "qcom,sa8155p-adp"; > + > + aliases { > + serial0 = &uart2; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + vreg_3p3: vreg_3p3_regulator { > + compatible = "regulator-fixed"; > + regulator-name = "vreg_3p3"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + }; > + > + /* > + * Apparently RPMh does not provide support for PM8150 S4 because it > + * is always-on; model it as a fixed regulator. > + */ You can reduce this to /* S4A is always on and not controllable through RPMh */ > + vreg_s4a_1p8: smps4 { > + compatible = "regulator-fixed"; > + regulator-name = "vreg_s4a_1p8"; > + > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + > + regulator-always-on; > + regulator-boot-on; > + > + vin-supply = <&vreg_3p3>; > + }; > +}; > + > +&apps_rsc { > + pmm8155au-1-rpmh-regulators { > + compatible = "qcom,pmm8155au-1-rpmh-regulators"; > + qcom,pmic-id = "a"; > + > + vdd-s1-supply = <&vreg_3p3>; > + vdd-s2-supply = <&vreg_3p3>; > + vdd-s3-supply = <&vreg_3p3>; > + vdd-s4-supply = <&vreg_3p3>; > + vdd-s5-supply = <&vreg_3p3>; > + vdd-s6-supply = <&vreg_3p3>; > + vdd-s7-supply = <&vreg_3p3>; > + vdd-s8-supply = <&vreg_3p3>; > + vdd-s9-supply = <&vreg_3p3>; > + vdd-s10-supply = <&vreg_3p3>; > + > + vdd-l1-l8-l11-supply = <&vreg_s6a_0p92>; > + vdd-l2-l10-supply = <&vreg_3p3>; > + vdd-l3-l4-l5-l18-supply = <&vreg_s6a_0p92>; > + vdd-l6-l9-supply = <&vreg_s6a_0p92>; > + vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>; > + vdd-l13-l16-l17-supply = <&vreg_3p3>; > + > + vreg_s5a_2p04: smps5 { > + regulator-min-microvolt = <1904000>; > + regulator-max-microvolt = <2000000>; > + }; > + > + vreg_s6a_0p92: smps6 { > + regulator-min-microvolt = <920000>; > + regulator-max-microvolt = <1128000>; > + }; > + > + vdda_wcss_pll: This is the "label" of the pad which the regulator typically is connected to (rather than a denotion of which regulator it is). So even though we have these in some of the other boards, I would prefer if you skip them and only use the vreg_xyz_abc variant. > + vreg_l1a_0p752: ldo1 { > + regulator-min-microvolt = <752000>; > + regulator-max-microvolt = <752000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + }; [..] > +&usb_1_dwc3 { > + dr_mode = "peripheral"; We have enough pieces to handle mode switching on this platform, but as discussed, lets leave it as "peripheral" until your local setup is back online. Thanks, Bjorn > +}; > + > +&qupv3_id_1 { > + status = "okay"; > +}; > -- > 2.31.1 >
On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > Add compatible strings for pmm8155au_1 and pmm8155au_2 pmics > found on SA8155p-adp board. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Andy Gross <agross@kernel.org> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-gpio@vger.kernel.org > Cc: bhupesh.linux@gmail.com > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > .../devicetree/bindings/regulator/qcom,rpmh-regulator.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > index e561a5b941e4..ea5cd71aa0c7 100644 > --- a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > @@ -55,6 +55,8 @@ properties: > - qcom,pm8009-1-rpmh-regulators > - qcom,pm8150-rpmh-regulators > - qcom,pm8150l-rpmh-regulators > + - qcom,pmm8155au-1-rpmh-regulators > + - qcom,pmm8155au-2-rpmh-regulators Looking at the component documentation and the schematics I think the component is "PMM8155AU" and we have two of them. Unless I'm mistaken we should have the compatible describe the single component and we should have DT describe the fact that we have 2 of them. Regards, Bjorn > - qcom,pm8350-rpmh-regulators > - qcom,pm8350c-rpmh-regulators > - qcom,pm8998-rpmh-regulators > -- > 2.31.1 >
On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > SA8155p-adp board is based on Qualcomm Snapdragon sm8150 > SoC. > > Add support for the same. The SA8155p is similar to SM8150 and we can reuse most things, but I think we can afford to add qcom,sa8155p in the DT bindings. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Andy Gross <agross@kernel.org> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-gpio@vger.kernel.org > Cc: bhupesh.linux@gmail.com > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > Documentation/devicetree/bindings/arm/qcom.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml > index 9b27e991bddc..b5897f1f9695 100644 > --- a/Documentation/devicetree/bindings/arm/qcom.yaml > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml > @@ -42,11 +42,13 @@ description: | > sdm660 > sdm845 > sdx55 > + sm8150 Naturally sm8150 should be part of this list, but please also add sa8155p as well. > sm8250 > sm8350 > > The 'board' element must be one of the following strings: > > + adp > cdp > cp01-c1 > dragonboard > @@ -198,6 +200,12 @@ properties: > - qcom,ipq6018-cp01-c1 > - const: qcom,ipq6018 > > + - items: > + - enum: > + - qcom,sa8155p-adp > + - qcom,sm8150-mtp > + - const: qcom,sm8150 And please split this in two (one qcom,sm8150-mtp and qcom,sm8150, and one qcom,sa8155p-adp and qcom,sa8155p). And note that this is saying that your compatible needs to be one of the enum entries, followed by the const, but in your dts you only specified qcom,sa8155p-adp. It needs to be: compatible = "qcom,sa8155p-adp", "qcom,sa8155p"; Thanks, Bjorn > + > - items: > - enum: > - qcom,qrb5165-rb5 > -- > 2.31.1 >
On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > Add base DTS file for pmm8155au_1 along with GPIOs, power-on, rtc and vadc > nodes. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Andy Gross <agross@kernel.org> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-gpio@vger.kernel.org > Cc: bhupesh.linux@gmail.com > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi | 134 ++++++++++++++++++++++ > 1 file changed, 134 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi > > diff --git a/arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi b/arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi As we describe our PMICs by including their definition to the top level of the .dts I don't see any alternative to duplicating this as _1 and _2. So let's go with this structure. [..] > + > +&spmi_bus { > + pmm8155au_1_0: pmic@0 { I don't think you need to give this a label. > + compatible = "qcom,pmm8155au-1", "qcom,spmi-pmic"; This is a "qcom,pmm8155au", "qcom,spmi-pmic", the labels are used to differentiate the two instances. > + reg = <0x0 SPMI_USID>; > + #address-cells = <1>; > + #size-cells = <0>; > + [..] > + > + pmm8155au_1_gpios: gpio@c000 { > + compatible = "qcom,pmm8155au-1-gpio"; "qcom,pmm8155au-gpio" > + reg = <0xc000>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + }; > + > + pmic@1 { > + compatible = "qcom,pmm8155au-1", "qcom,spmi-pmic"; "qcom,pmm8155au" Thanks, Bjorn > + reg = <0x1 SPMI_USID>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > +}; > -- > 2.31.1 >
Hello Bjorn, Thanks for the review comments. On Fri, 11 Jun 2021 at 08:18, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > > > Add compatible strings for pmm8155au_1 and pmm8155au_2 pmics > > found on SA8155p-adp board. > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Cc: Liam Girdwood <lgirdwood@gmail.com> > > Cc: Mark Brown <broonie@kernel.org> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Vinod Koul <vkoul@kernel.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Andy Gross <agross@kernel.org> > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-gpio@vger.kernel.org > > Cc: bhupesh.linux@gmail.com > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > .../devicetree/bindings/regulator/qcom,rpmh-regulator.yaml | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > index e561a5b941e4..ea5cd71aa0c7 100644 > > --- a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > @@ -55,6 +55,8 @@ properties: > > - qcom,pm8009-1-rpmh-regulators > > - qcom,pm8150-rpmh-regulators > > - qcom,pm8150l-rpmh-regulators > > + - qcom,pmm8155au-1-rpmh-regulators > > + - qcom,pmm8155au-2-rpmh-regulators > > Looking at the component documentation and the schematics I think the > component is "PMM8155AU" and we have two of them. > > Unless I'm mistaken we should have the compatible describe the single > component and we should have DT describe the fact that we have 2 of > them. If we refer to the PM8155AU device specifications, there are two regulators mentioned there PMM8155AU_1 and PMM8155AU_2. Although most parameters of the regulators seem similar the smps regulator summary for both appear different (Transient Load, mA ratings etc). Although most of these differences don't probably matter to the Linux world, others like the gpios on the pmic are different. So, IMO, it makes sense to mention the different pmic types on the board. Please let me know your views on the same. Thanks, Bhupesh > > > - qcom,pm8350-rpmh-regulators > > - qcom,pm8350c-rpmh-regulators > > - qcom,pm8998-rpmh-regulators > > -- > > 2.31.1 > >
Hello Bjorn, On Fri, 11 Jun 2021 at 08:29, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > > > SA8155p-adp board is based on Qualcomm Snapdragon sm8150 > > SoC. > > > > Add support for the same. > > The SA8155p is similar to SM8150 and we can reuse most things, but I > think we can afford to add qcom,sa8155p in the DT bindings. Sure will do the same in v2. > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Cc: Liam Girdwood <lgirdwood@gmail.com> > > Cc: Mark Brown <broonie@kernel.org> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Vinod Koul <vkoul@kernel.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Andy Gross <agross@kernel.org> > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-gpio@vger.kernel.org > > Cc: bhupesh.linux@gmail.com > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > Documentation/devicetree/bindings/arm/qcom.yaml | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml > > index 9b27e991bddc..b5897f1f9695 100644 > > --- a/Documentation/devicetree/bindings/arm/qcom.yaml > > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml > > @@ -42,11 +42,13 @@ description: | > > sdm660 > > sdm845 > > sdx55 > > + sm8150 > > Naturally sm8150 should be part of this list, but please also add > sa8155p as well. Ok. > > sm8250 > > sm8350 > > > > The 'board' element must be one of the following strings: > > > > + adp > > cdp > > cp01-c1 > > dragonboard > > @@ -198,6 +200,12 @@ properties: > > - qcom,ipq6018-cp01-c1 > > - const: qcom,ipq6018 > > > > + - items: > > + - enum: > > + - qcom,sa8155p-adp > > + - qcom,sm8150-mtp > > + - const: qcom,sm8150 > > And please split this in two (one qcom,sm8150-mtp and qcom,sm8150, and > one qcom,sa8155p-adp and qcom,sa8155p). > > And note that this is saying that your compatible needs to be one of the > enum entries, followed by the const, but in your dts you only specified > qcom,sa8155p-adp. It needs to be: > > compatible = "qcom,sa8155p-adp", "qcom,sa8155p"; Sure will do the same in v2. Regards, Bhupesh > > + > > - items: > > - enum: > > - qcom,qrb5165-rb5 > > -- > > 2.31.1 > >
Hello Bjorn, On Fri, 11 Jun 2021 at 07:55, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > > > Add base DTS file for sa8155p-adp and enable boot to console, > > Please spell out "sa8155-adp", i.e. "Add base DTS for SA8155p Automotive > Development Platform." Ok, will do. > > tlmm reserved range and also include pmic file(s). > > > > SA8155p-adp board is based on sm8150 Qualcomm Snapdragon SoC. > > > > It's not based on sm8150, it's based on sa8155p, so let's express this > as "The SA8155p platform is similar to the SM8150, so use this as base > for now", to document why we decided to do this. > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Cc: Liam Girdwood <lgirdwood@gmail.com> > > Cc: Mark Brown <broonie@kernel.org> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: Vinod Koul <vkoul@kernel.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Andy Gross <agross@kernel.org> > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-gpio@vger.kernel.org > > Cc: bhupesh.linux@gmail.com > > This would go into the git history as "I specifically asked for input > from these people", so please keep this list shorter (but for a change > like this it's probably better to omit it completely) Ok, will keep it shorter for future series. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > arch/arm64/boot/dts/qcom/Makefile | 1 + > > arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 363 +++++++++++++++++++++++ > > 2 files changed, 364 insertions(+) > > create mode 100644 arch/arm64/boot/dts/qcom/sa8155p-adp.dts > > > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > > index 456502aeee49..38d3a4728871 100644 > > --- a/arch/arm64/boot/dts/qcom/Makefile > > +++ b/arch/arm64/boot/dts/qcom/Makefile > > @@ -71,6 +71,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-xiaomi-beryllium.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sdm850-lenovo-yoga-c630.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sm8150-hdk.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sm8150-mtp.dtb > > +dtb-$(CONFIG_ARCH_QCOM) += sa8155p-adp.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sm8250-hdk.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sm8250-mtp.dtb > > dtb-$(CONFIG_ARCH_QCOM) += sm8350-hdk.dtb > > diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts > > new file mode 100644 > > index 000000000000..470d740e060a > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts > > @@ -0,0 +1,363 @@ > > +// SPDX-License-Identifier: BSD-3-Clause > > +/* > > + * Copyright (c) 2021, Linaro Limited > > + */ > > + > > +/dts-v1/; > > + > > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > > +#include <dt-bindings/gpio/gpio.h> > > +#include "sm8150.dtsi" > > +#include "pmm8155au_1.dtsi" > > +#include "pmm8155au_2.dtsi" > > + > > +/ { > > + model = "Qualcomm Technologies, Inc. SA8155P ADP"; > > + compatible = "qcom,sa8155p-adp"; > > + > > + aliases { > > + serial0 = &uart2; > > + }; > > + > > + chosen { > > + stdout-path = "serial0:115200n8"; > > + }; > > + > > + vreg_3p3: vreg_3p3_regulator { > > + compatible = "regulator-fixed"; > > + regulator-name = "vreg_3p3"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + }; > > + > > + /* > > + * Apparently RPMh does not provide support for PM8150 S4 because it > > + * is always-on; model it as a fixed regulator. > > + */ > > You can reduce this to > > /* S4A is always on and not controllable through RPMh */ > Ok, I wanted to keep it similar to the comment we have for sm815o-mtp, but this is fine as well. > > + vreg_s4a_1p8: smps4 { > > + compatible = "regulator-fixed"; > > + regulator-name = "vreg_s4a_1p8"; > > + > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + > > + regulator-always-on; > > + regulator-boot-on; > > + > > + vin-supply = <&vreg_3p3>; > > + }; > > +}; > > + > > +&apps_rsc { > > + pmm8155au-1-rpmh-regulators { > > + compatible = "qcom,pmm8155au-1-rpmh-regulators"; > > + qcom,pmic-id = "a"; > > + > > + vdd-s1-supply = <&vreg_3p3>; > > + vdd-s2-supply = <&vreg_3p3>; > > + vdd-s3-supply = <&vreg_3p3>; > > + vdd-s4-supply = <&vreg_3p3>; > > + vdd-s5-supply = <&vreg_3p3>; > > + vdd-s6-supply = <&vreg_3p3>; > > + vdd-s7-supply = <&vreg_3p3>; > > + vdd-s8-supply = <&vreg_3p3>; > > + vdd-s9-supply = <&vreg_3p3>; > > + vdd-s10-supply = <&vreg_3p3>; > > + > > + vdd-l1-l8-l11-supply = <&vreg_s6a_0p92>; > > + vdd-l2-l10-supply = <&vreg_3p3>; > > + vdd-l3-l4-l5-l18-supply = <&vreg_s6a_0p92>; > > + vdd-l6-l9-supply = <&vreg_s6a_0p92>; > > + vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>; > > + vdd-l13-l16-l17-supply = <&vreg_3p3>; > > + > > + vreg_s5a_2p04: smps5 { > > + regulator-min-microvolt = <1904000>; > > + regulator-max-microvolt = <2000000>; > > + }; > > + > > + vreg_s6a_0p92: smps6 { > > + regulator-min-microvolt = <920000>; > > + regulator-max-microvolt = <1128000>; > > + }; > > + > > + vdda_wcss_pll: > > This is the "label" of the pad which the regulator typically is > connected to (rather than a denotion of which regulator it is). So even > though we have these in some of the other boards, I would prefer if you > skip them and only use the vreg_xyz_abc variant. Ok. > > + vreg_l1a_0p752: ldo1 { > > + regulator-min-microvolt = <752000>; > > + regulator-max-microvolt = <752000>; > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > + }; > [..] > > +&usb_1_dwc3 { > > + dr_mode = "peripheral"; > > We have enough pieces to handle mode switching on this platform, but as > discussed, lets leave it as "peripheral" until your local setup is back > online. Sure, in later patches, I can try playing more with this configuration. Regards, Bhupesh
On Mon 14 Jun 03:05 CDT 2021, Bhupesh Sharma wrote: > Hello Bjorn, > > Thanks for the review comments. > > On Fri, 11 Jun 2021 at 08:18, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > > > > > Add compatible strings for pmm8155au_1 and pmm8155au_2 pmics > > > found on SA8155p-adp board. > > > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > Cc: Liam Girdwood <lgirdwood@gmail.com> > > > Cc: Mark Brown <broonie@kernel.org> > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > > Cc: Vinod Koul <vkoul@kernel.org> > > > Cc: Rob Herring <robh+dt@kernel.org> > > > Cc: Andy Gross <agross@kernel.org> > > > Cc: devicetree@vger.kernel.org > > > Cc: linux-kernel@vger.kernel.org > > > Cc: linux-gpio@vger.kernel.org > > > Cc: bhupesh.linux@gmail.com > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > > --- > > > .../devicetree/bindings/regulator/qcom,rpmh-regulator.yaml | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > > index e561a5b941e4..ea5cd71aa0c7 100644 > > > --- a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > > +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > > @@ -55,6 +55,8 @@ properties: > > > - qcom,pm8009-1-rpmh-regulators > > > - qcom,pm8150-rpmh-regulators > > > - qcom,pm8150l-rpmh-regulators > > > + - qcom,pmm8155au-1-rpmh-regulators > > > + - qcom,pmm8155au-2-rpmh-regulators > > > > Looking at the component documentation and the schematics I think the > > component is "PMM8155AU" and we have two of them. > > > > Unless I'm mistaken we should have the compatible describe the single > > component and we should have DT describe the fact that we have 2 of > > them. > > If we refer to the PM8155AU device specifications, there are two > regulators mentioned there PMM8155AU_1 and PMM8155AU_2. Although most > parameters of the regulators seem similar the smps regulator summary > for both appear different (Transient Load, mA ratings etc). > > Although most of these differences don't probably matter to the Linux > world, others like the gpios on the pmic are different. > > So, IMO, it makes sense to mention the different pmic types on the board. > > Please let me know your views on the same. > Afaict, they are both physically the same component, but there is some configuration differences between them. I don't see any differences that will show up in Linux, but afaict we would capture those in the DT anyways. Let me know if you see anything I'm missing, but I think we should have a single compatible. Regards, Bjorn > Thanks, > Bhupesh > > > > > > - qcom,pm8350-rpmh-regulators > > > - qcom,pm8350c-rpmh-regulators > > > - qcom,pm8998-rpmh-regulators > > > -- > > > 2.31.1 > > >
On Mon, 14 Jun 2021 at 21:58, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Mon 14 Jun 03:05 CDT 2021, Bhupesh Sharma wrote: > > > Hello Bjorn, > > > > Thanks for the review comments. > > > > On Fri, 11 Jun 2021 at 08:18, Bjorn Andersson > > <bjorn.andersson@linaro.org> wrote: > > > > > > On Mon 07 Jun 06:38 CDT 2021, Bhupesh Sharma wrote: > > > > > > > Add compatible strings for pmm8155au_1 and pmm8155au_2 pmics > > > > found on SA8155p-adp board. > > > > > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > > Cc: Liam Girdwood <lgirdwood@gmail.com> > > > > Cc: Mark Brown <broonie@kernel.org> > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > Cc: Vinod Koul <vkoul@kernel.org> > > > > Cc: Rob Herring <robh+dt@kernel.org> > > > > Cc: Andy Gross <agross@kernel.org> > > > > Cc: devicetree@vger.kernel.org > > > > Cc: linux-kernel@vger.kernel.org > > > > Cc: linux-gpio@vger.kernel.org > > > > Cc: bhupesh.linux@gmail.com > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > > > --- > > > > .../devicetree/bindings/regulator/qcom,rpmh-regulator.yaml | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > > > index e561a5b941e4..ea5cd71aa0c7 100644 > > > > --- a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > > > +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > > > @@ -55,6 +55,8 @@ properties: > > > > - qcom,pm8009-1-rpmh-regulators > > > > - qcom,pm8150-rpmh-regulators > > > > - qcom,pm8150l-rpmh-regulators > > > > + - qcom,pmm8155au-1-rpmh-regulators > > > > + - qcom,pmm8155au-2-rpmh-regulators > > > > > > Looking at the component documentation and the schematics I think the > > > component is "PMM8155AU" and we have two of them. > > > > > > Unless I'm mistaken we should have the compatible describe the single > > > component and we should have DT describe the fact that we have 2 of > > > them. > > > > If we refer to the PM8155AU device specifications, there are two > > regulators mentioned there PMM8155AU_1 and PMM8155AU_2. Although most > > parameters of the regulators seem similar the smps regulator summary > > for both appear different (Transient Load, mA ratings etc). > > > > Although most of these differences don't probably matter to the Linux > > world, others like the gpios on the pmic are different. > > > > So, IMO, it makes sense to mention the different pmic types on the board. > > > > Please let me know your views on the same. > > > > Afaict, they are both physically the same component, but there is some > configuration differences between them. I don't see any differences that > will show up in Linux, but afaict we would capture those in the DT > anyways. > > Let me know if you see anything I'm missing, but I think we should have > a single compatible. As discussed on IRC, let's go with the approach you suggested (I can propose followup patches if I find something amiss). I will send a v2 shortly. Thanks, Bhupesh
This series adds DTS for SA8155p-adp board which is based on Qualcomm snapdragon sm8150 SoC. This patchset also includes DTS for the two new PMICs PMM8155AU_1 and PMM8155AU_2 found on the adp board. Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Vinod Koul <vkoul@kernel.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Andy Gross <agross@kernel.org> Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-gpio@vger.kernel.org Cc: bhupesh.linux@gmail.com Bhupesh Sharma (8): dt-bindings: qcom: rpmh-regulator: Add compatible for SA8155p-adp board pmics dt-bindings: pinctrl: qcom,pmic-gpio: Add compatible for SA8155p-adp dt-bindings: arm: qcom: Add compatible for SA8155p-adp board regulator: qcom-rpmh: Add new regulator types found on SA8155p adp board pinctrl: qcom/pinctrl-spmi-gpio: Add compatibles for pmic-gpios on SA8155p-adp arm64: dts: qcom: pmm8155au_1: Add base dts file arm64: dts: qcom: pmm8155au_2: Add base dts file arm64: dts: qcom: sa8155p-adp: Add base dts file .../devicetree/bindings/arm/qcom.yaml | 8 + .../bindings/pinctrl/qcom,pmic-gpio.txt | 5 + .../regulator/qcom,rpmh-regulator.yaml | 2 + arch/arm64/boot/dts/qcom/Makefile | 1 + arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi | 134 +++++++ arch/arm64/boot/dts/qcom/pmm8155au_2.dtsi | 107 +++++ arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 375 ++++++++++++++++++ drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 4 + drivers/regulator/qcom-rpmh-regulator.c | 72 ++++ 9 files changed, 708 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pmm8155au_1.dtsi create mode 100644 arch/arm64/boot/dts/qcom/pmm8155au_2.dtsi create mode 100644 arch/arm64/boot/dts/qcom/sa8155p-adp.dts -- 2.31.1