[PATCH 1/2] dt-bindings: arm: qcom: Document the sc7280 CRD Pro boards

[PATCH 1/2] dt-bindings: arm: qcom: Document the sc7280 CRD Pro boards

[Rajendra Nayak]

Add compatibles for the Pro SKU of the sc7280 CRD boards

Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
---
 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

[PATCH 2/2] arm64: dts: qcom: sc7280: Add a new herobrine Pro SKU

[Rajendra Nayak]

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

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

Re: [PATCH 2/2] arm64: dts: qcom: sc7280: Add a new herobrine Pro SKU

[Matthias Kaehlcke]

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
> 

Re: [PATCH 2/2] arm64: dts: qcom: sc7280: Add a new herobrine Pro SKU

[Rajendra Nayak]

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

Re: [PATCH 1/2] dt-bindings: arm: qcom: Document the sc7280 CRD Pro boards

[Krzysztof Kozlowski]

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 <quic_rjendra@quicinc.com>
> ---
>  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

Re: [PATCH 1/2] dt-bindings: arm: qcom: Document the sc7280 CRD Pro boards

[Krzysztof Kozlowski]

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 <quic_rjendra@quicinc.com>
>> ---
>>  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

Re: [PATCH 1/2] dt-bindings: arm: qcom: Document the sc7280 CRD Pro boards

[Rajendra Nayak]

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 <quic_rjendra@quicinc.com>
>>> ---
>>>   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
>