linux-kernel - Re: [PATCH v5 2/9] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE-0n52Uzo47N5QzoKOwny3XTrArA4nM-KPqAVxAPewSAyNOWw@mail.gmail.com>
Date:   Thu, 17 Mar 2022 17:23:17 -0400
From:   Stephen Boyd <swboyd@...omium.org>
To:     Sankeerth Billakanti <quic_sbillaka@...cinc.com>,
        devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     robdclark@...il.com, seanpaul@...omium.org,
        quic_kalyant@...cinc.com, quic_abhinavk@...cinc.com,
        dianders@...omium.org, quic_khsieh@...cinc.com, agross@...nel.org,
        bjorn.andersson@...aro.org, robh+dt@...nel.org, krzk+dt@...nel.org,
        sean@...rly.run, airlied@...ux.ie, daniel@...ll.ch,
        thierry.reding@...il.com, sam@...nborg.org,
        dmitry.baryshkov@...aro.org, quic_vproddut@...cinc.com
Subject: Re: [PATCH v5 2/9] arm64: dts: qcom: sc7280: Add support for eDP
 panel on CRD

Quoting Sankeerth Billakanti (2022-03-16 10:35:47)
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..2df654e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -7,6 +7,7 @@
>
>  /dts-v1/;
>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>  #include "sc7280-idp.dtsi"
>  #include "sc7280-idp-ec-h1.dtsi"
>
> @@ -21,6 +22,27 @@
>         chosen {
>                 stdout-path = "serial0:115200n8";
>         };
> +
> +       edp_3v3_regulator: edp-3v3-regulator {
> +               compatible = "regulator-fixed";
> +               regulator-name = "edp_3v3_regulator";
> +
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +
> +               gpio = <&tlmm 80 GPIO_ACTIVE_HIGH>;
> +               enable-active-high;
> +
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&edp_panel_power>;
> +       };
> +
> +       vreg_edp_bp: vreg-edp-bp-regulator {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vreg_edp_bp";
> +               regulator-always-on;
> +               regulator-boot-on;
> +       };
>  };
>
>  &apps_rsc {
> @@ -76,6 +98,58 @@ ap_ts_pen_1v8: &i2c13 {
>         };
>  };
>
> +&mdss {
> +       status = "okay";
> +};
> +
> +&mdss_dp {
> +       status = "okay";
> +
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&dp_hot_plug_det>;
> +       data-lanes = <0 1>;
> +       vdda-1p2-supply = <&vreg_l6b_1p2>;
> +       vdda-0p9-supply = <&vreg_l1b_0p8>;
> +};
> +
> +&mdss_edp {
> +       status = "okay";
> +
> +       data-lanes = <0 1 2 3>;

Is this property necessary? It looks like the default.

> +       vdda-1p2-supply = <&vreg_l6b_1p2>;
> +       vdda-0p9-supply = <&vreg_l10c_0p8>;
> +
> +       aux-bus {

Can this move to sc7280.dtsi and get a phandle?

> +               edp_panel: edp-panel {

I'd prefer

	edp_panel: panel {

because there's only one panel node at this level.

> +                       compatible = "edp-panel";
> +
> +                       power-supply = <&edp_3v3_regulator>;

This is board specific, but I thought it was on the qcard so we should
move this to sc7280-qcard.dtsi?

> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               port@0 {
> +                                       reg = <0>;
> +                                       edp_panel_in: endpoint {

This can be shortened to

			port {
				edp_panel_in: endpoint {

according to panel-edp.yaml

> +                                               remote-endpoint = <&mdss_edp_out>;
> +                                       };
> +                               };
> +                       };
> +               };
> +       };
> +};
> +
> +&mdss_edp_out {
> +       remote-endpoint = <&edp_panel_in>;
> +};
> +
> +&mdss_edp_phy {
> +       status = "okay";
> +};
> +
> +&mdss_mdp {
> +       status = "okay";
> +};
> +
>  &nvme_3v3_regulator {
>         gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
>  };
> @@ -84,7 +158,26 @@ ap_ts_pen_1v8: &i2c13 {
>         pins = "gpio51";
>  };
>
> +&pm8350c_gpios {
> +       edp_bl_power: edp-bl-power {

Is this used in a later patch?

> +               pins = "gpio7";
> +               function = "normal";
> +               qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
> +       };
> +
> +       edp_bl_pwm: edp-bl-pwm {

Is this used in a later patch? Can it be moved to the patch that uses
it?

> +               pins = "gpio8";
> +               function = "func1";
> +               qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
> +       };
> +};
> +
>  &tlmm {
> +       edp_panel_power: edp-panel-power {
> +               pins = "gpio80";
> +               function = "gpio";

function of gpio is unnecessary. Where is the bias and drive-strength
settings?

> +       };
> +
>         tp_int_odl: tp-int-odl {
>                 pins = "gpio7";
>                 function = "gpio";
> --
> 2.7.4
>