Re: [PATCH V7 1/7] arm64: dts: sc7280: Add QSPI node

[rajpat]

On 2021-09-03 21:58, Doug Anderson wrote:
> Hi,
>
> On Thu, Sep 2, 2021 at 9:29 PM Rajesh Patil <rajpat@xxxxxxxxxxxxxx> 
> wrote:
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -415,6 +415,25 @@
> >                 method = "smc";
> >         };
> >
> > +       qspi_opp_table: qspi-opp-table {
> > +               compatible = "operating-points-v2";
> > +
> > +               opp-75000000 {
> > +                       opp-hz = /bits/ 64 <75000000>;
> > +                       required-opps = <&rpmhpd_opp_low_svs>;
> > +               };
> > +
> > +               opp-150000000 {
> > +                       opp-hz = /bits/ 64 <150000000>;
> > +                       required-opps = <&rpmhpd_opp_svs>;
> > +               };
> > +
>
> Any chance you could add a 200 MHz OPP point? It seems plausible that
> we might want to run the Quad SPI bus at 50 MHz and this OPP needs to
> be 4x that, so 200 MHz. ...or does it magically handle that case by
> one of the other OPPs?

Okay

> > +               opp-300000000 {
> > +                       opp-hz = /bits/ 64 <300000000>;
> > +                       required-opps = <&rpmhpd_opp_nom>;
> > +               };
> > +       };
> > +
> >         soc: soc@0 {
> >                 #address-cells = <2>;
> >                 #size-cells = <2>;
> > @@ -1318,6 +1337,23 @@
> >                         };
> >                 };
> >
> > +               qspi: spi@88dc000 {
> > +                       compatible = "qcom,qspi-v1";
>
> The above compatible should be:
>
> compatible = "qcom,sdm7280-qspi", "qcom,qspi-v1";
>
> ...and you should fix the devicetree bindings to handle that. You
> should also fix sc7180.
>
> Technically the "qcom,sdm7280-qspi" isn't really needed to make
> anything work today but having it is encouraged so that if we need to
> deal with a quirk in the future we can easily do it. Also note that
> your current dts will cause a bindings error because the current
> bindings _require_ you to have two compatible strings.

Okay