Re: [PATCH v2 3/3] arm64: dts: sc7180: Support Parade ps8640 edp bridge

Hi,

On Fri, Oct 8, 2021 at 11:46 AM Philip Chen [off-list ref] wrote:

Hi

On Thu, Oct 7, 2021 at 11:15 AM Philip Chen [off-list ref] wrote:

quoted

Hi,

On Thu, Sep 30, 2021 at 9:22 AM Doug Anderson [off-list ref] wrote:

quoted

Hi,

On Wed, Sep 29, 2021 at 9:02 PM Stephen Boyd [off-list ref] wrote:

quoted

quoted

+       pp3300_brij_ps8640: pp3300-brij-ps8640 {
+               compatible = "regulator-fixed";
+               status = "okay";
+               regulator-name = "pp3300_brij_ps8640";
+
+               regulator-min-microvolt = <3300000>;
+               regulator-max-microvolt = <3300000>;
+
+               gpio = <&tlmm 32 GPIO_ACTIVE_HIGH>;

Doesn't this need

                enable-active-high;

Looks like it. Without that it looks like it assumes active low.

Thanks for catching this.
I'll fix it in v3.

quoted

quoted

quoted

+
+               pinctrl-names = "default";
+               pinctrl-0 = <&en_pp3300_edp_brij_ps8640>;
+
+               vin-supply = <&pp3300_a>;
+       };
+};
+
+&dsi0_out {
+       remote-endpoint = <&ps8640_in>;

Should this also have data-lanes to be "complete"?

That's still back in the main trogdor.dtsi, isn't it?

Yes, I think so.
Plus, ti-sn65 dts doesn't define data-lanes for input either.

Sorry, I was wrong.
ti-sn65 dts actually defines data-lanes for input.
However, since ps8640 driver doesn't parse input data-lanes for now,
it's not useful to add data-lanes here anyway.

Ah, right. This one _isn't_ in the dtsi. Looking closer, I agree with
you that it's not useful. Specifically it should be noted that, unlike
ti-sn65dsi86, this bridge part looks to only support 2-lanes of DP
traffic. If both of these two lanes are routed to the panel then
there's really nothing to specify--that should be the default
assumption of the driver if/when it ever adds support for data-lanes.

-Doug