| Message ID | 20210604135439.19119-2-rojay@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add QSPI and QUPv3 DT nodes for SC7280 SoC | expand |
On Fri 04 Jun 08:54 CDT 2021, Roja Rani Yarubandi wrote: > Add QSPI DT node for SC7280 SoC. > > Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> > --- > Changes in V3: > - Broken the huge V2 patch into 3 smaller patches. > 1. QSPI DT nodes > 2. QUP wrapper_0 DT nodes > 3. QUP wrapper_1 DT nodes > > Changes in V2: > - As per Doug's comments removed pinmux/pinconf subnodes. > - As per Doug's comments split of SPI, UART nodes has been done. > - Moved QSPI node before aps_smmu as per the order. > > arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 +++++++++++++++++++++++++ > 2 files changed, 90 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > index 3900cfc09562..d0edffc15736 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > @@ -268,6 +268,22 @@ pmr735b_die_temp { > }; > }; > > +&qspi { > + status = "okay"; > + pinctrl-names = "default"; > + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; > + > + flash@0 { > + compatible = "jedec,spi-nor"; > + reg = <0>; > + > + /* TODO: Increase frequency after testing */ > + spi-max-frequency = <25000000>; > + spi-tx-bus-width = <2>; > + spi-rx-bus-width = <2>; > + }; > +}; > + > &qupv3_id_0 { > status = "okay"; > }; > @@ -278,6 +294,19 @@ &uart5 { > > /* PINCTRL - additions to nodes defined in sc7280.dtsi */ > > +&qspi_cs0 { > + bias-disable; > +}; > + > +&qspi_clk { > + bias-disable; > +}; > + > +&qspi_data01 { > + /* High-Z when no transfers; nice to park the lines */ > + bias-pull-up; > +}; > + > &qup_uart5_default { > tx { > pins = "gpio46"; > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 6c9d5eb93f93..3047ab802cd2 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { > }; > }; > > + qspi_opp_table: qspi-opp-table { This node doesn't represents anything on the mmio bus, so it shouldn't live in in /soc. Can't you move it into &qspi? Regards, Bjorn > + 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>; > + }; > + > + opp-300000000 { > + opp-hz = /bits/ 64 <300000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + }; > + > + qspi: spi@88dc000 { > + compatible = "qcom,qspi-v1"; > + reg = <0 0x088dc000 0 0x1000>; > + #address-cells = <1>; > + #size-cells = <0>; > + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&gcc GCC_QSPI_CNOC_PERIPH_AHB_CLK>, > + <&gcc GCC_QSPI_CORE_CLK>; > + clock-names = "iface", "core"; > + interconnects = <&gem_noc MASTER_APPSS_PROC 0 > + &cnoc2 SLAVE_QSPI_0 0>; > + interconnect-names = "qspi-config"; > + power-domains = <&rpmhpd SC7280_CX>; > + operating-points-v2 = <&qspi_opp_table>; > + status = "disabled"; > + };
On 2021-06-05 03:15, Stephen Boyd wrote: > Quoting Roja Rani Yarubandi (2021-06-04 06:54:37) >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> index 3900cfc09562..d0edffc15736 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> @@ -268,6 +268,22 @@ pmr735b_die_temp { >> }; >> }; >> >> +&qspi { >> + status = "okay"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; >> + >> + flash@0 { >> + compatible = "jedec,spi-nor"; >> + reg = <0>; >> + >> + /* TODO: Increase frequency after testing */ > > Is this going to change? Please set it to 37.5MHz if that's the max > supported. > Okay, will set it to 37.5MHz. Thanks, Roja >> + spi-max-frequency = <25000000>; >> + spi-tx-bus-width = <2>; >> + spi-rx-bus-width = <2>; >> + }; >> +}; >> + >> &qupv3_id_0 { >> status = "okay"; >> };
On 2021-06-06 09:25, Bjorn Andersson wrote: > On Fri 04 Jun 08:54 CDT 2021, Roja Rani Yarubandi wrote: > >> Add QSPI DT node for SC7280 SoC. >> >> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> >> --- >> Changes in V3: >> - Broken the huge V2 patch into 3 smaller patches. >> 1. QSPI DT nodes >> 2. QUP wrapper_0 DT nodes >> 3. QUP wrapper_1 DT nodes >> >> Changes in V2: >> - As per Doug's comments removed pinmux/pinconf subnodes. >> - As per Doug's comments split of SPI, UART nodes has been done. >> - Moved QSPI node before aps_smmu as per the order. >> >> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 >> +++++++++++++++++++++++++ >> 2 files changed, 90 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> index 3900cfc09562..d0edffc15736 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> @@ -268,6 +268,22 @@ pmr735b_die_temp { >> }; >> }; >> >> +&qspi { >> + status = "okay"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; >> + >> + flash@0 { >> + compatible = "jedec,spi-nor"; >> + reg = <0>; >> + >> + /* TODO: Increase frequency after testing */ >> + spi-max-frequency = <25000000>; >> + spi-tx-bus-width = <2>; >> + spi-rx-bus-width = <2>; >> + }; >> +}; >> + >> &qupv3_id_0 { >> status = "okay"; >> }; >> @@ -278,6 +294,19 @@ &uart5 { >> >> /* PINCTRL - additions to nodes defined in sc7280.dtsi */ >> >> +&qspi_cs0 { >> + bias-disable; >> +}; >> + >> +&qspi_clk { >> + bias-disable; >> +}; >> + >> +&qspi_data01 { >> + /* High-Z when no transfers; nice to park the lines */ >> + bias-pull-up; >> +}; >> + >> &qup_uart5_default { >> tx { >> pins = "gpio46"; >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> index 6c9d5eb93f93..3047ab802cd2 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { >> }; >> }; >> >> + qspi_opp_table: qspi-opp-table { > > This node doesn't represents anything on the mmio bus, so it shouldn't > live in in /soc. Can't you move it into &qspi? > > Regards, > Bjorn > Sure, will move it into qspi node. Thanks, Roja >> + 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>; >> + }; >> + >> + opp-300000000 { >> + opp-hz = /bits/ 64 <300000000>; >> + required-opps = <&rpmhpd_opp_nom>; >> + }; >> + }; >> + >> + qspi: spi@88dc000 { >> + compatible = "qcom,qspi-v1"; >> + reg = <0 0x088dc000 0 0x1000>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&gcc GCC_QSPI_CNOC_PERIPH_AHB_CLK>, >> + <&gcc GCC_QSPI_CORE_CLK>; >> + clock-names = "iface", "core"; >> + interconnects = <&gem_noc MASTER_APPSS_PROC 0 >> + &cnoc2 SLAVE_QSPI_0 0>; >> + interconnect-names = "qspi-config"; >> + power-domains = <&rpmhpd SC7280_CX>; >> + operating-points-v2 = <&qspi_opp_table>; >> + status = "disabled"; >> + };
On 2021-06-08 13:37, rojay@codeaurora.org wrote: > On 2021-06-06 09:25, Bjorn Andersson wrote: >> On Fri 04 Jun 08:54 CDT 2021, Roja Rani Yarubandi wrote: >> >>> Add QSPI DT node for SC7280 SoC. >>> >>> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> >>> --- >>> Changes in V3: >>> - Broken the huge V2 patch into 3 smaller patches. >>> 1. QSPI DT nodes >>> 2. QUP wrapper_0 DT nodes >>> 3. QUP wrapper_1 DT nodes >>> >>> Changes in V2: >>> - As per Doug's comments removed pinmux/pinconf subnodes. >>> - As per Doug's comments split of SPI, UART nodes has been done. >>> - Moved QSPI node before aps_smmu as per the order. >>> >>> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ >>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 >>> +++++++++++++++++++++++++ >>> 2 files changed, 90 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >>> index 3900cfc09562..d0edffc15736 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >>> @@ -268,6 +268,22 @@ pmr735b_die_temp { >>> }; >>> }; >>> >>> +&qspi { >>> + status = "okay"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; >>> + >>> + flash@0 { >>> + compatible = "jedec,spi-nor"; >>> + reg = <0>; >>> + >>> + /* TODO: Increase frequency after testing */ >>> + spi-max-frequency = <25000000>; >>> + spi-tx-bus-width = <2>; >>> + spi-rx-bus-width = <2>; >>> + }; >>> +}; >>> + >>> &qupv3_id_0 { >>> status = "okay"; >>> }; >>> @@ -278,6 +294,19 @@ &uart5 { >>> >>> /* PINCTRL - additions to nodes defined in sc7280.dtsi */ >>> >>> +&qspi_cs0 { >>> + bias-disable; >>> +}; >>> + >>> +&qspi_clk { >>> + bias-disable; >>> +}; >>> + >>> +&qspi_data01 { >>> + /* High-Z when no transfers; nice to park the lines */ >>> + bias-pull-up; >>> +}; >>> + >>> &qup_uart5_default { >>> tx { >>> pins = "gpio46"; >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> index 6c9d5eb93f93..3047ab802cd2 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { >>> }; >>> }; >>> >>> + qspi_opp_table: qspi-opp-table { >> >> This node doesn't represents anything on the mmio bus, so it shouldn't >> live in in /soc. Can't you move it into &qspi? >> >> Regards, >> Bjorn >> > > Sure, will move it into qspi node. > > Thanks, > Roja > Hi Bjorn, Moving "qspi_opp_table" inside &qspi node causing this warning: arch/arm64/boot/dts/qcom/sc7280.dtsi:1055.35-1072.6: Warning (spi_bus_reg): /soc@0/spi@88dc000/qspi-opp-table: missing or empty reg property Shall I keep the qspi-opp-table out of &qspi node? Thanks, Roja >>> + 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>; >>> + }; >>> + >>> + opp-300000000 { >>> + opp-hz = /bits/ 64 <300000000>; >>> + required-opps = <&rpmhpd_opp_nom>; >>> + }; >>> + }; >>> + >>> + qspi: spi@88dc000 { >>> + compatible = "qcom,qspi-v1"; >>> + reg = <0 0x088dc000 0 0x1000>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>; >>> + clocks = <&gcc GCC_QSPI_CNOC_PERIPH_AHB_CLK>, >>> + <&gcc GCC_QSPI_CORE_CLK>; >>> + clock-names = "iface", "core"; >>> + interconnects = <&gem_noc MASTER_APPSS_PROC 0 >>> + &cnoc2 SLAVE_QSPI_0 0>; >>> + interconnect-names = "qspi-config"; >>> + power-domains = <&rpmhpd SC7280_CX>; >>> + operating-points-v2 = <&qspi_opp_table>; >>> + status = "disabled"; >>> + };
Quoting rojay@codeaurora.org (2021-07-06 02:19:27) > On 2021-06-08 13:37, rojay@codeaurora.org wrote: > > On 2021-06-06 09:25, Bjorn Andersson wrote: > >> On Fri 04 Jun 08:54 CDT 2021, Roja Rani Yarubandi wrote: > >> > >>> Add QSPI DT node for SC7280 SoC. > >>> > >>> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> > >>> --- > >>> Changes in V3: > >>> - Broken the huge V2 patch into 3 smaller patches. > >>> 1. QSPI DT nodes > >>> 2. QUP wrapper_0 DT nodes > >>> 3. QUP wrapper_1 DT nodes > >>> > >>> Changes in V2: > >>> - As per Doug's comments removed pinmux/pinconf subnodes. > >>> - As per Doug's comments split of SPI, UART nodes has been done. > >>> - Moved QSPI node before aps_smmu as per the order. > >>> > >>> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ > >>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 > >>> +++++++++++++++++++++++++ > >>> 2 files changed, 90 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts > >>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > >>> index 3900cfc09562..d0edffc15736 100644 > >>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts > >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > >>> @@ -268,6 +268,22 @@ pmr735b_die_temp { > >>> }; > >>> }; > >>> > >>> +&qspi { > >>> + status = "okay"; > >>> + pinctrl-names = "default"; > >>> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; > >>> + > >>> + flash@0 { > >>> + compatible = "jedec,spi-nor"; > >>> + reg = <0>; > >>> + > >>> + /* TODO: Increase frequency after testing */ > >>> + spi-max-frequency = <25000000>; > >>> + spi-tx-bus-width = <2>; > >>> + spi-rx-bus-width = <2>; > >>> + }; > >>> +}; > >>> + > >>> &qupv3_id_0 { > >>> status = "okay"; > >>> }; > >>> @@ -278,6 +294,19 @@ &uart5 { > >>> > >>> /* PINCTRL - additions to nodes defined in sc7280.dtsi */ > >>> > >>> +&qspi_cs0 { > >>> + bias-disable; > >>> +}; > >>> + > >>> +&qspi_clk { > >>> + bias-disable; > >>> +}; > >>> + > >>> +&qspi_data01 { > >>> + /* High-Z when no transfers; nice to park the lines */ > >>> + bias-pull-up; > >>> +}; > >>> + > >>> &qup_uart5_default { > >>> tx { > >>> pins = "gpio46"; > >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> index 6c9d5eb93f93..3047ab802cd2 100644 > >>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { > >>> }; > >>> }; > >>> > >>> + qspi_opp_table: qspi-opp-table { > >> > >> This node doesn't represents anything on the mmio bus, so it shouldn't > >> live in in /soc. Can't you move it into &qspi? > >> > >> Regards, > >> Bjorn > >> > > > > Sure, will move it into qspi node. > > > > Thanks, > > Roja > > > > Hi Bjorn, > > Moving "qspi_opp_table" inside &qspi node causing this warning: > arch/arm64/boot/dts/qcom/sc7280.dtsi:1055.35-1072.6: Warning > (spi_bus_reg): /soc@0/spi@88dc000/qspi-opp-table: missing or empty reg > property If DT folks are OK I think we should hard-code 'opp-table' as not a device for spi to populate on the spi bus and relax the warning in the devicetree compiler (see [1] for more details). Technically, nodes that are bus controllers assume all child nodes are devices on that bus. In this case, we want to stick the opp table as a child of the spi node so that it can be called 'opp-table' and not be a node in the root of DT. > > Shall I keep the qspi-opp-table out of &qspi node? > If you do, please move it to / instead of putting it under /soc as it doesn't have an address or a reg property. [1] https://github.com/dgibson/dtc/blob/69595a167f06c4482ce784e30df1ac9b16ceb5b0/checks.c#L1844
On 2021-07-09 06:26, Stephen Boyd wrote: > Quoting rojay@codeaurora.org (2021-07-06 02:19:27) >> On 2021-06-08 13:37, rojay@codeaurora.org wrote: >> > On 2021-06-06 09:25, Bjorn Andersson wrote: >> >> On Fri 04 Jun 08:54 CDT 2021, Roja Rani Yarubandi wrote: >> >> >> >>> Add QSPI DT node for SC7280 SoC. >> >>> >> >>> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> >> >>> --- >> >>> Changes in V3: >> >>> - Broken the huge V2 patch into 3 smaller patches. >> >>> 1. QSPI DT nodes >> >>> 2. QUP wrapper_0 DT nodes >> >>> 3. QUP wrapper_1 DT nodes >> >>> >> >>> Changes in V2: >> >>> - As per Doug's comments removed pinmux/pinconf subnodes. >> >>> - As per Doug's comments split of SPI, UART nodes has been done. >> >>> - Moved QSPI node before aps_smmu as per the order. >> >>> >> >>> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ >> >>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 >> >>> +++++++++++++++++++++++++ >> >>> 2 files changed, 90 insertions(+) >> >>> >> >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> >>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> >>> index 3900cfc09562..d0edffc15736 100644 >> >>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> >>> @@ -268,6 +268,22 @@ pmr735b_die_temp { >> >>> }; >> >>> }; >> >>> >> >>> +&qspi { >> >>> + status = "okay"; >> >>> + pinctrl-names = "default"; >> >>> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; >> >>> + >> >>> + flash@0 { >> >>> + compatible = "jedec,spi-nor"; >> >>> + reg = <0>; >> >>> + >> >>> + /* TODO: Increase frequency after testing */ >> >>> + spi-max-frequency = <25000000>; >> >>> + spi-tx-bus-width = <2>; >> >>> + spi-rx-bus-width = <2>; >> >>> + }; >> >>> +}; >> >>> + >> >>> &qupv3_id_0 { >> >>> status = "okay"; >> >>> }; >> >>> @@ -278,6 +294,19 @@ &uart5 { >> >>> >> >>> /* PINCTRL - additions to nodes defined in sc7280.dtsi */ >> >>> >> >>> +&qspi_cs0 { >> >>> + bias-disable; >> >>> +}; >> >>> + >> >>> +&qspi_clk { >> >>> + bias-disable; >> >>> +}; >> >>> + >> >>> +&qspi_data01 { >> >>> + /* High-Z when no transfers; nice to park the lines */ >> >>> + bias-pull-up; >> >>> +}; >> >>> + >> >>> &qup_uart5_default { >> >>> tx { >> >>> pins = "gpio46"; >> >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> >>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> >>> index 6c9d5eb93f93..3047ab802cd2 100644 >> >>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> >>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> >>> @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { >> >>> }; >> >>> }; >> >>> >> >>> + qspi_opp_table: qspi-opp-table { >> >> >> >> This node doesn't represents anything on the mmio bus, so it shouldn't >> >> live in in /soc. Can't you move it into &qspi? >> >> >> >> Regards, >> >> Bjorn >> >> >> > >> > Sure, will move it into qspi node. >> > >> > Thanks, >> > Roja >> > >> >> Hi Bjorn, >> >> Moving "qspi_opp_table" inside &qspi node causing this warning: >> arch/arm64/boot/dts/qcom/sc7280.dtsi:1055.35-1072.6: Warning >> (spi_bus_reg): /soc@0/spi@88dc000/qspi-opp-table: missing or empty reg >> property > > If DT folks are OK I think we should hard-code 'opp-table' as not a > device for spi to populate on the spi bus and relax the warning in the > devicetree compiler (see [1] for more details). Technically, nodes that > are bus controllers assume all child nodes are devices on that bus. In > this case, we want to stick the opp table as a child of the spi node so > that it can be called 'opp-table' and not be a node in the root of DT. > >> >> Shall I keep the qspi-opp-table out of &qspi node? >> > > If you do, please move it to / instead of putting it under /soc as it > doesn't have an address or a reg property. > Hi Bjorn, Rob Can we move this "qspi_opp_table" to / from /soc? Thanks, Roja > [1] > https://github.com/dgibson/dtc/blob/69595a167f06c4482ce784e30df1ac9b16ceb5b0/checks.c#L1844
On Wed 14 Jul 02:47 CDT 2021, rojay@codeaurora.org wrote: > On 2021-07-09 06:26, Stephen Boyd wrote: > > Quoting rojay@codeaurora.org (2021-07-06 02:19:27) > > > On 2021-06-08 13:37, rojay@codeaurora.org wrote: > > > > On 2021-06-06 09:25, Bjorn Andersson wrote: > > > >> On Fri 04 Jun 08:54 CDT 2021, Roja Rani Yarubandi wrote: > > > >> > > > >>> Add QSPI DT node for SC7280 SoC. > > > >>> > > > >>> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> > > > >>> --- > > > >>> Changes in V3: > > > >>> - Broken the huge V2 patch into 3 smaller patches. > > > >>> 1. QSPI DT nodes > > > >>> 2. QUP wrapper_0 DT nodes > > > >>> 3. QUP wrapper_1 DT nodes > > > >>> > > > >>> Changes in V2: > > > >>> - As per Doug's comments removed pinmux/pinconf subnodes. > > > >>> - As per Doug's comments split of SPI, UART nodes has been done. > > > >>> - Moved QSPI node before aps_smmu as per the order. > > > >>> > > > >>> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ > > > >>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 > > > >>> +++++++++++++++++++++++++ > > > >>> 2 files changed, 90 insertions(+) > > > >>> > > > >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts > > > >>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > > > >>> index 3900cfc09562..d0edffc15736 100644 > > > >>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts > > > >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > > > >>> @@ -268,6 +268,22 @@ pmr735b_die_temp { > > > >>> }; > > > >>> }; > > > >>> > > > >>> +&qspi { > > > >>> + status = "okay"; > > > >>> + pinctrl-names = "default"; > > > >>> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; > > > >>> + > > > >>> + flash@0 { > > > >>> + compatible = "jedec,spi-nor"; > > > >>> + reg = <0>; > > > >>> + > > > >>> + /* TODO: Increase frequency after testing */ > > > >>> + spi-max-frequency = <25000000>; > > > >>> + spi-tx-bus-width = <2>; > > > >>> + spi-rx-bus-width = <2>; > > > >>> + }; > > > >>> +}; > > > >>> + > > > >>> &qupv3_id_0 { > > > >>> status = "okay"; > > > >>> }; > > > >>> @@ -278,6 +294,19 @@ &uart5 { > > > >>> > > > >>> /* PINCTRL - additions to nodes defined in sc7280.dtsi */ > > > >>> > > > >>> +&qspi_cs0 { > > > >>> + bias-disable; > > > >>> +}; > > > >>> + > > > >>> +&qspi_clk { > > > >>> + bias-disable; > > > >>> +}; > > > >>> + > > > >>> +&qspi_data01 { > > > >>> + /* High-Z when no transfers; nice to park the lines */ > > > >>> + bias-pull-up; > > > >>> +}; > > > >>> + > > > >>> &qup_uart5_default { > > > >>> tx { > > > >>> pins = "gpio46"; > > > >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > >>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > >>> index 6c9d5eb93f93..3047ab802cd2 100644 > > > >>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > >>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > > > >>> @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { > > > >>> }; > > > >>> }; > > > >>> > > > >>> + qspi_opp_table: qspi-opp-table { > > > >> > > > >> This node doesn't represents anything on the mmio bus, so it shouldn't > > > >> live in in /soc. Can't you move it into &qspi? > > > >> > > > >> Regards, > > > >> Bjorn > > > >> > > > > > > > > Sure, will move it into qspi node. > > > > > > > > Thanks, > > > > Roja > > > > > > > > > > Hi Bjorn, > > > > > > Moving "qspi_opp_table" inside &qspi node causing this warning: > > > arch/arm64/boot/dts/qcom/sc7280.dtsi:1055.35-1072.6: Warning > > > (spi_bus_reg): /soc@0/spi@88dc000/qspi-opp-table: missing or empty reg > > > property > > > > If DT folks are OK I think we should hard-code 'opp-table' as not a > > device for spi to populate on the spi bus and relax the warning in the > > devicetree compiler (see [1] for more details). Technically, nodes that > > are bus controllers assume all child nodes are devices on that bus. In > > this case, we want to stick the opp table as a child of the spi node so > > that it can be called 'opp-table' and not be a node in the root of DT. > > > > > > > > Shall I keep the qspi-opp-table out of &qspi node? > > > > > > > If you do, please move it to / instead of putting it under /soc as it > > doesn't have an address or a reg property. > > > > Hi Bjorn, Rob > > Can we move this "qspi_opp_table" to / from /soc? > If you have made a proper attempt to convince Rob and Mark that a child "opp-table" in a SPI master is not a SPI device - and the conclusion is that this is not a good idea...then yes it should live outside /soc. Thanks, Bjorn
On 2021-07-20 01:38, Bjorn Andersson wrote: > On Wed 14 Jul 02:47 CDT 2021, rojay@codeaurora.org wrote: > >> On 2021-07-09 06:26, Stephen Boyd wrote: >> > Quoting rojay@codeaurora.org (2021-07-06 02:19:27) >> > > On 2021-06-08 13:37, rojay@codeaurora.org wrote: >> > > > On 2021-06-06 09:25, Bjorn Andersson wrote: >> > > >> On Fri 04 Jun 08:54 CDT 2021, Roja Rani Yarubandi wrote: >> > > >> >> > > >>> Add QSPI DT node for SC7280 SoC. >> > > >>> >> > > >>> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> >> > > >>> --- >> > > >>> Changes in V3: >> > > >>> - Broken the huge V2 patch into 3 smaller patches. >> > > >>> 1. QSPI DT nodes >> > > >>> 2. QUP wrapper_0 DT nodes >> > > >>> 3. QUP wrapper_1 DT nodes >> > > >>> >> > > >>> Changes in V2: >> > > >>> - As per Doug's comments removed pinmux/pinconf subnodes. >> > > >>> - As per Doug's comments split of SPI, UART nodes has been done. >> > > >>> - Moved QSPI node before aps_smmu as per the order. >> > > >>> >> > > >>> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ >> > > >>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 >> > > >>> +++++++++++++++++++++++++ >> > > >>> 2 files changed, 90 insertions(+) >> > > >>> >> > > >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> > > >>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> > > >>> index 3900cfc09562..d0edffc15736 100644 >> > > >>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> > > >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> > > >>> @@ -268,6 +268,22 @@ pmr735b_die_temp { >> > > >>> }; >> > > >>> }; >> > > >>> >> > > >>> +&qspi { >> > > >>> + status = "okay"; >> > > >>> + pinctrl-names = "default"; >> > > >>> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; >> > > >>> + >> > > >>> + flash@0 { >> > > >>> + compatible = "jedec,spi-nor"; >> > > >>> + reg = <0>; >> > > >>> + >> > > >>> + /* TODO: Increase frequency after testing */ >> > > >>> + spi-max-frequency = <25000000>; >> > > >>> + spi-tx-bus-width = <2>; >> > > >>> + spi-rx-bus-width = <2>; >> > > >>> + }; >> > > >>> +}; >> > > >>> + >> > > >>> &qupv3_id_0 { >> > > >>> status = "okay"; >> > > >>> }; >> > > >>> @@ -278,6 +294,19 @@ &uart5 { >> > > >>> >> > > >>> /* PINCTRL - additions to nodes defined in sc7280.dtsi */ >> > > >>> >> > > >>> +&qspi_cs0 { >> > > >>> + bias-disable; >> > > >>> +}; >> > > >>> + >> > > >>> +&qspi_clk { >> > > >>> + bias-disable; >> > > >>> +}; >> > > >>> + >> > > >>> +&qspi_data01 { >> > > >>> + /* High-Z when no transfers; nice to park the lines */ >> > > >>> + bias-pull-up; >> > > >>> +}; >> > > >>> + >> > > >>> &qup_uart5_default { >> > > >>> tx { >> > > >>> pins = "gpio46"; >> > > >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> > > >>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> > > >>> index 6c9d5eb93f93..3047ab802cd2 100644 >> > > >>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> > > >>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> > > >>> @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { >> > > >>> }; >> > > >>> }; >> > > >>> >> > > >>> + qspi_opp_table: qspi-opp-table { >> > > >> >> > > >> This node doesn't represents anything on the mmio bus, so it shouldn't >> > > >> live in in /soc. Can't you move it into &qspi? >> > > >> >> > > >> Regards, >> > > >> Bjorn >> > > >> >> > > > >> > > > Sure, will move it into qspi node. >> > > > >> > > > Thanks, >> > > > Roja >> > > > >> > > >> > > Hi Bjorn, >> > > >> > > Moving "qspi_opp_table" inside &qspi node causing this warning: >> > > arch/arm64/boot/dts/qcom/sc7280.dtsi:1055.35-1072.6: Warning >> > > (spi_bus_reg): /soc@0/spi@88dc000/qspi-opp-table: missing or empty reg >> > > property >> > >> > If DT folks are OK I think we should hard-code 'opp-table' as not a >> > device for spi to populate on the spi bus and relax the warning in the >> > devicetree compiler (see [1] for more details). Technically, nodes that >> > are bus controllers assume all child nodes are devices on that bus. In >> > this case, we want to stick the opp table as a child of the spi node so >> > that it can be called 'opp-table' and not be a node in the root of DT. >> > >> > > >> > > Shall I keep the qspi-opp-table out of &qspi node? >> > > >> > >> > If you do, please move it to / instead of putting it under /soc as it >> > doesn't have an address or a reg property. >> > >> >> Hi Bjorn, Rob >> >> Can we move this "qspi_opp_table" to / from /soc? >> > > If you have made a proper attempt to convince Rob and Mark that > a child "opp-table" in a SPI master is not a SPI device - and the > conclusion is that this is not a good idea...then yes it should live > outside /soc. > > Thanks, > Bjorn Hi Rob/Mark, adding "qspi_opp_table" inside &qspi node causing warning we are getting "empty reg warning". qspi_opp_table contain frequencies for qspi and i think putting "opp-table" under qspi node is not a good idea because if we put under qspi it will treat "opp-table" as a device on the bus. in this scenario "opp-table" is not a device on the bus. so we moved qspi_opp_table from /soc to /. please give your inputs about this. Thanks and Regards, Rajesh
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts index 3900cfc09562..d0edffc15736 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts @@ -268,6 +268,22 @@ pmr735b_die_temp { }; }; +&qspi { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; + + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + + /* TODO: Increase frequency after testing */ + spi-max-frequency = <25000000>; + spi-tx-bus-width = <2>; + spi-rx-bus-width = <2>; + }; +}; + &qupv3_id_0 { status = "okay"; }; @@ -278,6 +294,19 @@ &uart5 { /* PINCTRL - additions to nodes defined in sc7280.dtsi */ +&qspi_cs0 { + bias-disable; +}; + +&qspi_clk { + bias-disable; +}; + +&qspi_data01 { + /* High-Z when no transfers; nice to park the lines */ + bias-pull-up; +}; + &qup_uart5_default { tx { pins = "gpio46"; diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 6c9d5eb93f93..3047ab802cd2 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -1061,6 +1061,42 @@ apss_merge_funnel_in: endpoint { }; }; + 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>; + }; + + opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + }; + + qspi: spi@88dc000 { + compatible = "qcom,qspi-v1"; + reg = <0 0x088dc000 0 0x1000>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&gcc GCC_QSPI_CNOC_PERIPH_AHB_CLK>, + <&gcc GCC_QSPI_CORE_CLK>; + clock-names = "iface", "core"; + interconnects = <&gem_noc MASTER_APPSS_PROC 0 + &cnoc2 SLAVE_QSPI_0 0>; + interconnect-names = "qspi-config"; + power-domains = <&rpmhpd SC7280_CX>; + operating-points-v2 = <&qspi_opp_table>; + status = "disabled"; + }; + system-cache-controller@9200000 { compatible = "qcom,sc7280-llcc"; reg = <0 0x09200000 0 0xd0000>, <0 0x09600000 0 0x50000>; @@ -1186,6 +1222,31 @@ tlmm: pinctrl@f100000 { gpio-ranges = <&tlmm 0 0 175>; wakeup-parent = <&pdc>; + qspi_clk: qspi-clk { + pins = "gpio14"; + function = "qspi_clk"; + }; + + qspi_cs0: qspi-cs0 { + pins = "gpio15"; + function = "qspi_cs"; + }; + + qspi_cs1: qspi-cs1 { + pins = "gpio19"; + function = "qspi_cs"; + }; + + qspi_data01: qspi-data01 { + pins = "gpio12", "gpio13"; + function = "qspi_data"; + }; + + qspi_data12: qspi-data12 { + pins = "gpio16", "gpio17"; + function = "qspi_data"; + }; + qup_uart5_default: qup-uart5-default { pins = "gpio46", "gpio47"; function = "qup13";
Add QSPI DT node for SC7280 SoC. Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> --- Changes in V3: - Broken the huge V2 patch into 3 smaller patches. 1. QSPI DT nodes 2. QUP wrapper_0 DT nodes 3. QUP wrapper_1 DT nodes Changes in V2: - As per Doug's comments removed pinmux/pinconf subnodes. - As per Doug's comments split of SPI, UART nodes has been done. - Moved QSPI node before aps_smmu as per the order. arch/arm64/boot/dts/qcom/sc7280-idp.dts | 29 ++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 61 +++++++++++++++++++++++++ 2 files changed, 90 insertions(+)