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@xxxxxxxxxxx>
---
[]..
+#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
