| Message ID | 20221118073017.26128-2-quic_rjendra@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] dt-bindings: arm: qcom: Document the sc7280 CRD Pro boards | expand |
Hi Rajendra, On Fri, Nov 18, 2022 at 01:00:17PM +0530, Rajendra Nayak wrote: > Subject: arm64: dts: qcom: sc7280: Add a new herobrine Pro SKU nit: this adds the herobrine *CRD* Pro SKU (though other Pro SKUs might follow), so 'CRD' should be part of the subject uber-nit: 'new' is redundant in this context > Some of the qualcomm qcard based herobrine devices can come with > a Pro variant of the chipset with some qcard level changes like > the smps9 from pm8350c which is ganged up with smps7 and smps8, > so we just end up removing smps9 from the herobrine pro sku dtsi. This is a very long sentence :) > We then use it to create a new dts for the Pro variant of the > herobrine CRD. Using 'we' is a a bit colloquial for a commit message, how a about something like this: 'Add a .dtsi for pro skus that deletes the smps9 node and include it from the new dts for the CRD Pro'. > Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > .../dts/qcom/sc7280-herobrine-crd-pro.dts | 35 +++++++++++++++++++ > .../dts/qcom/sc7280-herobrine-pro-sku.dtsi | 8 +++++ > 3 files changed, 44 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts > create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index afe496a93f94..c5ac51c3a383 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -108,6 +108,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-wormdingler-rev1-boe-rt5682s.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-r1.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-r1-lte.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-crd.dtb > +dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-crd-pro.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-evoker.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-evoker-lte.dtb > dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-herobrine-r1.dtb > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts > new file mode 100644 > index 000000000000..fe6b228e9e4b > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts > @@ -0,0 +1,35 @@ > +// SPDX-License-Identifier: BSD-3-Clause > +/* > + * sc7280 CRD 3+ Pro board device tree source > + * > + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include "sc7280-herobrine-crd.dts" > +#include "sc7280-herobrine-pro-sku.dtsi" > + > +/ { > + model = "Qualcomm Technologies, Inc. sc7280 CRD Pro platform (rev5+)"; > + compatible = "google,hoglin-sku1536", "qcom,sc7280"; > + > + /* FIXED REGULATORS */ > + > + /* > + * On most herobrine boards PPVAR_SYS directly provides VREG_EDP_BL. > + * However, on CRD there's an extra regulator in the way. Since this > + * is expected to be uncommon, we'll leave the "vreg_edp_bl" label > + * in the baseboard herobrine.dtsi point at "ppvar_sys" and then > + * make a "_crd" specific version here. > + */ > + vreg_edp_bl_crd: vreg-edp-bl-crd-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "vreg_edp_bl_crd"; > + > + gpio = <&pm8350c_gpios 6 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + pinctrl-names = "default"; > + pinctrl-0 = <&edp_bl_reg_en>; > + > + vin-supply = <&ppvar_sys>; > + }; > +}; Why is this node needed here, doesn't it already exist by including 'sc7280-herobrine-crd.dts'? > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi > new file mode 100644 > index 000000000000..fb4bbe8aeda0 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi > @@ -0,0 +1,8 @@ > +// SPDX-License-Identifier: BSD-3-Clause > +/* > + * Google Herobrine dts fragment for PRO SKUs > + * > + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +/delete-node/ &vreg_s9c_0p676; > -- > 2.17.1 >
On 11/18/2022 8:58 PM, Matthias Kaehlcke wrote: > Hi Rajendra, > > On Fri, Nov 18, 2022 at 01:00:17PM +0530, Rajendra Nayak wrote: > >> Subject: arm64: dts: qcom: sc7280: Add a new herobrine Pro SKU > > nit: this adds the herobrine *CRD* Pro SKU (though other Pro SKUs > might follow), so 'CRD' should be part of the subject > > uber-nit: 'new' is redundant in this context > >> Some of the qualcomm qcard based herobrine devices can come with >> a Pro variant of the chipset with some qcard level changes like >> the smps9 from pm8350c which is ganged up with smps7 and smps8, >> so we just end up removing smps9 from the herobrine pro sku dtsi. > > This is a very long sentence :) > >> We then use it to create a new dts for the Pro variant of the >> herobrine CRD. > > Using 'we' is a a bit colloquial for a commit message, how a about > something like this: 'Add a .dtsi for pro skus that deletes the > smps9 node and include it from the new dts for the CRD Pro'. Done, send a v2 with the changes. > >> Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com> >> --- [].. >> +#include "sc7280-herobrine-crd.dts" >> +#include "sc7280-herobrine-pro-sku.dtsi" >> + >> +/ { >> + model = "Qualcomm Technologies, Inc. sc7280 CRD Pro platform (rev5+)"; >> + compatible = "google,hoglin-sku1536", "qcom,sc7280"; >> + >> + /* FIXED REGULATORS */ >> + >> + /* >> + * On most herobrine boards PPVAR_SYS directly provides VREG_EDP_BL. >> + * However, on CRD there's an extra regulator in the way. Since this >> + * is expected to be uncommon, we'll leave the "vreg_edp_bl" label >> + * in the baseboard herobrine.dtsi point at "ppvar_sys" and then >> + * make a "_crd" specific version here. >> + */ >> + vreg_edp_bl_crd: vreg-edp-bl-crd-regulator { >> + compatible = "regulator-fixed"; >> + regulator-name = "vreg_edp_bl_crd"; >> + >> + gpio = <&pm8350c_gpios 6 GPIO_ACTIVE_HIGH>; >> + enable-active-high; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&edp_bl_reg_en>; >> + >> + vin-supply = <&ppvar_sys>; >> + }; >> +}; > > Why is this node needed here, doesn't it already exist by including > 'sc7280-herobrine-crd.dts'? yes, its not needed, removed it in v2. thanks, Rajendra
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile index afe496a93f94..c5ac51c3a383 100644 --- a/arch/arm64/boot/dts/qcom/Makefile +++ b/arch/arm64/boot/dts/qcom/Makefile @@ -108,6 +108,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-wormdingler-rev1-boe-rt5682s.dtb dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-r1.dtb dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-r1-lte.dtb dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-crd.dtb +dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-crd-pro.dtb dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-evoker.dtb dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-evoker-lte.dtb dtb-$(CONFIG_ARCH_QCOM) += sc7280-herobrine-herobrine-r1.dtb diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts new file mode 100644 index 000000000000..fe6b228e9e4b --- /dev/null +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: BSD-3-Clause +/* + * sc7280 CRD 3+ Pro board device tree source + * + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include "sc7280-herobrine-crd.dts" +#include "sc7280-herobrine-pro-sku.dtsi" + +/ { + model = "Qualcomm Technologies, Inc. sc7280 CRD Pro platform (rev5+)"; + compatible = "google,hoglin-sku1536", "qcom,sc7280"; + + /* FIXED REGULATORS */ + + /* + * On most herobrine boards PPVAR_SYS directly provides VREG_EDP_BL. + * However, on CRD there's an extra regulator in the way. Since this + * is expected to be uncommon, we'll leave the "vreg_edp_bl" label + * in the baseboard herobrine.dtsi point at "ppvar_sys" and then + * make a "_crd" specific version here. + */ + vreg_edp_bl_crd: vreg-edp-bl-crd-regulator { + compatible = "regulator-fixed"; + regulator-name = "vreg_edp_bl_crd"; + + gpio = <&pm8350c_gpios 6 GPIO_ACTIVE_HIGH>; + enable-active-high; + pinctrl-names = "default"; + pinctrl-0 = <&edp_bl_reg_en>; + + vin-supply = <&ppvar_sys>; + }; +}; diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi new file mode 100644 index 000000000000..fb4bbe8aeda0 --- /dev/null +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: BSD-3-Clause +/* + * Google Herobrine dts fragment for PRO SKUs + * + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +/delete-node/ &vreg_s9c_0p676;
Some of the qualcomm qcard based herobrine devices can come with a Pro variant of the chipset with some qcard level changes like the smps9 from pm8350c which is ganged up with smps7 and smps8, so we just end up removing smps9 from the herobrine pro sku dtsi. We then use it to create a new dts for the Pro variant of the herobrine CRD. Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com> --- arch/arm64/boot/dts/qcom/Makefile | 1 + .../dts/qcom/sc7280-herobrine-crd-pro.dts | 35 +++++++++++++++++++ .../dts/qcom/sc7280-herobrine-pro-sku.dtsi | 8 +++++ 3 files changed, 44 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-crd-pro.dts create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-pro-sku.dtsi