| Message ID | 1627306847-25308-1-git-send-email-rajpat@codeaurora.org |
|---|---|
| Headers | show |
| Series | Add QSPI and QUPv3 DT nodes for SC7280 SoC | expand |
On Mon, Jul 26, 2021 at 07:10:46PM +0530, Rajesh Patil wrote: > From: Roja Rani Yarubandi <rojay@codeaurora.org> > > Update QUPv3 Debug UART DT node with the interconnect names and > functions for SC7280 SoC. > > Split the Debug UART pin control functions. > > Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> > Signed-off-by: Rajesh Patil <rajpat@codeaurora.org> > --- > Changes in V4: > - As per Bjorn's comment, posting this debug-uart node update > as seperate patch > > arch/arm64/boot/dts/qcom/sc7280-idp.dts | 18 +++++++----------- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 28 ++++++++++++++++++++++++---- > 2 files changed, 31 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > index f63cf51..a50c9e5 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > @@ -383,18 +383,14 @@ > bias-pull-up; > }; > > -&qup_uart5_default { > - tx { > - pins = "gpio46"; > - drive-strength = <2>; > - bias-disable; > - }; > +&qup_uart5_tx { > + drive-strength = <2>; > + bias-disable; > +}; > > - rx { > - pins = "gpio47"; > - drive-strength = <2>; > - bias-pull-up; > - }; > +&qup_uart5_rx { > + drive-strength = <2>; > + bias-pull-up; > }; > > &sdc1_on { > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 455e58f..951818f 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -853,8 +853,13 @@ > clock-names = "se"; > clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>; > pinctrl-names = "default"; > - pinctrl-0 = <&qup_uart5_default>; > + pinctrl-0 = <&qup_uart5_cts>, <&qup_uart5_rts>, <&qup_uart5_tx>, <&qup_uart5_rx>; > interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&rpmhpd SC7280_CX>; > + operating-points-v2 = <&qup_opp_table>; > + interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>, > + <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>; > + interconnect-names = "qup-core", "qup-config"; Most of the above should be added by patch '[2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes'. I have to say I dislike that the SoC DT file dictates which UART to use for the serial console. Technically it could be any of them, right? uart5 is used because that's what the IDP does, and the rest of the world is expected to follow. Why not configure uart5 as "qcom,geni-uart" by default and overwrite the compatible string and pinctrl in the board file? You could even add 'qup-uartN-all' (or similar) pinconfigs to sc7280.dtsi, which would make the changes in the board file trivial.
On Mon, Jul 26, 2021 at 07:10:44PM +0530, Rajesh Patil wrote: > From: Roja Rani Yarubandi <rojay@codeaurora.org> > > Add QSPI DT node for SC7280 SoC. > > Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> > Signed-off-by: Rajesh Patil <rajpat@codeaurora.org> > --- > Changes in V4: > - As per Stephen's comment updated spi-max-frequency to 37.5MHz, moved > qspi_opp_table from /soc to / (root). > > 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 | 27 ++++++++++++++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 62 +++++++++++++++++++++++++++++++++ > 2 files changed, 89 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > index 73225e3..b0bfd8e 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts > @@ -269,6 +269,20 @@ > }; > }; > > +&qspi { > + status = "okay"; > + pinctrl-names = "default"; > + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; > + > + flash@0 { > + compatible = "jedec,spi-nor"; > + reg = <0>; > + spi-max-frequency = <37500000>; > + spi-tx-bus-width = <2>; > + spi-rx-bus-width = <2>; > + }; > +}; > + > &qupv3_id_0 { > status = "okay"; > }; > @@ -346,6 +360,19 @@ > > /* 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; > +}; > + This configures the SPI flash of the SC7280 IDP board, which is neither mentioned in the subject nor the body of the commit message. IMO this should be split out into a separate patch.
Quoting Rajesh Patil (2021-07-26 06:40:43) > Roja Rani Yarubandi (4): > arm64: dts: sc7280: Add QSPI node > arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes > arm64: dts: sc7280: Update QUPv3 Debug UART DT node > arm64: dts: sc7280: Add QUPv3 wrapper_1 nodes Please include a changelog in the cover letter and Cc me on the subsequent rounds of this series. Thanks. > > arch/arm64/boot/dts/qcom/sc7280-idp.dts | 133 +- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 3169 +++++++++++++++++++++++-------- > 2 files changed, 2482 insertions(+), 820 deletions(-) >
On 2021-07-26 21:12, Matthias Kaehlcke wrote: > On Mon, Jul 26, 2021 at 07:10:46PM +0530, Rajesh Patil wrote: >> From: Roja Rani Yarubandi <rojay@codeaurora.org> >> >> Update QUPv3 Debug UART DT node with the interconnect names and >> functions for SC7280 SoC. >> >> Split the Debug UART pin control functions. >> >> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> >> Signed-off-by: Rajesh Patil <rajpat@codeaurora.org> >> --- >> Changes in V4: >> - As per Bjorn's comment, posting this debug-uart node update >> as seperate patch >> >> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 18 +++++++----------- >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 28 >> ++++++++++++++++++++++++---- >> 2 files changed, 31 insertions(+), 15 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> index f63cf51..a50c9e5 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> @@ -383,18 +383,14 @@ >> bias-pull-up; >> }; >> >> -&qup_uart5_default { >> - tx { >> - pins = "gpio46"; >> - drive-strength = <2>; >> - bias-disable; >> - }; >> +&qup_uart5_tx { >> + drive-strength = <2>; >> + bias-disable; >> +}; >> >> - rx { >> - pins = "gpio47"; >> - drive-strength = <2>; >> - bias-pull-up; >> - }; >> +&qup_uart5_rx { >> + drive-strength = <2>; >> + bias-pull-up; >> }; >> >> &sdc1_on { >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> index 455e58f..951818f 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> @@ -853,8 +853,13 @@ >> clock-names = "se"; >> clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>; >> pinctrl-names = "default"; >> - pinctrl-0 = <&qup_uart5_default>; >> + pinctrl-0 = <&qup_uart5_cts>, <&qup_uart5_rts>, <&qup_uart5_tx>, >> <&qup_uart5_rx>; >> interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>; >> + power-domains = <&rpmhpd SC7280_CX>; >> + operating-points-v2 = <&qup_opp_table>; >> + interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt >> SLAVE_QUP_CORE_0 0>, >> + <&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>; >> + interconnect-names = "qup-core", "qup-config"; > > > Most of the above should be added by patch '[2/4] arm64: dts: sc7280: > Add QUPv3 > wrapper_0 nodes'. Based on the comments on v3 [1], I have added this as a separate patch [1] https://lore.kernel.org/patchwork/patch/1441257/ > > I have to say I dislike that the SoC DT file dictates which UART to use > for > the serial console. Technically it could be any of them, right? uart5 > is > used because that's what the IDP does, and the rest of the world is > expected > to follow. Why not configure uart5 as "qcom,geni-uart" by default and > overwrite the compatible string and pinctrl in the board file? You > could even > add 'qup-uartN-all' (or similar) pinconfigs to sc7280.dtsi, which would > make > the changes in the board file trivial. Okay, will make the compatible as "qcom,geni-uart" in SoC dt and later modify it in idp dts as "qcom,geni-debug-uart". Thanks, Rajesh
On 2021-07-26 21:37, Matthias Kaehlcke wrote: > On Mon, Jul 26, 2021 at 07:10:44PM +0530, Rajesh Patil wrote: >> From: Roja Rani Yarubandi <rojay@codeaurora.org> >> >> Add QSPI DT node for SC7280 SoC. >> >> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org> >> Signed-off-by: Rajesh Patil <rajpat@codeaurora.org> >> --- >> Changes in V4: >> - As per Stephen's comment updated spi-max-frequency to 37.5MHz, >> moved >> qspi_opp_table from /soc to / (root). >> >> 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 | 27 ++++++++++++++ >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 62 >> +++++++++++++++++++++++++++++++++ >> 2 files changed, 89 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> index 73225e3..b0bfd8e 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts >> @@ -269,6 +269,20 @@ >> }; >> }; >> >> +&qspi { >> + status = "okay"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>; >> + >> + flash@0 { >> + compatible = "jedec,spi-nor"; >> + reg = <0>; >> + spi-max-frequency = <37500000>; >> + spi-tx-bus-width = <2>; >> + spi-rx-bus-width = <2>; >> + }; >> +}; >> + >> &qupv3_id_0 { >> status = "okay"; >> }; >> @@ -346,6 +360,19 @@ >> >> /* 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; >> +}; >> + > > This configures the SPI flash of the SC7280 IDP board, which is neither > mentioned in the subject nor the body of the commit message. IMO this > should be split out into a separate patch. Okay.
On 2021-07-27 05:44, Stephen Boyd wrote: > Quoting Rajesh Patil (2021-07-26 06:40:43) >> Roja Rani Yarubandi (4): >> arm64: dts: sc7280: Add QSPI node >> arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes >> arm64: dts: sc7280: Update QUPv3 Debug UART DT node >> arm64: dts: sc7280: Add QUPv3 wrapper_1 nodes > > Please include a changelog in the cover letter and Cc me on the > subsequent rounds of this series. Thanks. > Okay. >> >> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 133 +- >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3169 >> +++++++++++++++++++++++-------- >> 2 files changed, 2482 insertions(+), 820 deletions(-) >>