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