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

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]

Date:   Fri, 8 Oct 2021 11:46:15 -0700
From:   Philip Chen <philipchen@...omium.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Stephen Boyd <swboyd@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH v2 3/3] arm64: dts: sc7180: Support Parade ps8640 edp bridge

Hi

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

>
> >
> >
> > > > +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