Add compatibles for the Pro SKU of the sc7280 CRD boards
Signed-off-by: Rajendra Nayak <[email protected]>
---
Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 463509f0f23a..2550ab251f7b 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -613,6 +613,11 @@ properties:
- const: google,hoglin
- const: qcom,sc7280
+ - description: Qualcomm Technologies, Inc. sc7280 CRD Pro platform (newest rev)
+ items:
+ - const: google,hoglin-sku1536
+ - const: qcom,sc7280
+
- description: Qualcomm Technologies, Inc. sc7280 IDP SKU1 platform
items:
- const: qcom,sc7280-idp
--
2.17.1
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 <[email protected]>
---
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>;
+ };
+};
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 18/11/2022 15:13, Krzysztof Kozlowski wrote:
> On 18/11/2022 08:30, Rajendra Nayak wrote:
>> Add compatibles for the Pro SKU of the sc7280 CRD boards
>>
>> Signed-off-by: Rajendra Nayak <[email protected]>
>> ---
>> Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>> index 463509f0f23a..2550ab251f7b 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>> @@ -613,6 +613,11 @@ properties:
>> - const: google,hoglin
>> - const: qcom,sc7280
>>
>> + - description: Qualcomm Technologies, Inc. sc7280 CRD Pro platform (newest rev)
>
> Such entry is already documented. I think this is not based on recent
> kernel.
My bad, I missed this is a Pro. Why it is not part of compatible? What
is the difference between hoglin and CRD Pro?
You have plenty of space in commit msg to explain this.
Best regards,
Krzysztof
On 18/11/2022 08:30, Rajendra Nayak wrote:
> Add compatibles for the Pro SKU of the sc7280 CRD boards
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 463509f0f23a..2550ab251f7b 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -613,6 +613,11 @@ properties:
> - const: google,hoglin
> - const: qcom,sc7280
>
> + - description: Qualcomm Technologies, Inc. sc7280 CRD Pro platform (newest rev)
Such entry is already documented. I think this is not based on recent
kernel.
> + items:
> + - const: google,hoglin-sku1536
> + - const: qcom,sc7280
> +
> - description: Qualcomm Technologies, Inc. sc7280 IDP SKU1 platform
> items:
> - const: qcom,sc7280-idp
Best regards,
Krzysztof
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 <[email protected]>
> ---
> 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 <[email protected]>
>> ---
[]..
>> +#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
On 11/18/2022 7:45 PM, Krzysztof Kozlowski wrote:
> On 18/11/2022 15:13, Krzysztof Kozlowski wrote:
>> On 18/11/2022 08:30, Rajendra Nayak wrote:
>>> Add compatibles for the Pro SKU of the sc7280 CRD boards
>>>
>>> Signed-off-by: Rajendra Nayak <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> index 463509f0f23a..2550ab251f7b 100644
>>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> @@ -613,6 +613,11 @@ properties:
>>> - const: google,hoglin
>>> - const: qcom,sc7280
>>>
>>> + - description: Qualcomm Technologies, Inc. sc7280 CRD Pro platform (newest rev)
>>
>> Such entry is already documented. I think this is not based on recent
>> kernel.
>
> My bad, I missed this is a Pro. Why it is not part of compatible? What
> is the difference between hoglin and CRD Pro?
>
> You have plenty of space in commit msg to explain this.
Sent a v2 adding the details on the difference captured in the commit log.
>
> Best regards,
> Krzysztof
>