Hi, On Fri, Oct 8, 2021 at 11:46 AM Philip Chen <philipchen@xxxxxxxxxxxx> wrote: > > Hi > > On Thu, Oct 7, 2021 at 11:15 AM Philip Chen <philipchen@xxxxxxxxxxxx> wrote: > > > > Hi, > > > > On Thu, Sep 30, 2021 at 9:22 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > > > > > Hi, > > > > > > On Wed, Sep 29, 2021 at 9:02 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > > > > > > > > + 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. > > > > > > > > > > > > > + > > > > > + 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