| Message ID | 20230421-fp4-bluetooth-v1-3-0430e3a7e0a2@fairphone.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add WCN3988 Bluetooth support for Fairphone 4 | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | warning | WARNING: line length of 113 exceeds 100 columns #147: FILE: arch/arm64/boot/dts/qcom/sm6350.dtsi:769: + pinctrl-0 = <&qup_uart1_cts>, <&qup_uart1_rts>, <&qup_uart1_tx>, <&qup_uart1_rx>; WARNING: line length of 109 exceeds 100 columns #151: FILE: arch/arm64/boot/dts/qcom/sm6350.dtsi:773: + interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>, WARNING: line length of 103 exceeds 100 columns #152: FILE: arch/arm64/boot/dts/qcom/sm6350.dtsi:774: + <&aggre1_noc MASTER_QUP_0 0 &clk_virt SLAVE_EBI_CH0 0>; total: 0 errors, 3 warnings, 81 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13220105.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | fail | "Bluetooth: " prefix is not specified in the subject |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
On Fri, Apr 21, 2023 at 9:12 AM Luca Weiss <luca.weiss@fairphone.com> wrote: > > Add the node describing uart1 incl. opp table and pinctrl. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 63 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 63 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi > index 18c4616848ce..16c5e9a6c98a 100644 > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi > @@ -378,6 +378,25 @@ opp-2073600000 { > }; > }; > > + qup_opp_table: opp-table-qup { > + compatible = "operating-points-v2"; > + > + opp-75000000 { > + opp-hz = /bits/ 64 <75000000>; > + required-opps = <&rpmhpd_opp_low_svs>; > + }; > + > + opp-100000000 { > + opp-hz = /bits/ 64 <100000000>; > + required-opps = <&rpmhpd_opp_svs>; > + }; > + > + opp-128000000 { > + opp-hz = /bits/ 64 <128000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + }; > + > pmu { > compatible = "arm,armv8-pmuv3"; > interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_LOW>; > @@ -741,6 +760,22 @@ i2c0: i2c@880000 { > status = "disabled"; > }; > > + uart1: serial@884000 { > + compatible = "qcom,geni-uart"; > + reg = <0 0x00884000 0 0x4000>; > + clock-names = "se"; > + clocks = <&gcc GCC_QUPV3_WRAP0_S1_CLK>; > + pinctrl-names = "default"; > + pinctrl-0 = <&qup_uart1_cts>, <&qup_uart1_rts>, <&qup_uart1_tx>, <&qup_uart1_rx>; > + interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&rpmhpd SM6350_CX>; > + operating-points-v2 = <&qup_opp_table>; > + interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>, > + <&aggre1_noc MASTER_QUP_0 0 &clk_virt SLAVE_EBI_CH0 0>; > + interconnect-names = "qup-core", "qup-config"; > + status = "disabled"; > + }; > + > i2c2: i2c@888000 { > compatible = "qcom,geni-i2c"; > reg = <0 0x00888000 0 0x4000>; > @@ -1726,6 +1761,34 @@ qup_i2c10_default: qup-i2c10-default-state { > drive-strength = <2>; > bias-pull-up; > }; > + > + qup_uart1_cts: qup-uart1-cts-default-state { > + pins = "gpio61"; > + function = "qup01"; > + drive-strength = <2>; > + bias-disable; > + }; > + > + qup_uart1_rts: qup-uart1-rts-default-state { > + pins = "gpio62"; > + function = "qup01"; > + drive-strength = <2>; > + bias-pull-down; > + }; > + > + qup_uart1_tx: qup-uart1-tx-default-state { > + pins = "gpio63"; > + function = "qup01"; > + drive-strength = <2>; > + bias-pull-up; > + }; > + tx should come after the rx, this caught me too when I was doing my bluetooth driver, it goes by name, not gpio#. > + qup_uart1_rx: qup-uart1-rx-default-state { > + pins = "gpio64"; > + function = "qup01"; > + drive-strength = <2>; > + bias-disable; > + }; > }; > > apps_smmu: iommu@15000000 { > > -- > 2.40.0 >
On 21/04/2023 16:11, Luca Weiss wrote: > Add the node describing uart1 incl. opp table and pinctrl. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 63 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 63 insertions(+) Please do not send DTS patches for net-next. DTS must go via Qualcomm SoC. Split the series and mention where is the bindings change in DTS patchset. Best regards, Krzysztof
On Sun Apr 23, 2023 at 12:51 PM CEST, Krzysztof Kozlowski wrote: > On 21/04/2023 16:11, Luca Weiss wrote: > > Add the node describing uart1 incl. opp table and pinctrl. > > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > > --- > > arch/arm64/boot/dts/qcom/sm6350.dtsi | 63 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 63 insertions(+) > > Please do not send DTS patches for net-next. DTS must go via Qualcomm > SoC. Split the series and mention where is the bindings change in DTS > patchset. Sorry, just saw now after already sending v2. Is this a special rule for linux-bluetooth@ / netdev@? Isn't it easier to keep it together so the status of series can be assessed easier? I've always submitted patches by topic, like input patches + dts patches and it was never mentioned. Regards Luca > > > Best regards, > Krzysztof
On 12/05/2023 16:30, Luca Weiss wrote: > On Sun Apr 23, 2023 at 12:51 PM CEST, Krzysztof Kozlowski wrote: >> On 21/04/2023 16:11, Luca Weiss wrote: >>> Add the node describing uart1 incl. opp table and pinctrl. >>> >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>> --- >>> arch/arm64/boot/dts/qcom/sm6350.dtsi | 63 ++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 63 insertions(+) >> >> Please do not send DTS patches for net-next. DTS must go via Qualcomm >> SoC. Split the series and mention where is the bindings change in DTS >> patchset. > > Sorry, just saw now after already sending v2. > > Is this a special rule for linux-bluetooth@ / netdev@? Isn't it easier > to keep it together so the status of series can be assessed easier? I've > always submitted patches by topic, like input patches + dts patches and > it was never mentioned. The rule that DTS must go via Qualcomm SoC (arm-soc) was there always, but other maintainers often do not pay attention to this. I don't blame them, don't get me wrong. I am just stating the observed actions. Usually netdev folks and Greg will take everything you throw at them, so for these subsystems it is recommended to split DTS to different patchset. For other maintainers it is usually also more useful to split, because then they can apply entire patchset with one command, instead of picking up specific patches (omitting DTS). Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi index 18c4616848ce..16c5e9a6c98a 100644 --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi @@ -378,6 +378,25 @@ opp-2073600000 { }; }; + qup_opp_table: opp-table-qup { + compatible = "operating-points-v2"; + + opp-75000000 { + opp-hz = /bits/ 64 <75000000>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-100000000 { + opp-hz = /bits/ 64 <100000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-128000000 { + opp-hz = /bits/ 64 <128000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + }; + pmu { compatible = "arm,armv8-pmuv3"; interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_LOW>; @@ -741,6 +760,22 @@ i2c0: i2c@880000 { status = "disabled"; }; + uart1: serial@884000 { + compatible = "qcom,geni-uart"; + reg = <0 0x00884000 0 0x4000>; + clock-names = "se"; + clocks = <&gcc GCC_QUPV3_WRAP0_S1_CLK>; + pinctrl-names = "default"; + pinctrl-0 = <&qup_uart1_cts>, <&qup_uart1_rts>, <&qup_uart1_tx>, <&qup_uart1_rx>; + interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&rpmhpd SM6350_CX>; + operating-points-v2 = <&qup_opp_table>; + interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>, + <&aggre1_noc MASTER_QUP_0 0 &clk_virt SLAVE_EBI_CH0 0>; + interconnect-names = "qup-core", "qup-config"; + status = "disabled"; + }; + i2c2: i2c@888000 { compatible = "qcom,geni-i2c"; reg = <0 0x00888000 0 0x4000>; @@ -1726,6 +1761,34 @@ qup_i2c10_default: qup-i2c10-default-state { drive-strength = <2>; bias-pull-up; }; + + qup_uart1_cts: qup-uart1-cts-default-state { + pins = "gpio61"; + function = "qup01"; + drive-strength = <2>; + bias-disable; + }; + + qup_uart1_rts: qup-uart1-rts-default-state { + pins = "gpio62"; + function = "qup01"; + drive-strength = <2>; + bias-pull-down; + }; + + qup_uart1_tx: qup-uart1-tx-default-state { + pins = "gpio63"; + function = "qup01"; + drive-strength = <2>; + bias-pull-up; + }; + + qup_uart1_rx: qup-uart1-rx-default-state { + pins = "gpio64"; + function = "qup01"; + drive-strength = <2>; + bias-disable; + }; }; apps_smmu: iommu@15000000 {
Add the node describing uart1 incl. opp table and pinctrl. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- arch/arm64/boot/dts/qcom/sm6350.dtsi | 63 ++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+)