[2/2] arm64: dts: qcom: sc7280: Add a new herobrine Pro SKU
Commit Message
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
Comments
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
@@ -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
new file mode 100644
@@ -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>;
+ };
+};
new file mode 100644
@@ -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;