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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

>
>
> > > +edp_brij_i2c: &i2c2 {
> > > +       status = "okay";
> > > +       clock-frequency = <400000>;
> > > +
> > > +       ps8640_bridge: edp-bridge@8 {
> >
> > Just bridge@8 to match ti bridge? Also, is the label used? If not
> > please drop it.
>
> I agree with Stephen about it being "bridge@8". Personally I don't
> mind labels like this even if they're not used since they don't hurt
> and it creates less churn to add them now, but I won't fight hard to
> keep them.
I will make it "bridge@8" to match ti-sn65 dts.
Meanwhile, can we keep "ps8640_bridge" label to match ti-sn65 dts?

>
>
> > > +               aux_bus: aux-bus {
> >
> > Is this label used either?
>
> Yeah, I'd get rid of this one since there you didn't add it in the
> sn65dsi86 dtsi file and it seems exceedingly unlikely we'd need it for
> any reason.
Sure, I will remove "aux_bus" label in v3.

>
> -Doug



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux