From: Bjorn Andersson <andersson@kernel.org>
To: Anand Tiwari <anand.tiwari@oss.qualcomm.com>
Cc: Konrad Dybcio <konradybcio@kernel.org>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, venkata.valluru@oss.qualcomm.com,
vishnu.saini@oss.qualcomm.com,
Jessica Zhang <jesszhan0024@gmail.com>
Subject: Re: [PATCH 1/2] arm64: dts: qcom: hamoa-iot-evk: Add eDP display overlay
Date: Fri, 7 Aug 2026 16:01:05 -0500 [thread overview]
Message-ID: <anZGDietz47mE2wp@baldur> (raw)
In-Reply-To: <anWsrXO9RztrF5H4@hu-anantiwa-hyd.qualcomm.com>
On Fri, Aug 07, 2026 at 03:30:13PM +0530, Anand Tiwari wrote:
> On Thu, Aug 06, 2026 at 07:05:37PM -0500, Bjorn Andersson wrote:
> > On Thu, Aug 06, 2026 at 10:01:20PM +0530, Anand Tiwari wrote:
> > > Move the eDP panel configuration and related power, backlight, and pinctrl
> > > nodes into a separate overlay. Keep the base DTB suitable for headless
> > > variants and provide a composite DTB for headed variants.
> >
> > To quote:
> > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> >
> > """
> > Describe your problem. Whether your patch is a one-line bug fix or 5000
> > lines of a new feature, there must be an underlying problem that
> > motivated you to do this work. Convince the reviewer that there is a
> > problem worth fixing and that it makes sense for them to read past the
> > first paragraph.
> > """
> >
> > In fact, I'm not even able to guess what the problem you're fixing here.
> >
> > Regards,
> > Bjorn
> >
>
> The problem is that the base device trees currently describe the eDP
> display hardware unconditionally. However, Hamoa and Purwa IoT EVKs
> are also available in headless variants without an eDP panel.
>
> With current base DTBs, headless variants are not stable where eDP panel is not
> physically connected with board and userspace is trying to use it.
>
> This series fixes the inaccurate hardware description by keeping the
> base DTBs limited to hardware common to both variants and moving the
> eDP-specific nodes into separate overlays. The base DTB can therefore
> be used for headless variants, while the overlay is selected for
> headed variants.
>
I see, then it seems your change does make sense. I don't understand why
you didn't explained this in the commit message though.
I further do not understand how it can be that nobody noticed that the
display was missing on their Hamoa EVK during the 11 months since I
merged the offending patch - this is completely unacceptable.
Regards,
Bjorn
> > >
> > > Signed-off-by: Anand Tiwari <anand.tiwari@oss.qualcomm.com>
> > > ---
> > > arch/arm64/boot/dts/qcom/Makefile | 3 +
> > > arch/arm64/boot/dts/qcom/hamoa-iot-evk-edp.dtso | 126 ++++++++++++++++++++++++
> > > arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts | 107 --------------------
> > > 3 files changed, 129 insertions(+), 107 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > > index 1c86e7e98f55..bb0da34983c8 100644
> > > --- a/arch/arm64/boot/dts/qcom/Makefile
> > > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > > @@ -20,6 +20,9 @@ dtb-$(CONFIG_ARCH_QCOM) += glymur-asus-zenbook-a16-ux3607oa.dtb
> > > dtb-$(CONFIG_ARCH_QCOM) += glymur-crd.dtb
> > > dtb-$(CONFIG_ARCH_QCOM) += hamoa-iot-evk.dtb
> > >
> > > +hamoa-iot-evk-edp-dtbs := hamoa-iot-evk.dtb hamoa-iot-evk-edp.dtbo
> > > +dtb-$(CONFIG_ARCH_QCOM) += hamoa-iot-evk-edp.dtb
> > > +
> > > hamoa-iot-evk-el2-dtbs := hamoa-iot-evk.dtb x1-el2.dtbo
> > >
> > > dtb-$(CONFIG_ARCH_QCOM) += hamoa-iot-evk-el2.dtb
> > > diff --git a/arch/arm64/boot/dts/qcom/hamoa-iot-evk-edp.dtso b/arch/arm64/boot/dts/qcom/hamoa-iot-evk-edp.dtso
> > > new file mode 100644
> > > index 000000000000..759d05342627
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/qcom/hamoa-iot-evk-edp.dtso
> > > @@ -0,0 +1,126 @@
> > > +// SPDX-License-Identifier: BSD-3-Clause
> > > +/*
> > > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > > + */
> > > +
> > > +/dts-v1/;
> > > +/plugin/;
> > > +
> > > +#include <dt-bindings/gpio/gpio.h>
> > > +
> > > +&{/} {
> > > + backlight: backlight {
> > > + compatible = "pwm-backlight";
> > > + pwms = <&pmk8550_pwm 0 5000000>;
> > > + enable-gpios = <&pmc8380_3_gpios 4 GPIO_ACTIVE_HIGH>;
> > > + power-supply = <&vreg_edp_bl>;
> > > +
> > > + pinctrl-0 = <&edp_bl_en>, <&edp_bl_pwm>;
> > > + pinctrl-names = "default";
> > > + };
> > > +
> > > + vreg_edp_3p3: regulator-edp-3p3 {
> > > + compatible = "regulator-fixed";
> > > +
> > > + regulator-name = "VREG_EDP_3P3";
> > > + regulator-min-microvolt = <3300000>;
> > > + regulator-max-microvolt = <3300000>;
> > > +
> > > + gpio = <&tlmm 70 GPIO_ACTIVE_HIGH>;
> > > + enable-active-high;
> > > +
> > > + pinctrl-0 = <&edp_reg_en>;
> > > + pinctrl-names = "default";
> > > +
> > > + regulator-boot-on;
> > > + };
> > > +
> > > + vreg_edp_bl: regulator-edp-bl {
> > > + compatible = "regulator-fixed";
> > > +
> > > + regulator-name = "VBL9";
> > > + regulator-min-microvolt = <3600000>;
> > > + regulator-max-microvolt = <3600000>;
> > > +
> > > + gpio = <&pmc8380_3_gpios 10 GPIO_ACTIVE_HIGH>;
> > > + enable-active-high;
> > > +
> > > + pinctrl-0 = <&edp_bl_reg_en>;
> > > + pinctrl-names = "default";
> > > +
> > > + regulator-boot-on;
> > > + };
> > > +};
> > > +
> > > +&mdss_dp3 {
> > > + /delete-property/ #sound-dai-cells;
> > > +
> > > + pinctrl-0 = <&edp0_hpd_default>;
> > > + pinctrl-names = "default";
> > > +
> > > + status = "okay";
> > > +
> > > + aux-bus {
> > > + panel {
> > > + compatible = "edp-panel";
> > > +
> > > + backlight = <&backlight>;
> > > + power-supply = <&vreg_edp_3p3>;
> > > +
> > > + port {
> > > + edp_panel_in: endpoint {
> > > + remote-endpoint = <&mdss_dp3_out>;
> > > + };
> > > + };
> > > + };
> > > + };
> > > +};
> > > +
> > > +&mdss_dp3_out {
> > > + data-lanes = <0 1 2 3>;
> > > + link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> > > +
> > > + remote-endpoint = <&edp_panel_in>;
> > > +};
> > > +
> > > +&mdss_dp3_phy {
> > > + vdda-phy-supply = <&vreg_l3j_0p8>;
> > > + vdda-pll-supply = <&vreg_l2j_1p2>;
> > > +
> > > + status = "okay";
> > > +};
> > > +
> > > +&pmc8380_3_gpios {
> > > + edp_bl_en: edp-bl-en-state {
> > > + pins = "gpio4";
> > > + function = "normal";
> > > + power-source = <1>;
> > > + input-disable;
> > > + output-enable;
> > > + };
> > > +
> > > + edp_bl_reg_en: edp-bl-reg-en-state {
> > > + pins = "gpio10";
> > > + function = "normal";
> > > + };
> > > +};
> > > +
> > > +&pmk8550_gpios {
> > > + edp_bl_pwm: edp-bl-pwm-state {
> > > + pins = "gpio5";
> > > + function = "func3";
> > > + };
> > > +};
> > > +
> > > +&pmk8550_pwm {
> > > + status = "okay";
> > > +};
> > > +
> > > +&tlmm {
> > > + edp_reg_en: edp-reg-en-state {
> > > + pins = "gpio70";
> > > + function = "gpio";
> > > + drive-strength = <16>;
> > > + bias-disable;
> > > + };
> > > +};
> > > diff --git a/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts b/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts
> > > index 9fa86bb6438e..78cefd5391b1 100644
> > > --- a/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts
> > > +++ b/arch/arm64/boot/dts/qcom/hamoa-iot-evk.dts
> > > @@ -19,16 +19,6 @@ aliases {
> > > serial1 = &uart14;
> > > };
> > >
> > > - backlight: backlight {
> > > - compatible = "pwm-backlight";
> > > - pwms = <&pmk8550_pwm 0 5000000>;
> > > - enable-gpios = <&pmc8380_3_gpios 4 GPIO_ACTIVE_HIGH>;
> > > - power-supply = <&vreg_edp_bl>;
> > > -
> > > - pinctrl-0 = <&edp_bl_en>, <&edp_bl_pwm>;
> > > - pinctrl-names = "default";
> > > - };
> > > -
> > > clocks {
> > > mcp2518fd_osc: clock-40000000 {
> > > compatible = "fixed-clock";
> > > @@ -213,38 +203,6 @@ pmic_glink_ss2_con_sbu_in: endpoint {
> > > };
> > > };
> > >
> > > - vreg_edp_3p3: regulator-edp-3p3 {
> > > - compatible = "regulator-fixed";
> > > -
> > > - regulator-name = "VREG_EDP_3P3";
> > > - regulator-min-microvolt = <3300000>;
> > > - regulator-max-microvolt = <3300000>;
> > > -
> > > - gpio = <&tlmm 70 GPIO_ACTIVE_HIGH>;
> > > - enable-active-high;
> > > -
> > > - pinctrl-0 = <&edp_reg_en>;
> > > - pinctrl-names = "default";
> > > -
> > > - regulator-boot-on;
> > > - };
> > > -
> > > - vreg_edp_bl: regulator-edp-bl {
> > > - compatible = "regulator-fixed";
> > > -
> > > - regulator-name = "VBL9";
> > > - regulator-min-microvolt = <3600000>;
> > > - regulator-max-microvolt = <3600000>;
> > > -
> > > - gpio = <&pmc8380_3_gpios 10 GPIO_ACTIVE_HIGH>;
> > > - enable-active-high;
> > > -
> > > - pinctrl-0 = <&edp_bl_reg_en>;
> > > - pinctrl-names = "default";
> > > -
> > > - regulator-boot-on;
> > > - };
> > > -
> > > vreg_nvme: regulator-nvme {
> > > compatible = "regulator-fixed";
> > >
> > > @@ -974,44 +932,6 @@ &mdss_dp2_out {
> > > link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> > > };
> > >
> > > -&mdss_dp3 {
> > > - /delete-property/ #sound-dai-cells;
> > > -
> > > - pinctrl-0 = <&edp0_hpd_default>;
> > > - pinctrl-names = "default";
> > > -
> > > - status = "okay";
> > > -
> > > - aux-bus {
> > > - panel {
> > > - compatible = "edp-panel";
> > > -
> > > - backlight = <&backlight>;
> > > - power-supply = <&vreg_edp_3p3>;
> > > -
> > > - port {
> > > - edp_panel_in: endpoint {
> > > - remote-endpoint = <&mdss_dp3_out>;
> > > - };
> > > - };
> > > - };
> > > - };
> > > -};
> > > -
> > > -&mdss_dp3_out {
> > > - data-lanes = <0 1 2 3>;
> > > - link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> > > -
> > > - remote-endpoint = <&edp_panel_in>;
> > > -};
> > > -
> > > -&mdss_dp3_phy {
> > > - vdda-phy-supply = <&vreg_l3j_0p8>;
> > > - vdda-pll-supply = <&vreg_l2j_1p2>;
> > > -
> > > - status = "okay";
> > > -};
> > > -
> > > &pcie3_port0 {
> > > vpcie12v-supply = <&vreg_pcie_12v>;
> > > vpcie3v3-supply = <&vreg_pcie_3v3>;
> > > @@ -1140,19 +1060,6 @@ led@2 {
> > > };
> > >
> > > &pmc8380_3_gpios {
> > > - edp_bl_en: edp-bl-en-state {
> > > - pins = "gpio4";
> > > - function = "normal";
> > > - power-source = <1>;
> > > - input-disable;
> > > - output-enable;
> > > - };
> > > -
> > > - edp_bl_reg_en: edp-bl-reg-en-state {
> > > - pins = "gpio10";
> > > - function = "normal";
> > > - };
> > > -
> > > pm_sde7_aux_3p3_en: pcie-aux-3p3-default-state {
> > > pins = "gpio8";
> > > function = "normal";
> > > @@ -1181,13 +1088,6 @@ usb0_pwr_1p15_reg_en: usb0-pwr-1p15-reg-en-state {
> > > };
> > > };
> > >
> > > -&pmk8550_gpios {
> > > - edp_bl_pwm: edp-bl-pwm-state {
> > > - pins = "gpio5";
> > > - function = "func3";
> > > - };
> > > -};
> > > -
> > > &pmk8550_pwm {
> > > status = "okay";
> > > };
> > > @@ -1357,13 +1257,6 @@ ec_int_n_default: ec-int-n-state {
> > > bias-disable;
> > > };
> > >
> > > - edp_reg_en: edp-reg-en-state {
> > > - pins = "gpio70";
> > > - function = "gpio";
> > > - drive-strength = <16>;
> > > - bias-disable;
> > > - };
> > > -
> > > eusb3_reset_n: eusb3-reset-n-state {
> > > pins = "gpio6";
> > > function = "gpio";
> > >
> > > --
> > > 2.54.0
> > >
next prev parent reply other threads:[~2026-08-07 21:01 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-06 16:31 [PATCH 0/2] arm64: dts: qcom: Add eDP display overlays for hamoa and purwa IoT EVKs Anand Tiwari
2026-08-06 16:31 ` [PATCH 1/2] arm64: dts: qcom: hamoa-iot-evk: Add eDP display overlay Anand Tiwari
2026-08-06 16:55 ` sashiko-bot
2026-08-07 0:05 ` Bjorn Andersson
2026-08-07 10:00 ` Anand Tiwari
2026-08-07 21:01 ` Bjorn Andersson [this message]
2026-08-06 16:31 ` [PATCH 2/2] arm64: dts: qcom: purwa-iot-evk: " Anand Tiwari
2026-08-06 16:54 ` sashiko-bot
2026-08-07 0:06 ` Bjorn Andersson
2026-08-07 9:22 ` [PATCH 0/2] arm64: dts: qcom: Add eDP display overlays for hamoa and purwa IoT EVKs Dmitry Baryshkov
2026-08-07 10:36 ` Anand Tiwari
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=anZGDietz47mE2wp@baldur \
--to=andersson@kernel.org \
--cc=anand.tiwari@oss.qualcomm.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=jesszhan0024@gmail.com \
--cc=konradybcio@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
--cc=venkata.valluru@oss.qualcomm.com \
--cc=vishnu.saini@oss.qualcomm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox