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

[rajpat@codeaurora.org]

On 2021-09-03 21:58, Doug Anderson wrote:
> Hi,
> 
> On Thu, Sep 2, 2021 at 9:29 PM Rajesh Patil <rajpat@codeaurora.org> 
> 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